kennknowles commented on a change in pull request #12225:
URL: https://github.com/apache/beam/pull/12225#discussion_r453818020
##########
File path:
sdks/java/core/src/main/java/org/apache/beam/sdk/io/LocalFileSystem.java
##########
@@ -90,9 +91,14 @@
@Override
protected List<MatchResult> match(List<String> specs) throws IOException {
+ return match(new File(".").getAbsolutePath(), specs);
+ }
+
+ @VisibleForTesting
+ List<MatchResult> match(String pwd, List<String> specs) throws IOException {
Review comment:
Changed to `baseDir`
##########
File path:
sdks/java/core/src/main/java/org/apache/beam/sdk/io/LocalFileSystem.java
##########
@@ -223,12 +229,22 @@ private MatchResult matchOne(String spec) throws
IOException {
// it considers it an invalid file system pattern. We should use
// new File(spec) to avoid such validation.
// See https://bugs.openjdk.java.net/browse/JDK-8197918
- final File file = new File(spec);
+ // However, new File(parent, child) resolves absolute `child` in a
system-dependent
+ // way that is generally incorrect, for example new File($PWD, "/tmp/foo")
resolves
+ // to $PWD/tmp/foo on many systems, unlike
Paths.get($PWD).resolve("/tmp/foo") which
+ // correctly resolves to "/tmp/foo". We add just this one piece of logic
here, without
+ // switching to Paths which could require a rewrite of this module to
support
+ // both Windows and correct file resolution.
+ // The root cause is that globs are not files but we are using file
manipulation libraries
+ // to work with them.
+ final File relativeFile = new File(spec);
Review comment:
Renamed to `specAsFile` since basically it is just cramming the string
into a `File` object, and renamed `file` into `absoluteFile` since it is now
rooted in a base directory unless it was already absolute.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
