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 ignores config.toml of dependencies #11492

Closed
BigBadE opened this issue Dec 16, 2022 · 7 comments
Closed

Cargo ignores config.toml of dependencies #11492

BigBadE opened this issue Dec 16, 2022 · 7 comments
Labels
A-configuration Area: cargo config files and env vars C-bug Category: bug

Comments

@BigBadE
Copy link

BigBadE commented Dec 16, 2022

Problem

Cargo internally only has one config, and will not load dependency configuration files when compiling them.
This causes a few issues with specific projects:

  1. Any dependencies that need to build with a different std than the project building them (for example, custom kernels writing a bootloader that need to run in 16bit mode).
  2. Any dependencies that need custom flags for a target (for example, if a library needs a special env variable for Windows or MacOS)

Steps

  1. Create a project "A"
  2. Create project "B"
  3. Make project "A" depend on project "B"
  4. Write "invalid" to the file .cargo/config.toml in project "B"
  5. Compile project "B", cargo errors with a TOML error
  6. Compile project "A", cargo succeeds with no TOML error

Possible Solution(s)

This may be hard to fix in Cargo, a lot of those settings may be applied before dependency resolution or not apply (like the [install] block setting where cargo install installs to), in which case they should be ignored. Other settings, like build-std, need to be applied.

I made a proof of concept at https://github.com/BigBadE/cargo, giving a possible implementation of this feature that adds the Config to the Unit, which means Config has to be clonable. For this proof of concept, a lot of LazyCells just use default, and some Unit methods just use the default Config. I'll open a PR if this is a usable implementation and not entirely naive.

Notes

No response

Version

cargo 1.64.0 (387270bc7 2022-09-16)
release: 1.64.0
commit-hash: 387270bc7f446d17869c7f208207c73231d6a252
commit-date: 2022-09-16
host: x86_64-pc-windows-msvc
libgit2: 1.4.2 (sys:0.14.2 vendored)
libcurl: 7.83.1-DEV (sys:0.4.55+curl-7.83.1 vendored ssl:Schannel)
os: Windows 10.0.19045 (Windows 10 Home) [64-bit]
@BigBadE BigBadE added the C-bug Category: bug label Dec 16, 2022
@epage
Copy link
Contributor

epage commented Dec 17, 2022

The config file is an environment configuration, not a project or package configuration.

One way to demonstrate this in practice is say you have a project foo with a config file inside of it

$ cd foo
$ cargo build  # uses the config
$ cd ..
$ cargo build --manifest-path foo/Cargo.toml  # doesn't use the config

Now, some project configuration is in the config and people have been identifying what those are and working to get them into manifests so it is tied to the package.

@epage epage added the A-configuration Area: cargo config files and env vars label Dec 17, 2022
@BigBadE
Copy link
Author

BigBadE commented Dec 17, 2022

The config file is an environment configuration, not a project or package configuration.

One way to demonstrate this in practice is say you have a project foo with a config file inside of it

$ cd foo
$ cargo build  # uses the config
$ cd ..
$ cargo build --manifest-path foo/Cargo.toml  # doesn't use the config

Now, some project configuration is in the config and people have been identifying what those are and working to get them into manifests so it is tied to the package.

Found that in #9096 (comment), but the problem is build-std isn't being moved to Cargo.toml. At #10308 (comment), ehuss says that build-std will likely not be moved to Cargo.toml, but that forces projects to manually call builds and removes some uses of the new artifact dependency feature (even though the goal is to prevent project manually calling builds).

So what is the planned solution for this? It seems pretty limiting for no good reason.

@BigBadE
Copy link
Author

BigBadE commented Dec 17, 2022

@ehuss It looks like there was a potential solution in #10330 but the feature seemingly has a different usage. Since you seem to have the best design idea about this, how would you propose to fix this issue? I'd like some sort of guidance before opening a PR.

@ehuss
Copy link
Contributor

ehuss commented Dec 17, 2022

It's not really clear to me what this issue is about, so it is hard to respond. If this issue is about having per-package config settings, then I think that is #2930. It is unlikely that we will add those directly. Some discussion around that is towards moving things that are absolutely required for the package to build to be placed in Cargo.toml, but those will need to be considered on a case-by-case basis. Otherwise there is #9769 which gives an overview of the config search issues.

If this is about fixing per-package-target with -Zbuild-std, then that seems like it is #9451, which just hasn't been fixed yet.

If you can provide a more specific example of your exact use case, we can maybe look at how that could be handled?

@BigBadE
Copy link
Author

BigBadE commented Dec 17, 2022

It's not really clear to me what this issue is about, so it is hard to respond. If this issue is about having per-package config settings, then I think that is #2930. It is unlikely that we will add those directly. Some discussion around that is towards moving things that are absolutely required for the package to build to be placed in Cargo.toml, but those will need to be considered on a case-by-case basis. Otherwise there is #9769 which gives an overview of the config search issues.

This issue is about some config.toml options are required for projects to build (like build-std), so any projects depending on them will fail. Build-std is the main one that will cause issues though

If this is about fixing per-package-target with -Zbuild-std, then that seems like it is #9451, which just hasn't been fixed yet.

I see. The author of that seemed to stop and endorse #10308 which got closed, but if per-package-target allowed setting per-package build-std, would that be the approach cargo will likely go with? I'm interested in picking up where the original issue creator left off.

If you can provide a more specific example of your exact use case, we can maybe look at how that could be handled?

There's a few config.toml settings that can't be set in Cargo.toml that force projects to chain cargo builds in a build script instead of a pure cargo implementation. One exact case is phil-opman's bootloader crate (which chains builds in the build script). This applies to a few other settings other than build-std, like [env] (for example, setting linker settings) and [target] (for example, per-target rust flags).

@weihanglo
Copy link
Member

… [target] (for example, per-target rust flags).

There are two unstable options may cover those use cases.

Though I am no sure when and how they get stabilized, especially for rustflags related option s.

@ehuss
Copy link
Contributor

ehuss commented Dec 30, 2022

I'm going to close as I believe there are several other issues tracking what is being asked here. For build-std support, that is tracked in rust-lang/wg-cargo-std-aware#43.

If you have a specific use case that does not seem to be already brought up, feel free to open a new issue. I recommend focusing on describing a real use case, preferably with an example. For example, I'm not really following the question about environment variables and linker settings, since those can be set in build scripts.

@ehuss ehuss closed this as completed Dec 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-configuration Area: cargo config files and env vars C-bug Category: bug
Projects
None yet
Development

No branches or pull requests

4 participants