Repository navigation
cargo: Generate a Cargo.lock if one wasn't found #44
Description
Spec from https://gitlab.com/BuildStream/bst-plugins-experimental/-/issues/42
Will behave like this:
track: Cargo.lock will be generated by the host'scargoIF(generate_lock is true)OR(generate_lock is 'if-missing' and Cargo.lock doesn't exist). If generate_lock is false and Cargo.lock doesn't exist, raise an error. Then the existing logic for tracking cargo sources will happen. The ref will now encode everything necessary to reconstruct the vendored libraries, so the generated Cargo.lock is discardedfetch: The sources are fetched according to the ref, as they are nowstage: The sources are staged according to the ref, as they are now- Buildstream will default to
generate_lock: false, and projects can (if they wish) globally override that default togenerate_lock: if-missingto make thecargoplugin work in all scenarios. Thengenerate_lock: truecan be used if Cargo.lock needs to be overwritten/ignored for whatever reason (i.e. a hypothetical repo that hasn't been updated in a while and Cargo.lock specifies dependencies w/ known security issues, so you want to force it to use newer libraries) - If a project doesn't want to grow a dependency on
cargofrom the host totrack, it can keepgenerate_lock: falseas the default, and then maintain its own Cargo.lock files & stage them before thecargosource:
sources:
- kind: git_tag
url: whatever:foobar.git
- kind: local
path: files/cargo/foobar/Cargo.lock
- kind: cargo
generate_lock: false # <-- from project defaults
Activity
nanonyme commentedon Dec 31, 2022
Another option is to create separate plugin cargo-lock-generator. Then you have
and cargo can consume lock as normal. cargo-lock-generator only generates cargo.lock when tracking but otherwise does nothing. It shall depend on previous source both for track and fetch.
nanonyme commentedon Dec 31, 2022
@gtristan any opinions?
nanonyme commentedon Dec 31, 2022
Creating new plugin does not change API or cache keys. Adding functionality to existing plugin does not necessarily need cache key change since the new functionality kicks in only with track.
AdrianVovk commentedon Jan 1, 2023
This is rather ugly, in my experience.
Here's how it ends up working out:
Building this into the
cargoplugin is optimal because it alleviates the need for the Cargo.lock file if not trackingEdit: Actually I suppose that
Cargo.lockonly existing during/after a track can be fine if thecargoplugin is willing to handle that. I seem to recall it blowing up for whatever reason.nanonyme commentedon Jan 1, 2023
I don't see any reason why cargo plugin would need Cargo.lock after tracking. It generates things to download based on it during tracking and records them in ref.
AdrianVovk commentedon Aug 8, 2023
An issue with the separate plugin: We have to actually store the Cargo.lock file and then stage it, so that the
cargoplugin can pick it up and use it. But this means that later, during the element's build, we actually place a Cargo.lock file into the sources. On a separate build machine, we would instead build with the Cargo.lock file missing (since it isn't really necessary). However, this behavior introduces a different build environment without changing the cache keys!nanonyme commentedon Aug 8, 2023
IMHO any generated cargo lock must be the ref of said plugin.
AdrianVovk commentedon Aug 8, 2023
So you mean that the generated cargo.lock file should be base64 encoded and then stored in the ref?
That generates a massive ref. I don't mind since it lives in project.refs, but it it ends up being necessary in freedesktop-sdk you'll end up with loads of chunk in your element
nanonyme commentedon Aug 9, 2023
I don't think it's that big a price to pay for determinism to store this information in ref. Also, freedesktop-sdk is not really needing this functionality in the first place as all the projects we use have Cargo.lock. This is more aimed for various downstreams and developers.
AdrianVovk commentedon Aug 9, 2023
It's already stored in the ref, at track time, by the
cargoplugin.Cargo.lockisn't necessary during the actual build, is it? I'm actually not sure now...Simply not including a Cargo.lock file isn't less deterministic than storing it in project.refs
For now...
nanonyme commentedon Aug 9, 2023
Oh, never mind, I misread your point. Ok, I guess I'm convinced with adding it to cargo plugin itself as long as this does not add dependency by default. I don't think you even need to change add generate_lock into cache keys since it's for tracking only here so this should be doable as a mostly backwards-compat change.
nanonyme commentedon Aug 9, 2023
@AdrianVovk freedesktop-sdk has been cooperating with any upstreams not shipping Cargo.lock and convincing them to do that. We plan to keep this strategy. Not shipping Cargo.lock with releases is a basically a bug.
AdrianVovk commentedon Aug 9, 2023
Some upstreams feel strongly about not shipping Cargo.lock... So unfortunately there's no escaping the need to generate it sometimes
i.e. systemd/zram-generator#65
nanonyme commentedon Aug 9, 2023
Yeah, I guess we live in an imperfect world where people sometimes do not follow best practices. Anyway, point stands. What that project is doing is not a general thing for people to do. I am not saying that Cargo.lock should always exist, it is very common for it not to. However, when generating tags, it usually does. You can even have same commit retagged multiple times with different Cargo.lock files if you so want. Once you have tested that this combination actually builds.
nanonyme commentedon Aug 9, 2023
This is essentially the thing which made me believe you claimed Cargo.lock needs to exist during build. It is perfectly fine to expect an element has to be tracked at least once and outputs committed into git for it to be buildable. However, refs stored in git must be sufficient for instrumenting source fetching and building.
nanonyme commentedon Aug 9, 2023
In any case, if you would like to proceed with this, I would suggesting making proposal in the form of a PR. I would really love to have tests added for cargo plugin as well in the same go, it currently has none making any changes to it very dangerous.
AdrianVovk commentedon Aug 9, 2023
I see how my working could have been confusing. I meant that if you just generate the Cargo.lock file (and store it in the mirror dir) during tracking (and commit the resulting
project.refsfile), someone on the other side wouldn't be able to build since they wouldn't have a Cargo.lock file to stage. As you say, refs stored in git must be sufficient to fetch and build.👍
What do you expect such a test to look like? A project that junctions off freedesktop-sdk to get a working
cargo, then tracks, compiles, and runs some hello-world rust project?How do we find out if the cache key changed? Hard-code it into the test and have to manually update it w/ major buildstream updates?
nanonyme commentedon Aug 9, 2023
I don't think I still get it. Do you or do you not need a Cargo.lock in build sandbox to build the project?
gtristan commentedon Aug 10, 2023
This is indeed the question.
Also the question is whether some projects require the
Cargo.lockat build time while others do not, and whether upstream will potentially change one day such as to start requiring theCargo.lockat build time.The uncertainty around these details leads me to intuit that it would be safer and simpler to just have a separate
cargo-generate-lockplugin which serializes theCargo.lockdata into itsref.As a side note, we would probably want such a lock generating plugin to only call
utils.get_host_tool()directly inSource.track()rather than inPlugin.preflight(), this will fail a bit later than usual when lacking hostcargobut will not require hostcargoto build unless you runbst source track.AdrianVovk commentedon Aug 18, 2023
Implemented here: https://gitlab.com/BuildStream/bst-plugins-experimental/-/merge_requests/239