rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.63k stars 12.74k forks source link

There is no cross-platform way to use `include_*!` #75075

Open Shnatsel opened 4 years ago

Shnatsel commented 4 years ago

Build scripts place their output in a folder created by Cargo. The path to it is passed in the environment variable OUT_DIR both to the build script and the main code. Since the file is placed in the directory, we need to add the path separator and then the name of the file. There seems to be no way to call include_bytes! with a platform-agnostic path separator, inserting / or \ depending on the host OS.

This works but is not portable: include_bytes!(concat!(env!("OUT_DIR"), "/myfile"));

This doesn't work: include_bytes!(concat!(env!("OUT_DIR"), std::path::MAIN_SEPARATOR, "myfile")); because MAIN_SEPARATOR is not a literal, and concat! only eats literals.

I've tried to to assemble a String in const fn, but that doesn't work either because String::push() requires a mutable borrow which are unsable in const fn.

#[cfg(unix)] and #[cfg(windows)] sort of work, but break on cross-compilation because these cfg attributes look at the target OS, and we need to add a separator that works on the host OS.

It's weird that this is either impossible to accomplish or the way to do so is very obscure. I'd expect this to be a relatively common operation.

rustc --version --verbose:

rustc 1.46.0-beta.2 (6f959902b 2020-07-23)
binary: rustc
commit-hash: 6f959902b3103c49ca981fbc01871589c3498489
commit-date: 2020-07-23
host: x86_64-unknown-linux-gnu
release: 1.46.0-beta.2
LLVM version: 10.0
retep998 commented 4 years ago

Can you use a build script to use cargo:rustc-env to set your own environment variable that contains the full path using the host specific path separators and whatnot?

Shnatsel commented 4 years ago

I'll try it. Thanks for pointing this out!

rodrimati1992 commented 4 years ago

You can use cfg-ed macros to pass conditional string literals to concat! (I can't say if this code specifically is good enough for cross-platform support)

#[cfg(not(windows))]
macro_rules! main_separator{
    ()=>{"/"}
}

#[cfg(windows)]
macro_rules! main_separator{
    ()=>{r#"\"#}
}

fn main(){
    println!("{}", include_str!(concat!(".", main_separator!(), "main.rs" )));
}
mati865 commented 4 years ago

/ should work on Windows, if it doesn't then that is the issue.

Shnatsel commented 4 years ago

The build.rs trick worked, thanks!

You can use cfg-ed macros to pass conditional string literals to concat!

I did that, but that only look at the target platform and should break when the host platform is different (i.e. when cross-compiling).

/ should work on Windows, if it doesn't then that is the issue.

include_bytes! is explicitly documented to use platform-dependent separators. The documentation doesn't say anything about / working on Windows. If include_bytes! allows / as the cross-platform separator, it would be great to document that.

retep998 commented 4 years ago

/ should work on Windows, if it doesn't then that is the issue.

On Windows if OUT_DIR for whatever reason uses \\?\ paths then using / as the separator will break. While it is nice to support / in paths as much as we can, we should be pushing to ensure code always uses the correct separator for the platform.

mati865 commented 4 years ago

Oh that's a bummer, I thought the same parsing here would apply as for std::path::Path which allows to use both separators on Windows.

retep998 commented 4 years ago

Well it is the same parsing. Concatenating foo/bar onto a Path that starts with \\?\ on Windows also runs into the exact same issue.

CryZe commented 4 years ago

Would it maybe make sense for the include_*! macros to allow syntax like include_bytes!(env!("OUT_DIR") / "myfile")?

danielhenrymantilla commented 4 years ago

I did that, but that only look at the target platform and should break when the host platform is different (i.e. when cross-compiling).

//! `build.rs`

#[cfg(windows)]
const HOST_FAMILY: &str = "windows";

#[cfg(unix)]
const HOST_FAMILY: &str = "unix";

fn main ()
{
    #[cfg(any(windows, unix))] {
        println!("cargo:rust-cfg=host_family={}", HOST_FAMILY);
    }
}

and then:

#[cfg(host_family = "windows")]
macro_rules! PATH_SEPARATOR {() => (
    r"\"
)}
#[cfg(not(host_family = "windows"))]
macro_rules! PATH_SEPARATOR {() => (
    r"/"
)}

include_bytes!(concat!(
    env!("OUT_DIR"), PATH_SEPARATOR!(), "my_file"
));
briansmith commented 3 years ago

include_bytes! is explicitly documented to use platform-dependent separators. The documentation doesn't say anything about / working on Windows. If include_bytes! allows / as the cross-platform separator, it would be great to document that.

I think the definition of include_bytes! should be changed to require that (1) the path must be relative, (2) include_bytes! will interpret '/' is the directory separator and translate as needed, (3) quotes ('"') and other similar escaping will be rejected with a compiler error.

therealbnut commented 3 years ago

I think the definition of include_bytes! should be changed to require that (1) the path must be relative, (2) include_bytes! will interpret '/' is the directory separator and translate as needed, (3) quotes ('"'`) and other similar escaping will be rejected with a compiler error.

1) I don't think changing them to be relative is a good idea: you need to use it with things like concat!(env!("OUT_DIR"), ...) for things like build scripts.

2) You might be able to solve this more cleanly if macros like include_bytes were variadic, and join their arguments with an appropriate separator, for example:

include_bytes!(env!("OUT_DIR"), "some_dir", "some_file.txt")

3) Rejecting invalid path characters sounds reasonable.

AZMCode commented 3 years ago

Perhaps to resolve this in an interesting manner, implement the second suggestion above by @therealbnut into a new "path!()" macro that resolves a set of path segments into a platform-specific path at compile time.

Edit: For example include_str!(path!("my","new","path")) Would be resolved to include_str!("my/new/path") on Linux and include_str!("my\\new\\path") on Windows Kinda like the concat!() macro does, but with extra path functionality.

Edit 2: It appears someone has beat me to the punch in the meantime, just with a different syntax

Edit 3: Just realized this would give the appearance that nested macros are a thing, which they most certainly are not. Perhaps not the greatest idea in hindsight. Maybe this behavior could be made into a new set of macros to mirror the old ones? include_path_* perhaps

AZMCode commented 3 years ago

For those just looking to use a cross-platform version of this, or those looking to better understand my proposal above, Here's a Crate

est31 commented 3 years ago

Just realized this would give the appearance that nested macros are a thing, which they most certainly are not.

Why are they not? include_str!(concat!(...)) is a pretty common pattern. Now it would be include_str!(path!(...)).

AZMCode commented 3 years ago

As far as I am aware these need special code to work, as all tokens inside a macro invocation need to be parsed by the macro itself, including other macro invocations, and I'm not sure giving the appearance that this pattern can work with any other combination of macros is a good idea.

ssokolow commented 3 years ago

I'd argue that the damage has already been done by include_str!(concat!(...)) and any confusion that would be caused by include_str!(path!(...)) is already present.

AZMCode commented 3 years ago

Not a reason to cause further confusion tbh. The better we can do the less confusion there'll be overall.

Edit: I feel like this is a whole separate issue we've stumbled upon here. Perhaps this should be addressed in a separate thread. For now, both proposals should be considered by the Rust team.

est31 commented 3 years ago

@AZMCode you have a point. Apparently the include_* macros work differently, as they expand their arguments before evaluating I think? Not sure. At least local tests show include_* not having issues with being passed a macro instead of a str expr. Anyways, I personally favour the proposal from above.

I think a path macro wouldn't be helpful for multiple reasons, at least in this instance. First, path as the bug report mentions, needs to have the host path, not the target path system. For cross compilation to work, the two may not be mixed up. Second, it would be most useful for users if the path macro emitted some object that is typed, like a Path or a PathBuf, instead of just a str literal. However, this would mean that the path macro is only available on the std crate, and not available in core, which the include_* family is. Also, the macro machinery can only deal with str slices. Now, it could probably "steal" the str slice from the expression, but I think this is a weird hack.

It seems to me the problem domain that the include_* macros need to solve is so different from what path is solving that just expanding the family's syntax to also accept multiple params that are then concatenated via the well known methods is the best thing.

AZMCode commented 3 years ago

Sounds reasonable enough, though I support commas over slashes for the separators for now. Edit: Perhaps this functionality of expanding macro arguments should be made available to proc_macros so nesting macro invocations becomes possible generally. Edit 2: Might just open an issue about that if one hasn't been already

est31 commented 3 years ago

@AZMCode it's explained here: https://rustc-dev-guide.rust-lang.org/macro-expansion.html#eager-expansion

est31 commented 3 years ago

Also: https://github.com/rust-lang/rfcs/pull/2320

Edit: I think it's getting a bit off topic though. This thread is not about eager expansion as a general language feature.

AZMCode commented 3 years ago

Understandable.

Be-ing commented 2 years ago

I have an alternative idea. What if there was an option to tell Cargo to use / path separators for all its path environment variables like OUT_DIR and CARGO_MANIFEST_DIR?