Skip to content

Conversation

@Thomas1664
Copy link
Contributor

@Thomas1664 Thomas1664 commented Dec 19, 2025

Fixes microsoft/vcpkg#48892

I think the root cause of the issue was that the default features of the top level manifest were initially unknown and the graph was then stuck resolving the circular dependency between the individual features. This issue is now avoided because all features of the top level manifest are now already known beforehand.

I also added an e2e test based on the manifest from the original bug report. Note that I removed the boost-test dependency. I verified that the original bug could still be reproduced.

@Thomas1664 Thomas1664 changed the title Manually revert 87adc3f47d5327558b92b6e6925f1706047f7acd "Fix default features control by top level manifest (#1331)" Manually revert 87adc3f "Fix default features control by top level manifest (#1331)" Dec 19, 2025
Note that I removed the boost-test dependency. I verified that the original bug could still be reproduced.
@Thomas1664
Copy link
Contributor Author

Thomas1664 commented Dec 19, 2025

Please note that this PR would again cause the bug that #1331 was trying to fix: microsoft/vcpkg#35694 (comment)

The failure mode there is that if you install a feature A of the top-level manifest that depends on another feature B of the top level manifest and feature B depends on a port with default features, vcpkg would ignore that you disabled default features when referencing that dependency in feature B. To get the intended behavior, you would need to also disable default features in feature A when referencing feature B - even if you don't have any default features in the top level manifest.

IMO, the original PR did not fix this issue. It just wasn't visible because the top level manifest wouldn't (fully) participate in the dependeny graph.

@Thomas1664
Copy link
Contributor Author

@BillyONeal I wouldn't recommend merging this as is because this would pull in more additional dependencies for manifest users which might be unexpected and cause problems.

Instead, the underlying issue with overwriting default features false in nested top level manifest features should be fixed. Unfortunately, I don't have time to address this. Feel free to push to this PR to make these changes.

@dg0yt
Copy link
Contributor

dg0yt commented Dec 19, 2025

A complete revert misses the point, in particular given the tests that came with #1331. The only behavioral change is much smaller, 4285366.

@Thomas1664
Copy link
Contributor Author

Closing in favor of #1890

@Thomas1664 Thomas1664 closed this Dec 20, 2025
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.

vcpkg doesn't fail and consumes RAM when a library lists itself as a dependency

2 participants