You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hamilton.apache.org by sk...@apache.org on 2026/06/17 05:55:58 UTC

(hamilton) 01/01: Harden release + verify scripts for SVN upload and local artifact dirs

This is an automated email from the ASF dual-hosted git repository.

skrawcz pushed a commit to branch stefan/harden-release-scripts
in repository https://gitbox.apache.org/repos/asf/hamilton.git

commit f1de0b19409d796df589a400917423e58d0b0ee5
Author: Stefan Krawczyk <st...@cs.stanford.edu>
AuthorDate: Tue Jun 16 23:52:55 2026 -0600

    Harden release + verify scripts for SVN upload and local artifact dirs
    
    These bugs surfaced while cutting the sub-package RC1 release.
    
    apache_release_helper.py:
    - create_release_artifacts: return absolute artifact paths. The function
      chdir's into the package working_dir to build, but svn_upload runs from
      the repo root, so the previously-returned relative paths failed to
      resolve (svn import 'No such file or directory').
    - svn_upload: tolerate an already-existing SVN RC directory so a re-run
      after a partial/failed upload can resume importing files instead of
      aborting on 'svn mkdir'.
    
    verify-sub-packages/*.sh:
    - Resolve ARTIFACTS_DIR to an absolute path right after acquisition. The
      functional step does 'cd /tmp' (to avoid the local checkout shadowing
      venv packages), which broke a relative local artifacts dir.
---
 scripts/apache_release_helper.py              | 18 +++++++++++++++---
 scripts/verify-sub-packages/verify_contrib.sh |  4 ++++
 scripts/verify-sub-packages/verify_lsp.sh     |  4 ++++
 scripts/verify-sub-packages/verify_sdk.sh     |  4 ++++
 scripts/verify-sub-packages/verify_ui.sh      |  4 ++++
 5 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/scripts/apache_release_helper.py b/scripts/apache_release_helper.py
index 84bb3ca8..d62d5330 100644
--- a/scripts/apache_release_helper.py
+++ b/scripts/apache_release_helper.py
@@ -456,7 +456,9 @@ def create_release_artifacts(package_config: dict, version, no_sign: bool = Fals
             wheel_signed_files = sign_artifacts(wheel_file)
 
         files_to_upload = [new_tar_ball, *new_tar_ball_signed, wheel_file, *wheel_signed_files]
-        return files_to_upload
+        # Return absolute paths: the caller (svn_upload) runs from the repo root,
+        # but these paths are relative to the package working_dir we chdir'd into.
+        return [os.path.abspath(f) for f in files_to_upload]
 
     finally:
         # Always return to original directory
@@ -474,10 +476,13 @@ def svn_upload(package_name: str, version, rc_num, files_to_import: list[str], a
 
     try:
         # Create a new directory for the release candidate.
+        # Tolerate an already-existing directory (e.g. re-running after a
+        # partial upload): mkdir failing because the path exists is not fatal,
+        # we still want to import the files below.
         print(
             f"Creating directory for {package_name} {version}-incubating-RC{rc_num}... at {svn_path}"
         )
-        subprocess.run(
+        mkdir_result = subprocess.run(
             [
                 "svn",
                 "mkdir",
@@ -486,8 +491,15 @@ def svn_upload(package_name: str, version, rc_num, files_to_import: list[str], a
                 f"Creating directory for {package_name} {version}-incubating-RC{rc_num}",
                 svn_path,
             ],
-            check=True,
+            capture_output=True,
+            text=True,
         )
+        if mkdir_result.returncode != 0:
+            if "already exists" in (mkdir_result.stderr or ""):
+                print(f"Directory already exists at {svn_path}, continuing to import files.")
+            else:
+                print(f"Error creating SVN directory: {mkdir_result.stderr.strip()}")
+                return None
 
         # Use svn import for the new directory.
         for file_path in files_to_import:
diff --git a/scripts/verify-sub-packages/verify_contrib.sh b/scripts/verify-sub-packages/verify_contrib.sh
index dfde3214..6b8eb31f 100755
--- a/scripts/verify-sub-packages/verify_contrib.sh
+++ b/scripts/verify-sub-packages/verify_contrib.sh
@@ -59,6 +59,10 @@ if [ -z "$ARTIFACTS_DIR" ]; then
     svn export -q "https://dist.apache.org/repos/dist/dev/incubator/hamilton/${PACKAGE}/${VERSION}-RC${RC}/" "$ARTIFACTS_DIR"
 fi
 
+# Resolve to an absolute path: later steps cd into /tmp, which would break
+# a relative artifacts dir.
+ARTIFACTS_DIR="$(cd "$ARTIFACTS_DIR" && pwd)"
+
 echo "Artifacts: $ARTIFACTS_DIR"
 ls "$ARTIFACTS_DIR"/*.tar.gz "$ARTIFACTS_DIR"/*.whl 2>/dev/null || { echo "ERROR: No artifacts found"; exit 1; }
 
diff --git a/scripts/verify-sub-packages/verify_lsp.sh b/scripts/verify-sub-packages/verify_lsp.sh
index 976e4e31..e286691f 100755
--- a/scripts/verify-sub-packages/verify_lsp.sh
+++ b/scripts/verify-sub-packages/verify_lsp.sh
@@ -59,6 +59,10 @@ if [ -z "$ARTIFACTS_DIR" ]; then
     svn export -q "https://dist.apache.org/repos/dist/dev/incubator/hamilton/${PACKAGE}/${VERSION}-RC${RC}/" "$ARTIFACTS_DIR"
 fi
 
+# Resolve to an absolute path: later steps cd into /tmp, which would break
+# a relative artifacts dir.
+ARTIFACTS_DIR="$(cd "$ARTIFACTS_DIR" && pwd)"
+
 echo "Artifacts: $ARTIFACTS_DIR"
 ls "$ARTIFACTS_DIR"/*.tar.gz "$ARTIFACTS_DIR"/*.whl 2>/dev/null || { echo "ERROR: No artifacts found"; exit 1; }
 
diff --git a/scripts/verify-sub-packages/verify_sdk.sh b/scripts/verify-sub-packages/verify_sdk.sh
index ea9cf4aa..0bbfb703 100755
--- a/scripts/verify-sub-packages/verify_sdk.sh
+++ b/scripts/verify-sub-packages/verify_sdk.sh
@@ -59,6 +59,10 @@ if [ -z "$ARTIFACTS_DIR" ]; then
     svn export -q "https://dist.apache.org/repos/dist/dev/incubator/hamilton/${PACKAGE}/${VERSION}-RC${RC}/" "$ARTIFACTS_DIR"
 fi
 
+# Resolve to an absolute path: later steps cd into /tmp, which would break
+# a relative artifacts dir.
+ARTIFACTS_DIR="$(cd "$ARTIFACTS_DIR" && pwd)"
+
 echo "Artifacts: $ARTIFACTS_DIR"
 ls "$ARTIFACTS_DIR"/*.tar.gz "$ARTIFACTS_DIR"/*.whl 2>/dev/null || { echo "ERROR: No artifacts found"; exit 1; }
 
diff --git a/scripts/verify-sub-packages/verify_ui.sh b/scripts/verify-sub-packages/verify_ui.sh
index 24a03d41..4d60b3d5 100755
--- a/scripts/verify-sub-packages/verify_ui.sh
+++ b/scripts/verify-sub-packages/verify_ui.sh
@@ -59,6 +59,10 @@ if [ -z "$ARTIFACTS_DIR" ]; then
     svn export -q "https://dist.apache.org/repos/dist/dev/incubator/hamilton/${PACKAGE}/${VERSION}-RC${RC}/" "$ARTIFACTS_DIR"
 fi
 
+# Resolve to an absolute path: later steps cd into /tmp, which would break
+# a relative artifacts dir.
+ARTIFACTS_DIR="$(cd "$ARTIFACTS_DIR" && pwd)"
+
 echo "Artifacts: $ARTIFACTS_DIR"
 ls "$ARTIFACTS_DIR"/*.tar.gz "$ARTIFACTS_DIR"/*.whl 2>/dev/null || { echo "ERROR: No artifacts found"; exit 1; }