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

Vendor Cargo dependencies #267

Open
wants to merge 1 commit into
base: msft-main
Choose a base branch
from
Open

Conversation

arc9693
Copy link

@arc9693 arc9693 commented Dec 10, 2024

Target rust folders: src/tardev-snapshotter, src/agent, src/utarfs, src/overlay

The change involves vendoring Cargo dependencies to enhance build reproducibility and security. All dependencies are downloaded and stored locally in a vendor directory, eliminating the need to fetch them from external sources during builds. The projeci's Cargo configuration is updated to prioritize these vendored sources, ensuring consistent, self-contained builds regardless of external factors like network availability or changes in dependency repositories. By committing the vendor directory to version control, the project gains improved security, faster build times, and determinism. Developers must refresh the vendored dependencies whenever updates are made to the Cargo.toml, by re-running cargo vendor.

Merge Checklist
  • Followed patch format from upstream recommendation: https://github.com/kata-containers/community/blob/main/CONTRIBUTING.md#patch-format
    • Included a single commit in a given PR - at least unless there are related commits and each makes sense as a change on its own.
  • Aware about the PR to be merged using "create a merge commit" rather than "squash and merge" (or similar)
  • The upstream/missing label (or upstream/not-needed) has been set on the PR.
Summary
Test Methodology

Spec file changes (draft): microsoft/azurelinux@mahuber/kata-3.2.0.azl4...archana1/remove-rust-deps
Buddy Build of kata-containers and kata-containers-cc: https://dev.azure.com/mariner-org/mariner/_build?definitionId=2190&_a=summary (https://dev.azure.com/mariner-org/mariner/_build/results?buildId=693264&view=results)

@arc9693 arc9693 requested review from a team as code owners December 10, 2024 12:00
@arc9693 arc9693 added the upstream/not-needed PRs that will not be upstreamed (e.g. internal) label Dec 10, 2024
@arc9693 arc9693 force-pushed the archana1/vendor-crates branch 2 times, most recently from cf07fbc to cc7ce2f Compare December 10, 2024 15:58
src/agent/.cargo/config Outdated Show resolved Hide resolved
.gitignore Show resolved Hide resolved
@arc9693 arc9693 force-pushed the archana1/vendor-crates branch from cc7ce2f to 8e2cea3 Compare December 11, 2024 11:20
.gitignore Show resolved Hide resolved
@Redent0r Redent0r requested a review from sprt December 16, 2024 22:57
@Redent0r
Copy link

Thoughts on splitting this into separate PRs per target folder? it might still be a lot of file changes but we may be able to load them through github UI (up to 3000 file changes)
image

Target rust folders: src/tardev-snapshotter, src/agent, src/utarfs, src/overlay

The change involves vendoring Cargo dependencies to enhance build
reproducibility and security. All dependencies are downloaded and
stored locally in a vendor directory, eliminating the need to fetch
them from external sources during builds. The projeci's Cargo
configuration is updated to prioritize these vendored sources,
ensuring consistent, self-contained builds regardless of external
factors like network availability or changes in dependency repositories.
By committing the vendor directory to version control, the project
gains improved security, faster build times, and determinism.
Developers must refresh the vendored dependencies whenever updates
are made to the Cargo.toml, by re-running cargo vendor.

Signed-off-by: Archana Choudhary <[email protected]>
@arc9693 arc9693 force-pushed the archana1/vendor-crates branch from 8e2cea3 to 7fa67f9 Compare December 19, 2024 14:25
@arc9693
Copy link
Author

arc9693 commented Dec 19, 2024

image
Updated config -> config.toml.
cc: @sprt

Copy link
Collaborator

@sprt sprt left a comment

Choose a reason for hiding this comment

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

Reviewed this by checking it out locally and LGTM.

But let's hold off on merging for now as @danmihai1 suggested an alternative approach that we might want to evaluate: don't vendor everything from the get go, but instead, for each package that needs to be patched, incrementally add it to the vendors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream/not-needed PRs that will not be upstreamed (e.g. internal)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants