Don't resolve symlinks in -Wconf:src #11192
Merged
+60
−8
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Updates the -Wconf:src filter to resolve each source path to its absolute, normalized form (instead of its canonicalized form) before matching it. Previously, filters intended to match symlinked paths would not apply when their canonicalized paths didn't contain the same path components. For example, Bazel 9 changed the location of symlinked source repositories for external dependencies such that their canonicalized paths no longer contained an `external/` component. This rendered `-Wconf:src=external/.*:s` filters to silence warnings in these external source repositories ineffective. This implementation closely matches the Scala3 implementation (as of 3.8.0-RC3), though it does not use `toURI`. Testing revealed that `toURI` prepends the current working directory to Windows-like paths unconditionally, and converts backslashes in such paths to `%5C` escape sequences. - https://github.com/scala/scala3/blob/3.8.0-RC3/compiler/src/dotty/tools/dotc/reporting/WConf.scala#L33
Contributor
Author
|
It looks like Jenkins ran out of space: Fetching changes from the remote Git repository
ERROR: Checkout failed
Also: hudson.remoting.Channel$CallSiteStackTrace: Remote call to jenkins-worker-behemoth-1
at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1916)
[ ...snip... ]
at java.base/java.lang.Thread.run(Unknown Source)
java.io.IOException: No space left on device
at java.base/java.io.FileOutputStream.writeBytes(Native Method)
[ ...snip... ]
ERROR: Maximum checkout retry attempts reached, aborting |
lrytz
reviewed
Dec 16, 2025
| def check(pos: Position) = cache.getOrElseUpdate(pos.source, { | ||
| val sourcePath = pos.source.file.canonicalPath.replace("\\", "/") | ||
| val sourceFile = pos.source.file.absolute.file | ||
| val sourcePath = sourceFile.toPath.normalize.toString.replace("\\", "/") |
Member
There was a problem hiding this comment.
source.file is an AbstractFile, which has a bunch of subtypes. We need to make sure what we do here works / is safe for all of them; in particular, VirtualFile.file seems to be null.
Sad that this didn't turn up in our test suite...
Suggested change
| val sourcePath = sourceFile.toPath.normalize.toString.replace("\\", "/") | |
| val sourcePath = Option(sourceFile).map(_.toPath.normalize.toString).getOrElse(pos.source.path).replace("\\", "/") |
Contributor
Author
There was a problem hiding this comment.
Done in ce5c0c6. Added WConfTest.sourcePatternMatchesVirtualFiles to validate the before and after behavior.
If the `AbstractFile` returned by `pos.source.file` is a `VirtualFile`, invoking `.absolute.file` on it will return `null`. In that case, we apply the filter to `pos.source.path` instead.
mbland
force-pushed
the
issue/13145-wconf-src-stop-resolving-symlinks
branch
from
2947ca0 to
ce5c0c6
Compare
Member
Is that something we should fix on the Scala 3 side? |
Contributor
Author
I can try it today and report back. |
mbland
added a commit
to mbland/scala3
that referenced
this pull request
Dec 17, 2025
Updates the -Wconf:src filter to avoid using `java.nio.file.Path.toURI` in order to fix Windows source path conversions. `Path.toURI` prepends the current working directory to Windows-like paths unconditionally, and converts backslashes in such paths to `%5C` escape sequences This can cause `-Wconf:src` filters that work on non-Windows platforms to fail on Windows. For example, before this change, the `Path.toURI` conversion in the `SourcePattern` case from `MessageFilter.matches()` produced: ```txt original: Optional[C:\foo\bar\myfile.scala] resolved: /Users/mbland/src/scala/scala3/C:%5Cfoo%5Cbar%5Cmyfile.scala ``` After this change, it produces the following, which still prepends the current working directory, but properly converts path separators to `/`: ```txt original: Optional[C:\foo\bar\myfile.scala] resolved: /Users/mbland/src/scala/scala3/C:/foo/bar/myfile.scala ``` This change is based on scala/scala#11192, and also adapts some test cases from that pull request to validate symlink and normalized path handling. This change also extracts the `diagnosticWarning` helper method to reduce duplication between new and existing test cases.
Contributor
Author
|
@lrytz I just filed scala/scala3#24771 and scala/scala3#24772 to avoid |
Member
|
Thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updates the -Wconf:src filter to resolve each source path to its absolute, normalized form (instead of its canonicalized form) before matching it.
Previously, filters intended to match symlinked paths would not apply when their canonicalized paths didn't contain the same path components. For example, Bazel 9 changed the location of symlinked source repositories for external dependencies such that their canonicalized paths no longer contained an
external/component. This rendered-Wconf:src=external/.*:sfilters to silence warnings in these external source repositories ineffective.This implementation closely matches the Scala3 implementation (as of 3.8.0-RC3), though it does not use
toURI. Testing revealed thattoURIprepends the current working directory to Windows-like paths unconditionally, and converts backslashes in such paths to%5Cescape sequences.Fixes scala/bug#13145. Review by @lrytz.