Tracking issue for config-include

[ehuss]

Implementation: #7649
Original proposal: #6699
Documentation: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#config-include
Issues: Z-config-include Nightly: `include` config key

Summary

Adds the include config key to include other config files.

Unresolved issues

• Should there be any restriction on the filename? Currently there isn't. I was initially considering restricting to .cargo/config-* in case we want to safely use other filenames in .cargo/ for other purposes. However, it seems like an unnatural restriction. Another restriction is that it should maybe end in .toml?

FCP

[ensc]

Some additional wishes (from #9306):

• allow to include multiple files

  include = "foo.toml"
  include = "bar.toml"
  include = "conf.d/*.toml"

• allow to ignore missing files; e.g. either by a special syntax like leading "-":

  include = "-can-be-missing.toml"
  include = "required.toml"

  or by using special keywords (e.g. required vs include)

  This feature is required for local configuration (e.g. the setup of build.target-dir) within a more compliex environment

• substitute placeholders within the filename; e.g.

  • %h ... local hostname
  • %u ... local username
  • %H ... hash of absolute Cargo.toml path
  • ...

[jonhoo]

Some quick thoughts on this:

Restrictions on the path. What's the motivation for restricting the paths? Cargo potentially having special files in .cargo/ in the future is a good one, though I also see an upside in being able to stash other config files under .cargo/ rather than adding another top-level directory to a project like .cargo-includes/. One option might be to say that we allow any path, except a path under .cargo/ that isn't prefixed with config-. So myconfig.toml, myconfig, .cargo/config-my.toml, .cargo/config-my are all okay, but .cargo/my.toml is not. Restrictions beyond that seem unwarranted to me (e.g. requiring .toml).

Multiple includes. I think the ability to have multiple includes is table-stakes for a feature like this. We could get that solely through globs (see below), but just allowing enumeration feels better. I think we should lean into TOML here though and say that the value can be either a string or a list of strings, rather than allowing the key to appear multiple times in a given context (it can, of course, appear multiple times in different tables).

Include globs. I'm torn on allowing globs. They tend to cause headaches as much as it fixes them. One option might be to instead say that Cargo implicitly has a top-level include for each file in .cargo/config.d/ instead? Which I think is a feature that could land independently of this. So my vote is to not include glob support.

Missing files. I think we should initially have just include, and say that it warns, but does not error, if the target file does not exist. We could then in the future add something like require (I don't love that name for this) if the need is great enough.

Placeholders. This feels like something worth punting on until we see use in practice, and not something that should block config-include. It may turn out to be useful, but I think realistically it could be added later. We may want to include checks in the code for config-include that checks the path for % though and warn about possible future-incompatibility (or just straight up disallow it).

[ensc]

• my main reason for this feature is local customization. E.g. I want .cargo/config as part of the project in git, but local setup (e.g. for build.target-dir) should be possible and only in my environment. This setup must be optional (e.g. no warnings when included files are not existing).

  Atm, my git work tree is always dirty in rust projects because there is some local setup (paths (see above) or other default features on Cargo.toml). Moving these settings in optional files would be very helpful

• when using distinct keywords (include for optional, require for mandatory includes), the syntax must allow multiple same keys in the same context. E.g.

   include: optional-00.conf        ## sets A = 0
   require: mandatory-00.conf       ## sets A = 1
   include: optional-01.conf        ## sets A = 2

  has a different load order than

   include:
   - optional-00.conf        ## sets A = 0
   - optional-01.conf        ## sets A = 2
   require:
   - mandatory-00.conf       ## sets A = 1

  "A" will be 2 in the first example but "1" in the latter one.

  That's why, some indication whether mandatory or not should be part of the file name. Perhaps

  include:
  - path: optional-00.conf
    optional: true
  - mandatory-00.conf
  - path: optional-01.conf
    optional: true

  Or just the - prefix (which is used e.g. by GNU Make in -include, or systemd in EnvironmentFile to mark optional files)

[jonhoo]

• This setup must be optional (e.g. no warnings when included files are not existing).

Why must it produce no warnings?

• paths (see above) or other default features on Cargo.toml

I don't think this feature will help you with includes for Cargo.toml. It's only for Cargo's configuration files as I understand it.

• the syntax must allow multiple same keys in the same context

I think that's simply just not allowed by the TOML spec, though I could be wrong. At the very least it would require some custom de/serialization to handle duplicate keys. Your alternative proposal of introducing table entries in the include array seems like a nice one to me, and crucially also seems like something that could be an extension of the feature, rather than something that's there from the get-go.

• Or just the - prefix (which is used e.g. by GNU Make in -include, or systemd in EnvironmentFile to mark optional files)

I personally strongly prefer explicit syntax (required: true) over "magic" strings. The latter are, among other things, much harder to search for, and not at all immediately obvious to an unfamiliar reader.

[ensc]

This setup must be optional (e.g. no warnings when included files are not existing).
Why must it produce no warnings?

Because the setup is not in git (it is local). When other people clone the project they will either be spammed by warnings about non-existing files, or they have to create them manually.

[weihanglo]

Try to push this toward stabilization a bit.

Here lists concerns that I feel like they could be a Future Extension an Expected Behavior, or a Blocker.

• Blocker: Name restriction
  I quite like the idea putting a restriction on config files name under .cargo/config-*. Requiring .toml makes no sense to me. Cargo will continue support .cargo/config forever, why only allow the standard config file to bypass that?

  There is the other unresolved question: If we put a rule on config file name, should we let the CLI --config bypass that rule, for example cargo --config .cargo/myconfig.toml? This is allowed today. My answer is yes, but should the behavior sticky to all include key loaded via --config or just the first one?

• Blocker: Loading Priority
  Not really a blocker just want it to be clarified. What is the loading precedence of the array include = ["a", "b", "c"]. I assume that it's like this: config in c gets the highest priority. Then its direct descendants, and down to leaf descendants. Then b and descendants. The last is a. Can als define it in reverse-order. We just need to confirm on a definition.

• Future Extension, Blocker: glob/gitgnore style syntax
  This can be added afterward. It may be a tiny breaking change if someone uses *!? in their filename but that shouldn't be too common. If we want this to happen, we need to reserve such characters before stabilization.

• Future Extension, Expected Behaivor, : ENOENT errors
  Currently this is an error when no file found at given config path. We can conservatively just keep it an error when stable. If someone checks config-include in their VCS, that must mean they want to share some configs. It's easy to fix by adding an empty file at that path. This can also be enhanced with [[include]] extension mentioned below. In addition, we could improve the error message a bit before stabilization.

  If you currently use include = […] and people uses your package via git clone may get lots of ENONET errors, you can change to include = "config-containing-other-include-array.toml", so people only need to create one empty config file to proceed to build.

• Future Extension: array of tables [[include]]
  This is not too hard to add after the stabilization. Needn't do it now.

• Future Extension: Placeholder for path
  Likewise, we can prohibit some special characters before stabilization. However, it does seem like a request similar to Support environment variable expansion in config.toml #10789. Maybe we should consider them as a whole? Anyway, I can think of one hacky patch: adding a new key env-substitution and when it's turned on Cargo use the new syntax to perform variable substitutions. I don't think we need to do it now.

[ensc]

If someone checks config-include in their VCS, that must mean they want to share some configs

This is a wrong assumption. People can use config-include for local configuration (e.g. for build.target-dir). Such local configuration should never end in the VCS.

It's easy to fix by adding an empty file at that path

This means, a simple git clone ... && cargo build or cargo install bin-crate will not work because people have to create empty files first.

[weihanglo]

This is a wrong assumption. People can use config-include for local configuration (e.g. for build.target-dir). Such local configuration should never end in the VCS.

Then I will argue why the crate author added an include pointing to nowhere in the first place, and committing it into VCS? I feel like it shouldn't have happened.

One benefit I can see is that it forces people to put their config files in a "dedicated" path the crate author wants. In that case, shouldn't it be better as a hard error?

But anyway, since we consider config files as a kind of local environment, I agree with you that it may also make sense starting from a warning.

[ensc]

Then I will argue why the crate author added an include pointing to nowhere in the first place, and committing it into VCS? I feel like it shouldn't have happened.

It will happen when people use include files for optional local setup. How will you handle it else, when .cargo/config expands unconditionally to

[build]
target-dir = "/home/ensc/.cache/rust/r-tftpd"

IMO such configuration fragments should be optional and do not cause a dirty vcs tree. E.g. place it in local-conf.toml and add

include = "-local-conf.toml"

to .cargo/config.

Btw, glob expansion would solve many problems; either explicitly like

include = "conf.d/*.toml"

or by looking automatically below .cargo/conf.d. It will:

• allow local setup without dirtying the vcs tree
• allow overriding by defining a load order

I agree with you that it may also make sense starting from a warning.

no; there should not be a warning. Missing files should be ignored silently.

[weihanglo]

Thanks for the feedback. The workflow of yours looks valid, though I don't agree on completely ignoring missing files. Downgrading to a warning seems fine to me. Ignoring is not.

If it were a silent ignore, how long would it take for people to figure out there is a typo in their include so that config [patch] never work? Cargo was changed to emit unused patches warning because they are really prone to typo. include without warning seems like repeating the old mistake to me.

That being said, one future work of -Zlints is that Cargo will have its own lint rules to turn on and off. Excessive warning can benefit from that.

Personally I lean toward not adding new feature at this time, they can be implemented afterward in a compatible way. -Zconfig-include was added three years ago and waiting another three years for newly added features doesn't look good to me.

[ensc]

If it were a silent ignore, how long would it take for people to figure out there is a typo in their include

Make it explicit whether file is required or optional. It was discussed some comments above:

• use a special - prefix (e.g. include = "-optional-config"); this is a common practice (e.g. in make or systemd)
• add a special annotation tag (optional: true)

three years ago and waiting another three years

I fear, that optional, local customization will never be implemented when some basic, half-baked include mechanism exists.

[joshtriplett]

@ensc We might be open to adding a feature for optional config files, such as include = { path = "some.toml", optional = true }. But that's a new feature proposal, and not something we should add at the last minute before stabilizing this existing feature.

[ensc]

@joshtriplett ok; by looking at the speed of implementing new features, this means that cargo will probably never support optional configuration files :(

[weihanglo]

The Cargo team discussed this on Tuesday. We are going to stabilize the feature as is.

We notice that there are some possible extensions, however, we believe they could be added later after the stabilization. The only last minute stuff added was #12298, which puts a restriction on config include path to allow only .toml extensions, reserving some spaces for Cargo to add its own directories.

Let's do a final check before the stabilization PR

@rfcbot fcp merge

[rfcbot]

Team member @weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Eh2406
• @Muscraft
• @arlosi
• @ehuss
• @epage
• @joshtriplett
• @weihanglo

Concerns:

• syntax (Tracking issue for config-include #7723 (comment))

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[ensc]

I'm also curious what those extra settings would actually be

[build]
target-dir = "/home/ensc/.cache/rust/r-tftpd"

or cargo vendor setup

[epage]

I'm also concerned about the lack of end-users driving this. Use cases from end-users help us make sure we are delivery features in the way needed rather than tying us down with compatibility with something no one uses. Looking at #6699 and #7649, this seems more driven as a "could this potentially be useful" rather than someone saying "I have problem X that I need solved". #9306 is linked to in this thread but that requires further extensions (glob or optional config support). If this were a personal project of mine, like clap, I'd either work towards #9306 or remove this feature.

[weihanglo]

Thanks for the feedback. There are some details I can share with.

People want to have some controls over configuration file loading for a long while. This is just one of those possible solutions. ##9769 is a collection of them.

Some configs in .cargo/config.toml file are "per-pakcage" configuration, like rustflags and $PATH. They shouldn't be shared across all packages. At $WORK, a "meta" build system generates Cargo configs on a per-package per-machine basis. Those configs shouldn't be shared across all pacakges because some configs like rustflags and $PATH are filled with local paths.

One workaround is to inject all the generated config into ./.cargo/config.toml under the package root. However, that implies a user now may need to check in generated configs with their own configs.

If we have -Zconfig-include. We can just generate a local .cargo/shared-config.toml that can then be included from users' project-local .cargo/config.toml (which we'd generate if it doesn't exist). users could then check in .cargo/config.toml (with the include field) to their VCS, but exclude the generated file.

---

If this is intended for local (per-machine) settings, then that implies that the per-machine settings wouldn't be checked into source control, which would mean the cargo packages would fail without first doing some extra tasks.

I was worried about this but forgot to write it out. I am more worried that cargo package succeeds but the tarball actually is broken due to the lack of some configs. Although broken packages exist on crates.io today, include may make the situation happening more often.

[Dessix]

The concept of .local and .dev configurations is pretty much a requirement as systems like alternate registries or Tokio's unstable mode begin coercing users into checking in .cargo/config.toml. As it stands, it's a hackfest to make something "optional" by instead generating another config.toml at the include target location if and only if it doesn't exist prior to builds in CI.

If you're concerned with satisfying end-user cases, all of our end-user cases rely on optionality, though it is something we've had to hack in, even with the addition of config-include.

[weihanglo]

• For truly user-specific settings, those should go in CARGO_HOME.

For a large monorepo that lots of people collaborate on, it is easier to educate people to put their project-local config.toml in the same location, say, .cargo/local-config.toml.

• If this is intended for local (per-machine) settings, then that implies that the per-machine settings wouldn't be checked into source control, which would mean the cargo packages would fail without first doing some extra tasks. I'm also curious what those extra settings would actually be (if there is perhaps a different solution for them).

A project that already checked .cargo/config.toml in VCS. It may have

[profile.test]
debug = false

to speed up the test build and reduce memory usage in CI pipeline.

For local development, user might want to turn it own at per-project-level, but without -Zconfig-include, their option is either using --config CLI arg, or set up a .cargo/config.toml one level up from the project root, or set the value in ~/.cargo/config.toml. These options are either not convenience for a large team, or doesn't reflect the truth of per-project-level setup.

Granted, we have git update-index --assume-unchanged or git update-index --skip-worktree to ignore local modified files and prevent from committing into git. These git options doesn't really good to learn and remember.

[Nemo157]

I have a usecase for something in this area now, but it requires the support for "optional includes". We have a shared workspace config committed into the repository for the xtask pattern, but we also commonly want to be able to apply patches while testing changes across multiple repositories:

# .cargo/config.toml
include = "config.local.toml"
[alias]
xtask = "run --package xtask --bin xtask --"

# Cargo.toml
...
# If you want to get `foobar` from your local machine then create a `.cargo/config.local.toml` in this directory with:
#
#   [patch."ssh://[email protected]/foobar/foobar.git"]
#   foobar.path = "../foobar"
#
# (adapting the path as needed relative to this directory)

Having to edit a version-controlled file and remember to not commit it (or keep reverting and reapplying the changes when using a VCS like Jujutsu that doesn't allow not committing) is painful.

Re-reading the docs now I also notice that the precedence is backwards for this sort of usecase, to get the correct precedence (local config can override shared config) requires committing two config files into the repo:

# .cargo/config.toml
include = [ "config.shared.toml", "config.local.toml" ]

# .cargo/config.shared.toml
[alias]
xtask = "run --package xtask --bin xtask --"

[twitu]

I just hit a use case where this feature will be very useful. I'm using pyo3 in my project and it frequently recompiles the whole project even for small change. This happens because of the difference in Python path in my cli and vscode rust analyzer. Running a cargo command in CLI invalidates the build artifacts from rust analyzer

The solution¹ is to make the rust analyzer and CLI use the same python path by setting the PYO3_PYTHON variable in .cargo/config.toml. As you can see though, the path is user specific so I can't commit it. Having local project specific config will be a big help here.

[env]
PYO3_PYTHON = "/home/twitu/.cache/pypoetry/virtualenvs/nautilus-trader-_5j1sq6Z-py3.10/bin/python"

I hope this gets merged soon 😄

Footnotes

1. https://github.com/PyO3/pyo3/issues/1708 ↩

[weihanglo]

. This happens because of the difference in Python path in my cli and vscode rust analyzer. Running a cargo command in CLI invalidates the build artifacts from rust analyzer

A workaround is to have an intermediate directory containing additional .cargo/config.toml so that you don't need to commit the file to VCS. It is not ergonomic though.

[bradjc]

For tock, it would be helpful for us to be able to include different config files (and hence different compiler/linker flags) for different hardware platforms and different architectures. For example, there are certain flags we only want to include on RISC-V platforms, and we would benefit from keeping those in one (optional) cargo config file rather than copying them for each individual platform (making maintenance more difficult). Right now we are accomplishing this using if statements in a Makefile.

[bradjc]

Also, we (tock) generally use nightly, so we can use the unstable feature, but it doesn't seem to work when enabled in a config file. For example:

❯ cargo config get
build.rustflags = ["-C", "linker-flavor=ld.lld"]
build.target = "thumbv7em-none-eabi"
unstable.build-std = ["core", "compiler_builtins"]
unstable.config-include = true
unstable.unstable-options = true
# The following environment variables may affect the loaded values.
# CARGO_HOME=/Users/bradjc/.cargo
❯ cargo -Zconfig-include config get
build.rustflags = ["-C", "linker=rust-lld", "-C", "linker-flavor=ld.lld", "-C", "relocation-model=static", "-C", "link-arg=-nmagic", "-C", "link-arg=-icf=all", "-C", "symbol-mangling-version=v0", "-C", "lto", "-C", "linker=rust-lld", "-C", "linker-flavor=ld.lld", "-C", "relocation-model=static", "-C", "link-arg=-nmagic", "-C", "link-arg=-icf=all", "-C", "symbol-mangling-version=v0", "-C", "lto", "-C", "linker-flavor=ld.lld"]
build.target = "thumbv7em-none-eabi"
unstable.build-std = ["core", "compiler_builtins"]
unstable.config-include = true
unstable.unstable-options = true
# The following environment variables may affect the loaded values.
# CARGO_HOME=/Users/bradjc/.cargo

My include key is only included with the command line flag and not the [unstable] setting.

[bradjc]

I can confirm that #14196 fixed my issue.

The config-include feature is very useful and it would be great to re-try stabilizing it.

[weihanglo]

Right now we are accomplishing this using if statements in a Makefile.

@bradjc Would you mind elaborating this more, and sharing the Makefile?
I am now at the point rethinking that the optionality of config-include feature is a requirement before stabilization.

[bradjc]

I think this open PR is the most clear explanation before and after: https://github.com/tock/tock/pull/4075/files

We optionally add -C flags that we pass to rustc in the makefile today, and that PR would move those flags to config.toml files optionally included by each binary crate (in the boards/ directory).

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[ehuss]

@rfcbot concern syntax

I would like to consider changing the syntax before stabilizing to make it easier to possibly extend in the future. Currently we have:

include = "path/to/file.toml"
# or
include = ["array/of/files.toml", "two.toml"]

However, if we intend to add other properties like optional, then we have to change the type which is a bit difficult with TOML and how our config processing works (particularly with backwards compatibility). We could always introduce new top-level keys, but this seems like a relatively easy change to make now.

I don't know exactly what syntax we should use. Above it was suggested to use a table, like:

include = { path = "some.toml", optional = true }
# or
include = { paths = ["a.toml", "b.toml"], optional = true }

Another option is to make it an array of tables, which would allow specifying settings for each file:

[[include]]
path = "a.toml"
optional = true
[[include]]
path = "b.toml"
optional = false

I also don't know exactly what it would look like if we want to add conditions (or even what conditions we might have). This feature was originally modeled after gitconfig, which uses a conditional syntax like:

[includeIf "hasconfig:remote.*.url:https://example.com/**"]
	path = foo.inc

Also note that git silently ignores missing includes, whereas cargo will error. I'm not sure which default is the right one.

[ehuss]

@rfcbot concern syntax