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

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

Open
Shnatsel opened this issue Aug 2, 2020 · 24 comments
Open

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

Shnatsel opened this issue Aug 2, 2020 · 24 comments
Labels
A-cross Area: Cross compilation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Shnatsel
Copy link
Member

Shnatsel commented Aug 2, 2020

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
Copy link
Member

retep998 commented Aug 2, 2020

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
Copy link
Member Author

Shnatsel commented Aug 2, 2020

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

@rodrimati1992
Copy link
Contributor

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
Copy link
Contributor

mati865 commented Aug 2, 2020

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

@Shnatsel
Copy link
Member Author

Shnatsel commented Aug 2, 2020

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.

@Shnatsel Shnatsel changed the title No way platform-independent way to include_bytes! data generated by a build script The way to use include_bytes! in a cross-platform way is not discoverable Aug 2, 2020
@retep998
Copy link
Member

retep998 commented Aug 2, 2020

/ 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
Copy link
Contributor

mati865 commented Aug 2, 2020

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
Copy link
Member

retep998 commented Aug 2, 2020

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
Copy link
Contributor

CryZe commented Aug 2, 2020

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

@danielhenrymantilla
Copy link
Contributor

danielhenrymantilla commented Aug 10, 2020

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);
    }
}
  • (Hopefully this ends up being an official cfg)

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"
));

@Shnatsel Shnatsel changed the title The way to use include_bytes! in a cross-platform way is not discoverable There is no cross-platform way to use include_bytes! Aug 11, 2020
@camelid camelid changed the title There is no cross-platform way to use include_bytes! There is no cross-platform way to use include_*! Oct 20, 2020
@camelid camelid added A-cross Area: Cross compilation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Oct 20, 2020
@briansmith
Copy link
Contributor

briansmith commented Nov 19, 2020

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
Copy link

therealbnut commented Jul 22, 2021

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")
  1. Rejecting invalid path characters sounds reasonable.

@AZMCode
Copy link

AZMCode commented Sep 18, 2021

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
Copy link

AZMCode commented Sep 19, 2021

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
Copy link
Member

est31 commented Sep 19, 2021

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
Copy link

AZMCode commented Sep 19, 2021

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
Copy link

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
Copy link

AZMCode commented Sep 20, 2021

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
Copy link
Member

est31 commented Sep 20, 2021

@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
Copy link

AZMCode commented Sep 20, 2021

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
Copy link
Member

est31 commented Sep 20, 2021

@est31
Copy link
Member

est31 commented Sep 20, 2021

Also: rust-lang/rfcs#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
Copy link

AZMCode commented Sep 20, 2021

Understandable.

@Be-ing
Copy link
Contributor

Be-ing commented Mar 9, 2022

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cross Area: Cross compilation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests