Skip to content

Conversation

@dsmiley
Copy link
Contributor

@dsmiley dsmiley commented Mar 20, 2025

span FileSystem.

https://issues.apache.org/jira/browse/IO-870

PathUtils.copyFileToDirectory is a simple method calling Files.copy, the latter of which supports cross-FileSystem copy. But copyFileToDirectory gets the file name from the source as a Path and then resolves this against the target, which is invalid. It needs to get the string form of the source file name.

This problem was encountered in Apache Solr which uses Apache Lucene's extensive test infrastructure, including custom FileSystem impls that emulate various behaviors to help tests see things that some users might experience.

@dsmiley
Copy link
Contributor Author

dsmiley commented Mar 20, 2025

I might actually be able to test this using Zip FileSystem from a JAR to copy to a temp dir (standard/default FileSystem). Could just use a dependency's JAR. Hmmm.

@garydgregory
Copy link
Member

garydgregory commented Mar 20, 2025

Hello @dsmiley

You did not follow the PR template since all builds fail:

Before you push a pull request, review this list:

  • Read the contribution guidelines for this project.
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible but is a best-practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that commits might be squashed by a maintainer on merge.

@dsmiley
Copy link
Contributor Author

dsmiley commented Mar 20, 2025

I didn't run "mvn" but otherwise I believe I'm compliant with this projects many rules. If there's still a problem after the tests pass but I haven't done something specifically, just let me know (which).

@garydgregory
Copy link
Member

Hello @dsmiley
The builds are still red. Run mvn; that's mvn on the command line, by itself. There is no sense in pushing broken commits if your build fails locally.

@dsmiley
Copy link
Contributor Author

dsmiley commented Mar 20, 2025

At first I thought "test" phase might be enough (I pushed) -- it wasn't. Then "verify" (I pushed) -- but apparently not! The project should hookup "spotbugs" to verify. It ran when I used "mvn" alone.

@garydgregory garydgregory merged commit a924140 into apache:master Mar 20, 2025
20 of 21 checks passed
@garydgregory garydgregory changed the title IO-870: PathUtils.copyFileToDirectory IO-870: PathUtils.copyFileToDirectory() across file systems Mar 20, 2025
@garydgregory garydgregory changed the title IO-870: PathUtils.copyFileToDirectory() across file systems [IO-870:] PathUtils.copyFileToDirectory() across file systems Mar 20, 2025
@garydgregory garydgregory changed the title [IO-870:] PathUtils.copyFileToDirectory() across file systems [IO-870] PathUtils.copyFileToDirectory() across file systems Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants