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.

Sign up for GitHub

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

Jump to bottom

Fail to format nested mod with relative path on windows #1873

Closed
BusyJay opened this issue Aug 11, 2017 · 9 comments
Closed

Fail to format nested mod with relative path on windows #1873

BusyJay opened this issue Aug 11, 2017 · 9 comments
Labels
bug Panic, non-idempotency, invalid code, etc. os-windows

Comments

@BusyJay
Copy link
Contributor

BusyJay commented Aug 11, 2017

Following snippets will fail to format on windows.

#[path = "../mod_b.rs"]
mod b;

rustfmt-nightly will complain

error: couldn't read "\\?\C:\src\../mod_b.rs": The filename, directory name, or volume label syntax is incorrect. (os error 123)

That's because rustfmt needs to canonicalize the source file paths first to be able to compare with file_lines configuration later. It works fine on linux. But on windows, canonicalize will return a UNC path, which doesn't accept relative join any more.

/cc rust-lang/rust#42869
@nrc nrc added the bug Panic, non-idempotency, invalid code, etc. label Aug 13, 2017
@nrc
Copy link
Member

nrc commented Aug 13, 2017

Do you know how we should fix this? I feel like there must be some way to get a canonical path that will work on Windows.

@nrc nrc added this to the impl period milestone Sep 18, 2017
@nrc nrc added the os-windows label Sep 18, 2017
@nrc nrc removed this from the impl period milestone Jan 11, 2018
@CasualX
Copy link

CasualX commented Aug 15, 2018 •
edited
Loading

After a post on reddit to encourage people to try out rustfmt in anticipation of a 1.0 release, I hit the same problem. Is this a blocking issue for a 1.0 release?

@povilasb
Copy link

I'm still seeing this issue :/

povilasb added a commit to povilasb/p2p that referenced this issue Nov 27, 2018
@povilasb
fix/ci: skip rustfmt on Windows
e5ee849 
We hit the rustfmt bug: rust-lang/rustfmt#1873 .
Currently, rustfmt fails to format when path attribute has relitive
paths.
Usually, our code does not have too much windows specific code and in
this case there's no such code at the moment. So, it's the easiest fix
is to disable rustfmt on Windows for now hoping for the fix in the future.
ustulation pushed a commit to ustulation/p2p that referenced this issue Nov 27, 2018
@povilasb @ustulation
More tests (#94)
c7f99c7 
* fix/docs: fix mio types we use

* test/udp_rendezvous_server: add integration test;
  also exposed UdpRendezvousClient (not as a crate API though,
  only for internal modules for testing).

* make UdpRendezvousServer return it's listen address (in case
  someone specifies port 0 for it)

* fixed UdpRendezvousServer on Windows where the client tried to
  connect with the server using 0.0.0.0 IP address.

* fix/ci: skip rustfmt on Windows

We hit the rustfmt bug: rust-lang/rustfmt#1873 .
Currently, rustfmt fails to format when path attribute has relative
paths.

Usually, our code does not have too much windows specific code and in
this case there's no such code at the moment. So, it's the easiest fix
is to disable rustfmt on Windows for now hoping for the fix in the future.
@rivy
Copy link

rivy commented Feb 5, 2019

I'm hitting this issue for coreutils (uutils/rust.coreutils) while attempting to add additional CI.

cc: @rivy

@topecongiro
Copy link
Contributor

IIUC we need an equivalent to std::filesystem::absolute from C++17.

@topecongiro topecongiro mentioned this issue Feb 6, 2019
Closed
@scampi scampi mentioned this issue Mar 12, 2019
Open
@Xanewok
Copy link
Member

Xanewok commented Apr 9, 2019

FWIW I just tested the std::filesystem::absolute under

  • clang version 6.0.0-1ubuntu2 (tags/RELEASE_600/final) (Ubuntu 18.04)
  • cl.exe 19.20.27508.1 (MSVC v142, Windows 10.0.17763)

and paths like /home/xanewok/../xanewok and \\?\C:\Users\Xanewok\t\..\t were not modified - this was only done by std::filesystem::canonical.

I'd also like to mention that std::fs::canonicalize seems to be wrong as it can't handle normalizing UNC paths with relative components (throws an error), while it's definitely possible in Windows (e.g. File Explorer can access such paths correctly). For Windows we're calling GetFinalPathNameByHandleW whereas we might want to explore calling PathCanonicalizeW instead, before passing the function later on.

@scampi
Copy link
Contributor

scampi commented May 27, 2019

I'd also like to mention that std::fs::canonicalize seems to be wrong as it can't handle normalizing UNC paths with relative components (throws an error), while it's definitely possible in Windows (e.g. File Explorer can access such paths correctly).

@Xanewok If possible, could you open an issue on the rust repo about this ? I believe you'd explain what is wrong better than me! Thanks

@scampi
Copy link
Contributor

scampi commented May 27, 2019

It seems a bunch of work is underway to tackle this:

@scampi scampi mentioned this issue May 27, 2019
Closed
@ghost ghost mentioned this issue Jul 10, 2019
Merged
@topecongiro
Copy link
Contributor

Closed via #4131.

@ghost ghost mentioned this issue Sep 25, 2020
Closed
bors added a commit to rust-lang/rust-clippy that referenced this issue Sep 25, 2020
@bors
Auto merge of #6083 - mikerite:dev-fmt-20200925, r=<try>
f42f7a0 
Remove rustfmt bug workaround

rust-lang/rustfmt#1873 was fixed earlier this year.

changelog: none
bors added a commit to rust-lang/rust-clippy that referenced this issue Sep 25, 2020
@bors
Auto merge of #6083 - mikerite:dev-fmt-20200925, r=<try>
b323bb7 
Remove rustfmt bug workaround

rust-lang/rustfmt#1873 was fixed earlier this year.

changelog: none
bors added a commit to rust-lang/rust-clippy that referenced this issue Oct 8, 2020
@bors
Auto merge of #6083 - mikerite:dev-fmt-20200925, r=<try>
ca31ddf 
Remove rustfmt bug workaround

rust-lang/rustfmt#1873 was fixed earlier this year.

changelog: none
bors added a commit to rust-lang/rust-clippy that referenced this issue Oct 8, 2020
@bors
Auto merge of #6083 - mikerite:dev-fmt-20200925, r=<try>
a9045b7 
Remove rustfmt bug workaround

rust-lang/rustfmt#1873 was fixed earlier this year.

changelog: none
@MartinKavik MartinKavik mentioned this issue Mar 29, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Panic, non-idempotency, invalid code, etc. os-windows
Projects
None yet
Milestone
No milestone
8 participants
@rivy @nrc @scampi @povilasb @BusyJay @CasualX @Xanewok @topecongiro