Skip to content

Conversation

@dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Dec 22, 2025

Collected while working on #1890.
Removes unused code.
Makes batch_load_vars() handle all dependencies from the current ConstraintFrame stack item. In particular, extends the initial batch when the stack is empty and the current frame already constains multiple dependencies from the top level manifest. OTOH it may have no effect at all because unbatched processing already collected the vars in commands.install.cpp.

@dg0yt dg0yt marked this pull request as ready for review December 22, 2025 13:21
Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

Thanks!

std::unordered_set<PackageSpec> spec_set = {spec};
// We want to batch as many dep_infos as possible, so look ahead in the frame and stack
std::unordered_set<PackageSpec> spec_set = {frame.spec};
for (auto&& d : frame.deps)
Copy link
Member

Choose a reason for hiding this comment

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

I hate the no {}s but that's what the other code in this function is doing so no change requested.

it->second.overlay_or_override = true;
it->second.scfl = p_overlay;
}
else if (const auto over_it = m_overrides.find(spec.name()); over_it != m_overrides.end())
Copy link
Member

Choose a reason for hiding this comment

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

GitHub's diff is freaking awful as always. This is just deleting the unused Version ver and changing else { if into else if

No change requested.

@BillyONeal BillyONeal merged commit a24853f into microsoft:main Jan 8, 2026
7 checks passed
BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this pull request Jan 8, 2026
Nitpicks I noticed while reviewing microsoft#1891
@dg0yt dg0yt deleted the livelock-opt branch January 8, 2026 22:09
vicroms pushed a commit that referenced this pull request Jan 8, 2026
Nitpicks I noticed while reviewing #1891
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