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.

Sign up for GitHub

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

Jump to bottom

Fix bug in .vcpkg-root detection that breaks --overlay-triplets #7954

Merged
merged 10 commits into from
Sep 13, 2019
Merged

Fix bug in .vcpkg-root detection that breaks --overlay-triplets #7954

merged 10 commits into from
Sep 13, 2019

Conversation

martin-s
Copy link
Contributor

@martin-s martin-s commented Aug 28, 2019 •
edited
Loading

I introduced a scripts variable (like download) to align the access to it throughout the cmake files. Please test, if it fits also to your --overlay-triplets use case (see #7943).

  • Added scripts variable to ports file.
  • Added check if triplet file is not available (NPE).
  • Fixed scripts command parameter in combination with --vcpkg-root

@martin-s
- Added scripts variable to ports file.
ddde7ae 
- Added check if triplet file is not available (NPE).
@cbezault
Copy link
Contributor

LGTM, though I would like to see the line ending changes get fixed so changes to the ports.cmake file are more obvious.

vicroms
vicroms requested changes Aug 29, 2019
View reviewed changes
@vicroms
Copy link
Member

vicroms commented Aug 29, 2019 •
edited
Loading

Still need to fix --overlay-triplets.

Fixed in ceed178

vicroms
vicroms reviewed Aug 29, 2019
View reviewed changes
@vicroms vicroms changed the title #7793 broke --overlay-triplets Fix bug in .vcpkg-root detection that breaks --overlay-ports Aug 29, 2019
@vicroms vicroms changed the title Fix bug in .vcpkg-root detection that breaks --overlay-ports Fix bug in .vcpkg-root detection that breaks --overlay-triplets Aug 29, 2019
@vicroms
Copy link
Member

vicroms commented Sep 4, 2019

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vicroms
Copy link
Member

vicroms commented Sep 12, 2019

At this time I have decided to make --scripts-root an experimental feature (renamed to --x-scripts-root). Experimental features, are features that we believe are good-to-have in the tool but that don't align with our currently prioritized scenarios (however, they could become official features eventually); we'll keep accepting PRs to fix issues with this feature, but won't test for regressions when other features are added/updated, for example, when fixing issues with --overlay-triplets.

That being said, I encountered an issue when using --x-scripts-root where packages fail to configure if no .vcpkg-root file exists in a directory above the specified scripts root.

I'll merge this PR in order to fix --overlay-triplets ASAP, and create an issue to track the bug with --x-scripts-root.

@vicroms vicroms self-requested a review September 12, 2019 22:34
@vicroms vicroms merged commit 191c864 into microsoft:master Sep 13, 2019
@NancyLi1013 NancyLi1013 mentioned this pull request Jan 21, 2020
Closed
@LilyWangL LilyWangL mentioned this pull request Jan 22, 2020
Merged
strega-nil pushed a commit to strega-nil/vcpkg that referenced this pull request May 5, 2021
@martin-s @vicroms
Fix bug in .vcpkg-root detection that breaks --overlay-triplets (m…
08192ec 
…icrosoft#7954)

* - Added scripts variable to ports file.
- Added check if triplet file is not available (NPE).

* - Fixed line endings.

* Provide location of .vcpkg-root to ports.cmake

* vcpkg sets VCPKG_ROOT_PATH in ports.cmake

* [vcpkg] Fail if VCPKG_ROOT_PATH is not passed from vcpkg.exe

* [vcpkg] Fix vcpkg rooth path

* [vcpkg] Make --x-scripts-root an experimental command
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vicroms vicroms Awaiting requested review from vicroms

Assignees

@vicroms vicroms

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@martin-s @cbezault @vicroms