matrei commented on code in PR #14878:
URL: https://github.com/apache/grails-core/pull/14878#discussion_r2186962790


##########
LICENSE:
##########
@@ -252,7 +252,7 @@ See licenses/LICENSE-CDDL.txt for the full license terms.
 
 This product includes the following files from Spring Framework 5.3, developed 
by Pivotal Software, Inc., licensed under the Apache License 2.0:
 
-    
grails-common/src/main/groovy/org/apache/grails/common/annotationAbstractRecursiveAnnotationVisitor.java
+    
grails-common/src/main/groovy/org/apache/grails/common/annotation/AbstractRecursiveAnnotationVisitor.java

Review Comment:
   Aren't the other files listed here also in the `annotation` package?



##########
gradle/java-config.gradle:
##########
@@ -49,6 +49,27 @@ tasks.withType(Jar).configureEach {
             'Implementation-Version': grailsVersion,
             'Implementation-Vendor': 'grails.apache.org'
     )
+
+    from(rootProject.layout.projectDirectory.file('DISCLAIMER')) {
+        into('META-INF')
+    }
+
+    def projectLicense = 
project.layout.projectDirectory.file('src/main/resources/META-INF/LICENSE')

Review Comment:
   No need for explicit `project.`?



##########
etc/bin/verify-jar-artifacts.sh:
##########
@@ -129,6 +129,22 @@ while IFS= read -r line; do
     exit 1
   fi
 
+  echo "... Verifying required files exist in jar..."
+  required_jar_contents=(META-INF/LICENSE META-INF/DISCLAIMER META-INF/NOTICE)
+  missing_jar_contents=()
+  for entry in "${required_jar_contents[@]}"; do
+      if ! jar tf "${JAR_FILE}" | grep -qF -- "${entry}"; then
+          missing_jar_contents+=("${entry}")
+      fi
+  done
+
+  if ((${#missing_jar_contents[@]})); then
+    printf '❌ %s missing from %s\n' "${missing_jar_contents[*]}" "${JAR_FILE}"
+    exit 1
+  else
+    printf '✅ Verified requires files %s are present in %s\n' 
"${required_jar_contents[*]}" "${JAR_FILE}"

Review Comment:
   Suggestion:
   `✅ Required files %s are present in %s\n`
   or
   `✅ All required files are present in %s\n`



##########
gradle/java-config.gradle:
##########
@@ -49,6 +49,27 @@ tasks.withType(Jar).configureEach {
             'Implementation-Version': grailsVersion,
             'Implementation-Vendor': 'grails.apache.org'
     )
+
+    from(rootProject.layout.projectDirectory.file('DISCLAIMER')) {
+        into('META-INF')
+    }
+
+    def projectLicense = 
project.layout.projectDirectory.file('src/main/resources/META-INF/LICENSE')
+    if(!projectLicense.asFile.exists()) {
+        def basicLicense = 
rootProject.layout.projectDirectory.file('licenses/LICENSE-Apache-2.0.txt')
+        from(basicLicense) {
+            into('META-INF')
+            rename { 'LICENSE' }
+        }
+    }
+
+    def projectNotice = 
project.layout.projectDirectory.file('src/main/resources/META-INF/NOTICE')
+    if(!projectNotice.asFile.exists()) {

Review Comment:
   Align on space after `if`?



##########
grails-forge/gradle/java-config.gradle:
##########
@@ -53,6 +53,27 @@ tasks.withType(Jar).configureEach {
             'Implementation-Vendor': 'grails.apache.org'
     )
     duplicatesStrategy = DuplicatesStrategy.INCLUDE
+
+    from(rootProject.layout.projectDirectory.file('../DISCLAIMER')) {
+        into('META-INF')
+    }
+
+    def projectLicense = 
project.layout.projectDirectory.file('src/main/resources/META-INF/LICENSE')

Review Comment:
   No need for explicit `project.`?



##########
grails-forge/gradle/java-config.gradle:
##########
@@ -53,6 +53,27 @@ tasks.withType(Jar).configureEach {
             'Implementation-Vendor': 'grails.apache.org'
     )
     duplicatesStrategy = DuplicatesStrategy.INCLUDE
+
+    from(rootProject.layout.projectDirectory.file('../DISCLAIMER')) {
+        into('META-INF')
+    }
+
+    def projectLicense = 
project.layout.projectDirectory.file('src/main/resources/META-INF/LICENSE')
+    if(!projectLicense.asFile.exists()) {

Review Comment:
   Align on space after `if`?



##########
gradle/java-config.gradle:
##########
@@ -49,6 +49,27 @@ tasks.withType(Jar).configureEach {
             'Implementation-Version': grailsVersion,
             'Implementation-Vendor': 'grails.apache.org'
     )
+
+    from(rootProject.layout.projectDirectory.file('DISCLAIMER')) {
+        into('META-INF')
+    }
+
+    def projectLicense = 
project.layout.projectDirectory.file('src/main/resources/META-INF/LICENSE')
+    if(!projectLicense.asFile.exists()) {
+        def basicLicense = 
rootProject.layout.projectDirectory.file('licenses/LICENSE-Apache-2.0.txt')
+        from(basicLicense) {
+            into('META-INF')
+            rename { 'LICENSE' }
+        }
+    }
+
+    def projectNotice = 
project.layout.projectDirectory.file('src/main/resources/META-INF/NOTICE')

Review Comment:
   No need for explicit `project.`?



##########
gradle/java-config.gradle:
##########
@@ -49,6 +49,27 @@ tasks.withType(Jar).configureEach {
             'Implementation-Version': grailsVersion,
             'Implementation-Vendor': 'grails.apache.org'
     )
+
+    from(rootProject.layout.projectDirectory.file('DISCLAIMER')) {
+        into('META-INF')
+    }
+
+    def projectLicense = 
project.layout.projectDirectory.file('src/main/resources/META-INF/LICENSE')
+    if(!projectLicense.asFile.exists()) {

Review Comment:
   Align on space after `if`?



##########
grails-forge/gradle/java-config.gradle:
##########
@@ -53,6 +53,27 @@ tasks.withType(Jar).configureEach {
             'Implementation-Vendor': 'grails.apache.org'
     )
     duplicatesStrategy = DuplicatesStrategy.INCLUDE
+
+    from(rootProject.layout.projectDirectory.file('../DISCLAIMER')) {
+        into('META-INF')
+    }
+
+    def projectLicense = 
project.layout.projectDirectory.file('src/main/resources/META-INF/LICENSE')
+    if(!projectLicense.asFile.exists()) {
+        def basicLicense = 
rootProject.layout.projectDirectory.file('../licenses/LICENSE-Apache-2.0.txt')
+        from(basicLicense) {
+            into('META-INF')
+            rename { 'LICENSE' }
+        }
+    }
+
+    def projectNotice = 
project.layout.projectDirectory.file('src/main/resources/META-INF/NOTICE')
+    if(!projectNotice.asFile.exists()) {

Review Comment:
   Align on space after `if`?



##########
grails-forge/gradle/java-config.gradle:
##########
@@ -53,6 +53,27 @@ tasks.withType(Jar).configureEach {
             'Implementation-Vendor': 'grails.apache.org'
     )
     duplicatesStrategy = DuplicatesStrategy.INCLUDE
+
+    from(rootProject.layout.projectDirectory.file('../DISCLAIMER')) {
+        into('META-INF')
+    }
+
+    def projectLicense = 
project.layout.projectDirectory.file('src/main/resources/META-INF/LICENSE')
+    if(!projectLicense.asFile.exists()) {
+        def basicLicense = 
rootProject.layout.projectDirectory.file('../licenses/LICENSE-Apache-2.0.txt')
+        from(basicLicense) {
+            into('META-INF')
+            rename { 'LICENSE' }
+        }
+    }
+
+    def projectNotice = 
project.layout.projectDirectory.file('src/main/resources/META-INF/NOTICE')

Review Comment:
   No need for explicit `project.`?



##########
gradle/java-config.gradle:
##########
@@ -49,6 +49,27 @@ tasks.withType(Jar).configureEach {
             'Implementation-Version': grailsVersion,
             'Implementation-Vendor': 'grails.apache.org'
     )
+
+    from(rootProject.layout.projectDirectory.file('DISCLAIMER')) {
+        into('META-INF')
+    }
+
+    def projectLicense = 
project.layout.projectDirectory.file('src/main/resources/META-INF/LICENSE')
+    if(!projectLicense.asFile.exists()) {
+        def basicLicense = 
rootProject.layout.projectDirectory.file('licenses/LICENSE-Apache-2.0.txt')
+        from(basicLicense) {
+            into('META-INF')
+            rename { 'LICENSE' }
+        }
+    }
+
+    def projectNotice = 
project.layout.projectDirectory.file('src/main/resources/META-INF/NOTICE')
+    if(!projectNotice.asFile.exists()) {
+        def basicNotice = 
rootProject.layout.projectDirectory.file('grails-core/src/main/resources/META-INF/NOTICE')
+        from(projectNotice.asFile.exists() ? projectNotice : basicNotice) {

Review Comment:
   Is it not already established here that `projectNotice.asFile.exists() == 
false`?
   I think the logic here might need another pass.



##########
grails-forge/gradle/java-config.gradle:
##########
@@ -53,6 +53,27 @@ tasks.withType(Jar).configureEach {
             'Implementation-Vendor': 'grails.apache.org'
     )
     duplicatesStrategy = DuplicatesStrategy.INCLUDE
+
+    from(rootProject.layout.projectDirectory.file('../DISCLAIMER')) {
+        into('META-INF')
+    }
+
+    def projectLicense = 
project.layout.projectDirectory.file('src/main/resources/META-INF/LICENSE')
+    if(!projectLicense.asFile.exists()) {
+        def basicLicense = 
rootProject.layout.projectDirectory.file('../licenses/LICENSE-Apache-2.0.txt')
+        from(basicLicense) {
+            into('META-INF')
+            rename { 'LICENSE' }
+        }
+    }
+
+    def projectNotice = 
project.layout.projectDirectory.file('src/main/resources/META-INF/NOTICE')
+    if(!projectNotice.asFile.exists()) {
+        def basicNotice = 
rootProject.layout.projectDirectory.file('../grails-core/src/main/resources/META-INF/NOTICE')
+        from(projectNotice.asFile.exists() ? projectNotice : basicNotice) {

Review Comment:
   Same problem with the logic as in the other `java-config.gradle` file.



##########
grails-gradle/gradle/java-config.gradle:
##########


Review Comment:
   Same comments as in the other `java-config.gradle` files.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscr...@grails.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to