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

Rename / Rearrange BinaryCache functions #1057

Merged
merged 4 commits into from
May 8, 2023

Conversation

ras0219-msft
Copy link
Contributor

This PR extracts ancillary changes from #998 so they can be reviewed separately from the primary rewrite of binarycaching.cpp.

  • create_binary_providers_from_configs_pure -> parse_binary_provider_configs

  • Different callers of Install::perform performed different amounts of "prep" (computing ABIs, etc). This work was then repeated inside perform.

    To resolve this, all callers are now expected to compute all ABIs ahead of time. I've removed the var_provider parameter, and made the action parameter const.

  • I've lifted the REQUIRE_LINES test macro so it can be used in more places.

@BillyONeal BillyONeal merged commit c2859a3 into microsoft:main May 8, 2023
@ras0219-msft ras0219-msft deleted the dev/roschuma/bincache1 branch May 8, 2023 21:35
@@ -552,8 +551,6 @@ namespace vcpkg
perform_install_plan_action(args, paths, action, status_db, binary_cache, build_logs_recorder));
}

compute_all_abis(paths, action_plan, var_provider, status_db);
binary_cache.prefetch(action_plan.install_actions);
Copy link
Contributor

Choose a reason for hiding this comment

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

Now the prefetch is done before Install::execute_plan, but in line 544/543 the packages folder is removed.

Copy link
Member

Choose a reason for hiding this comment

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

Those lines are only relevant for packages that are being removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

But if a package gets reinstalled with a different feature set it gets removed and then installed

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.

3 participants