cargo icon indicating copy to clipboard operation
cargo copied to clipboard

Allow crates to be published with cyclic dev-dependencies

Open Marwes opened this issue 8 years ago • 51 comments

In gluon I have the project split up into multiple crates with the intent that users can include only the crates they need if they desire a smaller binary footprint. For convenience I provide a main gluon crate which provides an easy to use interface when one uses all the crates.

While this split works fine for mostly everything I want to use the easy interface when writing documentation tests so I added the gluon crate as a dev-dependency in gluon_vm and this compiles without issue since a little bit back. It does however break when trying to publish gluon_vm as the gluon dev-dependency has not been published yet!

Currently I managed to work around it by specifying a version range but it would be nice if there was a way to publish a group of crates as a whole or to ignore the version check if a dev-dependency does not exist on crates.io (yet).

gluon = { path = "..", version = "<0.6.0, >=0.4.2" } # GLUON

Marwes avatar Jul 01 '17 16:07 Marwes

Yeah in general I think we should basically ignore dev-dependencies when we publish. We shouldn't attempt to resolve them, verify they're already published, etc.

alexcrichton avatar Jul 03 '17 22:07 alexcrichton

Maybe dev-dependencies don't have to be shown in the crate's page too.

BusyJay avatar Jul 25 '17 11:07 BusyJay

I bumped into this issue as well. From what I can tell the workaround above gets us by this for now. 👍

matthewkmayer avatar Jan 23 '18 06:01 matthewkmayer

+1 to this-- it now affects the futures crate.

cramertj avatar Mar 05 '18 22:03 cramertj

This affects a lot of crates, what do we need to do to fix this? Isn't there a check somewhere that checks for the dependencies before publishing, where we can just filter dev-dependencies away and call it a day? [0]

I mean, this would even affect core and std, where std could depend on core to build, but core would depend on std for tests. The check is spurious anyways, because this sort of dependencies are not circular: build core -> build std -> build core tests is ok. Tests and "dev-only" artifacts are their own crates (core+tests is a different crate than core, and so are every test/foo.rs in core, and core/examples, etc.).

I'll admit that the core/std analogy is imperfect because these crates are too magical, but when something wouldn't be correct for the std library, chances are it won't be correct for users either.


FWIW I just tried to prepare coresimd/stdsimd for a quick crates.io release, and ran into this, where coresimd builds standalone and stdsimd depends on coresimd, but coresimd depends on stdsimd for testing.

The only solution I came up with is the same that gluon uses which is insane: publish coresimd prunning dev-dependencies, publish stdsimd, re-publish coresimd with a minor version update to use the newly publish stdsimd as dev-dependencies.

Which is a lot of effort, for.. what exactly? Is it possible for users to execute cargo test in their whole dependency graph?


[0] cc @matklad, are you willing to mentor this?

It might be enough to just skip dev dependencies here: https://github.com/rust-lang/cargo/blob/af3f1cd29bc872b932a13083e531255aab233a7e/src/cargo/ops/registry.rs#L110

and also in packaging: https://github.com/rust-lang/cargo/blob/0e7a46e3276f56822b6ef48e341c522be27db17e/src/cargo/ops/cargo_package.rs

gnzlbg avatar Aug 15 '18 13:08 gnzlbg

The only solution I came up with is the same that gluon uses which is insane: publish coresimd prunning dev-dependencies, publish stdsimd, re-publish coresimd with a minor version update to use the newly publish stdsimd as dev-dependencies.

Hm, I think a simpler work-around is possible? Just comment-out dev-dependencies when publishing. This is what we do in Cargo, which has a path = dev-dep.

For mentoring instructions, I believe you've basically nailed it :) The only thing I have to add is that the test should go to this file, and it probably makes sense to start with a test and work through all errors.

For the tests, I think we should cover at least two scenarios:

  • publishing a path dev-dep should not fail (slight modification of this test)
  • publishing with a "cycle" via a dev-dep should not fail (the original motivation for the issue).

matklad avatar Aug 15 '18 13:08 matklad

Hm I'm actually not sure that the fix would go in that location nor that we could easily write a test for this. AFAIK the check for "does this dependency exist" is done on crates.io, not locally in Cargo. In that sense we'd need to test with crates.io, not with just local Cargo test suite business.

I also think the "fix" for this is to basically stop informing crates.io about dev-dependencies. Although we did that initially I'm not really sure why we need to do that, as they're never used and are otherwise just bloating the index. The fix for that would go around here by filtering out dev-dependencies. Cargo would then also need to be updated to not verify that dev-dependencies have versions (allowing path dependencies without a version).

This would be a relatively large change though (and somewhat of a policy shift) so it would likely need some more scrutiny and discussion before happening.

alexcrichton avatar Aug 15 '18 16:08 alexcrichton

I also think the "fix" for this is to basically stop informing crates.io about dev-dependencies.

Hm, probably a crazy idea, but, given that we already modify Cargo.toml on publishing to rewrite path dependencies (source), could we, at the same step, just drop dev-deps altogether as well?

matklad avatar Sep 08 '18 14:09 matklad

Yes I think such a fix would work, but it alone wouldn't be 100% sufficient because crates.io doesn't actually parse the manifest for dependency information (it only uses what was sent in JSON)

alexcrichton avatar Sep 10 '18 17:09 alexcrichton

I guess the proposed change would also now allow git dependencies in dev-dependencies?

jonhoo avatar Nov 23 '18 18:11 jonhoo

I just bumped into this issue as well. Is there a workaround?

hadronized avatar Aug 23 '19 12:08 hadronized

You can manually strip dev-dependencies from your Cargo.toml before publishing.

Nemo157 avatar Aug 23 '19 12:08 Nemo157

Fair enough, thank you!

hadronized avatar Aug 23 '19 12:08 hadronized

I’ll just do that for the first crate, then push the second, then make a patch version to the first one to restore everything.

hadronized avatar Aug 23 '19 12:08 hadronized

For some reason, cargo requires you to commit all your changes before publishing, but since crates.io doesn't require the code that's published to match the code in the linked repository, what I do is branch the crate, remove dev-dependencies everywhere, publish everything, and then delete the branch.

gnzlbg avatar Aug 23 '19 12:08 gnzlbg

You don’t need that: cargo publish --allow-dirty.

hadronized avatar Aug 23 '19 12:08 hadronized

I just blogged about what I did here. Thanks again for your help!

hadronized avatar Aug 23 '19 12:08 hadronized

Bump. The manual comment out dev deps seems like a real kludge. If that's the solution, cargo publish should just do it.

zippy avatar Oct 15 '19 17:10 zippy

trying to automate publishing to crates.io as part of CI/ops workflows

what's a good tool to safely comment a few dozen interdependent dev-dependencies across repos and then reinstate them from circleci after the cargo publish completes?

thedavidmeister avatar Oct 24 '19 08:10 thedavidmeister

what's a good tool to safely comment a few dozen interdependent dev-dependencies across repos and then reinstate them from circleci after the cargo publish completes?

FWIW I use a shell script to just rename the Cargo2.toml into Cargo.toml and publish with allow-dirty. I suppose a more robust solution could use the toml crate to parse the Cargo.toml file, remove all dev-dependencies keys, and write a different Cargo.toml file without them that gets used instead. Shouldn't be too hard, but unless you have a lot of files that you need to alter probably not worth the effort.

When I change my Cargo.toml, I just copy it over to Cargo2.toml, and cargo publish will complain of all the dev-dependencies anyways so I can't really forget to remove them there.

gnzlbg avatar Oct 24 '19 08:10 gnzlbg

futures/ci/remove-dev-dependencies uses toml_edit to remove dev-dependencies from a workspace in-place. It's used during CI for testing rather than publishing though, I'm not sure if you'd want to change anything about it for publish, but it's a pretty trivial tool.

Nemo157 avatar Oct 24 '19 08:10 Nemo157

Hm, probably a crazy idea, but, given that we already modify Cargo.toml on publishing to rewrite path dependencies (source), could we, at the same step, just drop dev-deps altogether as well?

How would you run tests that rely on dev-dependencies if cargo just trims all of them on publish?

nox avatar Nov 28 '19 11:11 nox

How would you run tests that rely on dev-dependencies if cargo just trims all of them on publish?

That won't work, because the dev-dependencies required won't be available under that solution. We need a better solution.

gnzlbg avatar Nov 28 '19 11:11 gnzlbg

i thought dev dependencies aren't used by anything published, and tests aren't run against what is published?

thedavidmeister avatar Dec 04 '19 16:12 thedavidmeister

Some users want the tests of the crates in crates.io to work, and that is kind of reasonable, e.g., if you are building a crate with many dependencies on a not widely tested platform, you might want to run the tests of all your dependencies to discover if there is a bug in any of those crates on that target.

Another application is crater, which downloads crates from crates.io and is able to run its tests.

The hack of removing the dev-dependencies to make cargo publish work means that if you download a crate from crates.io and run its tests, they won't compile because dependencies are missing.

gnzlbg avatar Dec 04 '19 17:12 gnzlbg

One major user of running tests on published packages is Crater, it's actually in the developers best interests to make sure their tests run from the package so that their code is tested when potentially-breaking changes are proposed in rustc. (Although if it's in a public git repo it may be tested by Crater from there as well)

Nemo157 avatar Dec 05 '19 09:12 Nemo157

i don't know what the solution is then, because its very common to need "circular dependencies" when doing integration testing across related crates because you want to spin up several fixtures etc. for context in the test

thedavidmeister avatar Dec 05 '19 10:12 thedavidmeister

i don't know what the solution is then

AFAICT, the first solution proposed in this thread satisfies all constraints without downsides.

gnzlbg avatar Dec 05 '19 22:12 gnzlbg

An alternative approach that might also solve the problem would be to allow crates.io to stage releases in a group, allowing the capacity to upload, without necessitating publishing, and then, when sufficient uploads are in place to solve dependency resolution, flip a switch to have them all become "published" simultaneously.

That's the real problem here right?

That circular dependencies require an atomic release of the relevant assets.

kentfredric avatar Dec 06 '19 07:12 kentfredric

Dev dependencies are kind of a weird concept. There isn't a single set of dependencies used for all development tasks. It depends on the environment, e.g. CI won't be running build in watch mode.

Kinrany avatar May 01 '20 02:05 Kinrany