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

Cargo fails to run when an *optional* dependency is missing #4544

Open
marcbowes opened this issue Sep 28, 2017 · 23 comments
Open

Cargo fails to run when an *optional* dependency is missing #4544

marcbowes opened this issue Sep 28, 2017 · 23 comments
Labels
A-optional-dependencies Area: dependencies with optional=true C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. Z-build-plan Nightly: --build-plan feature

Comments

@marcbowes
Copy link
Contributor

#!/usr/bin/env bash

set -e

cd /tmp
[ -d cargo-optional-deps ] && rm -rf cargo-optional-deps
cargo new cargo-optional-deps
cd cargo-optional-deps

cargo build
cat >> Cargo.toml <<EOF
optional_dep_does_not_exist = { version = "1.0", optional = true }
EOF
cargo build

Fails with

error: no matching package named optional_dep_does_not_exist found (required by cargo-optional-deps)

I did not expect this because the dependency is not required as per http://doc.crates.io/manifest.html#the-features-section:

optional dependencies, which enhance a package, but are not required; and

I'm running into this behavior while integrating Cargo into a private build system that wants control over dependencies. As such, I need to elide optional dependencies by default in this system's dependency list.

I'm opening this issue to get some input on:

  1. Would ignoring missing optional dependencies for inactive features be a problem for Cargo?
  2. I tried to look through existing issues - has something like this been discussed before?
  3. An idea about resourcing/priority of the feature and how it fits into the roadmap for Cargo's integration with existing systems.
@alexcrichton
Copy link
Member

Thanks for the report! This is currently expected behavior, however, as Cargo will asset that all dependencies exist (optional, platform-specific, or not) to generate a lock file.

Can you describe your use case a bit for depending on optional dependencies that don't exist? There may be another way to tackle the same problem!

@marcbowes
Copy link
Contributor Author

marcbowes commented Sep 28, 2017

The build system at my company is dependency aware. It uses this knowledge to trigger rebuilds. For example, if A depends on B and then B is built, so too will A. In this manner, a commit to a package will land up running a build (e.g. run tests, static analysis) for everything that is using it. The dependencies are listed in a package config file and are broken down by "build", "normal", "test" and "runtime" dependencies.

The "third party" import process for Rust crates involves a step where we use the crates.io API to map the dependencies onto internal packages. Thus, the dependencies in Cargo.toml land up being in the package config (normal) dependencies section while dev-dependencies land up in build/test dependencies. When we build a package, our build system processes the dependencies and builds a Cargo registry (the "soon to be deprecated" variant) that has precisely the crates we need.

The impedance mismatch is that our build system doesn't have a concept of optional dependencies. Thus we're stuck between two extremes: "all optional dependencies go in the package config" and "no optional dependencies go in the package config". After an internal discussion, we decided the latter was much better. The story in this world is that if you (package A) want to consume a crate (package B) and turn on an optional dependency (package C), then you need to depend on A + C and enable the feature in your Cargo.toml.

If we went for the "put all optional dependencies in the package config" we'd have a bunch of issues such as triggering unnecessary rebuilds, potentially running into licensing issues and so forth. Less is more.

Just to add some more color:

Our build system is pervasive, old and opinionated. When I read rust-lang/rfcs#2136 I assumed we'd need to turn every knob and dial you provided. However, despite being old and opinionated, the integration we have now is actually pretty neat. We have an import process that worries about turning Cargo.toml dependencies into our dependency manifest format and we have a custom registry build process. That's it. Cargo does everything else and that means that our developers get to use everything they find on stackoverflow without any weirdness.

Also note that there is no way we're going to use crates.io. Not only for security/policy reasons, but because we have internal build system concepts that allow developers to "have their own universe" - this makes any "central" or "single" source nonsensical. Each developer can have their own crates.io for their own universe. Another interesting fact is that our build system handles all types of dependencies, so we don't have any issues with native code, e.g. ensuring that the right version of openssl is present.

@marcbowes
Copy link
Contributor Author

It's likely that "platform specific" will also cause similar issues, but I haven't thought through that too deeply.

@carols10cents carols10cents added Z-build-plan Nightly: --build-plan feature A-optional-dependencies Area: dependencies with optional=true C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` labels Oct 3, 2017
@alexcrichton
Copy link
Member

Ok thanks for the explanation @marcbowes! I wonder if there may be a good balance to strike here perhaps.

So the reason that Cargo needs optional dependencies is that when generating a lock file for a crate it'll ensure that the lock file doesn't change depending on what you do with the crate (test, build with features, etc). This means that even though some compiles don't use all the dependencies it instead requires all deps to be present at all times.

This sounds like a case, though, where there's definitely an impedance mismatch between your internal build system and "idiomatic Cargo". In that sense it may be best to follow the "build system integration" threads and features to see how we can better accommodate your build system. When you say "there is no way we're going to use crates.io" my guess is you don't mean that you won't use crates from crates.io but rather you won't be directly downloading from crates.io (which is totally ok!). We basically want to make sure that we're empowering and enabling anyone who just wants to use code from crates.io!

One thought I had when writing this is that maybe some "shim crates" could work for now? You're right that pulling in unused crates could have license problems and such like that, but you may also be able to have a set of crates that are "stubbed out" which are empty and contain no dependencies and fail to compile. That way if you accidentally pull it in you know that something needs to be updated to really import the crate.

@marcbowes
Copy link
Contributor Author

So the reason that Cargo needs optional dependencies is that when generating a lock file

I think you've hit the nail on the head here. I'm not sure "lock files" are necessary in our system which has its ways of ensuring reproducible builds. Our package template for Rust projects has a .gitignore for Cargo.lock.

but rather you won't be directly downloading from crates.io

Yup, exactly. Sorry for the ambiguity. We have an "import" process to pull an external dependency into our build system. That process is aware of crates.io.

One thought I had when writing this is that maybe some "shim crates" could work for now?

I'm open to the idea but also skeptical. Right now I don't care about 32 bit, Windows, etc. There are trees of dependencies I don't need to worry about if I just remove them. That works great because I can just scrub them from the Cargo.toml and nothing breaks. Optionals are harder because enabling the feature becomes impossible without "unscrubbing" the manifest on demand.

In that sense it may be best to follow the "build system integration" threads and features to see how we can better accommodate your build system

Yeah, I am (trying to). If something in particular is worth investigation, I'd appreciate if you could ping me.

@cramertj
Copy link
Member

Has any further thought been given to what a solution here might look like? I'd be happy to help with implementation if there's a decision about what should be done. Currently, we're checking in a huge number of vendored crates into the fuchsia tree because they're pulled in as optional dependencies of something or other, and I'd like to make it possible to omit them.

@alexcrichton
Copy link
Member

@cramertj that may or may not be a bug in Cargo and/or the Cargo integration, optional dependencies are only pulled in for members of a workspace, which typically isn't true for crates from crates.io (just those you yourself are writing). Is that the behavior you're seeing?

@bryteise
Copy link

So I likely have a similar issue as @marcbowes in that I'm trying to avoid packaging optional dependencies as it just creates more work for myself (as then I need to package all of the optional dependency packages and their dependencies).

I also noticed something unexpected based on what I understand from lock file generation. I have a package shipping a lock file so I go to package up the content in the lock file (the lock file content should contain the full dependency resolution if I understand correctly). As part of this process I notice one of the dependencies itself has an optional dependency but that is not part of the primary package's lock file. Is that expected?

(side story, in the libc repo, it ships a Cargo.lock file which I slighly remember buzz around libc breaking folks and maybe that's why the library is shipping a lock file but I don't see any of the dependencies listed in the Cargo.toml so I'm just left with adding all of dependencies I see from running cargo vendor for libc is that the expected workflow?)

@cramertj
Copy link
Member

@alexcrichton I'm not sure yet-- I've made a note to myself to investigate further. I started pushing after this because the Fuchsia repo currently includes 57 megs worth of winapi dependencies.

@alexcrichton
Copy link
Member

@cramertj heh I've thought we'd hit a problem like this at some point.

A random solution which may help solve this though is to perhaps list the valid targets for a project in a workspace root Cargo.toml. That way Cargo could be smarter and just vendor dependencies for those targets (and alter resolution so it won't include winapi in the lock file)

@cramertj
Copy link
Member

@alexcrichton That would be perfect! I'll message you on IRC and we can chat about the details.

@siscia
Copy link

siscia commented Apr 17, 2018

Hi Folks,

I hit the same problem with an use case that I believe is quite reasonable.

My project redisql has an open source part and a close source part.

For my own sanity, I was keeping everything in the same cargo folder and using a git submodule to a private repo for the close source part. Like this: https://github.com/RedBeardLab/rediSQL

Now users correctly report that they cannot build it: RedBeardLab/rediSQL#29 (comment)

Is there any way around it?

Cheers,

Simone

@alexcrichton
Copy link
Member

@siscia unfortunately there's no workaround at this time, although #5133 may be workable if the dependency is placed behind a Cargo feature

@siscia
Copy link

siscia commented Apr 17, 2018

I see, thank you :)

@acfoltzer
Copy link

I've just hit the exact same situation as @siscia, where I have an open source project with an optional proprietary component that adds a few extra features. I'm currently working around it by checking in an empty package to the open source project, and then instructing users to overwrite that with the proprietary code if they need it, but this is pretty bad ergonomically.

@joshtriplett
Copy link
Member

joshtriplett commented May 23, 2018

@acfoltzer Cargo does avoid requiring optional dependencies when you use cargo install. So if you create a "directory registry" with the one crate in it, and then cargo install the crate by name from that registry, cargo will build and install the crate without requiring optional dependencies.

I'd like to have a general solution to the problem, though.

@Michel-Haber
Copy link

Has a decision been made for this issue?
I have encountered a case where it is really needed.
In fact, one of my dependencies is optional, and needs a crate that is generated locally. I want to be able to build my project without this dependency when its feature isn't used, and get a compile error if I try to build with this feature and the generated code is missing.

Is there no possible workaround for now?

@keszybz
Copy link

keszybz commented Aug 19, 2018

I'd like to add another "me too" case. I'm working on packaging crates in Fedora. We cannot use crates.io directly, and all crates that are needed during the package build process must be packaged as RPMs. Not all crates are packaged (for various reasons, sometimes lack of resources, sometimes we don't want to package something), and we want to use cargo build with some optional features disabled and without those crates installed. This is very close to the situation described in the original report for this bug, except that this is a public distro so everything is in the open.

Right now the only workable solution seems to be to edit Cargo.toml files to remove the optional dependencies we don't want to enable, but that is a terrible hack and makes it much harder to automate the whole process.

@emilio
Copy link

emilio commented Sep 17, 2018

Somehow Firefox manages to build without optional dependencies in the tree... But I'm not sure how that even works :).

Edit: Note that only optional path dependencies.

@emilio
Copy link

emilio commented Sep 17, 2018

Here's why the Firefox thing works if anyone's curious: https://mozilla.logbot.info/servo/20180917#c15322773

Fi3 pushed a commit to Fi3/stratum-1 that referenced this issue Sep 2, 2021
Two fake crates (quickcheck and quickcheck_macros) are added in oreder
to give satisfy the cargo build inside the guix container. They are
needed only to let `cargo build` craete a valid Cargo.lock file but are
not compiled as thery are optional.
rust-lang/cargo#4544
@delan
Copy link
Contributor

delan commented Feb 15, 2023

Here's why the Firefox thing works if anyone's curious: https://mozilla.logbot.info/servo/20180917#c15322773

LogBot was decommissioned, but here’s that conversation extracted from the archive:
2018-09-17T09:56:19 #servo <emilio> SimonSapin: around? Can I ask you a cargo question?
2018-09-17T09:56:40 #servo <emilio> SimonSapin: so I wanted to unify Gecko's and Servo's style/Cargo.toml
2018-09-17T09:56:51 #servo <emilio> SimonSapin: which right now differ on:
2018-09-17T09:57:36 #servo <emilio> https://www.irccloud.com/pastebin/77W5Ampk/t.diff
2018-09-17T09:58:27 #servo <emilio> SimonSapin: I thought moving the gecko_debug bit somewhere else I'd be able to unify them, since Gecko uses without problems crates that refer to non-existing path that are optional
2018-09-17T09:59:01 #servo <emilio> SimonSapin: but even after that I'm not able to make cargo cope :)
2018-09-17T09:59:08 #servo <emilio> SimonSapin: any idea?
2018-09-17T09:59:13 #servo <emilio> nox: ^ In case you're around
2018-09-17T09:59:40 #servo <SimonSapin> looking
2018-09-17T10:02:33 #servo <emilio> SimonSapin: (in particular, note how gecko builds just fine with `atoms = {path = "../atoms", optional = true}` even though that path is nowhere in m-c). I _assumed_ that that was because of the feature cargo needed to bother looking at the nsstring Cargo.toml, but I'm wrong apparently.
2018-09-17T10:02:42 #servo <SimonSapin> Does Gecko really do that? I think Cargo wants to resolve the full potential dependency graph before doing feature selection
2018-09-17T10:03:26 #servo <emilio> SimonSapin: well it does afaik, but the gecko build may be more complex than just `cargo build` I guess
2018-09-17T10:03:42 #servo <emilio> SimonSapin: maybe I should ask #build instead :)
2018-09-17T10:03:53 #servo <SimonSapin> uh
2018-09-17T10:04:26 #servo <SimonSapin> emilio: I was gonna say, I think I made those changes when removing nsstring bindings from servo/servo, but we could make it an empty crate there instead
2018-09-17T10:04:41 #servo <SimonSapin> but now I wonder how things work out for servo_atoms in gecko
2018-09-17T10:05:11 #servo <emilio> SimonSapin: yeah, that's the other alternative, but I thought there should be a way to make this work... I'll figure out how Gecko does the dependency graph bits.
2018-09-17T10:06:24 #servo <SimonSapin> servo atom is indeed not present in https://searchfox.org/mozilla-central/source/Cargo.lock
2018-09-17T10:23:38 #servo <emilio> SimonSapin: 12:22 <@ted> i have no idea why that works, sorry!
2018-09-17T10:23:47 #servo <SimonSapin> eh
2018-09-17T10:23:55 #servo <emilio> SimonSapin: magic.gif :)
2018-09-17T10:25:07 #servo <emilio> https://github.com/rust-lang/cargo/issues/4544 is open, as expected
2018-09-17T10:25:07 #servo <crowbot> Issue #4544: Cargo fails to run when an *optional* dependency is missing - https://github.com/rust-lang/cargo/issues/4544
2018-09-17T10:25:33 #servo <emilio> oh well
2018-09-17T10:26:16 #servo <emilio> SimonSapin: I may look at what's going on in there if I find some spare time, oh well
2018-09-17T10:26:17 #servo <ted> i would not be surprised to find that it works by accident
2018-09-17T10:26:36 #servo <emilio> ted: yeah, me neither... To bad that now we depend on it heavily :(
2018-09-17T10:27:13 #servo <emilio> ted: do you know what are the biggest differences from a normal `cargo build` in `m-c`? I guess mostly vendored stuff I guess?
2018-09-17T10:27:25 #servo <emilio> s/I guess?//
2018-09-17T10:27:56 #servo <ted> we do write out .cargo/config to do source replacement for vendoring
2018-09-17T10:28:01 #servo <ted> but we also pass a ton of options to cargo
2018-09-17T10:28:14 #servo <ted> https://dxr.mozilla.org/mozilla-central/source/config/rules.mk#957
2018-09-17T10:28:55 #servo <ted> i guess we always run `cargo rustc` not `cargo build`
2018-09-17T10:33:22 #servo <paul> standups: working on making the compositor more embedder friendly: optional at startup, support multiple windows, not bound to the embedder thread.
2018-09-17T10:33:22 #servo <standups> Ok, submitted #59981 for https://www.standu.ps/user/paul/
2018-09-17T10:40:25 #servo <emilio> SimonSapin: ted: Oh, figured out
2018-09-17T10:40:45 #servo <emilio> SimonSapin: ted: (RUST_LOG=cargo is amazing)
2018-09-17T10:41:06 #servo <emilio> SimonSapin: ted: It was erroring in cargo::core::workspace: find_members
2018-09-17T10:41:16 #servo <emilio> SimonSapin: ted: Gecko excludes `servo/` from the workspace
2018-09-17T10:41:23 #servo <ted> aha
2018-09-17T10:41:59 #servo <SimonSapin> uh… so in gecko, the style crate is a path dependency but not a workspace member?
2018-09-17T10:42:14 #servo <SimonSapin> I’m not even sure what that means
2018-09-17T10:42:19 #servo <emilio> SimonSapin: if I add `exclude = ["components/style"]` to servo, it works. Now, I guess we don't really want to do that, but maybe excluding just the non-existing path works well enough
2018-09-17T10:43:08 #servo <emilio> SimonSapin: well, the side effects are nice, imho...
2018-09-17T10:43:28 #servo <ted> SimonSapin: i think it's just because servo defines its own workspace?
2018-09-17T10:43:33 #servo <ted> or maybe because of this, i can't remember
2018-09-17T10:43:42 #servo <SimonSapin> emilio: I mean I don’t know what the other effects are
2018-09-17T10:44:06 #servo <SimonSapin> emilio: does it it work to exclude only support/ ?
2018-09-17T10:45:11 #servo <emilio> SimonSapin: nope, `Unable to update file:///home/emilio/src/moz/servo/support/gecko/nsstring`
2018-09-17T10:46:15 #servo <emilio> SimonSapin: curious what that does to the lockfile, here's the patch:
2018-09-17T10:46:30 #servo <emilio> https://www.irccloud.com/pastebin/4cgrcEGa/t.diff
2018-09-17T10:46:44 #servo <emilio> SimonSapin: I guess it's expected
2018-09-17T10:47:09 #servo <emilio> SimonSapin: (but still, lol)
2018-09-17T10:48:43 #servo <emilio> ted: so Servo used to define its own workspace when we vendored the whole thing, but now we don't, yet we rely on that for everything not to blow up :D
2018-09-17T10:48:58 #servo <ted> heh
2018-09-17T10:49:24 #servo <SimonSapin> I think nested workspaces are ignored, though
2018-09-17T10:51:13 #servo <emilio> SimonSapin: I guess the best way forward for me is to land a cargo patch to not hard-error when broken optional dependencies are found... Does that sound right?
2018-09-17T10:51:36 #servo <emilio> SimonSapin: that should allow us to remove the weird `exclude =` thing as well in Gecko
2018-09-17T10:52:09 #servo <SimonSapin> emilio: I don’t know if the cargo maintainers will want to take that patch, but it may be worth a try
2018-09-17T10:52:38 #servo <SimonSapin> maybe file an issue first in case they don’t, so you don’t do the work if it’s not gonna land
2018-09-17T10:54:37 #servo <emilio> SimonSapin: looks like the patch is trivial to write so I may as well test it and submit it.

copybara-service bot pushed a commit to chromeos/adhd that referenced this issue Jun 20, 2023
Cargo tries to download dependencies even if it's optional.
Stub it to stop download the platform2 repository.

See also upstream bug:
rust-lang/cargo#4544

BUG=None
TEST=audio-qv

Change-Id: If5e66313d3ed688a940843b6bff627ed23232bf6
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/4622503
Commit-Queue: Li-Yu Yu <aaronyu@google.com>
Tested-by: ChromeOS Audio Quick Verifier <audio-qv@chromeos-audio-qv.iam.gserviceaccount.com>
Tested-by: chromeos-cop-builder@chromeos-cop.iam.gserviceaccount.com <chromeos-cop-builder@chromeos-cop.iam.gserviceaccount.com>
Reviewed-by: Chih-Yang Hsia <paulhsia@chromium.org>
ntBre added a commit to ntBre/openff-toolkit that referenced this issue Sep 11, 2023
these fail because cargo checks that the openmm dep is available even though
it's optional and not enabled. see rust-lang/cargo#4544
@epage epage added the S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. label Oct 17, 2023
@tklajnscek
Copy link

We also hit this just now.

We have a few internal crates (as path dependencies) that we'd like to optionally enable when a local dev feature is enabled, but we want those crates to never be required in any capacity by our public users.

We now have to either:

  • patch Cargo.toml locally and add those dependencies in, which is not ergonomic on the dev side.
  • add dummy crates and require the users to work form a workspace with those in, which is also very very ugly

I believe this is a very common use case that I've seen and used in most of the projects I've worked on in my career so far.

I am also open to any horrible hacks and/or suggestions on how to work around this 😅

@warfaj
Copy link

warfaj commented Jan 23, 2024

We hit the same issue with an mix of open source and private optional dependencies.
We have a public crate A that has an optional dependencies on a private crate B (behind a feature). Cargo is failing to resolve the private crate for anyone who uses A even though it's behind a feature flag that not enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-optional-dependencies Area: dependencies with optional=true C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. Z-build-plan Nightly: --build-plan feature
Projects
None yet
Development

No branches or pull requests