contrary to manifest, cargo does not build src/bin/*.rs when toml contains [[bin]]

[boxofrox]

According to http://doc.crates.io/manifest.html#the-project-layout...

Cargo will also treat any files located in src/bin/*.rs as executables.

... which is presented as an invariant. There is no mention that this is conditional.

My project started out with with a few binaries src/bin/*.rs. When src/bin/daemon.rs ballooned, I refactored it into src/bin/daemon/main.rs and organized extracted code into modules under src/bin/daemon.

Once I added a [[bin]] entry to Cargo.toml, cargo build only builds the daemon binary, and no longer adheres to the quote above.

I see two options here:

1. Update the quoted portion of the manifest to indicate that *Cargo will treat any files located in /src/bin/*.rs as executables unless the toml contains a [[bin]] section.

2. Modify the behavior of cargo to comply with the docs. cargo treats any files located in src/bin/*.rs that are not otherwise defined in a [[bin]] section as executables.

   a. cargo fetches list of binaries from src/bin/*.rs.
   b. cargo replaces any binaries from src/bin/*.rs with respective [[bin]] entries found in toml.
   c. cargo builds all executables in said list.

[boxofrox]

Place thumbs-up/down votes for option 2 (modify cargo behavior) here.

[alexcrichton]

Ah yes the docs definitely at least need to be updated! The current behavior is intentional in the sense that it wasn't just an accident, but I'd be open to extending it as well if possible!

[boxofrox]

Excellent. I'm happy to work on this. Will submit a PR to update the docs before next Monday, and then start working on a PR for option 2.

[alexcrichton]

Awesome thanks! Let me know if you need any help.

Note that there are backwards compatibility concerns with option 2 above, so we may want to dig into those before jumping to a PR

[boxofrox]

Now that the manifest is updated, what are the backward compatibility concerns?

The only issue I see (off the top of my head) is some users may not want to autobuild src/bin/*.rs, and rely on [[bin]] to stop that. Seems like a flag (opt-in or opt-out?) is necessary to maintain that behavior.
Something like...

[package]
...
auto-build-binaries = false

[[bin]]
...

Case for opt-out automatic building:

1. Intuitive for new users. New and early projects will likely rely on the autobuild feature at first. Adding a [[bin]] section later to customize a build shouldn't unexpectedly disable the autobuild, unless the developer explicitly asks for it.

2. Consistency. Cargo already autobuilds by default, so the the auto-build-binaries option should also default to true to remain consistent with that behavior.

Case for opt-in automatic building:

1. Preserves existing behavior for existing projects using [[bin]] sections. People might be surprised when their build times jump after a cargo upgrade when all the src/bin/*.rs are added to the build. An opt-in flag avoids this situation.

Is there a mechanism in place to patch existing Cargo.toml's with auto-build-binaries = false so mature projects aren't affected by the new opt-out behavior? Or at least notify users when they run cargo of this change (preferably something that can be disabled with an acknowledgement).

[alexcrichton]

Yeah that's basically my worry, for example Cargo itself has src/bin/*.rs but only wants one binary (cargo itself) built. We could add a key like auto-build-binaries but that would take a long time to propagate throughout the ecosystem I fear and may be difficult to evaluate.

We could perhaps add auto-build-binaries and require it to be set to true though to get the behavior here, though? Eventually we could start warning that you should write down false if you want today's behavior, and then way down the road we could perhaps switch the defaults.

[boxofrox]

Sounds great. I imagine we'll want to also update cargo new and cargo init to include auto-build-binaries = true for new projects. Which means existing projects relying on autobuild will have to add the key themselves, otherwise nothing builds for them... hmmmm.

One last consideration before I start work on the opt-in approach, because this plan should work for either opt-in or opt-out.

1. Add the auto-build-binaries key to Cargo, at this point Cargo ignores it and doesn't complain about an unused manifest key.
2. Warn users to add the key to their projects. cargo new and cargo init will add the key with a default value (opt-in = false, opt-out = true).
3. At some future date, update Cargo to use the key. If the key doesn't exist, cargo build terminates with an error message asking the user to add the key.
4. At some much later date, allow the user to omit the key, and quietly use the default value.

I'm not certain about (4), because anyone who neglects a project for a long period will be in for a surprise when they decide to dig into the project again, and nothing builds. Adding a FAQ for "cargo build builds zero binaries" to the Cargo docs may be sufficient to alleviate this concern.

[alexcrichton]

cc @rust-lang/cargo

Those steps sound reasonable to me @boxofrox (modulo bikeshedding and such), I'm cc'ing some others to see if they've got thoughts on the transition strategy here or whether we should do this.

[withoutboats]

I'd like to brainstorm to see if we can't find a solution that doesn't involve exposing another flag to users (at least permanently).

It seems like the case we're trying to avoid breaking is that only one file in src/bin is actually a binary, and the rest are submodules of it. I think what we should do is make some more structured recommendations about how the src/bin directory should be laid out so that, if you follow that structure, cargo will be able to correctly able to distinguish binaries from their submodules.

This would still require a migration path most likely that might involve a flag for a while, but in the long term it would be automatically. Possibly it could annul the need for a [[bin]] in most cases also, a net reduction in flags.

[matklad]

I'd like to brainstorm to see if we can't find a solution that doesn't involve exposing another flag to users (at least permanently).

👍 for this. In particular, I think that the original use case of "I want bin/daemon/main.rs to be binary" could be a default behavior.

That is, for the layout

bin/
  client.rs
  server/
    main.rs
    aux.rs

Cargo, without any configuration, infers two binaries: client and server. Would be interesting to collect existing rust repositories and to find out if this is a common layout.

[withoutboats]

@matklad Yes exactly! I got hung up on figuring out what the root file of server would be called, but main.rs is an obvious choice.

Then if cargo were to migrate to src/bin/cargo it would be able to delete its [[bin]] key as well, and I think we should recommend this strategy over adding a [[bin]].

[matklad]

Would be interesting to collect existing rust repositories and to find out if this is a common layout.

Huh, I've found at lest one repository that follows this convention: https://github.com/matklad/learnOpenGL/blob/master/Cargo.toml 😜

[alexcrichton]

Oh note that this isn't just related to the case of one binary having submodules, but also a case that sometimes you want to configure one [[bin]]. For example you might nowadays add required-features to only build some binaries when fatures are enabled. The problem with this is that even if you have the conventional layout of source files you've still got to specify all other [[bin]] sections because specifying any of them turns off Cargo's auto-inference.

[matklad]

@alexcrichton indeed! Split off #4086 because it seems orthogonal to this issue (although it makes it less pressing), and is a nice quality of life improvement.

I don't think it's possible to make required features use case always work by "convention", so I agree that we should infer [[bin]]s by default.

However, I am not sure that the current multi-staged plan will be executed properly to the end, and that we won't get stuck in the middle, with both a wrong default, a configuration option, and broken old crates!

I think we need a more rigid process for removing deprecated stuff from Cargo and switching defaults, rust-lang/rfcs#1953 perhaps?

[boxofrox]

I do like #4086.

Initially, I imagined a couple use cases:

1. With regards to @alexcrichton's comment above, Cargo has many binary files, but only wants to build one, and thus would need a way to disable automatically building all binaries. (I completely misread this, withoutboats clarifies below).

2. My project is a single repo/crate with one binary serving as a daemon (the soul of the project), and the other binaries are various tools for troubleshooting, configuration, and what-not. At some point, I'd like to only build the daemon binary and one or two utilities, and a flag in Cargo.toml would facilitate that.

However, I could just split my project up into a lib crate, daemon crate, and crate of misc binaries to achieve the same. Or I could just use a shell script to cargo build --bin NAME for those binaries I'm working on. So I have other options available without the need for an auto-build-binary flag.

[withoutboats]

The problem with this is that even if you have the conventional layout of source files you've still got to specify all other [[bin]] sections because specifying any of them turns off Cargo's auto-inference.

Yeah, what I want to do is migrate to continue to infer the binaries, but that in the final state (and I realize this may involve epochs and such) we should not expose a flag to change that behavior back to the current behavior.

With regards to @alexcrichton's comment above, Cargo has many binary files, but only wants to build one, and thus would need a way to disable automatically building all binaries.

To clarify: cargo has only one binary, all the other files in the bin directory are submodules of it. The problem cargo's dealing with is that cargo build would always fail because it would try to build modules that aren't binaries.

I think users who want to only build one binary but have multiple can suffice with cargo build --bin <arg>.

[boxofrox]

🤦‍♂️ Thanks for clarifying. Striking that comment from my post, so no one else is confused by it.

[dwijnand]

With #5330 shipped is there more left to do here?

[alexcrichton]

Indeed I think this is fixed!