Enable binary crates as dependencies

[npmccallum]

This is useful in a variety of situations such as:

• Embedding one binary into another (i.e. BIOS, kernel)
• Testing using a third-party utility (i.e. nc)

Fixes #4316.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[npmccallum]

FYI @haraldh @cuviper @jcranmer @Twey

[npmccallum]

Ugh. I forgot about .exe on Windows. I'll push a fix for Windows tonight.

[npmccallum]

@ehuss This is now ready for review.

[ehuss]

Hi, thanks for the PR!

This seems like a pretty significant change, that I would expect may need an RFC to consider the motivation and impact. A package can have multiple binaries, and as-written it randomly picks the first one. It also doesn't handle if the package has both a library and a binary. Dependencies may have a large number of binaries, and unconditionally building all of them does not seem like the right thing to do.

[npmccallum]

@ehuss

Hi, thanks for the PR!

You're very welcome!

This seems like a pretty significant change, that I would expect may need an RFC to consider the motivation and impact.

I'm not sure I agree. Though I think expanding the case to specifying a subset of the binaries in a dependent crate would probably justify an RFC.

A package can have multiple binaries, and as-written it randomly picks the first one.

Hrm... Let me expand the test to cover this case.

It also doesn't handle if the package has both a library and a binary.

We can test for this case. But I don't understand what the problem is. The library will be built and made available for linking. The binaries will be built and available for execution. That seems like the only sensible behavior to me.

Dependencies may have a large number of binaries, and unconditionally building all of them does not seem like the right thing to do.

If you have specified that package as a dependency, then sure. I think the case of having a single crate with a large number of binaries is probably not common. And then using that crate as a dependency is then even more uncommon.

I do think it may be legit to develop a new dependency field to specify an array of binaries within a dependent crate to build. And I think this should have an RFC. But this seems to me an expanded scope of the simple case which covers probably 80% of usage: a package depending on a crate that has a single binary.

In our case, we have a VMM which depends on a BIOS and a kernel. Both are dependent crates. Each produces a single binary. I suspect this is the most common flow. Cargo itself encourages small well-defined crates.

[npmccallum]

I may have changed my mind on this needing an RFC. Here's my sketch of a proposal:

[build-dependencies]
# Library is built (at the latest) before `build.rs` is linked.
# Library is available for linking during `build.rs` linking.
# (No change in existing behavior.)
lib_only = "*"

# Binaries are built (at the latest) before `build.rs` is executed.
# Binaries are available for execution during `build.rs` execution.
# Change to existing behavior:
#   * Binaries not previously built are now built.
bins_only = "*"

# Library is built (at the latest) before `build.rs` is linked.
# Library is available for linking during `build.rs` linking.
# Binaries are built (at the latest) before `build.rs` is executed.
# Binaries are available for execution during `build.rs` execution.
# Change to existing behavior:
#   * Binaries not previously built are now built. (Controversial?)
lib_and_bins = "*"

[dependencies]
# Library is built (at the latest) before the crate is linked.
# Library is available for linking during crate linking.
# (No change in existing behavior.)
lib_only = "*"

# Binaries are built (at the latest) during `cargo build`.
# Binaries are installed during `cargo install`.
# Change to existing behavior:
#   * Binaries not previously built are now built.
#   * Binaries not previously installed are now installed. (Controversial?)
bins_only = "*"

# Library is built (at the latest) before the crate is linked.
# Library is available for linking during crate linking.
# Binaries are built (at the latest) during `cargo build`.
# Binaries are installed during `cargo install`.
# Change to existing behavior:
#   * Binaries not previously built are now built. (Controversial?)
#   * Binaries not previously installed are now installed. (Controversial?)
lib_and_bins = "*"

[dev-dependencies]
# Library is built (at the latest) before tests are linked.
# Library is available for linking during test linking.
# (No change in existing behavior.)
lib_only = "*"

# Binaries are built (at the latest) before tests are executed.
# Binaries are available for execution during test execution.
# Change to existing behavior:
#   * Binaries not previously built are now built.
bins_only = "*"

# Library is built (at the latest) before tests are linked.
# Library is available for linking during test linking.
# Binaries are built (at the latest) before tests are executed.
# Binaries are available for execution during test execution.
# Change to existing behavior:
#   * Binaries not previously built are now built. (Controversial?)
lib_and_bins = "*"

I think the most controversial aspect of this proposal is to install dependency binaries during cargo install. The idea behind this is that the binary dependency may be needed during execution, so they need to be installed with the crate. Some alternative proposals are as follows.

One alternate proposal would be to install dependency binaries only if the crate itself installs a binary. However, even a library might do something akin to fork() and exec(). So we can't correlate on this.

A much better alternative is to never install dependency binaries unless specified. In this case, the dependency would gain a new property which contains a list of binaries to install. I rather like this option, particularly because it could be developed as an additional feature. If a whitelist of binaries to build were added as a dependency field, we should create a union between the specified build set and the install set (since installed binaries need to be built).

Thoughts?

[npmccallum]

@joshtriplett @haraldh I'd love your thoughts on the above proposal.

[haraldh]

I guess, @phil-opp might be interested in this, too with his bootloader build.rs.

[haraldh]

This is bins_only = [ "…", … ] right?

[npmccallum]

@cuviper Might be interested too.

[npmccallum]

This is bins_only = [ "…", … ] right?

No. In this case bins_only represents the name of a crate which contains only binaries.

[haraldh]

This is bins_only = [ "…", … ] right?

No. In this case bins_only represents the name of a crate which contains only binaries.

so it is in the package section?

[build-dependencies.<pkgname>]
bins_only = "*" ... or true ?

or

[build-dependencies]
pkgname = { …, bins_only = "*" }

[haraldh]

I still don't know how the build.rs is going to find out about the location of the binary, if that build dependency is built with another target.

[npmccallum]

This is bins_only = [ "…", … ] right?

No. In this case bins_only represents the name of a crate which contains only binaries.

so it is in the package section?

[build-dependencies.<pkgname>]
bins_only = "*" ... or true ?

or

[build-dependencies]
pkgname = { …, bins_only = "*" }

Neither. Let's take the case of a source code generator written in Rust named mkcode. It outputs C (not Rust) source code which will be compiled and linked with the rest of the crate. This command line tool needs to be run during build.rs. Therefore, it needs to be built before build.rs is executed. Likewise, build.rs will use the cc library crate to compile the code generated by mkcode. The snippet might look like this:

[build-dependencies]
cc = "1.0.50"    # A library crate
mkcode = "0.1.2" # A binary crate (all binaries built)

[npmccallum]

I still don't know how the build.rs is going to find out about the location of the binary, if that build dependency is built with another target.

I assume you mean the case where the dependent crate is built for a different target than the depending crate. I'm not sure we can support this. The dependent and depending crates must be compiled for the same target.

[npmccallum]

I have pushed a new commit which builds all binaries, not merely the first one. I've updated tests to reflect this change. One test currently fails due to the fact that dependency build ordering is not predictable. I'm not sure what to do about that. 88887d4#diff-6f6836eb687bcd20bec3f99c805c0180R1691-R1697

I'm also working on an RFC.

[est31]

@ehuss

It also doesn't handle if the package has both a library and a binary.

I think it's also important to be forward compatible with [binary-dependencies] in Cargo.toml, because sometimes crates have both libraries and binaries, and you might only want to use one of the two but not the other. If there is no opt in to depend only on the library component of a dependency, [binary-dependencies] won't make any sense in the future (issue for binary dependencies: #1982).

[bors]

☔ The latest upstream changes (presumably #7820) made this pull request unmergeable. Please resolve the merge conflicts.

[ehuss]

I'm going to close this since this PR has gotten a little old, and this feature will likely need an RFC. Once it goes through the RFC process, we can revisit the implementation.