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.

Sign up for GitHub

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

Jump to bottom

Provide a flag for package to avoid regenerating/generating Cargo.lock, thus decoupling packaging from the registry #15159

Closed
NoisyCoil opened this issue Feb 8, 2025 · 7 comments · Fixed by #15234
Closed

Provide a flag for package to avoid regenerating/generating Cargo.lock, thus decoupling packaging from the registry #15159

NoisyCoil opened this issue Feb 8, 2025 · 7 comments · Fixed by #15234
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-package S-triage Status: This issue is waiting on initial triage.

Comments

@NoisyCoil
Copy link
Contributor

NoisyCoil commented Feb 8, 2025 •
edited
Loading

Problem

I would like to work on adding a new flag to cargo package to avoid regenerating Cargo.lock when packaging, or to avoid generating it at all in case there is none.

The main use-case I have in mind for this flag is the packaging work we do in Debian. In Debian we adopt two main approaches when it comes to rust packaging. The first is packaging single crates, with source code obtained from crates.io. The second is packaging from upstream sources (usually git repos), which is particularly useful for multi-crate projects, cross-language projects, or projects that simply don't publish to crates.io. Now, the latter may contain crates with rust libraries, and these rust libraries must be packaged as librust-$CRATENAME-dev Debian packages in order for them to be used by other packages as build dependencies. When it comes to this, cargo package (used either as a command line tool or via the cargo library) is extremely useful in that it creates a canonicalized version of the crate, with the desired included/excluded files, which can then be simply copied to the desired directory and packaged as a .deb. Before this is done (regardless of whether one uses cargo package or not), Cargo.lock is removed from the directory because there is no use for it in Debian (verification is done at the level of the Debian archive -- that is, by verifying .debs themselves -- and versioning is locked to whatever we have in the archive at a certain point in time).

Unfortunately, the way cargo package currently works -- especially after 1.84 which started regenerating/generating lock files for all crates -- couples the packaging process to the registry in a way that makes it quite hard for us to use it in practice the way we would like. Specifically, if Cargo.lock needs to be regenerated/generated, then all the crates' dependencies must be in the registry in order to use cargo package. This causes some notable issues:

  1. it forces us to deal with ordering. Namely, we need to install packages in the local registry in the correct order, so that they can be found by packages that depend on them. At this time I've only seen evidence of ordering issues between crates, but my guess is this will extend to ordering issues between crate features, which will be much harder to solve

  2. due to the above, it forces us to deal with circular dependencies. An example of this was already reported for cargo publish in publishing a crate that depends on itself as a dev dependency #15151, pointing to the same underlying issue I'm reporting here (but possibly needing a different solution). I have a strong feeling that the issue of circular dependencies will not be solved easily

  3. in general, it prevents us from packaging crates unless all dependencies are installed in the local registry (this was also reported in cargo package --no-verify fails if a package's version req is too high for the registry (but works locally) #15059). This is not a logic necessity since, were it not for Cargo.lock, canonicalization and file inclusion/exclusion could proceed with no input from the registry. Packaging crates without dependencies being present locally saves us from downloading and installing dependencies unless they are really needed

  4. since not only dependencies, but also dev-dependencies must be installed in the local registry, test/example/benchmark dependencies turn into actual build-dependencies. This in itself is a separate issue than circular dependencies, but can ultimately cause (cross-workspace) circular build-dependencies (see e.g. the scenario in my comment below).

Proposed Solution

Since as I said we don't care about Cargo.lock in Debian (again, we actually remove it from the final package), all of this would be solved for us if one could just skip lock file regeneration/generation. I've already tested that, when used together with --no-verify, a new --no-gen-lockfile flag that implements such skipping would decouple cargo package from the registry: in practice one can purge the registry from all crates and use cargo package with no dependencies present locally at all. This way the issue of dependency ordering/cyles obviously never arises.

A proof of concept is at https://github.com/NoisyCoil/cargo/tree/no-lockfile, but I would like to hear your thoughts before submitting an actual PR.

Notes

I am not sure this proposal is suitable for solving #15151, since publish should probably have different requirements than package. Indeed, in my proof of concept, the no-gen-lockfile flag is turned off for the (internal) use that publish makes of package. On the other hand, it may help (or even completely solve) #15059.
@NoisyCoil NoisyCoil added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage. labels Feb 8, 2025
@cpg314 cpg314 mentioned this issue Feb 13, 2025
Closed
@epage
Copy link
Contributor

epage commented Feb 18, 2025

Before 1.84, the lockfile was already being generated if it had a bin or an example. How did you workaround that problem?

Are you dealing with packaging-order with packages within a workspace or does this cross workspaces? If the latter, could you explain more about that process?

If its for workspaces, could you test and report back on whether -Zpackage-workspace resolves the problem? That allows packaging more than one package at a time in a workspace in the correct order.

@NoisyCoil
Copy link
Contributor Author

Before 1.84, the lockfile was already being generated if it had a bin or an example. How did you workaround that problem?

I didn't personally, most of the work I do for Debian is within the first scenario (packaging single crates from crates.io), plus a couple of cdylibs, and cargo package is not needed for either of these use-cases. To my knowledge, the only user of cargo package in Debian at this time is dh-rust, which was written and is maintained by a developer outside of Rust Team. From what I understand he was aware of issues caused by the lockfile before 1.84 and he implemented a simple dependency resolution algorithm which (again to my understanding) broke after 1.84 because more packages became affected and started to FTBFS. The last time I checked (that is, shortly after opening this issue) the current workaround was an improved dependency resolution algorithm which however still breaks with circular dependencies because of cargo package's behavior, causing some packages to still FTBFS. Moreover, the workaround doesn't deal with the fact that dependencies must be present to use cargo package (AFAIK, the maintainer dropped support for building library packages without installing (useless, in this context) build-dependencies in response to this, in tens of packages). I don't know if the situation has improved since.

Within Rust Team, we are not currently affected by lockfile issues. However, we plan to support packaging workspaces using debcargo and dh-cargo (the first of which builds the source package, while the second builds the actual .deb), and cargo package is probably the best option for this use-case, in a very similar fashion in which dh-rust is using it at this time. I see us going through the very same problems, and them being fully avoidable since we don't really care about the lockfile. The present issue is mainly for future applications in Rust Team (but if it results in making life easier for dh-rust and its maintainer I would be very happy that it did).

Are you dealing with packaging-order with packages within a workspace or does this cross workspaces? If the latter, could you explain more about that process?

AFAIK dh-rust is having packaging-order issues only within workspaces, and I am anticipating the same issues in workspaces too, so yes to the first question. As for cross-workspace packaging-order issues, no, I don't expect any1, but that of dependencies needing to be installed just to canonicalize a crate still remains, and we want to avoid it.

If its for workspaces, could you test and report back on whether -Zpackage-workspace resolves the problem? That allows packaging more than one package at a time in a workspace in the correct order.

For the sake of giving you an answer based on a pre-existing implementation I hacked dh-rust 0.0.10 to make it use current rustup's cargo nightly, and it did not work. However, I think the implementation matters here (dh-rust wasn't written to use -Zpackage-workspace, it calls cargo package --package on each crate in turn, so I guess that flag not working is fully expected). For future implementations, I can see how -Zpackage-workspace may solve the ordering issues (e.g. by using the --workspace flag too and packaging all the workspace at once?), but, again, if I understood correctly what it does, it will not solve the issue of avoiding useless dependencies (namely, the rest of the registry besides the workspace). I guess the bottom line is cargo package should have a way to avoid depending on the registry for use-cases that do not require it -- like those which don't really care about Cargo.lock, see Debian -- and that's not the problem -Zpackage-workspace tries to solve.

More generally, what I think should work and does not is: empty registry, cargo package --offline --no-verify, either within or outside of a workspace. If one actually needs a well-formed and up-to-date Cargo.lock then this request is meaningless, for obvious reasons. But if one doesn't need it then I think this is a sensible request, and one which may be expressed by an additional flag.

Footnotes

  1. In official Debian packaging at least. Outside of Debian, I was told by a Rust Team members he's using cargo package to build debian packages for their day job, and he's forced to have all crates installed in the local registry just to build the source package. ↩

@NoisyCoil
Copy link
Contributor Author

Actually, I think I do see a fairly possible scenario where cross-workspace packaging-order issues may arise in Debian, still having to do with circular dependencies. Library crate a in workspace A depends on library crate b in workspace B, library crate b in workspace B dev-depends (e.g. for tests) on library crate a in workspace A, neither A nor B have ever been packaged in Debian, we want to use cargo package in internal tooling (that is, at package build time, like dh-rust does) to package both A and B.

This is currently hard to do because of the change in 1.84: building the .deb for a from source A requires b's .deb (which is fine, b is a real build-dependency of A because a depends on b), but building the .deb for b from source B requires a's .deb because installing a's .deb is the only way to have a in the registry used by cargo package to create b's Cargo.lock (this Cargo.lock will never end up in the Debian package for b in the first place as it is deleted immediately). Note that B needing a to build means that a mere test dependency (we have those in Debian too) has now become an actual build-dependency: generating the useless lockfile caused an actual circular build-dependency.

What we would do in this case, I guess, is patch out the dev-dependency of b on a by any means necessary (e.g. by removing tests) and upload the B and then the A source. Then, if we wanted to re-enable b's tests, assuming we are ok with introducing a circular build-dependency in the archive, we would wait a few hours for A and B to build, remove the patch, make B build-depend on a and re-upload it. Will this work? Probably. Does this make me wish we could decouple cargo package from the registry, given that a useless lockfile cost us a circular build-dependency? Yes.

/cc @Fabian-Gruenbichler to see if I overlooked something in the scenario above

@Fabian-Gruenbichler
Copy link

In official Debian packaging at least. Outside of Debian, I was told by a Rust Team members he's using cargo package to build debian packages for their day job, and he's forced to have all crates installed in the local registry just to build the source package.

that would be me, and indeed, this is quite annoying for us at Proxmox. we've written quite a bit of software in Rust over the past few years which is shipped as Debian packages (we are a Debian derivative). most of our crates are not (yet) published on crates.io for various reasons, and debcargo calls cargo's package action internally when (Debian-)packaging crates from a local tree. we use a local fake registry (consisting of packaged crates) to replace crates.io via .cargo/config.toml.

this means if we have a crate A depending on B and dev-depending on C, we need to have Debian packages for B and C and their transitive dependencies installed, just to run debcargo package (which in turn basically does cargo package) to generate a source package which then gets passed to the build environment to build the binary package(s). this is very cumbersome, as building the package doesn't require the dev-dependencies at all (they are only required for the seaprate, post-build testing step that happens in a different environment), but the first step in the build pipeline (generating the source package), which doesn't actually need any crates installed at all, requires all of them to be installed instead.

I know this probably all sounds a bit involved/cumbersome/weird from upstream's PoV - I can go into more detail regarding the rationale for why things work as they do there, but to make the rather long story short - there definitely is a real world use case for running cargo package (both the command, as well as the corresponding cargo library interface) without touching/requiring Cargo.lock, and we'd appreciate it if it became possible (an unstable flag is totally fine, and crates.io rejecting crates without one would also be fine in my book!).

@Fabian-Gruenbichler
Copy link

just to add: for us -Zpackage-workspace doesn't help at all, as only some of the (superfluously required) crates are part of the same workspace, the issue affects every dependency.

@Fabian-Gruenbichler
Copy link

/cc @Fabian-Gruenbichler to see if I overlooked something in the scenario above

no, that example seems correct - this is the equivalent of cargo package with a reference to an unpublished crate (version), just that "publish" in this case means "create and install a Debian package" as that is what creates the entry in the local (fake) registry replacement for crates.io. I guess a hacky workaround would be to somehow generate fake entries for dev-dependencies if they are missing (which would of course mean bogus entries in the resulting Cargo.lock file, but since that gets thrown away anyway..).

the -Zavoid-dev-deps trick we use to run cargo build/install/clean in the absence of dev-dependencies doesn't work here either.

@NoisyCoil
Copy link
Contributor Author

no, that example seems correct

Then I'll add turning test dependencies into build-dependencies as a 4th issue in the problem statement above.

@weihanglo weihanglo mentioned this issue Feb 25, 2025
Merged
github-merge-queue bot pushed a commit that referenced this issue Mar 14, 2025
@epage
feat(package): add --exclude-lockfile flag (#15234)
6cf8267 
### What does this PR try to resolve?

Fixes #15059
Fixes #15159

This provides an escape hatch `--exclude-lockfile`for uncommon workflows
that don't verify (`--no-verify` is passed) the build with their
unpublished packages
In effect, this takes the heuristic removed in #14815 and replaces it
with a flag

When `--exclude-lockfile` is enabled,
`cargo package` will not verify the lock file if present,
nor will it generate a new one if absent.
Cargo.lock will not be included in the resulting tarball.

Together with `--no-verify`,
this flag decouples packaging from checking the registry index.
While this is useful for some non-normal workflows that requires
to assemble packages having unpublished dependencies.
It is recommended to use `-Zpackage-workspace` to package the entire
workspace, instead of opting out lockfile.

### How should we test and review this PR?

The first commit was stolen from
<NoisyCoil@1a104b5>
(credit to @NoisyCoil!)

The second added two failing cases we observed in #15059.

### Additional information
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-package S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(package): add --exclude-lockfile flag weihanglo/cargo
4 participants
@epage @Fabian-Gruenbichler @weihanglo @NoisyCoil