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

Cargo scripts try to build a nearby src/lib.rs if present #14476

Closed
kornelski opened this issue Sep 1, 2024 · 2 comments · Fixed by #14591
Closed

Cargo scripts try to build a nearby src/lib.rs if present #14476

kornelski opened this issue Sep 1, 2024 · 2 comments · Fixed by #14591
Labels
A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. Z-script Nightly: cargo script

Comments

@kornelski
Copy link
Contributor

Problem

If I place a script in the same directory as a crate, it uses crate's source files, but does not use crate's dependencies, causing unexpected compilation errors.

Steps

./Cargo.toml:

[package]
name = "mainpackage"
version = "0.1.0"
edition = "2021"

[dependencies]
hex = "0.4.3"

./src/lib.rs:

use hex;

./script.rs:

#!/usr/bin/env cargo +nightly -Zscript

//! ```cargo
//! [package]
//! edition = "2021"
//! ```

fn main() {
}

Running ./script.rs causes:

warning: `package.edition` is unspecified, defaulting to `2021`
   Compiling script v0.0.0 (…/mainpackage)
error[E0432]: unresolved import `hex`
 --> src/lib.rs:1:5
  |
1 | use hex;
  |     ^^^ no external crate `hex`

This looks broken, because src/lib.rs is compiled with scripts dependencies, not its usual crate dependencies.

Possible Solution(s)

I would expect Cargo.toml and src/ to be completely ignored by the scripts. Or if they're not ignored, to be compiled correctly as a library and included as an external crate in the script (like tests/ or main.rs get). But compiling the crate, but with a wrong Cargo.toml context seems like the worst of both.

Notes

BTW, "warning: package.edition is unspecified, defaulting to 2021" is printed, even though I've tried to specify it in all places.

Version

cargo 1.82.0-nightly (8f40fc59f 2024-08-21)
release: 1.82.0-nightly
commit-hash: 8f40fc59fb0c8df91c97405785197f3c630304ea
commit-date: 2024-08-21
host: aarch64-apple-darwin
libgit2: 1.8.1 (sys:0.19.0 vendored)
libcurl: 8.7.1 (sys:0.4.74+curl-8.9.0 system ssl:(SecureTransport) LibreSSL/3.3.6)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Mac OS 15.0.0 [64-bit]
@kornelski kornelski added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Sep 1, 2024
@epage epage added Z-script Nightly: cargo script A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. and removed S-triage Status: This issue is waiting on initial triage. labels Sep 1, 2024
@epage
Copy link
Contributor

epage commented Sep 1, 2024

BTW, "warning: package.edition is unspecified, defaulting to 2021" is printed, even though I've tried to specify it in all places.

It looks like this is because you are using an older syntax

//! ```cargo
//! [package]
//! edition = "2021"
//! ```

We dropped support for the doc-comment syntax in favor of rust-lang/rfcs#3503

I've verified the following does not produce the warning:

#!/usr/bin/env nargo
---
[package]
edition = "2021"
---

fn main () {
}

@epage
Copy link
Contributor

epage commented Sep 1, 2024

I would expect Cargo.toml and src/ to be completely ignored by the scripts. Or if they're not ignored, to be compiled correctly as a library and included as an external crate in the script (like tests/ or main.rs get). But compiling the crate, but with a wrong Cargo.toml context seems like the worst of both.

We do, for bins, tests, benches, examples, and build scripts I overlooked that we don't have a way to control auto-discovery of lib targets. We'll either need to hack this in or add a control mechanism for lib auto-discovery.

@epage epage changed the title Cargo script has unexpected interaction with src/lib.rs Cargo scripts try to build a nearby src/lib.rs if present Sep 1, 2024
epage added a commit to epage/cargo that referenced this issue Sep 24, 2024
epage added a commit to epage/cargo that referenced this issue Sep 24, 2024
PR rust-lang#5335 added `autobins`, etc for rust-lang#5330.  Nowhere in there is
discussion of `autolib`.

Cargo script disables support for additional build-targets by disabling
discovery.
Except we don't have a way to disable discovery of `autolib`, leading to rust-lang#14476.
By adding `autolib`, we can continue in that direction.

This also allows us to bypass inferring of libs on published packages,
like all other build-targets which were handled in rust-lang#13849.

As this seems fairly low controversy, this insta-stabilizes the field.
In prior versions of Cargo, users will get an "unused manifest key"
warning.
For packags where this is set by `cargo publish`, the warning will be suppressed and things will work as normal.
For `cargo vendor`, the same except there will be some churn in the
vendored source as this field will now be set.
For local development, it should be rare to set `autolib` so the lack of
error by discovering a file when this is set shouldn't be a problem.

Fixes rust-lang#14476
epage added a commit to epage/cargo that referenced this issue Sep 24, 2024
PR rust-lang#5335 added `autobins`, etc for rust-lang#5330.  Nowhere in there is
discussion of `autolib`.

Cargo script disables support for additional build-targets by disabling
discovery.
Except we don't have a way to disable discovery of `autolib`, leading to rust-lang#14476.
By adding `autolib`, we can continue in that direction.

This also allows us to bypass inferring of libs on published packages,
like all other build-targets which were handled in rust-lang#13849.

As this seems fairly low controversy, this insta-stabilizes the field.
In prior versions of Cargo, users will get an "unused manifest key"
warning.
For packags where this is set by `cargo publish`, the warning will be suppressed and things will work as normal.
For `cargo vendor`, the same except there will be some churn in the
vendored source as this field will now be set.
For local development, it should be rare to set `autolib` so the lack of
error by discovering a file when this is set shouldn't be a problem.

Fixes rust-lang#14476
epage added a commit to epage/cargo that referenced this issue Sep 25, 2024
epage added a commit to epage/cargo that referenced this issue Sep 25, 2024
PR rust-lang#5335 added `autobins`, etc for rust-lang#5330.  Nowhere in there is
discussion of `autolib`.

Cargo script disables support for additional build-targets by disabling
discovery.
Except we don't have a way to disable discovery of `autolib`, leading to rust-lang#14476.
By adding `autolib`, we can continue in that direction.

This also allows us to bypass inferring of libs on published packages,
like all other build-targets which were handled in rust-lang#13849.

As this seems fairly low controversy, this insta-stabilizes the field.
In prior versions of Cargo, users will get an "unused manifest key"
warning.
For packags where this is set by `cargo publish`, the warning will be suppressed and things will work as normal.
For `cargo vendor`, the same except there will be some churn in the
vendored source as this field will now be set.
For local development, it should be rare to set `autolib` so the lack of
error by discovering a file when this is set shouldn't be a problem.

Fixes rust-lang#14476
epage added a commit to epage/cargo that referenced this issue Sep 26, 2024
PR rust-lang#5335 added `autobins`, etc for rust-lang#5330.  Nowhere in there is
discussion of `autolib`.

Cargo script disables support for additional build-targets by disabling
discovery.
Except we don't have a way to disable discovery of `autolib`, leading to rust-lang#14476.
By adding `autolib`, we can continue in that direction.

This also allows us to bypass inferring of libs on published packages,
like all other build-targets which were handled in rust-lang#13849.

As this seems fairly low controversy, this insta-stabilizes the field.
In prior versions of Cargo, users will get an "unused manifest key"
warning.
For packags where this is set by `cargo publish`, the warning will be suppressed and things will work as normal.
For `cargo vendor`, the same except there will be some churn in the
vendored source as this field will now be set.
For local development, it should be rare to set `autolib` so the lack of
error by discovering a file when this is set shouldn't be a problem.

Fixes rust-lang#14476
epage added a commit to epage/cargo that referenced this issue Sep 26, 2024
bors added a commit that referenced this issue Sep 27, 2024
feat(toml): Add `autolib`

### What does this PR try to resolve?

PR #5335 added `autobins`, etc for #5330.  Nowhere in there is
discussion of `autolib`.

Cargo script disables support for additional build-targets by disabling
discovery.
Except we don't have a way to disable discovery of `autolib`, leading to #14476.
By adding `autolib`, we can continue in that direction.

This also allows us to bypass inferring of libs on published packages,
like all other build-targets which were handled in #13849.

Fixes #14476

### How should we test and review this PR?

### Additional information

As this seems fairly low controversy, this insta-stabilizes the field.
In prior versions of Cargo, users will get an "unused manifest key"
warning.
For packages where this is set by `cargo publish`, the warning will be suppressed and things will work as normal.
For `cargo vendor`, the same except there will be some churn in the
vendored source as this field will now be set.
For local development, it should be rare to set `autolib` so the lack of
error by discovering a file when this is set shouldn't be a problem.
@bors bors closed this as completed in 5e35e27 Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cargo-targets Area: selection and definition of targets (lib, bins, examples, tests, benches) C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. Z-script Nightly: cargo script
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants