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

Allow "--extern" rustc flags in build script #14065

Open
notdanilo opened this issue Jun 13, 2024 · 10 comments
Open

Allow "--extern" rustc flags in build script #14065

notdanilo opened this issue Jun 13, 2024 · 10 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

@notdanilo
Copy link

notdanilo commented Jun 13, 2024

Problem

I have a significant goal of integrating the whole pip ecosystem to Cargo only by adding:

Cargo.toml

[package.metadata.pip]
diffusers = "0.28.2"

build.rs

fn main() {
    todo!("pip install");
    todo!("parse pip package");
    todo!("generate rust crate");
    todo!("build rust crate");

    // Include it in the current crate
    println!("cargo:rustc-flags=--extern diffusers=CUSTOM_FOLDER/diffusers//target/debug/libdiffusers.rlib");
}

But this isn't allowed due to:

error: Only `-l` and `-L` flags are allowed in build script of `pip-example v0.1.0 (D:\dev\sensorial\systems\pip-example)`: `--extern pip_crate=external/pip-crate//target/debug/libpip_crate.rlib`

Proposed Solution

Allow --extern flag in build script.

Notes

Even though I can create an external tool (like cargo-pip) to add the required flags to RUSTFLAGS, this would not be orthogonal. If I want to implement the same feature to a different registry besides pip (say conda), how could I mix them in the project?

cargo pip build // would make [package.metadata.pip] dependencies available
cargo conda build // would make [package.metadata.conda] dependencies available
But how to make both [package.metadata.conda] and [package.metadata.pip] available?

Another important point is that the goal is to make pip dependencies seamlessly available without the need of a third-party tool. Making it possible in build.rs guarantees easy redistribution on crates.io

Tasks

No tasks being tracked yet.
@notdanilo notdanilo added 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. labels Jun 13, 2024
@epage epage added the A-build-scripts Area: build.rs scripts label Jun 13, 2024
@notdanilo
Copy link
Author

As an additional note:

I considered publishing all the generated crates from pip on crates.io, but that would pollute crates.io with lots of automatically generated code, which I believe is undesired/unnecessary.

The proposed solution above is the ideal way to avoid publishing the automatically generated crates to crates.io.

My ultimate goal is to enrich Rust's ecosystem with AI libraries from Python.

@epage
Copy link
Contributor

epage commented Jun 14, 2024

Could you clarify a little more on what you mean in the steps "generate rust crate" and "build rust crate"?

@notdanilo
Copy link
Author

Could you clarify a little more on what you mean in the steps "generate rust crate" and "build rust crate"?

I can, but these steps aren't important to the issue.

These steps will be done with my binding generator https://github.com/sensorial-systems/ligen

It will:

  1. Parse the Python code to an intermediate representation listing all the types and exposeable interfaces (public functions and methods)
  2. Generate a new Cargo library with pyo3 bindings in an arbitrary folder
  3. Build the Cargo project to make a rlib file which can then be used by the --extern parameter

@epage
Copy link
Contributor

epage commented Jun 14, 2024

I can, but these steps aren't important to the issue.

Understanding use cases is important to make sure we are solving the right problem.

This Issue seems to be suggesting a specific solution to #8709. Generally, we encourage Issues to focus on problems and discuss trade offs on alternative solutions on those issues. In that context, I would be inclined to close this in favor of #8709 with the suggestion of you posting this idea as an alternative solution.

Is there something I'm missing in that?

@notdanilo
Copy link
Author

The ability to programmatically set dependencies in a project through build.rs is of extreme importance for codegen tools.

#8709 doesn't solve this issue as its workaround requires a pre-Cargo process to codegen the crate and build it to make available through

[dependencies]
example = { path = "path/to/example" } 

The proposed solution here would make Cargo more extensible.

@notdanilo
Copy link
Author

I will start a PR for the proposed solution soon.

@notdanilo
Copy link
Author

I also would like to understand why not all rustc flags are supported in build.rs and config file? Why it's more limited than using RUSTFLAGS?

I see that the current implementation has two properties in BuildOutput holding '-L' and '-l' values, respectively and thus the limitation. But why wasn't it generalized to get all flags?

@weihanglo
Copy link
Member

I might be wrong. From my observation the cargo::rustc-flags instruction predates 1.0 of the Rust/Cargo (see cargo#792, specifically this commit), and now there are equivalent like rustc-link-lib and rustc-link-search. The existence of cargo::rustc-flags might just be a wrong abstraction slipping into 1.0.

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

3 participants