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

Need a reliable way to get the target dir from the build script #9661

Open
kennykerr opened this issue Jul 7, 2021 · 63 comments
Open

Need a reliable way to get the target dir from the build script #9661

kennykerr opened this issue Jul 7, 2021 · 63 comments
Labels
A-build-scripts Area: build.rs scripts A-layout Area: target output directory layout, naming, and organization C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-propose-close Status: A team member has nominated this for closing, pending further input from the team

Comments

@kennykerr
Copy link

kennykerr commented Jul 7, 2021

The problem: I need to locate the target dir from the build script.

The solution: I'd love a CARGO_TARGET_DIR environment variable. There's clearly precedent for this as cargo doc depends on such a thing today.

@jyn514 suggested I reach out to the Cargo team for advice.

Currently the Windows crate has been using various hacky solutions that are problematic.

rust-lang/docs.rs#1444 (comment)

@kennykerr kennykerr added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Jul 7, 2021
@kennykerr
Copy link
Author

And grabbing the target_directory reported by "cargo metadata" doesn't work. If the crate is coming from crates.io then Cargo builds it in C:\Users\kenny\.cargo\registry\src\<random>\<some-crate> and the "target_directory" that cargo metadata reports is C:\Users\kenny\.cargo\registry\src\<random>\<some-crate>\target - what I need is the target directory for the crate the developer is building directly, not an intermediate target directly only known to the dependency.

@zeerooth
Copy link

zeerooth commented Jul 9, 2021

I've been recently working on a project where there's need to link the generated binary in user project to another directory and I couldn't find a reliable way to get the target directory either.
Currently, the code just guesses that the target directory is set as default <project dir>/target/ but if the project is located in a workspace or the build.target-dir is set to something else then this linking simply fails.
Such feature to get the target dir reliably would be greatly appreciated ❤️

@jyn514
Copy link
Member

jyn514 commented Jul 9, 2021

The solution: I'd love a CARGO_TARGET_DIR environment variable. There's clearly precedent for this as cargo doc depends on such a thing today.

This has cause and effect backwards. cargo doc works fine without CARGO_TARGET_DIR set in the environment. Docs.rs uses that as an input to cargo because the source directory is sandboxed, and cargo happens to pass it through to any build scripts it invokes.

@kennykerr
Copy link
Author

Yes, I since noticed that this is an optional input to Cargo:

https://doc.rust-lang.org/cargo/reference/environment-variables.html

Indeed, that's not what I want. I really need the target dir for the most dependent crate being directly built. I've tried numerous unsatisfying hacks. The one that works the most reliably is to read the first path from the PATH environment variable. This always appears to be:

<target_dir>\<profile>\deps

Although this only works on Windows. And I don't know how "supported" this is but I haven't found a better solution.

@ehuss
Copy link
Contributor

ehuss commented Aug 3, 2021

Can you provide more information on exactly why you need the target directory? Cargo is designed so that build scripts are intentionally constrained on what they should do, and their only interaction should be through the OUT_DIR.

@riverar
Copy link

riverar commented Aug 3, 2021

@ehuss Kenny is out this week but some scenarios are listed here microsoft/windows-rs#979 (comment).

I'll take a stab here at paraphrasing:

The windows crate has a build script that generates bindings for the developer, sourcing from its built-in metadata or local workspace metadata. Things get tricky when this metadata is delivered via multiple crates in the dependency graph.

For example, envision a developer's crate depending on windows, windows-fizz, and windows-buzz each with their own unique metadata. The build script (macro) running in the context of the developer's crate needs to have access to all the dependent crates' metadata to generate the developer's desired bindings.

One proposed change is to align the normal build script behavior with the cargo doc build script behavior. CARGO_TARGET_DIR would then exist and point to the developer's crate target directory and all dependent crates could write their metadata to that path for consumption.

One proposed change is to provide a pointer of sorts to the developer's crate target directory and all dependent crates could write their metadata to that path for consumption. Open to other ideas, of course.

@jyn514
Copy link
Member

jyn514 commented Aug 3, 2021

One proposed change is to align the normal build script behavior with the cargo doc build script behavior.

@riverar you're making the same mistake as #9661 (comment), cargo doc is not related to the target dir. Docs.rs just happens to set CARGO_TARGET_DIR and you've never tried setting it locally.

@riverar
Copy link

riverar commented Aug 3, 2021

@jyn514 Ah right, forgot. Thanks!

This doesn't affect the proposed change much. The bottom line is we'd like a pointer to the target directory for the aforementioned reasons.

@matklad
Copy link
Member

matklad commented Jan 25, 2022

One use-case I have is to store build-script's own caches. I can use a temporary directory as a scratch space (though it might still be nice to place this somewhere inside ./target rather than /tmp), I can use OUT_DIR for a real output, but I am not sure what's the right place for intermediate artifacts generated by the build script, which are not necessary output, but which I nonetheless want to preserve between invocations.

For example, if I am building some C code, I want to put intermediate .o files somewhere. Or, my specific use-case, I recursively invoke Cargo to compile some helpers to wasm. I mistakenly tried to use target dir for that, but that broke in all kinds of interesting ways down the line. Looking at how cc crate just puts .o into OUT_DIR, I think it's correct to use that for intermediate artifacts as well, but it isn't obvious from the docs. Will send a PR shortly (#10326).

matklad added a commit to matklad/cargo that referenced this issue Jan 25, 2022
Eg, storing `.o` files in OUT_DIR is ok! See
rust-lang#9661 (comment)
for some discussion.
nearprotocol-ci pushed a commit to near/nearcore that referenced this issue Jan 25, 2022
near-test-contracts builds some wasm contracts for use in testing. It
does so by recursively invoking `cargo` from `build.rs`. Before this
commit, we tried to re-use parent's `CARGO_TARGET_DIR` to figure out
where we should put the data. That was rather hacky, as cargo doesn't
expose that information to the build scripts in a reliable way:

rust-lang/cargo#9661 (comment)

Naturally, our hacks broken when when the `CARGO_TARGET_DIR` was set to
a relative path, because build.rs doesn't know where workspace root
lives.

The fix is to use `OUT_DIR` rather than `CARGO_TARGET_DIR`, which I
think is the supported way to this in the first place. Eg, the `cc`
crate uses `OUT_DIR` to store intermediate `.o` files, which I think
matches our use case pretty closely.
nearprotocol-ci pushed a commit to near/nearcore that referenced this issue Jan 25, 2022
near-test-contracts builds some wasm contracts for use in testing. It
does so by recursively invoking `cargo` from `build.rs`. Before this
commit, we tried to re-use parent's `CARGO_TARGET_DIR` to figure out
where we should put the data. That was rather hacky, as cargo doesn't
expose that information to the build scripts in a reliable way:

rust-lang/cargo#9661 (comment)

Naturally, our hacks broken when when the `CARGO_TARGET_DIR` was set to
a relative path, because build.rs doesn't know where workspace root
lives.

The fix is to use `OUT_DIR` rather than `CARGO_TARGET_DIR`, which I
think is the supported way to this in the first place. Eg, the `cc`
crate uses `OUT_DIR` to store intermediate `.o` files, which I think
matches our use case pretty closely.
bors added a commit that referenced this issue Jan 25, 2022
doc: it's valid to use OUT_DIR for intermediate artifacts

Eg, storing `.o` files in OUT_DIR is ok! See
#9661 (comment)
for some discussion.
near-bulldozer bot pushed a commit to near/nearcore that referenced this issue Feb 1, 2022
near-test-contracts builds some wasm contracts for use in testing. It
does so by recursively invoking `cargo` from `build.rs`. Before this
commit, we tried to re-use parent's `CARGO_TARGET_DIR` to figure out
where we should put the data. That was rather hacky, as cargo doesn't
expose that information to the build scripts in a reliable way:

rust-lang/cargo#9661 (comment)

Naturally, our hacks broken when when the `CARGO_TARGET_DIR` was set to
a relative path, because build.rs doesn't know where workspace root
lives.

The fix is to use `OUT_DIR` rather than `CARGO_TARGET_DIR`, which I
think is the supported way to this in the first place. Eg, the `cc`
crate uses `OUT_DIR` to store intermediate `.o` files, which I think
matches our use case pretty closely.
@kmod-midori
Copy link

+1 for this issue as we are distributing dynamic libraries that are hard to build (especially on Windows) internally (by automatically downloading them from a server), and these libs needs to be in the same directory as the final executable.

Without a reliable way to determine TARGET_DIR (e.g. the user might be cross compiling, or using non-default toolchains), cargo run will not work out-of-the-box.

@dureuill
Copy link

FWIW, cxx_build has the following module to determine target_dir: https://docs.rs/cxx-build/latest/src/cxx_build/lib.rs.html#1-473.

It expects the path stored in the OUT_DIR environment variable as input. I'm not certain it handles all possible situations though.

@Be-ing
Copy link

Be-ing commented Aug 22, 2022

I'm working on cxx-qt, a code generator build on top of cxx. I need a stable path to output generated C++ headers from build.rs for C++ build systems to be able to find them. I've seen what cxx_build does by walking up from OUT_DIR but that feels quite hacky to me. The least worst idea I've come up with is explicitly setting the CARGO_TARGET_DIR environment variable from the C++ build system so it gets passed to build.rs. I wish I did not have to do this though. Of course, this workaround isn't available if you're not working with an automated system that invokes cargo.

@Be-ing
Copy link

Be-ing commented Aug 23, 2022

On further thought, for cxx-qt (and cxx), perhaps setting the CARGO_TARGET_DIR environment variable isn't the best idea. I don't want to depend on unstable implementation details of the structure of the target directory. I think it would be better to use an environment variable specific to this purpose. Or better yet, a designated place to put exported build artifacts (#5457).

I think this is quite a different use case from the windows crate. Please correct me if I'm wrong, but I think what @kennykerr and @riverar are asking for is a way to pass data from the build script of a dependency to downstream build scripts. I'm not sure exposing the target directory to build scripts is a great way to do that either. cxx_build uses a public static mut struct together with links in Cargo.toml for this purpose... which also feels hacky, but I guess it's less hacky than relying on the layout of the target directory? 🤷

@sam0x17
Copy link

sam0x17 commented Jun 7, 2023

I think it is quite silly that at present it is difficult for a crate to discover where it is currently being built

@weihanglo weihanglo added A-build-scripts Area: build.rs scripts A-layout Area: target output directory layout, naming, and organization S-needs-team-input Status: Needs input from team on whether/how to proceed. labels Jun 7, 2023
@kennykerr
Copy link
Author

so are just needing a way to find the final artifact of a dependency, right?

@epage - Yes, although it may be more than one file - for example you typically need both server.dll and server.pdb...

@Be-ing - I am not talking about interop with other build systems or interop with C++. This is all just Rust code and crates. When its all tested and done, I may end up taking the DLL and shipping it to customers outside of the Rust universe but that's a separate issue that is outside the scope of this problem, at least for me.

I will check out artifact dependencies.

@kennykerr
Copy link
Author

I did some experimenting with artifact dependencies. Very promising! I can use the CARGO_CDYLIB_DIR_SERVER environment variable from the client's build script to find the server DLL but it's still quite a challenge to copy it to the directory where the client executable will be placed. If the client is a test I need to copy two directories up, but if the client is a binary then I need to copy it three directories up. It will probably also be different if I need to test with an alternative --target. It would be nice to have another build script environment variable for where the client executable will be run from for either cargo run -p client or cargo test -p client.

This is required for safe DLL loading on Windows via LoadLibraryExW(w!("server.dll"), 0, LOAD_LIBRARY_SEARCH_DEFAULT_DIRS).

@kennykerr
Copy link
Author

Should anyone else stumble upon this, here's an example.

  • server\Cargo.toml
[package]
name = "server"
edition = "2021"

[lib]
crate-type = ["cdylib"]
  • server\src\lib.rs
#[no_mangle]
unsafe extern "system" fn server() -> i32 {
    println!("hello server");
    123
}
  • client\Cargo.toml
[package]
name = "client"
edition = "2021"

[build-dependencies.server]
path = "../server"
artifact = "cdylib"

[dependencies.windows-sys]
version = "0.52"
features = ["Win32_Foundation", "Win32_System_LibraryLoader"]
  • client\build.rs
fn main() {
    let var = std::env::var("CARGO_CDYLIB_DIR_SERVER").expect("var not found");
    let from = std::path::Path::new(&var);

    let to = from
        .parent()
        .expect("parent not found")
        .parent()
        .expect("parent not found")
        .parent()
        .expect("parent not found");

    for entry in std::fs::read_dir(from).expect("from not found") {
        let from = entry.expect("entry not found").path();

        std::fs::copy(
            &from,
            to.with_file_name(from.file_name().expect("file name not found")),
        )
        .expect("copy failed");
    }
}
  • client\src\main.rs
fn main() {
    unsafe {
        use windows_sys::{core::*, Win32::System::LibraryLoader::*};

        let library = LoadLibraryExW(w!("server.dll"), 0, LOAD_LIBRARY_SEARCH_DEFAULT_DIRS);
        let address = GetProcAddress(library, s!("server")).expect("server not found");

        let server: extern "system" fn() -> i32 = std::mem::transmute(address);
        assert_eq!(server(), 123);
    }
}

Run as follows: cargo run -p client -Z bindeps

It's the client build script that continues to be problematic.

Is there a way to reliably determine where Cargo will place the client binary/test executable?

@pronebird
Copy link

What we do in debug builds is set the env variable and use it to locate the DLL, in release builds we rely on executable dir.

@cpick
Copy link

cpick commented May 5, 2024

@kennykerr maybe the dynamic library paths that can be set from your build.rs build script via cargo::rustc-link-search could work without having to copy the DLL for cargo run and cargo test?

@kennykerr
Copy link
Author

What path would I add?

@VorpalBlade
Copy link

I would very much rather not frame these discussions more generally; the concrete details for how a solution is wanting to be used and why can make a big difference.

@epage I would, I'm not interested in generating C++ headers, but I do want to generate man pages and shell completion files from build.rs of my crate, in a predictable location for external packaging tools (e.g. debian, arch linux, etc). OUT_DIR is a poor choice since it has a hash appended to it that change all the time. I don't want to bloat my binary by including the man page or completion generation code into the final binary.

There doesn't seem to be any good solution currently.

@epage
Copy link
Contributor

epage commented Jun 10, 2024

@VorpalBlade it sounds like you are disagreeing with me but I'm not sure on what. My comment was about us not talking abstractly about this problem but speaking in terms of concrete use cases, like your man page use case.

@VorpalBlade
Copy link

@epage Ah then I believe I misunderstood you as wanting to only concentrate on the specific case of generating headers.

This is what I currently do:

This is quite hacky on the PKGBUILD side:

  1. In prepare() I need to clean out the build dir: rm -rf target/release/build/${pkgname}-* in case we are not running on a clean build tree (as might happen when you develop the packaging)
  2. Later on in the package() phase (the bash functions are written in the order they are called for clarity) I locate the build dir using $(ls -d target/release/build/${pkgname}-*/out) (because the bash options are set a bit screwy in PKGBUILDs I needed that ls workaround to expand properly).

This breaks down if there are multiple out dirs as happens when you are developing and changing the package. During development it also leaves cruft in the build tree that is useless:

find target/debug/ -name paketkoll.1 | wc -l
26

That really shouldn't be there in so many copies. In fact I see no reason for cargo retaining all of that. Some sort of garbage collection would be in order for any packages that are part of your workspace (and thus are unlikely to be around in multiple versions at the same time and are also like to change a lot).

@epage
Copy link
Contributor

epage commented Jun 10, 2024

That really shouldn't be there in so many copies. In fact I see no reason for cargo retaining all of that. Some sort of garbage collection would be in order for any packages that are part of your workspace (and thus are unlikely to be around in multiple versions at the same time and are also like to change a lot).

GC is being tracked at #12633. Our initial focus is on global resources and then target directories as a whole. We'll then evaluate what doing it inside of a target-dir looks like. However, I think its unlikely that we'd leave only one instance present.

@VorpalBlade
Copy link

Fair enough, what about the rest of the use case though? As you can see, the current solution is hacky and suboptimal.

@epage
Copy link
Contributor

epage commented Jun 10, 2024

Yes, if I'm understanding correctly, this is like the C++ use cases where they are wanting to have build.rs generate and stage final artifacts. I did a quick search in our backlog and #13663 looks to be a more specific issue for that and I recommend discussion move over there.

@epage
Copy link
Contributor

epage commented Jun 10, 2024

I propose to the Cargo team that we close this issue with the encouragement that people move their discussion to more specific, use-case focused issues, creating them where needed.

As mentioned in #9661 (comment), there are fundamental problems with build.rss accessing CARGO_TARGET_DIR and we need to look for what solutions align with where Cargo is at and where it is going.

Tracked use cases:

Use cases where I could not find an Issue tracking it:

@epage epage added S-propose-close Status: A team member has nominated this for closing, pending further input from the team and removed S-needs-team-input Status: Needs input from team on whether/how to proceed. labels Jun 10, 2024
@alerque
Copy link
Contributor

alerque commented Jun 10, 2024

Having watched this issue and a number of the related ones for a while, that sounds to me like a plan that will effectively make sure this is not addressed. When the individual use cases discuss this topic the answer is usually along the lines of "this may affect other things too so we can't change it". Having at least one meta issue open tracking the multiple use cases and how they interact seems like the only way likely to actually reach a conclusion.

@epage
Copy link
Contributor

epage commented Jun 10, 2024

In looking at the use cases I just summarized, they are so unrelated to each other that I don't see how having a tracking issue for them would help with coordination across them. As for your concern, I can't speak too much to that without examples as that failure case does not ring a bell.

@sam0x17
Copy link

sam0x17 commented Jun 11, 2024

The main issue gives us a way to consolidate power and make it clear a number of different use cases could all benefit from the same improved functionality in build.rs. Dividing these obfuscates the collective community need for an improvement here, and will make it much more difficult to coordinate across different use-cases as a solution is designed

@iago-lito
Copy link

I think I've just stumbled here with another use case.. ?

My cargo project is going to be distributed as an executable, as my colleagues don't understand or need rust or libs.
Documentation for this executable takes the form of a mdBook within a doc/ folder next to src/ and target/.
I'm using mdbook-cmdrun to integrate error messages / help messages etc. into my documentation by producing them instead of duplicating them.
To get further, I wish to leverage rustdoc-types to extract other kind of information about my project for integration within the doc/: constant values, docstrings..
To this end, I'm writing some src/bin/extrac_doc.rs that would use rustdoc-types to parse the target/doc/myproject.json file produced by rustdoc --output-format json, for eventual call by mdbook-cmdrun for integration within my doc.

I think code in this later binary needs a reliable way to retrieve a valid path to the target/ dir.. ?

@epage
Copy link
Contributor

epage commented Sep 27, 2024

For a use case like that, I assume what cargo semver-checks does would work for you: run cargo metadata to get the target-dir and then pass that to cargo doc with --target-dir.

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 A-layout Area: target output directory layout, naming, and organization C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-propose-close Status: A team member has nominated this for closing, pending further input from the team
Projects
None yet
Development

No branches or pull requests