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

Unbreak fmt upgrades #783

Merged
merged 1 commit into from
Nov 2, 2022
Merged

Unbreak fmt upgrades #783

merged 1 commit into from
Nov 2, 2022

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Nov 1, 2022

VCPKG_FMT_URL must not be cached because it the cached value breaks contributors when fmt and its SHA512 sum is updated.

@BillyONeal
Copy link
Member

To clarify, we have 1 scenario ever for this which is that a signed build needs to be able to parachute in a different URI on the command line because there are restrictions on what servers signed builds are allowed to talk to. For example:

cmake -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=OFF -DVCPKG_DEVELOPMENT_WARNINGS=ON -DVCPKG_WARNINGS_AS_ERRORS=ON -DVCPKG_BUILD_FUZZING=OFF -DVCPKG_EMBED_GIT_SHA=ON -DVCPKG_OFFICIAL_BUILD=ON -DCMAKE_OSX_DEPLOYMENT_TARGET=10.13 -DCMAKE_OSX_ARCHITECTURES="arm64;x86_64" "-DVCPKG_FMT_URL=$FMT_TARBALL_URL" -DVCPKG_BASE_VERSION=$VCPKG_BASE_VERSION -DVCPKG_STANDALONE_BUNDLE_SHA=$VCPKG_STANDALONE_BUNDLE_SHA -DVCPKG_CE_SHA=$VCPKG_CE_SHA -B "$(Build.BinariesDirectory)/build"

Can this be done more reasonably without making it a cache variable in the first place? This seems to be constantly breaking folks, however little I care about saying "just delete cache and reconfigure"

Copy link
Contributor Author

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be done more reasonably without making it a cache variable in the first place?

This is what this patch does...

if("$CACHE{VCPKG_FMT_URL}" MATCHES "^https://github.com/fmtlib/fmt/archive/refs/tags")
unset(VCPKG_FMT_URL CACHE) # Fix upgrade
endif()
if(NOT VCPKG_FMT_URL)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you pass the variable from outside, it is used and not overriden...

@@ -4,7 +4,13 @@ option(VCPKG_DEPENDENCY_EXTERNAL_FMT "Use an external version of the fmt library
# builds which have restricted internet access; see azure-pipelines/signing.yml
# Note that the SHA512 is the same, so vcpkg-tool contributors need not be concerned that we built
# with different content.
set(VCPKG_FMT_URL "https://github.com/fmtlib/fmt/archive/refs/tags/9.1.0.tar.gz" CACHE STRING "URL to the fmt release tarball to use.")
# A cache variable cannot be used it here because it will break contributors' builds on fmt update.
if("$CACHE{VCPKG_FMT_URL}" MATCHES "^https://github.com/fmtlib/fmt/archive/refs/tags")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...unless it is assumed to be an old default value: This cache value must be removed in order to fix upgrades.

@BillyONeal BillyONeal merged commit 5aaea46 into microsoft:main Nov 2, 2022
@BillyONeal
Copy link
Member

Thanks :)

@dg0yt dg0yt deleted the fmt_url branch November 3, 2022 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants