You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2020/12/24 06:10:39 UTC
[GitHub] [maven-ear-plugin] mabrarov opened a new pull request #33: [MEAR-166] Handling EAR modules of all types when modifying Class-Path and when skinnyWars / skinnyModules is used
mabrarov opened a new pull request #33:
URL: https://github.com/apache/maven-ear-plugin/pull/33
[MEAR-166] - Removing EAR modules of all types when skinnyWars / skinnyModules is on. Updating reference to the EAR modules of all types in the Class-Path entry of MANIFEST.mf when modifying that entry.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #33: [MEAR-166] Handling EAR modules of all types when modifying Class-Path and when skinnyWars / skinnyModules is used
Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #33:
URL: https://github.com/apache/maven-ear-plugin/pull/33#discussion_r548752603
##########
File path: src/main/java/org/apache/maven/plugins/ear/AbstractEarModule.java
##########
@@ -101,6 +101,16 @@
*/
protected String libDirectory;
+ /**
+ * If module is considered for inclusion into the Class-Path entry of MANIFEST.mf of other modules. {@code false}
+ * value leads to removal of the module from the Class-Path entry. {@code true} value leads to modification of the
+ * reference to the module in the Class-Path entry if such reference exists or leads to adding of the module into
+ * the Class-Path entry if such reference doesn't exist. Removal, modification or adding of the reference in the
+ * Class-Path entry depends on libDirectory property of another module and on skinnyWars / skinnyModules parameters
+ * of EAR Plugin.
+ */
+ protected Boolean classPathItem = Boolean.FALSE;
Review comment:
`boolean` was my original decision, but there is `excluded` field which has `Boolean` type by some (not clear to me) reason.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [maven-ear-plugin] elharo commented on a change in pull request #33: [MEAR-166] Handling EAR modules of all types when modifying Class-Path and when skinnyWars / skinnyModules is used
Posted by GitBox <gi...@apache.org>.
elharo commented on a change in pull request #33:
URL: https://github.com/apache/maven-ear-plugin/pull/33#discussion_r548751870
##########
File path: src/main/java/org/apache/maven/plugins/ear/AbstractEarModule.java
##########
@@ -101,6 +101,16 @@
*/
protected String libDirectory;
+ /**
+ * If module is considered for inclusion into the Class-Path entry of MANIFEST.mf of other modules. {@code false}
+ * value leads to removal of the module from the Class-Path entry. {@code true} value leads to modification of the
+ * reference to the module in the Class-Path entry if such reference exists or leads to adding of the module into
+ * the Class-Path entry if such reference doesn't exist. Removal, modification or adding of the reference in the
+ * Class-Path entry depends on libDirectory property of another module and on skinnyWars / skinnyModules parameters
+ * of EAR Plugin.
+ */
+ protected Boolean classPathItem = Boolean.FALSE;
Review comment:
Boolean --> boolean
##########
File path: src/main/java/org/apache/maven/plugins/ear/AbstractEarMojo.java
##########
@@ -272,8 +272,8 @@ public void execute()
// Now we have everything let's built modules which have not been excluded
ScopeArtifactFilter filter = new ScopeArtifactFilter( Artifact.SCOPE_RUNTIME );
- allJarModules = new ArrayList<JarModule>();
- providedJarModules = new ArrayList<JarModule>();
+ allEarModules = new ArrayList<EarModule>();
Review comment:
`<EarModule> --> <>`
##########
File path: src/site/apt/modules.apt.vm
##########
@@ -220,6 +234,50 @@ EAR Modules
* <<includeInApplicationXml>> - set to true to if you want to generate an entry
of this module in <<<application.xml>>>. Default is false.
+ * <<classPathItem>> - defines if the module is an element of the <<<Class-Path>>> setting
+ of <<<MANIFEST.MF>>> of other modules. Default is true.
+
+ The module is removed from the <<<Class-Path>>> setting of <<<MANIFEST.MF>>> of another module
+ if the <<<classPathItem>>> property is false and one of the following conditions is met:
+
+ * Another module doesn't contain all of it dependencies (refer to the <<<libDirectory>>>
Review comment:
it --> its
##########
File path: src/main/java/org/apache/maven/plugins/ear/EarModule.java
##########
@@ -119,4 +119,17 @@ void resolveArtifact( Set<Artifact> artifacts )
*/
String getLibDir();
+ /**
+ * Returns the bundle file name. If {@code null}, the artifact's file name is returned.
+ *
+ * @return the bundle file name
+ */
+ String getBundleFileName();
+
+ /**
+ * If module should present in the Class-Path entry of MANIFEST.mf. Doesn't impact Class-Path entry of MANIFEST.mf
Review comment:
present --> be included
##########
File path: src/main/java/org/apache/maven/plugins/ear/AbstractEarMojo.java
##########
@@ -272,8 +272,8 @@ public void execute()
// Now we have everything let's built modules which have not been excluded
ScopeArtifactFilter filter = new ScopeArtifactFilter( Artifact.SCOPE_RUNTIME );
- allJarModules = new ArrayList<JarModule>();
- providedJarModules = new ArrayList<JarModule>();
+ allEarModules = new ArrayList<EarModule>();
+ providedEarModules = new ArrayList<EarModule>();
Review comment:
`<EarModule> --> <>`
##########
File path: src/site/apt/modules.apt.vm
##########
@@ -220,6 +234,50 @@ EAR Modules
* <<includeInApplicationXml>> - set to true to if you want to generate an entry
of this module in <<<application.xml>>>. Default is false.
+ * <<classPathItem>> - defines if the module is an element of the <<<Class-Path>>> setting
+ of <<<MANIFEST.MF>>> of other modules. Default is true.
+
+ The module is removed from the <<<Class-Path>>> setting of <<<MANIFEST.MF>>> of another module
+ if the <<<classPathItem>>> property is false and one of the following conditions is met:
+
+ * Another module doesn't contain all of it dependencies (refer to the <<<libDirectory>>>
+ property of particular module type).
+
+ * {{{./ear-mojo.html#skinnyWars}skinnyWars}} parameter is true and another module is
+ a {{{webModule}webModule}}.
+
+ * {{{./ear-mojo.html#skinnyModules}skinnyModules}} parameter is true.
+
+ Existing reference to the module in the <<<Class-Path>>> setting of <<<MANIFEST.MF>>>
+ of another module is updated to match location of the module in EAR if
+ the <<<classPathItem>>> property is true and one of the following conditions is met:
+
+ * Another module doesn't contain all of it dependencies (refer to the <<<libDirectory>>>
Review comment:
it --> its
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #33: [MEAR-166] Handling EAR modules of all types when modifying Class-Path and when skinnyWars / skinnyModules is used
Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #33:
URL: https://github.com/apache/maven-ear-plugin/pull/33#discussion_r548758675
##########
File path: src/main/java/org/apache/maven/plugins/ear/EarModule.java
##########
@@ -119,4 +119,17 @@ void resolveArtifact( Set<Artifact> artifacts )
*/
String getLibDir();
+ /**
+ * Returns the bundle file name. If {@code null}, the artifact's file name is returned.
+ *
+ * @return the bundle file name
+ */
+ String getBundleFileName();
+
+ /**
+ * If module should present in the Class-Path entry of MANIFEST.mf. Doesn't impact Class-Path entry of MANIFEST.mf
Review comment:
Applied suggestion in 804f94f. Thank you.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #33: [MEAR-166] Handling EAR modules of all types when modifying Class-Path and when skinnyWars / skinnyModules is used
Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #33:
URL: https://github.com/apache/maven-ear-plugin/pull/33#discussion_r548752603
##########
File path: src/main/java/org/apache/maven/plugins/ear/AbstractEarModule.java
##########
@@ -101,6 +101,16 @@
*/
protected String libDirectory;
+ /**
+ * If module is considered for inclusion into the Class-Path entry of MANIFEST.mf of other modules. {@code false}
+ * value leads to removal of the module from the Class-Path entry. {@code true} value leads to modification of the
+ * reference to the module in the Class-Path entry if such reference exists or leads to adding of the module into
+ * the Class-Path entry if such reference doesn't exist. Removal, modification or adding of the reference in the
+ * Class-Path entry depends on libDirectory property of another module and on skinnyWars / skinnyModules parameters
+ * of EAR Plugin.
+ */
+ protected Boolean classPathItem = Boolean.FALSE;
Review comment:
`boolean` was my original decision, but there is `excluded` field which has `Boolean` type by some (not clear to me reason).
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #33: [MEAR-166] Handling EAR modules of all types when modifying Class-Path and when skinnyWars / skinnyModules is used
Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #33:
URL: https://github.com/apache/maven-ear-plugin/pull/33#discussion_r548753138
##########
File path: src/main/java/org/apache/maven/plugins/ear/EarModule.java
##########
@@ -119,4 +119,17 @@ void resolveArtifact( Set<Artifact> artifacts )
*/
String getLibDir();
+ /**
+ * Returns the bundle file name. If {@code null}, the artifact's file name is returned.
+ *
+ * @return the bundle file name
+ */
+ String getBundleFileName();
+
+ /**
+ * If module should present in the Class-Path entry of MANIFEST.mf. Doesn't impact Class-Path entry of MANIFEST.mf
Review comment:
`be included` looks a little bit inaccurate (while I prefer this wording). The module won't `be added` into the Class-Path if it's already referenced there. Won't `be included` mess with `be added` in this case?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #33: [MEAR-166] Handling EAR modules of all types when modifying Class-Path and when skinnyWars / skinnyModules is used
Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #33:
URL: https://github.com/apache/maven-ear-plugin/pull/33#discussion_r548762605
##########
File path: src/main/java/org/apache/maven/plugins/ear/AbstractEarModule.java
##########
@@ -101,6 +101,16 @@
*/
protected String libDirectory;
+ /**
+ * If module is considered for inclusion into the Class-Path entry of MANIFEST.mf of other modules. {@code false}
+ * value leads to removal of the module from the Class-Path entry. {@code true} value leads to modification of the
+ * reference to the module in the Class-Path entry if such reference exists or leads to adding of the module into
+ * the Class-Path entry if such reference doesn't exist. Removal, modification or adding of the reference in the
+ * Class-Path entry depends on libDirectory property of another module and on skinnyWars / skinnyModules parameters
+ * of EAR Plugin.
+ */
+ protected Boolean classPathItem = Boolean.FALSE;
Review comment:
Done in 9866585
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #33: [MEAR-166] Handling EAR modules of all types when modifying Class-Path and when skinnyWars / skinnyModules is used
Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #33:
URL: https://github.com/apache/maven-ear-plugin/pull/33#discussion_r548752603
##########
File path: src/main/java/org/apache/maven/plugins/ear/AbstractEarModule.java
##########
@@ -101,6 +101,16 @@
*/
protected String libDirectory;
+ /**
+ * If module is considered for inclusion into the Class-Path entry of MANIFEST.mf of other modules. {@code false}
+ * value leads to removal of the module from the Class-Path entry. {@code true} value leads to modification of the
+ * reference to the module in the Class-Path entry if such reference exists or leads to adding of the module into
+ * the Class-Path entry if such reference doesn't exist. Removal, modification or adding of the reference in the
+ * Class-Path entry depends on libDirectory property of another module and on skinnyWars / skinnyModules parameters
+ * of EAR Plugin.
+ */
+ protected Boolean classPathItem = Boolean.FALSE;
Review comment:
`boolean` was my original decision, but there is `excluded` field which has `Boolean` type by some (not clear to me) reason, so I decided to follow existing design (if any).
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [maven-ear-plugin] elharo commented on a change in pull request #33: [MEAR-166] Handling EAR modules of all types when modifying Class-Path and when skinnyWars / skinnyModules is used
Posted by GitBox <gi...@apache.org>.
elharo commented on a change in pull request #33:
URL: https://github.com/apache/maven-ear-plugin/pull/33#discussion_r548756357
##########
File path: src/main/java/org/apache/maven/plugins/ear/AbstractEarModule.java
##########
@@ -101,6 +101,16 @@
*/
protected String libDirectory;
+ /**
+ * If module is considered for inclusion into the Class-Path entry of MANIFEST.mf of other modules. {@code false}
+ * value leads to removal of the module from the Class-Path entry. {@code true} value leads to modification of the
+ * reference to the module in the Class-Path entry if such reference exists or leads to adding of the module into
+ * the Class-Path entry if such reference doesn't exist. Removal, modification or adding of the reference in the
+ * Class-Path entry depends on libDirectory property of another module and on skinnyWars / skinnyModules parameters
+ * of EAR Plugin.
+ */
+ protected Boolean classPathItem = Boolean.FALSE;
Review comment:
per Effective Java, boolean is preferred
##########
File path: src/main/java/org/apache/maven/plugins/ear/AbstractEarMojo.java
##########
@@ -272,8 +272,8 @@ public void execute()
// Now we have everything let's built modules which have not been excluded
ScopeArtifactFilter filter = new ScopeArtifactFilter( Artifact.SCOPE_RUNTIME );
- allJarModules = new ArrayList<JarModule>();
- providedJarModules = new ArrayList<JarModule>();
+ allEarModules = new ArrayList<EarModule>();
Review comment:
Yes, we now require Java 7. In general, you do not need to be consistent with existing code that has problems.
##########
File path: src/main/java/org/apache/maven/plugins/ear/EarModule.java
##########
@@ -119,4 +119,17 @@ void resolveArtifact( Set<Artifact> artifacts )
*/
String getLibDir();
+ /**
+ * Returns the bundle file name. If {@code null}, the artifact's file name is returned.
+ *
+ * @return the bundle file name
+ */
+ String getBundleFileName();
+
+ /**
+ * If module should present in the Class-Path entry of MANIFEST.mf. Doesn't impact Class-Path entry of MANIFEST.mf
Review comment:
I'm not sure but "should present" is wrong.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [maven-ear-plugin] elharo merged pull request #33: [MEAR-166] Handling EAR modules of all types when modifying Class-Path and when skinnyWars / skinnyModules is used
Posted by GitBox <gi...@apache.org>.
elharo merged pull request #33:
URL: https://github.com/apache/maven-ear-plugin/pull/33
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #33: [MEAR-166] Handling EAR modules of all types when modifying Class-Path and when skinnyWars / skinnyModules is used
Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #33:
URL: https://github.com/apache/maven-ear-plugin/pull/33#discussion_r548752876
##########
File path: src/main/java/org/apache/maven/plugins/ear/AbstractEarMojo.java
##########
@@ -272,8 +272,8 @@ public void execute()
// Now we have everything let's built modules which have not been excluded
ScopeArtifactFilter filter = new ScopeArtifactFilter( Artifact.SCOPE_RUNTIME );
- allJarModules = new ArrayList<JarModule>();
- providedJarModules = new ArrayList<JarModule>();
+ allEarModules = new ArrayList<EarModule>();
Review comment:
What's the reason old lines use `new ArrayList<JarModule>()`? Is it because they where written before migration to Java 7 as minimal supported version?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [maven-ear-plugin] mabrarov commented on pull request #33: [MEAR-166] Handling EAR modules of all types when modifying Class-Path and when skinnyWars / skinnyModules is used
Posted by GitBox <gi...@apache.org>.
mabrarov commented on pull request #33:
URL: https://github.com/apache/maven-ear-plugin/pull/33#issuecomment-751003513
@khmarbaise, @hboutemy, @elharo and @rfscholte, your review and feedback is appreciated.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #33: [MEAR-166] Handling EAR modules of all types when modifying Class-Path and when skinnyWars / skinnyModules is used
Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #33:
URL: https://github.com/apache/maven-ear-plugin/pull/33#discussion_r548762561
##########
File path: src/main/java/org/apache/maven/plugins/ear/AbstractEarMojo.java
##########
@@ -272,8 +272,8 @@ public void execute()
// Now we have everything let's built modules which have not been excluded
ScopeArtifactFilter filter = new ScopeArtifactFilter( Artifact.SCOPE_RUNTIME );
- allJarModules = new ArrayList<JarModule>();
- providedJarModules = new ArrayList<JarModule>();
+ allEarModules = new ArrayList<EarModule>();
+ providedEarModules = new ArrayList<EarModule>();
Review comment:
Done in 9866585
##########
File path: src/main/java/org/apache/maven/plugins/ear/AbstractEarMojo.java
##########
@@ -272,8 +272,8 @@ public void execute()
// Now we have everything let's built modules which have not been excluded
ScopeArtifactFilter filter = new ScopeArtifactFilter( Artifact.SCOPE_RUNTIME );
- allJarModules = new ArrayList<JarModule>();
- providedJarModules = new ArrayList<JarModule>();
+ allEarModules = new ArrayList<EarModule>();
Review comment:
Done in 9866585
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [maven-ear-plugin] elharo commented on pull request #33: [MEAR-166] Handling EAR modules of all types when modifying Class-Path and when skinnyWars / skinnyModules is used
Posted by GitBox <gi...@apache.org>.
elharo commented on pull request #33:
URL: https://github.com/apache/maven-ear-plugin/pull/33#issuecomment-751350304
Running through Jenkins: https://ci-builds.apache.org/job/Maven/job/maven-box/job/maven-ear-plugin/job/mear-166/
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #33: [MEAR-166] Handling EAR modules of all types when modifying Class-Path and when skinnyWars / skinnyModules is used
Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #33:
URL: https://github.com/apache/maven-ear-plugin/pull/33#discussion_r548758580
##########
File path: src/site/apt/modules.apt.vm
##########
@@ -220,6 +234,50 @@ EAR Modules
* <<includeInApplicationXml>> - set to true to if you want to generate an entry
of this module in <<<application.xml>>>. Default is false.
+ * <<classPathItem>> - defines if the module is an element of the <<<Class-Path>>> setting
+ of <<<MANIFEST.MF>>> of other modules. Default is true.
+
+ The module is removed from the <<<Class-Path>>> setting of <<<MANIFEST.MF>>> of another module
+ if the <<<classPathItem>>> property is false and one of the following conditions is met:
+
+ * Another module doesn't contain all of it dependencies (refer to the <<<libDirectory>>>
Review comment:
Fixed in 804f94f
##########
File path: src/site/apt/modules.apt.vm
##########
@@ -220,6 +234,50 @@ EAR Modules
* <<includeInApplicationXml>> - set to true to if you want to generate an entry
of this module in <<<application.xml>>>. Default is false.
+ * <<classPathItem>> - defines if the module is an element of the <<<Class-Path>>> setting
+ of <<<MANIFEST.MF>>> of other modules. Default is true.
+
+ The module is removed from the <<<Class-Path>>> setting of <<<MANIFEST.MF>>> of another module
+ if the <<<classPathItem>>> property is false and one of the following conditions is met:
+
+ * Another module doesn't contain all of it dependencies (refer to the <<<libDirectory>>>
+ property of particular module type).
+
+ * {{{./ear-mojo.html#skinnyWars}skinnyWars}} parameter is true and another module is
+ a {{{webModule}webModule}}.
+
+ * {{{./ear-mojo.html#skinnyModules}skinnyModules}} parameter is true.
+
+ Existing reference to the module in the <<<Class-Path>>> setting of <<<MANIFEST.MF>>>
+ of another module is updated to match location of the module in EAR if
+ the <<<classPathItem>>> property is true and one of the following conditions is met:
+
+ * Another module doesn't contain all of it dependencies (refer to the <<<libDirectory>>>
Review comment:
Fixed in 804f94f
----------------------------------------------------------------
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:
users@infra.apache.org