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

cargo fmt failing in Windows with MAX_PATH problem. can you add manifest with longPathAware? #5975

Open
kazuhiko-kikuchi opened this issue Dec 7, 2023 · 8 comments
Labels
needs-mcve needs a Minimal Complete and Verifiable Example os-windows

Comments

@kazuhiko-kikuchi
Copy link

longPathAware manifest item is documented here.

https://learn.microsoft.com/en-us/windows/win32/sbscs/application-manifests#longPathAware

and embedding manifest resource in build, can be with embed-resource crate. https://crates.io/crates/embed-resource

@ytmimi ytmimi added os-windows needs-mcve needs a Minimal Complete and Verifiable Example labels Dec 7, 2023
@ytmimi
Copy link
Contributor

ytmimi commented Dec 7, 2023

@kazuhiko-kikuchi thanks for reaching out. Any chance you can provide us with some more detail. What version of cargo fmt are you using? You can run cargo fmt --version to figure that out. Additionally, can you put together a minimal reproducible test case, though if this is an windows OS issue then I don't know if there's much we can do on the rustfmt side.

@kazuhiko-kikuchi
Copy link
Author

$ cargo fmt --version
rustfmt 1.6.0-stable (79e9716c 2023-11-13)

We use deep directory structure. cargo fmt succeeded in Linux, but Windows in fails.

$ cargo fmt
ファイル名または拡張子が長すぎます。 (os error 206)
This utility formats all bin and lib files of the current crate using rustfmt.

Usage: cargo fmt [OPTIONS] [-- <rustfmt_options>...]

Arguments:
<snip>

ファイル名または拡張子が長すぎます。 (os error 206) is os error message for MAX_PATH problem.

@PatchMixolydic
Copy link
Contributor

ファイル名または拡張子が長すぎます。 (os error 206)

To aid investigation, the English version of this error message seems to be "The file name or extension is too long."

@ytmimi
Copy link
Contributor

ytmimi commented Dec 9, 2023

@kazuhiko-kikuchi are you able to link to the project where you're trying to run cargo fmt? If not, are you able to put together a minimal test case that we can use to reproduce the issue that you're running into.

@RossSmyth
Copy link

RossSmyth commented Feb 27, 2024

@ytmimi This was actually found with rust-lang/miri#3317 recently as well. It is most likely unrelated to MAX_PATH. Though it possible for that to be an issue.

The issue is the following:

  1. The miri repo runs rustfmt on every .rs file in the repo. This includes tests and every crate.
  2. Windows has a hard limit on the length of CLI arguments + environment variables of 32k
  3. The way miri's formatting tool currently works is collecting every file in the repo and throwing it at rustfmt
  4. This leads to the formatting tool not working on Windows (error with The filename or extension is too long. (os error 206)), as the total CLI argument length is around 65k

Note that the miri repo currently has 897 Rust files.

It looks like cargo-fmt is also susceptible to this issue as well.

let mut command = rustfmt_command()
.stdout(stdout)
.args(files)
.args(["--edition", edition.as_str()])
.args(fmt_args)
.spawn()
.map_err(|e| match e.kind() {
io::ErrorKind::NotFound => io::Error::new(
io::ErrorKind::Other,
"Could not run rustfmt, please make sure it is in your PATH.",
),
_ => e,
})?;

I have a fix up for the miri tool, which batches rustfmt calls by argument length.
https://github.com/rust-lang/miri/blob/c72e73f4deab94cb433e64b4c863124eb893ef6c/miri-script/src/util.rs#L153-L229

Basically the "minimal" test case is:

  1. Have a Windows machine
  2. Generate a whole bunch of files to throw at rustfmt so that the arguments + env var length goes above (2^15 - 1)
  3. Run cargo-fmt

Other OS's have limits as well, but they are much higher.

It looks like rustc bootstrap got around it by only formatting a few files at a time in parallel to walking the repo directories.
https://github.com/rust-lang/rust/blob/9afdb8d1d55f7ee80259009c39530d163d24dc65/src/bootstrap/src/core/build_steps/format.rs#L279-L317

Interestingly this is actually a decent time to bring this up because it naturally leads to parallel formatting if done at the cargo-fmt level.

@kazuhiko-kikuchi
Copy link
Author

I know cargo have longPathAware manifest now.

rust-lang/cargo#7986

@RossSmyth
Copy link

RossSmyth commented Feb 28, 2024

Yeah, though cargo-fmt is a seperate binary. Since you have not provided a minimal reproducer there are potentially two different issues here:

  1. It could be longPathAware
  2. It could be passing too many characters via CLI

They have the same error from Windows it is impossible to disambiguate them from the information provided. But both should be mitigated as they are both potential issues.

@kazuhiko-kikuchi
Copy link
Author

Since you have not provided a minimal reproducer

sorry, I work on closed source repo. Can not to provide it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-mcve needs a Minimal Complete and Verifiable Example os-windows
Projects
None yet
Development

No branches or pull requests

4 participants