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

Windows \\?\ verbatim paths break idiomatic use of OUT_DIR and include! #13919

Open
dtolnay opened this issue May 16, 2024 · 5 comments
Open
Labels
A-build-scripts Area: build.rs scripts C-bug Category: bug O-windows OS: Windows S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@dtolnay
Copy link
Member

dtolnay commented May 16, 2024

Problem

When Cargo is invoked on Windows in either of the following 2 scenarios, and possibly others:

  • a current directory that begins with \\?\
  • --manifest-path that begins with \\?\

then the following widespread idiom for including build.rs-generated code does not work.

include!(concat!(env!("OUT_DIR"), "/repro.rs"));

Because / is not a directory separator in verbatim paths, this fails to compile with an error like this:

error: couldn't read \\?\D:\a\repro\repro\target\debug\build\repro-59592f29f689633e\out/repro.rs: The filename, directory name, or volume label syntax is incorrect. (os error 123)
 --> src\lib.rs:3:1
  |
3 | include!(concat!(env!("OUT_DIR"), "/repro.rs"));
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: this error originates in the macro `include` (in Nightly builds, run with -Z macro-backtrace for more info)

Normally in Windows paths / works just as well as \, but from Maximum Path Length Limitation: File I/O functions in the Windows API convert "/" to "\" as part of converting the name to an NT-style name, except when using the "\\?\" prefix as detailed in the following sections.

Steps

The problem can be reproduced in GitHub Actions with the following source code.

# Cargo.toml

[package]
name = "repro"
version = "0.0.0"
// build.rs

fn main() {
    let out_dir = std::env::var_os("OUT_DIR").unwrap();
    let repro = std::path::Path::new(&out_dir).join("repro.rs");
    std::fs::write(repro, "").unwrap();
}
// src/lib.rs

include!(concat!(env!("OUT_DIR"), "/repro.rs"));
# .github/workflows/ci.yml

name: CI
on: [push]
jobs:
  repro:
    runs-on: windows-latest
    steps:
      - uses: actions/checkout@v4
      - uses: dtolnay/rust-toolchain@nightly

      - run: pwd
      - run: echo ${{github.workspace}}
      - run: echo \\?\${{github.workspace}}

      - run: cargo check --verbose && cargo clean

      - run: cargo check --manifest-path \\?\${{github.workspace}}\Cargo.toml --verbose
        if: always()

      - run: cargo check --verbose
        working-directory: \\?\${{github.workspace}}
        if: always()

The first cargo check will succeed. The second two will each fail as shown above.

Possible Solution(s)

I hope that someone can make a successful proposal for something like:

include!(concat!(env!("OUT_DIR") / "repro.rs"));

which would work in a cross-platform manner by using the correct path separator for rustc's host OS.

Until then, #13914 (comment) proposes a possible way that Cargo could work around this issue some of the time.

Internally to Cargo, we exclusively work with regular paths and not verbatim paths. It sounds like the problem is that we accept paths from users in a couple of places which may be verbatim paths.

Should we instead run dunce on those paths so they are no longer verbatim? This would solve it for Cargo and all other cases.

(https://docs.rs/dunce/1/dunce/fn.simplified.html)

This is only a partial fix because not all verbatim paths can be converted to something without \\?\. If a path is longer than 260 characters, \\?\ is the only way to refer to it.

#13914 shows another possible workaround, but that one amounts to boiling the ocean to replace the OUT_DIR/include idiom with a substantially more verbose one.

Notes

Meta-issue: #9770

@dtolnay dtolnay added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels May 16, 2024
@epage epage added O-windows OS: Windows A-build-scripts Area: build.rs scripts labels May 16, 2024
@ChrisDenton
Copy link
Member

ChrisDenton commented May 16, 2024

If a path is longer than 260 characters, \\?\ is the only way to refer to it.

This is only true when calling OS APIs (and only if longpathaware isn't enabled). Paths can be stored and passed around as a user path then converted to a verbatim path at the point of use. The std will even do the latter for you.

@weihanglo
Copy link
Member

This is only true when calling OS APIs (and only if longpathaware isn't enabled). Paths can be stored and passed around as a user path then converted to a verbatim path at the point of use. The std will even do the latter for you.

Since Cargo has set longpathaware and std handles for us, does it mean it's generally safe for cargo to take advantage of non-UNC paths via dunce?

@ChrisDenton
Copy link
Member

Hm, I don't think that's used for build scripts? But in any case, when using Rust's std APIs it should work out fine either way.

The issue with dunce is that it intentionally won't work in a number of cases, including for long paths and \\?\UNC\ style paths. Nushell ran into issues with this so I created omnipath which they've been using successfully for awhile now. Essentially the philosophy of omnipath is to have a distinction between "user" paths and "verbatim" paths where converting between the two is always fine so long as it's possible to round-trip (e.g. converting to verbatim->user->verbatim will return the same path). The only downside is that it needs to use OS APIs to be sure it can round-trip the path. So it is slower.

@ChrisDenton
Copy link
Member

I would just add that for a long time now PathBuf::push, etc should work fine if pushing like a/b to a verbatim path. It will convert the separator for you. It's just naively appending strings which doesn't work. The problem, as the OP says, is that there isn't a path API for include!.

@weihanglo weihanglo added S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. and removed S-triage Status: This issue is waiting on initial triage. labels Jun 19, 2024
@weihanglo
Copy link
Member

Some other workarouds appear in rust-lang/rust#75075.

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-bug Category: bug O-windows OS: Windows S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

4 participants