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

Tracking issue for PR #3361 #3391

Closed
pwoolcoc opened this issue Dec 12, 2016 · 7 comments · Fixed by #3664
Closed

Tracking issue for PR #3361 #3391

pwoolcoc opened this issue Dec 12, 2016 · 7 comments · Fixed by #3664

Comments

@pwoolcoc
Copy link
Contributor

pwoolcoc commented Dec 12, 2016

Per @alexcrichton in this comment, I am opening a tracking issue for the plan to eventually change cargo to assume that a build.rs file in the same directory as a Cargo.toml file is a build script. Right now cargo just issues a warning when it sees a build.rs file in the same directory as a Cargo.toml file that lacks a build = ... value.

Original PR:
#3361

@pwoolcoc
Copy link
Contributor Author

pwoolcoc commented Feb 2, 2017

@alexcrichton hello again! now that 1.15 is out with this change, what do you think of completing the change on the current nightly? Assuming it gets in to 1.17, that would mean users would get the build = false warning for 2 stable releases before the behavior changes.

@alexcrichton
Copy link
Member

Good question! @rust-lang/tools -- thoughts about whether we're far enough along to implement this change?

@nrc
Copy link
Member

nrc commented Feb 6, 2017

Could you clarify what exactly the change is? My understanding is that currently if build = false and there exists a build.rs in the project root, then we warn and we would change this to a hard error? Or does the warning happen when there is no build entry and a build.rs file and we would change this to silently accepting?

Is this a backwards incompatible change?

@alexcrichton
Copy link
Member

@nrc sure yeah, the state of play is this:

  • We'd like to automatically infer build.rs as the build script
  • This is backwards-incompatible to projects which have build.rs but it's not a build script
  • We added the ability to specify build = false to Cargo.toml to work around this.
  • Cargo currently warns if build.rs exists, isn't a build script, and build = false is missing.
  • Older Cargos will reject build = false as an invalid manifest.

The concrete change to make here is to automatically infer build.rs as a build script unless otherwise configured. This would also entail removing the warning. Crates which have build.rs, it's not a build script, and haven't configured build = false will likely break.

A preliminary list of crates with build.rs but not having it as a build script were compiled. This may not be an exhaustive list.

@nrc
Copy link
Member

nrc commented Feb 6, 2017

How does Cargo know if build.rs is a build script or not?

To be clear, this is only crates with build.rs in the root? Or would this break for src/foo/build.rs too?

@alexcrichton
Copy link
Member

Cargo does no inference based on file contents, just location. And yeah it's only build.rs at the root (next to Cargo.toml), which is quite rare in practice.

Cargo's proposed logic is that if build.rs exists next to Cargo.toml it's automatically inferred as a build script, no configuration necessary.

@nrc
Copy link
Member

nrc commented Feb 6, 2017

Thanks for the clarifications!

I think this is OK to go ahead with - we'll have had 2 whole warning cycles and the fix is pretty trivial.

pwoolcoc pushed a commit to pwoolcoc/cargo that referenced this issue Feb 7, 2017
If cargo sees a `build.rs` file in the same directory as the current
`Cargo.toml`, it will assume that the `build.rs` file is a build script,
_unless there is_ `build = false` _in the _ `Cargo.toml` _file_.

Closes rust-lang#3391
bors added a commit that referenced this issue Feb 7, 2017
Assume that a `build.rs` file is a build script

If cargo sees a `build.rs` file in the same directory as the current
`Cargo.toml`, it will assume that the `build.rs` file is a build script,
unless there is `build = false` in the  `Cargo.toml` file.

Closes #3391
@bors bors closed this as completed in #3664 Feb 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants