Skip to content
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

Upgrade the minimum required Java version to 7 and Plexus IO to 3.0.0 #56

Closed
wants to merge 3 commits into from
Closed

Upgrade the minimum required Java version to 7 and Plexus IO to 3.0.0 #56

wants to merge 3 commits into from

Conversation

plamentotev
Copy link
Member

This pull request updates the minimum required Java version to 7 and the Plexus IO version to 3.0.0

After the update to Java 7 useJvmChmod is just ignored. I've added deprecation annotation so it can be removed in future.

*/
@Deprecated
Copy link
Member Author

@plamentotev plamentotev May 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is obsolete but since it's public I think it's better to keep it for now (maybe until the next major release?).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct.

@plamentotev
Copy link
Member Author

I suppose I should update ReleaseNotes.md as well?

@michael-o
Copy link
Member

That would be nice.

@michael-o michael-o self-assigned this May 10, 2017
@michael-o michael-o added this to the 3.5 milestone May 10, 2017
@@ -114,11 +110,6 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<systemPropertyVariables>
<useJvmChmod>${useJvmChmod}</useJvmChmod>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about Surefire? That code did not change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The useJvmChmod system property is no longer used, so there is no point
in setting it. That is why I've removed it from the Surefire configuration.

@@ -351,7 +351,7 @@ else if ( isDirectory )

if ( !isIgnorePermissions() && mode != null && !isDirectory )
{
ArchiveEntryUtils.chmod( f, mode, getLogger(), isUseJvmChmod() );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logger is not necessary anymore?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it was used to log the output of the 'chmod' command, so it is no
longer needed.

@plamentotev
Copy link
Member Author

@michael-o excuse me for the comments, those were meant to be replies to your inline comments. Looks like the reply by mail feature does not work as good as I expected. I've added them as inline comments.

@michael-o
Copy link
Member

No big deal.

It may seem that there are lot of changes but actually most of them are:

* Rename `Java7FileAttributes` to `FileAttributes`. In the new version
  of  Plexus IO `FileAttributes` is replaced by `Java7FileAttributes`
* Remove calls to `Java7Reflector.isAtLeastJava7()`.
  `Java7Reflector` is deleted from the new version of Plexus IO
   because now at least Java 7 is required

The most significant change is made inside `ArchiveEntryUtils#chmod` - now
the Java version is at least 7 so together with the removal of the
call to `Java7Reflector.isAtLeastJava7()` the legacy code is removed as well.

Remove the `commons-io` dependency. Plexus IO 3.0.0 was updated to
use `commons-io` 2.5 so there is no longer a need to override
the transitive dependency.
Prior to Java 7,  the `chmod` command was used to change the files mode.
`useJvmChmod` was a way to force the usage of the JVM instead of
the external command.

Now Java 7 is the minimum required version and no external commands
are needed to change the files mode and useJvmChmod` is just ignored.
Deprecate this setting so it can be removed from future versions.
@plamentotev
Copy link
Member Author

I've updated the PR with the Plexus 3.0.0 release version instead of SNAPSHOT. Also I've removed the commons-io dependency. In Plexus 3.0.0 it is updated to 2.5 already so there is no need to override it.

@michael-o
Copy link
Member

Merged, thank you.

@michael-o michael-o closed this May 25, 2017
@plamentotev plamentotev deleted the upgrade-to-java-7 branch May 26, 2017 04:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants