[vcpkg] Fix name clash (e.g. "all" build for triplet x64-windows-release and "release" build for triplet x64-windows)#22501
[vcpkg] Fix name clash (e.g. "all" build for triplet x64-windows-release and "release" build for triplet x64-windows)#22501mkhon wants to merge 1 commit intomicrosoft:masterfrom soylent-io:fix/z_vcpkg_get_cmake_vars-name-clash
Conversation
and "release" build for triplet x64-windows).
|
|
||
| if(DEFINED VCPKG_BUILD_TYPE) | ||
| set(cmake_vars_file "${CURRENT_BUILDTREES_DIR}/cmake-vars-${TARGET_TRIPLET}-${VCPKG_BUILD_TYPE}.cmake.log") | ||
| set(cmake_vars_file "${CURRENT_BUILDTREES_DIR}/cmake-vars-${TARGET_TRIPLET}.${VCPKG_BUILD_TYPE}.cmake.log") |
There was a problem hiding this comment.
I don't quite understand; doesn't this need to match the paths on 54 and 55? That one uses dashes rather than dots.
There was a problem hiding this comment.
No, it should not have to match these because this file includes one (or both) of defined at lines 54 and 55
There was a problem hiding this comment.
Maybe then we should add a name like "combined" rather than expecting a . rather than - to convey all the meaning here?
There was a problem hiding this comment.
It was "cmake-vars-${TARGET_TRIPLET}.cmake.log" before 7534d24
I only wanted to change it to "cmake-vars-${TARGET_TRIPLET}.${VCPKG_BUILD_TYPE}.cmake.log"
There was a problem hiding this comment.
What I mean is, having two files with semantically different contents looks like a bug, and I can easily see the next person recreating the bug you're trying to fix here by renaming it back. I don't care what was before, I only care about making this code understandable and making it unlikely that we will recreate the problem again.
|
Can we push this minor fix? |
|
requires:author-response tag should also be removed from here |
I don't believe #22501 (comment) has a response |
|
@mkhon Can you reply the review suggestions? Thanks! |
|
Closing this PR since it seems that no progress is being made. Please reopen if work is still being done. |
Describe the pull request
This fixes name clash still left after 7534d24
What does your PR fix?
Which triplets are supported/not supported? Have you updated the CI baseline?
all
Does your PR follow the maintainer guide?
Yes
If you have added/updated a port: Have you run
./vcpkg x-add-version --alland committed the result?N/A
If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/