Skip to content

Conversation

namanlp
Copy link
Contributor

@namanlp namanlp commented Aug 28, 2025

This PR is continuation of my work on GSoC Project : Build Script Delegation

What does this PR try to resolve?

Through this PR, I want the user to be able to access each build script's OUT_DIR.

How to test and review this PR?

There is a feature gate multiple-build-scripts that can be passed via cargo-features in Cargo.toml. So, you have to add

cargo-features = ["multiple-build-scripts"]

Preferably on the top of the Cargo.toml and use nightly toolchain to use the feature

Then, you can access the OUT_DIR of given build script by accessing it like a compile time environment variable, for example:

const BUILD1_OUT_DIR: &'static str = env!("BUILD1_OUT_DIR");
const BUILD2_OUT_DIR: &'static str = env!("BUILD2_OUT_DIR");

fn main() {
    println!("{}", BUILD1_OUT_DIR);
    println!("{}", BUILD2_OUT_DIR);
}

@namanlp namanlp force-pushed the multiple-build-scripts-4 branch from 75ff71c to 0d5b7b0 Compare August 28, 2025 14:50
@rustbot rustbot added the A-build-scripts Area: build.rs scripts label Aug 28, 2025
@namanlp namanlp force-pushed the multiple-build-scripts-4 branch 3 times, most recently from c0d23d2 to 6c8bc08 Compare September 7, 2025 12:37
@rustbot rustbot added the A-build-execution Area: anything dealing with executing the compiler label Sep 7, 2025
Comment on lines 235 to 245
let env = self
.compilation
.extra_env
.entry(script_meta)
.or_insert_with(Vec::new)
.push(("OUT_DIR".to_string(), out_dir));
.or_insert_with(Vec::new);
env.push(("OUT_DIR".to_string(), out_dir.clone()));
env.push((out_dir_name, out_dir));
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In a separate commit maybe? Or should I update the latest commit itself?

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless the commit is ginormous (e.g. adding all of cargo add into cargo), I would generally update docs as part of commit that added the feature, like tests.

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 think only docs are remaining. Everything else should be done.

@namanlp namanlp force-pushed the multiple-build-scripts-4 branch 4 times, most recently from 5581004 to c9e3081 Compare September 11, 2025 20:34
@namanlp namanlp force-pushed the multiple-build-scripts-4 branch from c9e3081 to 08f83e3 Compare September 12, 2025 18:50
@namanlp namanlp force-pushed the multiple-build-scripts-4 branch from 65b6c7f to 4117277 Compare September 15, 2025 21:11
Comment on lines +911 to +916
anyhow::bail!(
"found duplicate build script file stem {}, \
but all build scripts must have a unique file stem",
stem
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we gather this into a HashMap<Name, Vec<Path>> and then iterate through and if Vec<Path> has more than one element, we then error? That would led us report all errors at once and report the full path for each conflicting build script so users can better understand what they need to change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I was just following how earlier validate_unique_name functions were written.

@namanlp namanlp force-pushed the multiple-build-scripts-4 branch from 4117277 to d48d421 Compare September 17, 2025 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-execution Area: anything dealing with executing the compiler A-build-scripts Area: build.rs scripts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants