Fix processing mod with multi-level path on Windows #51278

EPashkin · 2018-06-01T17:22:32Z

Fix error in rustfmt because libsyntax can not handle mod with multilevel path on Windows.

Alternative is do almost same in https://github.com/rust-lang/rust/blob/master/src/libstd/sys/windows/fs.rs#L717 to allow work on paths with different separators, Ex. "\\?\c:\windows/temp"

rust-highfive · 2018-06-01T17:22:43Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @michaelwoerister (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

rust-highfive · 2018-06-01T18:03:57Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.


[00:05:55] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:05:56] tidy error: /checkout/src/libsyntax/parse/parser.rs:6156: tab character
[00:05:57] some tidy checks failed
[00:05:57] 
[00:05:57] 
[00:05:57] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:05:57] 
[00:05:57] 
[00:05:57] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:05:57] Build completed unsuccessfully in 0:01:57
[00:05:57] Build completed unsuccessfully in 0:01:57
[00:05:57] Makefile:79: recipe for target 'tidy' failed
[00:05:57] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0955d2d4
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

michaelwoerister · 2018-06-05T09:44:23Z

r? @nikomatsakis for re-assignment

nikomatsakis · 2018-06-05T16:55:16Z

@EPashkin — hmm, so the problem is that an annotation like #[path = "foo/bar.rs"] needs to work also on windows (but doesn't?).

cc @rust-lang/libs @retep998 — just doing s%/%\\% feels a bit hacky here, is there a better fix?

EPashkin · 2018-06-05T17:34:05Z

@nikomatsakis You right. Rust works with it fine but libsyntax is not: because it used canonicalized base path (like "\\?\c:\work\lib") and it only path form on Windows that not accept mixed separator (see rust-lang/rustfmt#1754 (comment))

retep998 · 2018-06-05T20:08:25Z

If you have a relative path, it is completely valid to do replacements such as changing / to \, although to be completely correct we'd also need to strip . components and handle .. as well, because they're perfectly valid in a relative path but lose their special meaning if naively joined onto a \\?\ path. Really this is how path joining should always be done, but it's probably too late to change libstd in that manner.

nikomatsakis · 2018-06-06T20:21:07Z

@bors r+ -- ok, I .. guess this is a good start then.

bors · 2018-06-06T20:21:08Z

📌 Commit 459e223 has been approved by nikomatsakis

nikomatsakis · 2018-06-06T20:21:15Z

@bors r- -- whoops

nikomatsakis · 2018-06-06T20:23:06Z

@EPashkin can we add some form of test for this? It looks like there are some examples of tests that use #[path] in src/test/run-pass/mod_dir_path2.rs, maybe we can construct a test there? (It'd be good to build the test first to ensure it fails on windows; this strikes me as the sort of thing where it is easy to have a test that isn't actually testing what you think it is)

nikomatsakis · 2018-06-06T20:23:23Z

At worst we could do a run-make test (are those tested on windows? presumably yes?)

rust-highfive · 2018-06-06T20:35:24Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.


[00:04:58] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:04:59] tidy error: /checkout/src/libsyntax/parse/parser.rs:6222: trailing whitespace
[00:05:00] some tidy checks failed
[00:05:00] 
[00:05:00] 
[00:05:00] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:05:00] 
[00:05:00] 
[00:05:00] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:05:00] Build completed unsuccessfully in 0:01:51
[00:05:00] Build completed unsuccessfully in 0:01:51
[00:05:00] Makefile:79: recipe for target 'tidy' failed
[00:05:00] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:00306168
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

EPashkin · 2018-06-06T21:13:55Z

@nikomatsakis I almost sure that test don't use libsyntax or canonicalized base paths
as modified version of it passed on not fixed version.
Also I don't see any test that uses libsyntax directly.

#[path = "mod_dir_simple"]
mod pancakes {
    #[path = "test.rs"]
    pub mod syrup;
}

#[path = "mod_dir_simple/test.rs"]
mod gravy;

pub fn main() {
    assert_eq!(pancakes::syrup::foo(), 10);
    assert_eq!(gravy::foo(), 10);
}

rust-highfive · 2018-06-08T05:09:18Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:46:34] ...i..............................................................................i.................
[00:46:39] ....................................................................................................
[00:46:45] ....................................................................................................
[00:46:51] ....................................................................................................
[00:46:56] ...............i.................iiiiiiiii...................................................
[00:46:56] 
[00:46:56] travis_fold:start:test_ui_nll
travis_time:start:test_ui_nll
Check compiletest suite=ui mode=ui compare_mode=nll (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
[00:47:47] ...i..............................................................................i.................
[00:47:51] ....................................................................................................
[00:47:56] ....................................................................................................
[00:48:02] ....................................................................................................
[00:48:07] ...............i.................iiiiiiiii...................................................
[00:48:07] 
[00:48:07]  finished in 70.725
[00:48:07] travis_fold:end:test_ui_nll

---
[00:52:27] .....................................................................test [run-pass] run-pass/issue-29227.rs has been running for over 60 seconds
[00:52:36] ...............................
[00:53:01] ....................................................................................................
[00:53:13] ....................................................................................................
[00:53:30] ...................................................F....i...........................................
[00:54:17] ....................................................................................................
[00:54:18] .test [run-pass] run-pass/mir_heavy_promoted.rs has been running for over 60 seconds
[00:54:33] ...................................................................................................
[00:55:10] .......................ii...........................................................i....ii.........
---
[00:57:08] ---- [run-pass] run-pass/mod_dir_path_canonicalized.rs stdout ----
[00:57:08] 
[00:57:08] error: compilation failed!
[00:57:08] status: exit code: 101
[00:57:08] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass/mod_dir_path_canonicalized.rs" "--target=x86_64-unknown-linux-gnu" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/mod_dir_path_canonicalized/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/mod_dir_path_canonicalized/auxiliary"
[00:57:08] ------------------------------------------
[00:57:08] 
[00:57:08] ------------------------------------------
[00:57:08] stderr:
[00:57:08] stderr:
[00:57:08] ------------------------------------------
[00:57:08] error: couldn't read "/checkout/src/test/run-pass/mod_dir_simple\\test.rs": No such file or directory (os error 2)
[00:57:08]   --> /checkout/src/test/run-pass/mod_dir_path_canonicalized.rs:24:5
[00:57:08]    |
[00:57:08] 24 | mod gravy;
[00:57:08] 
[00:57:08] error: aborting due to previous error
[00:57:08] 
[00:57:08] 
---
[00:57:08] 
[00:57:08] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:498:22
[00:57:08] 
[00:57:08] 
[00:57:08] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/run-pass" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "run-pass" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:57:08] 
[00:57:08] 
[00:57:08] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:57:08] Build completed unsuccessfully in 0:12:53
[00:57:08] Build completed unsuccessfully in 0:12:53
[00:57:08] Makefile:58: recipe for target 'check' failed
[00:57:08] make: *** [check] Error 1

The comma3916684 .
2642188 ./obj
2642156 ./obj/build
---
147120 ./.git/modules/src
143060 ./obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu
143056 ./obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release
123264 ./obj/build/bootstrap/debug/incremental/bootstrap-1r3bppl29tbrj
123260 ./obj/build/bootstrap/debug/incremental/bootstrap-1r3bppl29tbrj/s-f1s7n6pasb-x83fbl-t4oe0he9btpl
107456 ./obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release
103816 .

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

rust-highfive · 2018-06-08T06:16:53Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:47:36] ...i..............................................................................i.................
[00:47:41] ....................................................................................................
[00:47:47] ....................................................................................................
[00:47:53] ....................................................................................................
[00:47:58] ...............i.................iiiiiiiii...................................................
[00:47:58] 
[00:47:58] travis_fold:start:test_ui_nll
travis_time:start:test_ui_nll
Check compiletest suite=ui mode=ui compare_mode=nll (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
[00:48:49] ...i..............................................................................i.................
[00:48:54] ....................................................................................................
[00:48:59] ....................................................................................................
[00:49:04] ....................................................................................................
[00:49:09] ...............i.................iiiiiiiii...................................................
[00:49:09] 
[00:49:09]  finished in 71.408
[00:49:09] travis_fold:end:test_ui_nll

---
[00:53:38] .............................................................test [run-pass] run-pass/issue-29227.rs has been running for over 60 seconds
[00:53:49] .......................................
[00:54:14] ....................................................................................................
[00:54:27] ....................................................................................................
[00:54:44] ....................................................F...i...........................................
[00:55:32] .............................................................................................test [run-pass] run-pass/mir_heavy_promoted.rs has been running for over 60 seconds
[00:55:34] .......
[00:55:50] ....................................................................................................
[00:56:30] .......................ii...........................................................i....ii.........
---
[00:58:29] ---- [run-pass] run-pass/mod_dir_path_canonicalized.rs stdout ----
[00:58:29] 
[00:58:29] error: compilation failed!
[00:58:29] status: exit code: 101
[00:58:29] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass/mod_dir_path_canonicalized.rs" "--target=x86_64-unknown-linux-gnu" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/mod_dir_path_canonicalized/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/mod_dir_path_canonicalized/auxiliary"
[00:58:29] ------------------------------------------
[00:58:29] 
[00:58:29] ------------------------------------------
[00:58:29] stderr:
---
[00:58:29] error: aborting due to previous error
[00:58:29] 
[00:58:29] For more information about this error, try `rustc --explain E0463`.
[00:58:29] 
[00:58:29] .1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:58:29] 
[00:58:29] 
[00:58:29] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:58:29] Build completed unsuccessfully in 0:13:13
[00:58:29] Build completed unsuccessfully in 0:13:13
[00:58:29] Makefile:58: recipe for target 'check' failed
[00:58:29] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:24be705e
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

nikomatsakis · 2018-06-08T15:11:05Z

@EPashkin nice job going above and beyond with the test :) I think it's really good to test these sorts of properties, as they are easily overlooked during refactorings and things. (I presume that test fails without your PR?)

EPashkin · 2018-06-08T16:15:08Z

@nikomatsakis Thanks.
Yes, test will fails on windows with removed fix ;)

EPashkin · 2018-06-08T16:59:43Z

@nikomatsakis This is final test version.
Want me to squash commits?

EPashkin · 2018-06-12T17:56:28Z

@nikomatsakis Friendly reminder of this PR

pietroalbini · 2018-06-18T09:04:44Z

Ping from triage @nikomatsakis! This PR needs your review.

nikomatsakis · 2018-06-18T18:46:07Z

@bors r+

bors · 2018-06-18T18:46:07Z

📌 Commit cb5e973 has been approved by nikomatsakis

bors · 2018-06-19T02:18:03Z

⌛ Testing commit cb5e973 with merge ed39523...

…ows, r=nikomatsakis Fix processing mod with multi-level path on Windows Fix error in [rustfmt](rust-lang/rustfmt#1754) because libsyntax can not handle `mod` with multilevel path on Windows. Alternative is do almost same in https://github.com/rust-lang/rust/blob/master/src/libstd/sys/windows/fs.rs#L717 to allow work on paths with different separators, Ex. "\\\\?\\c:\\windows/temp"

bors · 2018-06-19T04:33:42Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing ed39523 to master...

rust-highfive assigned michaelwoerister Jun 1, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 1, 2018

Fix processing mod with multi-level path on Windows

bd6c81a

EPashkin force-pushed the fix_mod_with_multilevel_paths_on_windows branch from fc0aaed to bd6c81a Compare June 1, 2018 18:13

rust-highfive assigned nikomatsakis and unassigned michaelwoerister Jun 5, 2018

nikomatsakis approved these changes Jun 6, 2018

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 6, 2018

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 6, 2018

add an explanatory comment

b417701

EPashkin force-pushed the fix_mod_with_multilevel_paths_on_windows branch from 459e223 to b417701 Compare June 6, 2018 21:26

EPashkin force-pushed the fix_mod_with_multilevel_paths_on_windows branch from da972b1 to 8e4c046 Compare June 8, 2018 05:16

EPashkin force-pushed the fix_mod_with_multilevel_paths_on_windows branch from 8e4c046 to 06939f4 Compare June 8, 2018 10:00

EPashkin force-pushed the fix_mod_with_multilevel_paths_on_windows branch from 06939f4 to 65fb2a4 Compare June 8, 2018 11:50

Add test for libsyntax with canonicalized base path

cb5e973

EPashkin force-pushed the fix_mod_with_multilevel_paths_on_windows branch from 65fb2a4 to cb5e973 Compare June 8, 2018 15:06

pietroalbini added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 18, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 18, 2018

bors merged commit cb5e973 into rust-lang:master Jun 19, 2018

EPashkin deleted the fix_mod_with_multilevel_paths_on_windows branch June 19, 2018 04:40

Fix processing mod with multi-level path on Windows #51278

Fix processing mod with multi-level path on Windows #51278

Uh oh!

Conversation

EPashkin commented Jun 1, 2018

Uh oh!

rust-highfive commented Jun 1, 2018

Uh oh!

rust-highfive commented Jun 1, 2018

Uh oh!

michaelwoerister commented Jun 5, 2018

Uh oh!

nikomatsakis commented Jun 5, 2018

Uh oh!

EPashkin commented Jun 5, 2018

Uh oh!

retep998 commented Jun 5, 2018

Uh oh!

nikomatsakis commented Jun 6, 2018

Uh oh!

bors commented Jun 6, 2018

Uh oh!

nikomatsakis commented Jun 6, 2018

Uh oh!

nikomatsakis commented Jun 6, 2018

Uh oh!

nikomatsakis commented Jun 6, 2018

Uh oh!

rust-highfive commented Jun 6, 2018

Uh oh!

EPashkin commented Jun 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Jun 8, 2018

Uh oh!

rust-highfive commented Jun 8, 2018

Uh oh!

nikomatsakis commented Jun 8, 2018

Uh oh!

EPashkin commented Jun 8, 2018

Uh oh!

EPashkin commented Jun 8, 2018

Uh oh!

EPashkin commented Jun 12, 2018

Uh oh!

pietroalbini commented Jun 18, 2018

Uh oh!

nikomatsakis commented Jun 18, 2018

Uh oh!

bors commented Jun 18, 2018

Uh oh!

bors commented Jun 19, 2018

Uh oh!

bors commented Jun 19, 2018

Uh oh!

Uh oh!

EPashkin commented Jun 6, 2018 •

edited

Loading