Support following symbolic links to files #38
Conversation
This file contains 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
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 <mirabilos@evolvis.org>
mirabilos
added a commit
to qvest-digital/plexus-archiver
that referenced
this pull request
Jan 8, 2021
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
plamentotev
reviewed
Mar 3, 2021
| Path p = f.toPath(); | ||
| if ( isFollowingSymLinks() ) | ||
| { | ||
| p = p.toRealPath(); |
There was a problem hiding this comment.
I think f.getCanonicalPath() would do the same without introducing Path.
plamentotev
reviewed
Mar 3, 2021
| @@ -158,8 +159,13 @@ private void addResources( List<PlexusIoResource> result, String[] resources ) | |||
| { | |||
| String sourceDir = name.replace( '\\', '/' ); | |||
| File f = new File( dir, sourceDir ); | |||
| Path p = f.toPath(); | |||
| if ( isFollowingSymLinks() ) | |||
There was a problem hiding this comment.
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).
|
slachiewicz
approved these changes
Jun 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
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.