-
Couldn't load subscription status.
- Fork 10
Support following symbolic links to files #38
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.
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
base: master
Are you sure you want to change the base?
Conversation
This implements the necessary support to allow plexus-archiver to archive the contents of symlinks (instead of the symbolic links themselves), depending on the (already-extant) follow-symlinks flag. Closes: codehaus-plexus#37 Signed-off-by: mirabilos <[email protected]>
default is still true here, will be false when submitting upstream REQUIRES a version of plexus-io with the following PR merged: codehaus-plexus/plexus-io#38 advances codehaus-plexus#160 even if I don’t know how to make this configurable from Maven
| Path p = f.toPath(); | ||
| if ( isFollowingSymLinks() ) | ||
| { | ||
| p = p.toRealPath(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think f.getCanonicalPath() would do the same without introducing Path.
| String sourceDir = name.replace( '\\', '/' ); | ||
| File f = new File( dir, sourceDir ); | ||
| Path p = f.toPath(); | ||
| if ( isFollowingSymLinks() ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is true by default isFollowingSymLinks. I'm a bit concerned that this change would change the behavior of existing applications. I know the existing behavior is odd, but it was existing for quite some time and maybe there are applications that depend on it. Do you think adding additional flag would made sense? In general I hate fix bug kind of flags, but this code was here for quite some time and maybe there is code out there depending on PlexusIoFileResourceCollection not following symlinks by default.
|
Adding a test case would be great if you have the time. @mirabilos thanks for the contribution. The symbolic link handling in Plexus IO and Plexus Archiver could be improved, it is great to see contributors helping with that, |
|
Plamen Totev dixit:
I think `f.getCanonicalPath()` would do the same without introducing `Path`.
Hrm. It does say it follows symlinks on Unix… (I hope it also does that
on Windows… ouch, we’ll probably have to test that…)
|
|
Plamen Totev dixit:
Adding a test case would be great if you have the time.
I’m very much out of my depth here. I don’t use this directly, but as…
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-source-plugin</artifactId>
<version>${maven.source.version}</version>
<dependencies>
<dependency>
<!-- codehaus-plexus/plexus-archiver#160 -->
<groupId>org.evolvis.tartools.forked</groupId>
<artifactId>plexus-archiver</artifactId>
<version>4.2.1</version>
</dependency>
</dependencies>
</plugin>
… so the only testcase I have is a full project.
***@***.*** thanks for the contribution. The symbolic link handling in
@plexus IO and Plexus Archiver could be improved, it is great to see
***@***.*** helping with that,
Thanks. Unfortunately I’ve very little experience with developing
Maven itself, and I basically took that suggestion from the comment
codehaus-plexus/plexus-archiver#47 (comment)
and hacked on it until I got it. I’m not really in a situation to
bring it further.
bye,
//mirabilos
--
Infrastrukturexperte • tarent solutions GmbH
Am Dickobskreuz 10, D-53121 Bonn • http://www.tarent.de/
Telephon +49 228 54881-393 • Fax: +49 228 54881-235
HRB AG Bonn 5168 • USt-ID (VAT): DE122264941
Geschäftsführer: Dr. Stefan Barth, Kai Ebenrett, Boris Esser, Alexander Steeg
|
|
Plamen Totev dixit:
This is `true` by default `isFollowingSymLinks`. I'm a bit concerned
I think it even *should* be true. It’s a breaking change, yes,
so a major version bump is necessary (see the other mail).
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for following symbolic links when archiving files by leveraging the existing follow-symlinks flag. When enabled, the archive will contain the actual file contents that symlinks point to rather than the symbolic links themselves.
Key changes:
- Modified file resource collection to resolve symbolic links to their real paths when the follow-symlinks flag is enabled
- Refactored
FileAttributesconstructor to acceptPathobjects directly, enabling symlink resolution
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| PlexusIoFileResourceCollection.java | Resolves symbolic links to real paths when follow-symlinks is enabled before creating file attributes |
| FileAttributes.java | Adds new constructor accepting Path parameter to support resolved symlink paths |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Path p = f.toPath(); | ||
| if ( isFollowingSymLinks() ) | ||
| { | ||
| p = p.toRealPath(); |
Copilot
AI
Oct 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toRealPath() requires LinkOption varargs but none are provided. When following symlinks, this will throw NoSuchFileException if the symlink target doesn't exist. Consider adding exception handling or using toRealPath(LinkOption.NOFOLLOW_LINKS) with custom symlink resolution logic to provide better error messages when symlink targets are missing.
| p = p.toRealPath(); | |
| try { | |
| p = p.toRealPath(); | |
| } catch (java.nio.file.NoSuchFileException e) { | |
| throw new IOException("Symlink target does not exist for: " + p.toString(), e); | |
| } |
This implements the necessary support to allow plexus-archiver to archive the contents of symlinks (instead of the symbolic links themselves), depending on the (already-extant) follow-symlinks flag.
Closes: #37
I’ve tested this in a patch against 3.2.0 which slightly differs from this patch against git master, but I believe there to be no issues.