Skip to content

fixed relativePath on Windows #10448

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

Merged
merged 1 commit into from
Nov 24, 2020
Merged

fixed relativePath on Windows #10448

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/PostTyper.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,12 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase
val jpath = file.jpath
val relativePath =
if jpath eq null then file.path // repl and other custom tests use abstract files with no path
else if jpath.isAbsolute then sourcerootPath.relativize(jpath.normalize).toString
else if jpath.isAbsolute then
val cunitPath = jpath.normalize
// On Windows we can only relativize paths if root component matches
// (see implementation of sun.nio.fs.WindowsPath#relativize)
try sourcerootPath.relativize(cunitPath).toString
catch case _: IllegalArgumentException => cunitPath.toString
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering why it is not caught by the Windows CI. Can we add a test for this in Windows CI?

Copy link
Contributor Author

@michelou michelou Nov 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll notice it only if the drive letter of your project directory and the drive letter of the temporary directory used for the test suite are different.

As sketched above, my project directory is W:\scala3-windows\ (W: contains my local GitHub repo for michelou/dotty-examples; W: was created with command subst) while temporary files created by the test suite are located in C:\Users\michelou\AppData\Local\Temp\ (aka. %LOCALAPPDATA%).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Answering your question "Can we add a test for this in Windows CI?"

As a reminder the two variables LOCALAPPDATA and TEMP have the following default values on Windows:

LOCALAPPDATA=C:\Users\%USERNAME%\AppData\Local
TEMP=%LOCALAPPDATA%\Temp

NB. With a Github-hosted runner I have USERNAME=runneradmin, locally I have USERNAME=michelou.

Let's assume we can execute command subst in a CI workflow; then it should possible to write something like the following (eg. in batch file cmdTests.bat):

subst X: %LOCALAPPDATA%
set TEMP=X:\Temp
call "%_SBT_CMD%" ";scalac ..."
set TEMP=%LOCALAPPDATA%\Temp
subst /D X:

The idea is to reassign the TEMP variable to some path with a drive letter different from C: (in this case X:) before running scalac with a Tasty related test example (derived from one of the above failing tests).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed explanation 👍

I just checked that our Windows CI machine supports the command subst. Let me know if you'd like to add a test in this PR or later.

else jpath.normalize.toString
sym.addAnnotation(Annotation.makeSourceFile(relativePath))
else (tree.rhs, sym.info) match
Expand Down