Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[vcpkg] Fix ci: suggest vcpkg_*_cmake -> vcpkg_cmake_* migration #20218

Merged

Conversation

autoantwort
Copy link
Contributor

@autoantwort autoantwort commented Sep 17, 2021

Fixes the bugs reported in #20142

HEAD^^ refers to the parent of the parent (HEAD^2 refers to the second parent)
If the ci is running, the branch is merged into the master branch, so we for example have the following graph:

*   c21dd13cb (HEAD -> master) Merge branch 'document-deprecated-functions'
|\  
| * aa8e5cc56 (document-deprecated-functions) Document deprecated functions from #20142 in the maintainer guide.
|/  
* 574c125d6 [openimageio] Re-fix the usage (#20092)
* fbe07843a [gettext] Remove `SUBPATH`, add iconv linking info (#20090)

The command git diff --name-status --merge-base HEAD^^ HEAD now compared c21dd13cb(HEAD) against the base fbe07843a, so it detects the file changes in aa8e5cc56 and 574c125d6, which is wrong (it always also included the changes of the latest master commit). Now it compares c21dd13cb(HEAD) against 574c125d6(latest master). This explains the suggestion in #20217

@autoantwort autoantwort force-pushed the request-cmake_vcpkg-conversion branch from 4258beb to ebcca4f Compare September 17, 2021 02:10
@autoantwort
Copy link
Contributor Author

The wrong suggestion in #19756 was caused by wrongly using grep 🤦

@PhoebeHui PhoebeHui added the category:infrastructure Pertaining to the CI/Testing infrastrucutre label Sep 17, 2021
@PhoebeHui PhoebeHui changed the title Fix ci: suggest vcpkg_*_cmake -> vcpkg_cmake_* migration [vcpkg] Fix ci: suggest vcpkg_*_cmake -> vcpkg_cmake_* migration Sep 17, 2021
@autoantwort autoantwort force-pushed the request-cmake_vcpkg-conversion branch from ebcca4f to 9b3b0a7 Compare September 17, 2021 03:01
@PhoebeHui PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Sep 17, 2021
@BillyONeal BillyONeal merged commit 2056203 into microsoft:master Sep 17, 2021
@BillyONeal
Copy link
Member

Thanks for poking the bot :)

@wrobelda
Copy link
Contributor

wrobelda commented Sep 17, 2021

Something is off, still – the bot now fails with a non-zero exit code and no comment left:
https://github.com/microsoft/vcpkg/pull/19828/checks?check_run_id=3633952227#step:6:17

@autoantwort
Copy link
Contributor Author

Uff yeah grep returns 1 when it finds nothing which is then interpreted as pipeline failure.

@BillyONeal
Copy link
Member

Uff yeah grep returns 1 when it finds nothing which is then interpreted as pipeline failure.

Whack a mole continues 👍

@autoantwort
Copy link
Contributor Author

Hopefully last fix pr in #20236 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:infrastructure Pertaining to the CI/Testing infrastrucutre info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants