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

Invoking cargo run from a build script #6412

Open
CAD97 opened this issue Dec 9, 2018 · 13 comments
Open

Invoking cargo run from a build script #6412

CAD97 opened this issue Dec 9, 2018 · 13 comments
Labels
A-build-scripts Area: build.rs scripts C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage.

Comments

@CAD97
Copy link
Contributor

CAD97 commented Dec 9, 2018

In the pest project, I currently have "solved" circular dependencies of bootstrap by "dynamically linking" to a version built from crates-io in order to do codegen, which is then published with cargo publish and the codegen isn't done when building from crates-io. (So when building from git sources, you first build the current crates-io version, use that to pre-process the self-dependencies in the project, which generates some .rs files which are dumped in the appropriate places in the workspace. This state is what is on crates-io, and skips the generation step.)

This is done so in a very brittle way

            // This "dynamic linking" is probably so fragile I don't even want to hear it
            let status = Command::new(manifest_dir.join("../target/debug/pest_bootstrap"))

because we cannot just use execute cargo run --package pest_bootstrap (or the alias cargo bootstrap) as a child Command, as this needs to lock the build directory, which is already locked because we're in build.rs.

Ideally, I'd like some way to run cargo bootstrap before building the package that currently has this build script. But equally important is that this isn't run when building from crates-io, as a) pest_bootstrap is a repository-local non-published script and b) it would result in endless dependency recursion if published to crates-io.

A smaller improvement is a way to run --package pest_bootstrap without relying on this fragile "find the built binary", even if it still requires the user to build it beforehand. (I'm currently debugging it not working under tarpaulin. (As it turns out, I just missed that it does a clean first, so of course the file isn't going to be there.))

@alexcrichton
Copy link
Member

Thanks for the report! I don't think I quite follow though what this is a report about though? Is there an example of an error you're hitting? Or is this a feature request you'd like Cargo to add?

@CAD97
Copy link
Contributor Author

CAD97 commented Dec 10, 2018

Feature request for a really awkward use case. And probably a little XY problem obfuscated as well.

The actual "problem" is that it's not possible to invoke a cargo command that involves building a crate in the workspace from a buildscript, due to the lock on the target directory.

(Doing Command::new("cargo run --package unrelated").run() (modulo syntax, writing this on my phone) causes the new cargo instance to deadlock the parent one waiting for the directory lock (at least, when it's not already built).)

The direct solution is to give buildscripts some way to invoke cargo run without spawning a new process which needs to require the build directory lock.

The reason I want to use cargo run from within a buildscript is to break the bootstrap cycle for pest. This is only done when building from git sources; the output of this mess is vendored when published, but having the buildscript run automatically during development is ideal for not forgetting to do so.

This is a messy fringe use case, so I can understand if it's not something cargo wants to support. I'm already breaking two taboos: writing to the source directory in a buildscript (but only when building from git sources) and publishing files that aren't committed. This executable based "dynamic linking" works even if it isn't the prettiest thing in the world.

(I'm probably going to end up writing a blog post about the strategy, as ugly as it is, because it is, all things considered, a very good example of how bootstrapping works and doesn't take infinite time.)

TL;DR feature request: some way to cargo run a different crate in the workspace before building a crate when building from source control.

@alexcrichton alexcrichton added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Dec 11, 2018
@vitiral
Copy link

vitiral commented Jan 18, 2019

@CAD97 I have a bit of a "less fringe" use case in #6563 if you're interested.

@jethrogb
Copy link
Contributor

jethrogb commented Oct 9, 2019

You can work around this by specifying --target-dir

@ehuss ehuss added the A-build-scripts Area: build.rs scripts label Apr 6, 2020
@rafaelcaricio
Copy link

Good to see other people having similar issues. I followed the same approach in my project. Thanks @CAD97 !

@ghost
Copy link

ghost commented Aug 8, 2022

Any updates? I think this would be really useful. In my case, I want to integrate Wasm compilation into my regular build:

use std::process::Command;

fn main() {
    // cargo build --lib --target wasm32-unknown-unknown
    let _child = Command::new("cargo")
        .arg("build")
        .arg("--lib")
        .arg("--target")
        .arg("wasm32-unknown-unknown")
        .spawn()
        .expect("failed to start wasm build");
    // wasm-bindgen --target web --no-typescript --out-dir public target/wasm32-unknown-unknown/debug/wasm.wasm
    let _child = Command::new("wasm-bindgen")
        .arg("--target")
        .arg("web")
        .arg("--no-typescript")
        .arg("--out-dir public")
        .arg("target/wasm32-unknown-unknown/debug/wasm.wasm")
        .spawn()
        .expect("failed to start wasm-bindgen");
}

Which currently results in a deadlock too.

@CAD97
Copy link
Contributor Author

CAD97 commented Aug 9, 2022

Strangely enough I've not run into this problem myself recently, despite running cargo inside cargo; perhaps it's because I have CARGO_TARGET_DIR set? Or perhaps I've just conditioned myself to always set --target-dir within OUT_DIR. (Note that in your example there, .arg("--out-dir public" doesn't do what you want it to; you want .args(["--out-dir", "public"]) instead.)

@DaAitch
Copy link

DaAitch commented May 18, 2023

Ran into the same issue today: secondary build script does not terminate presumably because of the lock.

I have a parent project that should load .wasm files, generated by sub-projects. Everything should build from parent project cargo build.

The following example, as mentioned by @jethrogb, works for me.
Important: the target path has to be absolute as some downstream wasm binary has problems to find the .wasm.

// build.rs
// ..
let mut opts = wasm_pack::command::build::BuildOptions::default();
opts.release = !cfg!(debug_assertions);
opts.path = Some(path.clone());
opts.extra_options = vec![
    "--target-dir".to_owned(),
    format!("{}/different_target", env!("CARGO_MANIFEST_DIR")),
];
wasm_pack::command::build::Build::try_from_opts(opts)
    .unwrap()
    .run()
    .unwrap();
// ..

@weihanglo weihanglo added the S-triage Status: This issue is waiting on initial triage. label May 18, 2023
@kennykerr
Copy link

Ran into this with windows-rs where I need to build and run other tools from within the same workspace from build scripts. Seems like an oversight that you cannot easily do so. The workaround works ok, but it breaks down when you want to ensure a rebuild and cargo clean doesn't know to find the alternative target dir.

@weihanglo
Copy link
Member

build and run other tools from within the same workspace from build scripts

Can artifact-dependencies help with this matter?

@kennykerr
Copy link

A sort-of workaround is to set the target dir to a sub directory of the workspace target dir.

@0xffffharry
Copy link

Any progress?

I tried to use the above method to separate target_dir, but this will increase the disk space usage.

I think it is possible to introduce unlock command in build.rs to temporarily provide the lock to the cargo command in build.rs

example:

fn main() {
  // Unlock Cargo
  println!("cargo:unlock");

  // Run Cargo Command Here
  let output = std::process::Command::new("cargo")
    .args(["build", "--package", "other_package", "--release"])
    .output()
    .expect("Failed to run cargo build");

  // Lock Again
  println!("cargo:lock");
}

@epage
Copy link
Contributor

epage commented Dec 6, 2024

#8938 is a more general issue about build scripts calling into Cargo.

As was mentioned, it seems like artifact dependencies would be the solution to this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-scripts Area: build.rs scripts C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

No branches or pull requests