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

Presence of binaries or examples causes "dev-dependencies" metadata to be required for cargo package #7487

Closed
kentfredric opened this issue Oct 7, 2019 · 12 comments
Labels
A-lockfile Area: Cargo.lock issues C-bug Category: bug Command-package

Comments

@kentfredric
Copy link

I've been trying to track down an annoying issue in vendoring, where we're using the crate-dir layout as described here

As part of the build process, we invoke cargo package at some point to give us an image that we then roll out to the crate-dir to the vendor layout prefix.

An annoying recurring problem is dependencies listed as "dev-dependencies" turn out to be necessary to be pre-provided somewhere( for metadata purposes, due to the absence of the crates.io registry index/cache thing ), and this is leading to fun problems with them becoming cyclical dependencies that thwart the build system.

Testing has shown that the presence of the "examples/" directory with literally any ".rs" file is sufficient for this to happen.

One could possibly argue that examples and tests are similar in both requiring dev-dependencies in some regard, but oddly, only examples are tripping up the "dev-dependencies required" issue.

You can soft-demonstrate this issue in some regards by checking out a copy of a crate (itertools is my example), and making sure there is no Cargo.lock file, and running:

   cargo package --no-verify --no-metadata

With the examples/ directory present, every invocation of that command will trip it up with emitting:

    Updating crates.io index

At the end of the task.

Removing the examples directory makes this problem go away.

@kentfredric kentfredric added the C-bug Category: bug label Oct 7, 2019
@kentfredric kentfredric changed the title Presence of "examples" directory causes "dev-dependencies" metadata to be required for cargo package Presence of "examples/*.rs" causes "dev-dependencies" metadata to be required for cargo package Oct 7, 2019
@ehuss
Copy link
Contributor

ehuss commented Oct 7, 2019

Sorry, I'm having trouble understanding the use case here. cargo vendor will indeed need to vendor dev-dependencies if they are listed in the manifest. I'm not sure I follow the process with cargo package here, since vendoring needs a source distribution, not a .crate file. I'm also not sure what the significance of cargo package and updating the index is.

It might be helpful to explain what you're trying to do and what issues/errors you are having. Using cargo package is a pretty rare thing since its use case is somewhat niche.

@kentfredric
Copy link
Author

To clarify, its not using "cargo vendor", all dependencies are forcibly communicated to vendor (as in, linux-vendor) dependencies, and must be installed prior to any steps.

Our build chain starts with official copies that are pre-downloaded from crates.io, and unrolls them to a source tree to allow patching (which includes user controlled patching), and in depending on conditions, run tests (its a source-based distribution, and so end users have control to patch things, not all end users run tests, but some elect to do so).

Then at some point after patching and (optionally) testing, cargo package is invoked to re-roll into a viable crate to deploy to the filesystem. ( @gyakovlev can probably help expand on why )

The desire is that end users who aren't electing to run tests, aren't pulling in dev-dependencies forcibly, (our package manager contols this), and only those who do run tests are forced to satisfy dev-dependencies first.

Currently a usable workaround is just rm -rf examples/, which returns dependencies into expected behaviour.

Though I'd prefer to leave examples in the crate just in case somebody wants them for something later.

@kentfredric
Copy link
Author

( NB: We already patch-out target-specific dependencies just to keep the graph small, because target-specific dependencies metadata are still required to satisfy various cargo stages, even if they're not ultimately used. And the absence of the crates.io index/cache is what otherwise causes these crates to be needed just to satisfy metadata )

@kentfredric
Copy link
Author

The extra confusion is why it is that examples and seemingly, only examples that makes this happen.

It feels like there's some sort of intentional logic behind this, but what that is is beyond me.

@kentfredric
Copy link
Author

Further probing shows its not just dev-dependencies that the presence of examples/ causes to be tripped into requirements for cargo package, it seems to trip all dependencies to be required, including all optional deps. I just hadn't noticed this earlier because I was already pre-provisioning the optional deps.

Its really bizzare that you can have dependencies missing, and cargo package works just fine, ... until you do:

 mkdir examples/
 touch examples/foo.rs

And without even changing Cargo.toml, you suddenly get a much more breaky cargo.

@kentfredric
Copy link
Author

Found another path that trips this behaviour as well, and it helps somewhat diagnose this behaviour:

  • src/main.rs

Possibly for the same underlying rationale.

I'm speculating here, but it might be some aspiration about what "cargo run" requires, and/or something about the requirements of cargo install.

@kentfredric
Copy link
Author

I get the impression that the code-path that activates this requirement should be gated out by --no-verify or something.

Its fine and all to have checks for this sort of thing, just there should also be a way to turn them off.

@ehuss ehuss changed the title Presence of "examples/*.rs" causes "dev-dependencies" metadata to be required for cargo package Presence of binaries or examples causes "dev-dependencies" metadata to be required for cargo package Nov 8, 2019
@ehuss
Copy link
Contributor

ehuss commented Nov 8, 2019

I'm still having a hard time understanding the exact issue here. Is it that you are using source replacement, and don't have the crates.io index? It would help to have an MCVE and the resulting error message. When binaries are in a package, it rebuilds Cargo.lock which requires metadata for all of the dependencies in the manifest.

@kentfredric
Copy link
Author

I'm still having a hard time understanding the exact issue here. Is it that you are using source replacement, and don't have the crates.io index? It would help to have an MCVE and the resulting error message. When binaries are in a package, it rebuilds Cargo.lock which requires metadata for all of the dependencies in the manifest.

That does sound like a good redux of the problem.

You can synthetically present the circumstances that trigger it using an intentionally invalid dependency.

Invalid dependencies declared in dev.dependencies aren't inherently a problem for running cargo package --no-verify --no-metadata, it just chugs along.

But as soon as you have any kind of binaries, a Cargo.lock generation becomes mandatory, which in turn requires metadata for all dependencies listed in the manifest.

And its this mandatory step that makes matters annoying.

@kentfredric
Copy link
Author

Also worth mentioning, that for us at least, Cargo.lock serves no purpose. Freezing of dependencies is something that has to be handled in our vendor package manager.

@iliana
Copy link
Contributor

iliana commented Nov 19, 2019

We ran into this issue over at rusoto/rusoto#1603 — we'd like to add cargo package --no-verify on all our packages to CI to avoid problems in the middle of releasing a couple hundred crates. Two crates with examples fail.

@epage
Copy link
Contributor

epage commented Nov 2, 2023

Dev-dependencies are only verified if you tell cargo that you can pull them from the registry by including the version field. If you drop the version field, the dependency will be ignored for generating the lockfile and won't be included in the packaged Cargo.toml.

As I believe this resolves the need in this issue, I'm closing it. If there is a reason for us to reconsider, let us know!

@epage epage closed this as not planned Won't fix, can't repro, duplicate, stale Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lockfile Area: Cargo.lock issues C-bug Category: bug Command-package
Projects
None yet
Development

No branches or pull requests

4 participants