SL-18837: Use multi-line GitHub outputs to upload artifacts.

Having observed installer upload failures, I discovered the warning in
actions/upload-artifact/README.md about multiple concurrent jobs trying to
post the same pathname to the same artifact name. Try to disambiguate
artifacts not only for different platforms, but for different jobs running on
the same platform.

This change also reflects my understanding that an artifact is (effectively) a
distinct zip file that can contain multiple uploaded files. Because we'll want
to download metadata without having to download enormous installers, create a
separate metadata artifact per platform. Similarly, symbol files can get large:
use a third distinct artifact for symbol files.

But with those artifacts defined, leverage actions/upload-artifact's ability
to upload multiple paths to the same artifact. In build.sh, define bash arrays
installer, metadata, symbolfile and set up so that, on exit, each is written
to a GITHUB_OUTPUT variable with the corresponding name. This involves a
little magic to get macOS bash 3 to indirectly access an array.

These multi-line output variables are then used to drive the upload-artifact
step for each of the defined artifacts.

2 files changed, 65 insertions, 70 deletions

diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
index e55a799825..574a83246c 100644
--- a/.github/workflows/build.yaml
+++ b/.github/workflows/build.yaml
@@ -186,58 +186,48 @@ jobs:
 
           ./build.sh
 
-          # Find artifacts
-          if [[ "$RUNNER_OS" == "Windows" ]]; then
-            installer_path=$(find ./build-*/newview/ | grep '_Setup\.exe')
-            installer_name="$(basename $installer_path)"
-          elif [[ "$RUNNER_OS" == "macOS" ]]; then
-            installer_path=$(find ./build-*/newview/ | grep '\.dmg')
-            installer_name="$(basename $installer_path)"
-          fi
-
-          echo "installer_path=$installer_path" >> $GITHUB_OUTPUT
-          echo "installer_name=$installer_name" >> $GITHUB_OUTPUT
+          # Each artifact is downloaded as a distinct .zip file. Multiple jobs
+          # (per the matrix above) writing the same filepath to the same
+          # artifact name will *overwrite* that file. Moreover, they can
+          # interfere with each other, causing the upload to fail.
+          # https://github.com/actions/upload-artifact#uploading-to-the-same-artifact
+          # Given the size of our installers, and the fact that we typically
+          # only want to download just one instead of a single zip containing
+          # several, generate a distinct artifact name for each installer.
+          # Since the matrix above can run multiple builds on the same
+          # platform, we must disambiguate on more than the platform name.
+          # If we were still running Windows 32-bit builds, we'd need to
+          # qualify the artifact with bit width.
+          # DEVELOPER_DIR="/Applications/Xcode_14.0.1.app/Contents/Developer"
+          # or the empty string, so this produces dev="Xcode_14.0.1" or ".".
+          dev="$(basename "$(dirname "$(dirname "$DEVELOPER_DIR")")" .app)"
+          artifact="$RUNNER_OS $dev"
+          # For empty DEVELOPER_DIR, dev is ".", so artifact can end up with
+          # appended " ." -- ditch that if present.
+          artifact="${artifact% .}"
+          echo "artifact=$artifact" >> $GITHUB_OUTPUT
 
       - name: Upload installer
         uses: actions/upload-artifact@v3
         with:
-          name: ${{ steps.build.outputs.installer_name }}
-          path: ${{ steps.build.outputs.installer_path }}
-
-      - name: Upload metadata
-        uses: actions/upload-artifact@v3
-        with:
-          name: ${{ steps.build.outputs.autobuild_package_name }}
-          path: ${{ steps.build.outputs.autobuild_package_path }}
-
-      - name: Upload version
-        uses: actions/upload-artifact@v3
-        with:
-          name: viewer_version.txt
-          path: ${{ steps.build.outputs.viewer_version_name }}
+          name: "${{ steps.build.outputs.artifact }} installer"
+          # emitted by build.sh, possibly multiple lines
+          path: |
+            ${{ steps.build.outputs.installer }}
 
-      - name: Upload Doxygen Log
-        if: steps.build.outputs.doxygen_log_path
-        uses: actions/upload-artifact@v3
-        with:
-          name: ${{ steps.build.outputs.doxygen_log_name }}
-          path: ${{ steps.build.outputs.doxygen_log_path }}
-
-      - name: Upload Doxygen Tarball
-        if: steps.build.outputs.doxygen_tarball_path
-        uses: actions/upload-artifact@v3
-        with:
-          name: ${{ steps.build.outputs.doxygen_tarball_name }}
-          path: ${{ steps.build.outputs.doxygen_tarball_path }}
-
-      - name: Upload viewer package installers
+      # The other upload of nontrivial size is the symbol file. Use a distinct
+      # artifact for that too.
+      - name: Upload symbol file
         uses: actions/upload-artifact@v3
         with:
-          name: ${{ steps.build.outputs.installer_name }}
-          path: ${{ steps.build.outputs.packages }}
+          name: "${{ steps.build.outputs.artifact }} symbols"
+          path: |
+            ${{ steps.build.outputs.symbolfile }}
 
-      - name: Upload symbol file
+      - name: Upload metadata
         uses: actions/upload-artifact@v3
         with:
-          name: ${{ steps.build.outputs.symbolfile_name }}
-          path: ${{ steps.build.outputs.symbolfile_path }}
+          name: "${{ steps.build.outputs.artifact }} metadata"
+          # emitted by build.sh, possibly multiple lines
+          path: |
+            ${{ steps.build.outputs.metadata }}
diff --git a/build.sh b/build.sh
index c50bf1df9d..bd3e84d7bd 100755
--- a/build.sh
+++ b/build.sh
@@ -16,6 +16,8 @@
 # * The special style in which python is invoked is intentional to permit
 #   use of a native python install on windows - which requires paths in DOS form
 
+cleanup="true"
+
 retry_cmd()
 {
     max_attempts="$1"; shift
@@ -111,24 +113,28 @@ installer_CYGWIN()
 }
 
 [[ -n "$GITHUB_OUTPUT" ]] || fatal "Need to export GITHUB_OUTPUT"
-
-addoutput()
-{
-    local varname="$1"
-    local value="$2"
-    echo "${varname}_name=$(basename "$value")" >> "$GITHUB_OUTPUT"
-    echo "${varname}_path=$value" >> "$GITHUB_OUTPUT"
-}
-
-# For variable numbers of output values, separate them one per line.
-addarrayoutput()
+# The following is based on the Warning for GitHub multiline output strings:
+# https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#multiline-strings
+EOF=$(dd if=/dev/urandom bs=15 count=1 status=none | base64)
+
+# Build up these arrays as we go
+installer=()
+metadata=()
+symbolfile=()
+# and dump them to GITHUB_OUTPUT when done
+cleanup="$cleanup ; arrayoutput installer ; arrayoutput metadata ; arrayoutput symbolfile"
+trap "$cleanup" EXIT
+
+arrayoutput()
 {
-    local varname="$1"
-    # indirection: specify the *name* of the array to emit
-    local -n arrayname="$2"
+    local outputname="$1"
+    # append "[*]" to the array name so array indirection works
+    local array="$1[*]"
     local IFS='
 '
-    echo "$varname=${arrayname[*]}" >> "$GITHUB_OUTPUT"
+    echo "$outputname<<$EOF
+${!array}
+$EOF" >> "$GITHUB_OUTPUT"
 }
 
 pre_build()
@@ -216,7 +222,8 @@ package_llphysicsextensions_tpv()
       
       # capture the package file name for use in upload later...
       PKGTMP=`mktemp -t pgktpv.XXXXXX`
-      trap "rm $PKGTMP* 2>/dev/null" 0
+      cleanup="$cleanup ; rm $PKGTMP* 2>/dev/null"
+      trap "$cleanup" EXIT
       "$autobuild" package --quiet --config-file "$tpvconfig" --results-file "$(native_path $PKGTMP)" || fatal "failed to package llphysicsextensions_tpv"
       tpv_status=$?
       if [ -r "${PKGTMP}" ]
@@ -390,7 +397,7 @@ do
                   begin_section "Autobuild metadata"
                   python_cmd "$helpers/codeticket.py" addoutput "Autobuild Metadata" "$build_dir/autobuild-package.xml" --mimetype text/xml \
                       || fatal "Upload of autobuild metadata failed"
-                  addoutput autobuild_package "$build_dir/autobuild-package.xml"
+                  metadata+=("$build_dir/autobuild-package.xml")
                   if [ "$arch" != "Linux" ]
                   then
                       record_dependencies_graph "$build_dir/autobuild-package.xml" # defined in buildscripts/hg/bin/build.sh
@@ -406,7 +413,7 @@ do
                   begin_section "Viewer Version"
                   python_cmd "$helpers/codeticket.py" addoutput "Viewer Version" "$(<"$build_dir/newview/viewer_version.txt")" --mimetype inline-text \
                       || fatal "Upload of viewer version failed"
-                  addoutput viewer_version "$(<"$build_dir/newview/viewer_version.txt")"
+                  metadata+=("$build_dir/newview/viewer_version.txt")
                   end_section "Viewer Version"
               fi
               ;;
@@ -415,14 +422,14 @@ do
               then
                   record_event "Doxygen warnings generated; see doxygen_warnings.log"
                   python_cmd "$helpers/codeticket.py" addoutput "Doxygen Log" "$build_dir/doxygen_warnings.log" --mimetype text/plain ## TBD
-                  addoutput doxygen_log "$build_dir/doxygen_warnings.log"
+                  metadata+=("$build_dir/doxygen_warnings.log")
               fi
               if [ -d "$build_dir/doxygen/html" ]
               then
                   tar -c -f "$build_dir/viewer-doxygen.tar.bz2" --strip-components 3  "$build_dir/doxygen/html"
                   python_cmd "$helpers/codeticket.py" addoutput "Doxygen Tarball" "$build_dir/viewer-doxygen.tar.bz2" \
                       || fatal "Upload of doxygen tarball failed"
-                  addoutput doxygen_tarball "$build_dir/viewer-doxygen.tar.bz2"
+                  metadata+=("$build_dir/viewer-doxygen.tar.bz2")
               fi
               ;;
             *)
@@ -538,7 +545,7 @@ then
       retry_cmd 4 30 python_cmd "$helpers/codeticket.py" addoutput Installer "$package"  \
           || fatal "Upload of installer failed"
       wait_for_codeticket
-      packages=("$package")
+      installer+=("$package")
 
       # Upload additional packages.
       for package_id in $additional_packages
@@ -549,14 +556,12 @@ then
           retry_cmd 4 30 python_cmd "$helpers/codeticket.py" addoutput "Installer $package_id" "$package" \
               || fatal "Upload of installer $package_id failed"
           wait_for_codeticket
-          packages+=("$package")
+          installer+=("$package")
         else
           record_failure "Failed to find additional package for '$package_id'."
         fi
       done
 
-      addarrayoutput viewer_packages packages
-
       if [ "$last_built_variant" = "Release" ]
       then
           # nat 2016-12-22: without RELEASE_CRASH_REPORTING, we have no symbol file.
@@ -566,7 +571,7 @@ then
               retry_cmd 4 30 python_cmd "$helpers/codeticket.py" addoutput "Symbolfile" "$VIEWER_SYMBOL_FILE" \
                   || fatal "Upload of symbolfile failed"
               wait_for_codeticket
-              addoutput symbolfile "$VIEWER_SYMBOL_FILE"
+              symbolfile+=("$VIEWER_SYMBOL_FILE")
           fi
 
           # Upload the llphysicsextensions_tpv package, if one was produced