Re: [PR] Automated bundled license check [incubator-xtable]

["vinishjail97 (via GitHub)" <gi...@apache.org>]

vinishjail97 commented on code in PR #820:
URL: https://github.com/apache/incubator-xtable/pull/820#discussion_r3190475721


##########
pom.xml:
##########
@@ -974,6 +994,29 @@
                     </execution>
                 </executions>
             </plugin>
+            <plugin>
+                <groupId>org.codehaus.mojo</groupId>
+                <artifactId>exec-maven-plugin</artifactId>
+                <version>3.1.0</version>
+                <executions>
+                    <execution>
+                        <id>check-bundled-licenses</id>
+                        <phase>verify</phase>
+                        <goals>
+                            <goal>exec</goal>
+                        </goals>
+                        <configuration>
+                            <skip>${skipTests}</skip>
+                            <executable>bash</executable>
+                            <workingDirectory>${project.basedir}</workingDirectory>

Review Comment:
   Setting `workingDirectory` to the module dir means the script can't find `mvnw` (which is repo-root only). Either set this to `${maven.multiModuleProjectDirectory}` (and pass `${project.basedir}` as an arg, which the script already accepts), or pass the multi-module dir as an extra argument and resolve `mvnw` from there inside the script.



##########
release/scripts/check-bundled-licenses.sh:
##########
@@ -0,0 +1,262 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+set -euo pipefail
+
+# Color and timestamp logging helpers
+log() {
+  local color="$1"; shift
+  local msg="$1"; shift
+  local now
+  now="$(date '+%Y-%m-%d %H:%M:%S')"
+  printf "\033[%sm[%s] %s\033[0m\n" "$color" "$now" "$msg" "$@"
+}
+log_info() { log "1;34" "$1"; }
+log_success() { log "1;32" "$1"; }
+log_warn() { log "1;33" "$1"; }
+log_error() { log "1;31" "$1"; }
+
+cleanup_files=()
+
+cleanup() {
+  if [[ ${#cleanup_files[@]} -gt 0 ]]; then
+    rm -f "${cleanup_files[@]}"
+  fi
+}
+trap cleanup EXIT
+
+make_temp_file() {
+  local tmp_file
+  tmp_file="$(mktemp)"
+  cleanup_files+=("${tmp_file}")
+  printf '%s\n' "${tmp_file}"
+}
+
+ensure_single_bundled_jar() {
+  local module_dir="$1"
+  local jar_count
+  local jar_path
+
+  jar_count="$(find "${module_dir}/target" -maxdepth 1 -type f -name '*-bundled.jar' | wc -l | tr -d ' ')"
+
+  if [[ "${jar_count}" -eq 0 ]]; then
+    return 0
+  fi
+  if [[ "${jar_count}" -ne 1 ]]; then
+    log_error "Expected exactly one bundled jar in ${module_dir}/target, found ${jar_count}." >&2
+    return 2
+  fi
+
+  jar_path="$(find "${module_dir}/target" -maxdepth 1 -type f -name '*-bundled.jar' | sort | head -n 1)"
+  printf '%s\n' "${jar_path}"
+}
+
+jar_has_entry() {
+  local jar_path="$1"
+  local jar_entry="$2"
+  unzip -Z1 "${jar_path}" "${jar_entry}" >/dev/null 2>&1
+}
+
+verify_jar_entry_matches_source() {
+  local jar_path="$1"
+  local jar_entry="$2"
+  local source_path="$3"
+  local module_dir="$4"
+
+  if ! jar_has_entry "${jar_path}" "${jar_entry}"; then
+    log_error "[${module_dir}] Missing required entry ${jar_entry} in ${jar_path}"
+    exit 1
+  fi
+
+  local extracted_path
+  extracted_path="$(make_temp_file)"
+  unzip -p "${jar_path}" "${jar_entry}" > "${extracted_path}"
+
+  if ! cmp -s "${source_path}" "${extracted_path}"; then
+    log_error "[${module_dir}] ${jar_entry} in ${jar_path} does not match ${source_path}"
+    diff -u "${source_path}" "${extracted_path}" || true
+    exit 1
+  fi
+}
+
+verify_module_meta_inf_resources() {
+  local jar_path="$1"
+  local module_dir="$2"
+  local meta_inf_dir="$3"
+
+  while IFS= read -r source_path; do
+    local relative_path
+    local jar_entry
+    relative_path="${source_path#"${meta_inf_dir}/"}"
+    jar_entry="META-INF/${relative_path}"
+    verify_jar_entry_matches_source "${jar_path}" "${jar_entry}" "${source_path}" "${module_dir}"
+  done < <(find "${meta_inf_dir}" -type f | sort)
+}
+
+extract_bundled_coordinates() {
+  local jar_path="$1"
+  while IFS= read -r pom_properties; do
+    local coordinate
+    coordinate="$(
+      unzip -p "${jar_path}" "${pom_properties}" | tr -d "\r" | awk -F= '
+        $1 == "groupId" { group_id = $2 }
+        $1 == "artifactId" { artifact_id = $2 }
+        $1 == "version" { version = $2 }
+        END {
+          if (group_id != "" && artifact_id != "" && version != "" && group_id !~ /^org\.apache\.xtable$/) {
+            print group_id ":" artifact_id ":" version
+          }
+        }
+      '
+    )"
+    if [[ -n "${coordinate}" ]]; then
+      printf '%s\n' "${coordinate}"
+    fi
+  done < <(zipinfo -1 "${jar_path}" | awk '/^META-INF\/maven\/.*\/pom.properties$/') | sort -u
+}
+
+extract_license_coordinates() {
+  local license_path="$1"
+  tr -d '\r' < "${license_path}" \
+    | grep -Eo '[[:alnum:]_.-]+:[[:alnum:]_.-]+:(jar:)?[[:alnum:]_.+-]+' \
+    | awk -F: '
+      NF == 3 { print $1 ":" $2 ":" $3 }
+      NF == 4 && $3 == "jar" { print $1 ":" $2 ":" $4 }
+    ' \
+    | sort -u
+}
+
+# Extracts runtime dependencies using Maven for the given module
+extract_runtime_dependencies() {
+  local module_dir="$1"
+  local module_name
+  module_name=$(basename "$module_dir")
+  local dep_tree_output
+  dep_tree_output=$(./mvnw -pl "$module_name" dependency:tree -Dscope=runtime 2>/dev/null)

Review Comment:
   This is the cause of the CI exit 127. `./mvnw` is resolved against `cwd`, which is `${project.basedir}` per the pom config — but `mvnw` only exists at the repo root, so bash exits with 127 ("command not found"). It only triggers on `xtable-hudi-support-extensions_2.12` because every other module skips before reaching this code path.
   
   Minimal fix: pass `${maven.multiModuleProjectDirectory}` from the pom as a second script arg and use `"${project_root}/mvnw" -f "${module_dir}/pom.xml" dependency:tree -Dscope=runtime`. Using `-f <pom>` instead of `-pl <basename>` also avoids the silent breakage for nested modules (`-pl xtable-hudi-support-extensions` doesn't resolve when the module lives at `xtable-hudi-support/xtable-hudi-support-extensions`).



##########
release/scripts/check-bundled-licenses.sh:
##########
@@ -0,0 +1,262 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+set -euo pipefail
+
+# Color and timestamp logging helpers
+log() {
+  local color="$1"; shift
+  local msg="$1"; shift
+  local now
+  now="$(date '+%Y-%m-%d %H:%M:%S')"
+  printf "\033[%sm[%s] %s\033[0m\n" "$color" "$now" "$msg" "$@"
+}
+log_info() { log "1;34" "$1"; }
+log_success() { log "1;32" "$1"; }
+log_warn() { log "1;33" "$1"; }
+log_error() { log "1;31" "$1"; }
+
+cleanup_files=()
+
+cleanup() {
+  if [[ ${#cleanup_files[@]} -gt 0 ]]; then
+    rm -f "${cleanup_files[@]}"
+  fi
+}
+trap cleanup EXIT
+
+make_temp_file() {
+  local tmp_file
+  tmp_file="$(mktemp)"
+  cleanup_files+=("${tmp_file}")
+  printf '%s\n' "${tmp_file}"
+}
+
+ensure_single_bundled_jar() {
+  local module_dir="$1"
+  local jar_count
+  local jar_path
+
+  jar_count="$(find "${module_dir}/target" -maxdepth 1 -type f -name '*-bundled.jar' | wc -l | tr -d ' ')"
+
+  if [[ "${jar_count}" -eq 0 ]]; then
+    return 0
+  fi
+  if [[ "${jar_count}" -ne 1 ]]; then
+    log_error "Expected exactly one bundled jar in ${module_dir}/target, found ${jar_count}." >&2
+    return 2
+  fi
+
+  jar_path="$(find "${module_dir}/target" -maxdepth 1 -type f -name '*-bundled.jar' | sort | head -n 1)"
+  printf '%s\n' "${jar_path}"
+}
+
+jar_has_entry() {
+  local jar_path="$1"
+  local jar_entry="$2"
+  unzip -Z1 "${jar_path}" "${jar_entry}" >/dev/null 2>&1
+}
+
+verify_jar_entry_matches_source() {
+  local jar_path="$1"
+  local jar_entry="$2"
+  local source_path="$3"
+  local module_dir="$4"
+
+  if ! jar_has_entry "${jar_path}" "${jar_entry}"; then
+    log_error "[${module_dir}] Missing required entry ${jar_entry} in ${jar_path}"
+    exit 1
+  fi
+
+  local extracted_path
+  extracted_path="$(make_temp_file)"
+  unzip -p "${jar_path}" "${jar_entry}" > "${extracted_path}"
+
+  if ! cmp -s "${source_path}" "${extracted_path}"; then
+    log_error "[${module_dir}] ${jar_entry} in ${jar_path} does not match ${source_path}"
+    diff -u "${source_path}" "${extracted_path}" || true
+    exit 1
+  fi
+}
+
+verify_module_meta_inf_resources() {
+  local jar_path="$1"
+  local module_dir="$2"
+  local meta_inf_dir="$3"
+
+  while IFS= read -r source_path; do
+    local relative_path
+    local jar_entry
+    relative_path="${source_path#"${meta_inf_dir}/"}"
+    jar_entry="META-INF/${relative_path}"
+    verify_jar_entry_matches_source "${jar_path}" "${jar_entry}" "${source_path}" "${module_dir}"
+  done < <(find "${meta_inf_dir}" -type f | sort)
+}
+
+extract_bundled_coordinates() {

Review Comment:
   `extract_bundled_coordinates` reads `META-INF/maven/*/pom.properties` directly from the produced jar — i.e. exactly what was actually shaded — and already filters `org.apache.xtable`. It's defined here but never called; `verify_bundled_coordinates_are_in_license` uses `extract_runtime_dependencies` instead.
   
   Using the runtime tree as the source of truth produces both false positives (deps the shade plugin excluded — delta-kernel, iceberg-common, scala-library, xtable's own modules) and false negatives (anything shade pulls in via relocations that's not on the runtime classpath). I think the previous bundled-coords approach was correct; happy to discuss if there's a case the dep-tree catches that bundled-coords misses.



-- 
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: commits-unsubscribe@xtable.apache.org

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