Published crate fails to compile tests due to absence of jemalloc-ctl about jemallocator HOT 29 CLOSED

The dependency change for a jemallocator test is the following:

jemallocator->jemalloc-ctl->jemallocator-tests

However, cargo has a bug in which it considers dev-dependencies to be direct dependencies when publishing to crates.io only, so you end up with a cyclic dependency of the form:

jemalloc-ctl->jemallocator->jemalloc-ctl 

and such dependencies cannot be published to crates.io, so we need to hack the Cargo.toml manually before every release before publishing to crates.io (which has other problems, like the source code that you can see here does not match the source code on crates.io).

Since there are many crates having this problem, and having to manually edit Cargo.toml before release to work around the many flaws and bugs in Cargo's dependency resolution, I don't think it makes much sense to try to solve this problem here.

The fix is simple: dev-dependencies are not dependencies of a crate, but dependencies of its "dev crates" (e.g. tests, examples, benches, etc.), so it should be possible for a dev-dependency to depend on the crate that uses it.

This cargo bug has been open for a long time: rust-lang/cargo#4242

Closing this as "not actionable", since there is nothing we can do to fix this cargo bug here.

from jemallocator.

Curious, I battle with circular dependencies every day with crates.io stuff in dev-dependencies, clearly these crates aren't blocked by this.

In general I just despise circular dependencies, so ain't going on a bender to encourage more of these.

But I also suspect there's a good compromise that doesn't require circular dependencies.

from jemallocator.

As mentioned, there is no circular dependency in jemallocator, but cargo has a bug that makes it think that there are. If you believe this is incorrect, and that there is a circular dependency in this crate, please, go ahead, and let us know about it.

But IIUC, what you want is for us to fix a circular dependency issue that actually does not exist. Your time is better spent fixing the cargo bug that makes cargo think that there is a circular dependency here. Complicating this crate, or other crates, to work around that cargo bug, makes the situation worse.

from jemallocator.

There is a circular dependency though. Just because it's a dev dependency doesn't make the circularity go away.

OTOH, not doing anything means vendors such as distros cannot blindly run tests, which sounds pretty bad for a thing as important as a memory allocator.

And neither of @kentfredric suggested fixes would complicate the crate, in my opinion. Au contraire, it will remove a circular dev dependency, which is definitely a simplification.

from jemallocator.

The jemallocator tests/ crates depend on jemallocator and jemalloc-ctl, and jemalloc-ctl depend on jemallocator. Where is the cycle?

from jemallocator.

Fwiw, I locally patched in the dependency in jemallocator, and everything works JustFine for me, can you show what problem you're having in a different way? So far your descriptions of the problem are just confusing me :/

from jemallocator.

• jemallocator depends on jemalloc-ctl
• jemalloc-ctl depends on jemallocator

Right? Am I missing something obvious here?

from jemallocator.

Right? Am I missing something obvious here?

Wrong: jemallocator does not depend on jemalloc-ctl. jemallocator-tests-crates in the test/ directory do, and those are different crates than the jemallocator crate (EDIT: the proof is trivial: you can't access a private root module of the jemallocator crate from within a test crate because they are different crates).

from jemallocator.

Wrong: jemallocator does not depend on jemalloc-ctl. jemallocator-tests-crates in the test/ directory do, and those are different crates than the jemallocator crate.

I literally linked you to the jemalloc-ctl dev-dependency of jemallocator.

from jemallocator.

I literally linked you to the jemalloc-ctl dev-dependency of jemallocator.

I know, you linked there, but incorrectly claimed that this means that the jemallocator crate has a dependency on jemalloc-ctl. That is not the case, and cargo devs have acknowledged that behavior that assumes that is incorrect.

from jemallocator.

My claim is definitely correct, we disagree on the consequences of that fact but I don't appreciate being told things that are false.

Where is the Cargo issue?

from jemallocator.

Where is the Cargo issue?

I linked it in my reply to this post that closed this issue, where I explained this in detail.

My claim is definitely correct,

Sadly, it isn't. A test is a different crate than a library, or a binary, or an example, or a bench, or...

from jemallocator.

So my original point stands: there is a cyclic dev-dependency, and cyclic dependencies, be them dev or not, are a huge increase in complecity and process. Removing the cycle would simplify things, especially for vendors and distros.

from jemallocator.

@nox No, your original point is still incorrect.

The jemallocator crate does not depend on jemalloc-ctl.

A Cargo.toml describes multiple crates. A test crate within the jemallocator repository depends on both jemallocator and jemallocator-ctl, and cargo incorrectly interprets this as jemallocator having a cyclic dependency, and this is an acknowledged bug by the cargo maintainers.

If you disagree that this is a bug, it's pointless to try to argue with me here, go and argue with the cargo maintainers upstream that disagree with you.

from jemallocator.

jemallocators tests require jemalloc-ctl though, this is important.

Even though you're talking about crates as the sense of "compilation unit", there's also the sense of crates as "bundled aggregate of source code".

And there is no way to stipulate dependencies for source code among those without using the top level Cargo.toml

Thus, stipulating dependencies for tests must be done in Cargo.toml, the only problem here I'm seeing is that "dev-dependencies" is over-general, when what you need is "test-dependencies".

So while terminologically correct that "test/" and "jemallocator" are "different logical crates", jemalloc-ctl is still a "test dependency" of the crate in question, as it is required to "compile and run the tests of the crate". ( The tests themselves just happen to be different compile-unit crates, but that's an implementation detail )

from jemallocator.

Even though you're talking about crates as the sense of "compilation unit", there's also the sense of crates as "bundled aggregate of source code".

That's a package, not a crate.

Fwiw, I locally patched in the dependency in jemallocator, and everything works JustFine for me, can you show what problem you're having in a different way?

What patch did you apply? Notice that our Cargo.toml has a dev-dependency, which corresponds to your second proposed solution, and that works just fine, with the exception that cargo doesn't allow such crates to be uploaded to crates.io by default, which is why the crates.io version of jemallocator, which differs from the source code in this repo, doesn't have any dev-dependencies.

from jemallocator.

To side step a lot of faff:

Can you show us ( eg: the error you get, cargo whining etc ) what happens when jemallocator's Cargo.toml includes the section:

[dev-dependencies.jemalloc-ctl]
version = "^0.3"
default-features = false

Because I don't see any issues when I do this, and this is all that's required to get things working.

from jemallocator.

The error only happens when you try cargo publish, and is described in detail in the reported issue in cargo upstream.

from jemallocator.

I would like to see this, because as it is, there are so many crates currently and recently published on crates.io with messy circular dependencies, that it conspires to be a bit of a nightmare.

from jemallocator.

I would like to see this,

Then execute cargo publish ? That's literally the only step required.

from jemallocator.

At some point I may take a stab at writing a patch that makes this less painful in some regards.

Though I'll probably take the path of putting the tests that require circular dependencies in such a location that you can exclude them automatically with an exclude = rule.

This at least would make things usable.

from jemallocator.

Would those tests then run when you try to test the jemallocator crate from crates.io?

from jemallocator.

Those tests would continue to run on the git clone, but would be non-existent ( and thus, couldn't fail to compile ) in the published crate.

from jemallocator.

But this is already the case ?

from jemallocator.

No, not at all, that's why I had to file this bug.

from jemallocator.

So I'd like to see your patch, because the master branch already has a dev-dependency that makes the tests work. If you only care about running tests on master, this already works. The only thing that doesn't work is running tests on the source code that you download from crates.io, which is different from the one in master.

from jemallocator.

Ping me when you publish next then, and if it still poses a problem I can look into it :)

If the change to Cargo.toml currently in master fixes the problem, then I don't need to :)

from jemallocator.

As mentioned a 100 times, cargo publish fails for master, because of the dev-dependency key, as explained in my first reply to this thread and the cargo issue, so it must be removed before publishing, so that crates.io accepts it. I'm unsubscribing from this thread, since I don't have time for this, and I feel my time isn't being valued and is better spent doing something else.

from jemallocator.

And again, as long as the published crate works, I won't have a problem.

That's why I'm going to look into a solution that works for published crates.

"The fix is in master" doesn't translate to "the fix is on crates.io"

:)

from jemallocator.