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

Ability to specify the output name for a bin target different from the crate name #9627

Merged
merged 1 commit into from
Aug 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions src/cargo/core/compiler/build_context/target_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,18 @@ impl FileType {
/// The filename for this FileType that Cargo should use when "uplifting"
/// it to the destination directory.
pub fn uplift_filename(&self, target: &Target) -> String {
let name = if self.should_replace_hyphens {
target.crate_name()
} else {
target.name().to_string()
let name = match target.binary_filename() {
Some(name) => name,
None => {
// For binary crate type, `should_replace_hyphens` will always be false.
if self.should_replace_hyphens {
target.crate_name()
} else {
target.name().to_string()
}
}
};

format!("{}{}{}", self.prefix, name, self.suffix)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it's automatically appending a prefix/suffix such as lib, .a, and .exe I think. If that's the case I'm not sure that filename is the right name for the key in the manifest since it's only influencing part of the filename, not the whole filename.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about something like binary_name instead of filename? would that be more suitable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed some ideas for the name, but none seemed particularly good. file_stem is a strong option, but it may not work well with libraries. Another option is to allow name to support arbitrary values, but if it is a non-identifier, require a second field of crate-name.

We didn't feel strongly about any of the options we thought of, so I think leaving this as filename for now is fine, and we can maybe consider other options later.

}

Expand Down
7 changes: 6 additions & 1 deletion src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,12 @@ impl<'cfg> Compilation<'cfg> {
/// that are only relevant in a context that has a unit
fn fill_rustc_tool_env(mut cmd: ProcessBuilder, unit: &Unit) -> ProcessBuilder {
if unit.target.is_bin() {
cmd.env("CARGO_BIN_NAME", unit.target.name());
let name = unit
.target
.binary_filename()
.unwrap_or(unit.target.name().to_string());

cmd.env("CARGO_BIN_NAME", name);
}
cmd.env("CARGO_CRATE_NAME", unit.target.crate_name());
cmd
Expand Down
3 changes: 3 additions & 0 deletions src/cargo/core/compiler/context/compilation_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,9 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
let meta = &self.metas[unit];
let meta_opt = meta.use_extra_filename.then(|| meta.meta_hash.to_string());
let path = out_dir.join(file_type.output_filename(&unit.target, meta_opt.as_deref()));

// If, the `different_binary_name` feature is enabled, the name of the hardlink will
// be the name of the binary provided by the user in `Cargo.toml`.
let hardlink = self.uplift_to(unit, &file_type, &path);
let export_path = if unit.target.is_custom_build() {
None
Expand Down
5 changes: 4 additions & 1 deletion src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,10 @@ fn build_base_args(
let exe_path = cx
.files()
.bin_link_for_target(bin_target, unit.kind, cx.bcx)?;
let key = format!("CARGO_BIN_EXE_{}", bin_target.name());
let name = bin_target
.binary_filename()
.unwrap_or(bin_target.name().to_string());
let key = format!("CARGO_BIN_EXE_{}", name);
cmd.env(&key, exe_path);
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,9 @@ features! {

// Allow to specify which codegen backend should be used.
(unstable, codegen_backend, "", "reference/unstable.html#codegen-backend"),

// Allow specifying different binary name apart from the crate name
(unstable, different_binary_name, "", "reference/unstable.html#different-binary-name"),
}

pub struct Feature {
Expand Down
14 changes: 13 additions & 1 deletion src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ pub struct Target {
struct TargetInner {
kind: TargetKind,
name: String,
// Note that `bin_name` is used for the cargo-feature `different_binary_name`
bin_name: Option<String>,
// Note that the `src_path` here is excluded from the `Hash` implementation
// as it's absolute currently and is otherwise a little too brittle for
// causing rebuilds. Instead the hash for the path that we send to the
Expand Down Expand Up @@ -350,6 +352,7 @@ compact_debug! {
[debug_the_fields(
kind
name
bin_name
src_path
required_features
tested
Expand Down Expand Up @@ -627,6 +630,7 @@ impl Target {
inner: Arc::new(TargetInner {
kind: TargetKind::Bin,
name: String::new(),
bin_name: None,
src_path,
required_features: None,
doc: false,
Expand Down Expand Up @@ -662,6 +666,7 @@ impl Target {

pub fn bin_target(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is something that we'll want to handle for at least the cdylib output as well, perhaps the staticlib output too. Overall I think that the filename handling from the TOML profile may need to happen at a lower layer than as a new method on constructors here (as otherwise it would need to be added as an argument to all constructors)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to re-use the name field in the same struct, instead of creating a new field as I have done here. But, @ehuss, suggested not to do that as we don't know what kind of implications that might have.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what Alex is referring to here is that it could be generalized for all the target types, and that could be handled in some place like configure. I don't feel strongly about supporting that in the initial PR, though it would be good to keep that in mind.

name: &str,
bin_name: Option<String>,
src_path: PathBuf,
required_features: Option<Vec<String>>,
edition: Edition,
Expand All @@ -670,6 +675,7 @@ impl Target {
target
.set_kind(TargetKind::Bin)
.set_name(name)
.set_binary_name(bin_name)
.set_required_features(required_features)
.set_doc(true);
target
Expand Down Expand Up @@ -911,11 +917,17 @@ impl Target {
Arc::make_mut(&mut self.inner).name = name.to_string();
self
}
pub fn set_binary_name(&mut self, bin_name: Option<String>) -> &mut Target {
Arc::make_mut(&mut self.inner).bin_name = bin_name;
self
}
pub fn set_required_features(&mut self, required_features: Option<Vec<String>>) -> &mut Target {
Arc::make_mut(&mut self.inner).required_features = required_features;
self
}

pub fn binary_filename(&self) -> Option<String> {
self.inner.bin_name.clone()
}
pub fn description_named(&self) -> String {
match self.kind() {
TargetKind::Lib(..) => "lib".to_string(),
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1968,6 +1968,8 @@ struct TomlTarget {
crate_type2: Option<Vec<String>>,

path: Option<PathValue>,
// Note that `filename` is used for the cargo-feature `different_binary_name`
filename: Option<String>,
test: Option<bool>,
doctest: Option<bool>,
bench: Option<bool>,
Expand Down
17 changes: 15 additions & 2 deletions src/cargo/util/toml/targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,14 @@ fn clean_bins(
"autobins",
);

// This loop performs basic checks on each of the TomlTarget in `bins`.
for bin in &bins {
// For each binary, check if the `filename` parameter is populated. If it is,
// check if the corresponding cargo feature has been activated.
if bin.filename.is_some() {
features.require(Feature::different_binary_name())?;
}

validate_target_name(bin, "binary", "bin", warnings)?;

let name = bin.name();
Expand Down Expand Up @@ -321,8 +328,14 @@ fn clean_bins(
Err(e) => anyhow::bail!("{}", e),
};

let mut target =
Target::bin_target(&bin.name(), path, bin.required_features.clone(), edition);
let mut target = Target::bin_target(
&bin.name(),
bin.filename.clone(),
path,
bin.required_features.clone(),
edition,
);

configure(features, bin, &mut target)?;
result.push(target);
}
Expand Down
27 changes: 27 additions & 0 deletions src/doc/src/reference/unstable.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ Each new feature described below should explain how to use it.
* Output behavior
* [out-dir](#out-dir) — Adds a directory where artifacts are copied to.
* [terminal-width](#terminal-width) — Tells rustc the width of the terminal so that long diagnostic messages can be truncated to be more readable.
* [Different binary name](#different-binary-name) — Assign a name to the built binary that is seperate from the crate name.
* Compile behavior
* [mtime-on-use](#mtime-on-use) — Updates the last-modified timestamp on every dependency every time it is used, to provide a mechanism to delete unused artifacts.
* [doctest-xcompile](#doctest-xcompile) — Supports running doctests with the `--target` flag.
Expand Down Expand Up @@ -1288,6 +1289,32 @@ The primary use case is to run `cargo rustc --print=cfg` to get config values
for the appropriate target and influenced by any other RUSTFLAGS.


### Different binary name

* Tracking Issue: [#9778](https://github.com/rust-lang/cargo/issues/9778)
* PR: [#9627](https://github.com/rust-lang/cargo/pull/9627)

The `different-binary-name` feature allows setting the filename of the binary without having to obey the
restrictions placed on crate names. For example, the crate name must use only `alphanumeric` characters
or `-` or `_`, and cannot be empty.

The `filename` parameter should **not** include the binary extension, `cargo` will figure out the appropriate
extension and use that for the binary on its own.

The `filename` parameter is only available in the `[[bin]]` section of the manifest.

```toml
cargo-features = ["different-binary-name"]

[project]
name = "foo"
version = "0.0.1"

[[bin]]
name = "foo"
filename = "007bar"
path = "src/main.rs"
```

## Stabilized and removed features

Expand Down
Loading