From fdfd85463c5919d5ec52ee0389af92771b42709f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mat=C3=ADas=20A=2E=20Bellone?=
 <matias.bellone@buildkite.com>
Date: Wed, 24 Jul 2024 18:49:51 -0300
Subject: [PATCH 1/2] Better handling of ignore missing option

---
 hooks/pre-command | 64 ++++++++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 28 deletions(-)

diff --git a/hooks/pre-command b/hooks/pre-command
index 70c5a60..55e0622 100755
--- a/hooks/pre-command
+++ b/hooks/pre-command
@@ -64,16 +64,17 @@ bk_agent() {
   options+=("${@}")  # all the rest
 
   echo "~~~ Downloading artifacts ${EXTRA_MESSAGE}"
-  if ! buildkite-agent artifact "${options[@]}"; then
-    if [[ "${BUILDKITE_PLUGIN_ARTIFACTS_IGNORE_MISSING:-"false"}" != "false" ]]; then
-      echo "Ignoring error in download of" "${@: -2}"
-    else
-      echo "Error in download of" "${@: -2}"
-      return 1
-    fi
-  fi
+  buildkite-agent artifact "${options[@]}"
+}
 
-  return 0
+handle_bk_error() {
+  local filename="$1"
+  if [[ "${BUILDKITE_PLUGIN_ARTIFACTS_IGNORE_MISSING:-"false"}" != "false" ]]; then
+    echo "Ignoring error in download of ${filename}"
+  else
+    echo "Error in download of ${filename}"
+    exit 1
+  fi
 }
 
 handle_relocation() {
@@ -114,23 +115,25 @@ fi
 workdir="${BUILDKITE_PLUGIN_ARTIFACTS_WORKDIR:-.}"
 
 if [[ "${COMPRESSED}" == "true" ]]; then
-  bk_agent "${step_option}" "${build_option}" "${BUILDKITE_PLUGIN_ARTIFACTS_COMPRESSED}" "${workdir}"
+  if ! bk_agent "${step_option}" "${build_option}" "${BUILDKITE_PLUGIN_ARTIFACTS_COMPRESSED}" "${workdir}"; then
+    handle_bk_error "${BUILDKITE_PLUGIN_ARTIFACTS_COMPRESSED}"
+  else
+    echo "~~~ Uncompressing ${paths[*]} from ${BUILDKITE_PLUGIN_ARTIFACTS_COMPRESSED}"
+    "${compress[@]}" "${paths[@]}"
 
-  echo "~~~ Uncompressing ${paths[*]} from ${BUILDKITE_PLUGIN_ARTIFACTS_COMPRESSED}"
-  "${compress[@]}" "${paths[@]}"
+    # single relocation
+    if [[ -n "${BUILDKITE_PLUGIN_ARTIFACTS_DOWNLOAD_TO:-}" ]]; then
+      handle_relocation ""
+    fi
 
-  # single relocation
-  if [[ -n "${BUILDKITE_PLUGIN_ARTIFACTS_DOWNLOAD_TO:-}" ]]; then
-    handle_relocation ""
+    # multiple relocations
+    index=0
+    for path in "${paths[@]}"; do
+      handle_relocation "${index}"
+      ((index+=1))
+    done
   fi
 
-  # multiple relocations
-  index=0
-  for path in "${paths[@]}"; do
-    handle_relocation "${index}"
-    ((index+=1))
-  done
-
 elif [[ "${SINGULAR_DOWNLOAD_OBJECT}" == "true" ]]; then
   if [[ "${RELOCATION}" == "true" ]]; then
     source="${BUILDKITE_PLUGIN_ARTIFACTS_DOWNLOAD_FROM}"
@@ -138,10 +141,12 @@ elif [[ "${SINGULAR_DOWNLOAD_OBJECT}" == "true" ]]; then
     source="${paths[*]}"
   fi
 
-  bk_agent "${step_option}" "${build_option}" "${source}" "${workdir}"
-
-  if [[ -n "${BUILDKITE_PLUGIN_ARTIFACTS_DOWNLOAD_TO:-}" ]]; then
-    handle_relocation ""
+  if ! bk_agent "${step_option}" "${build_option}" "${source}" "${workdir}"; then
+    handle_bk_error "${source}"
+  else
+    if [[ -n "${BUILDKITE_PLUGIN_ARTIFACTS_DOWNLOAD_TO:-}" ]]; then
+      handle_relocation ""
+    fi
   fi
 elif [[ "${MULTIPLE_DOWNLOADS}" == "true" ]]; then
   index=0
@@ -159,9 +164,12 @@ elif [[ "${MULTIPLE_DOWNLOADS}" == "true" ]]; then
       source="${path}"
     fi
 
-    bk_agent "${!step_env_var:-${step_option}}" "${!build_env_var:-${build_option}}" "${source}" "${workdir}"
+    if ! bk_agent "${!step_env_var:-${step_option}}" "${!build_env_var:-${build_option}}" "${source}" "${workdir}"; then
+      handle_bk_error "${source}"
+    else
+      handle_relocation "${index}"
+    fi
 
-    handle_relocation "${index}"
     ((index+=1))
   done
 fi

From 6a4d64e39e076066cb6878097b290276d775049e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mat=C3=ADas=20A=2E=20Bellone?=
 <matias.bellone@buildkite.com>
Date: Wed, 24 Jul 2024 18:50:06 -0300
Subject: [PATCH 2/2] Updated tests due to removed message

---
 tests/download-ignore-missing.bats | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tests/download-ignore-missing.bats b/tests/download-ignore-missing.bats
index 71c3552..9ff0e1e 100644
--- a/tests/download-ignore-missing.bats
+++ b/tests/download-ignore-missing.bats
@@ -36,7 +36,6 @@ setup() {
   assert_success
   assert_output --partial "Downloading artifacts"
   assert_output --partial "Ignoring error in download of /tmp/foo.log"
-  assert_output --partial "Ignoring missing file /tmp/foo.log for relocation"
 
   unstub buildkite-agent
 }
@@ -102,7 +101,6 @@ setup() {
   assert_success
   assert_output --partial "Downloading artifacts"
   assert_output --partial "Ignoring error in download of /tmp/foo.log"
-  assert_output --partial "Ignoring missing file /tmp/foo.log for relocation"
   refute_output --partial "download artifact /tmp/foo.log"
 
   assert [ ! -e /tmp/foo2.log ]
@@ -111,7 +109,6 @@ setup() {
   unstub buildkite-agent
 }
 
-
 @test "Pre-command downloads multiple > 10 artifacts with build and relocation and some failures" {
   stub_calls=()
   for i in $(seq 0 10); do
@@ -139,10 +136,9 @@ setup() {
       assert [ ! -e /tmp/foo-r-"${i}".log ]
       assert [ ! -e /tmp/foo-"${i}".log ]
       assert_output --partial "Ignoring error in download of /tmp/foo-${i}.log"
-      assert_output --partial "Ignoring missing file /tmp/foo-${i}.log"
       refute_output --partial "downloaded artifact /tmp/foo-${i}.log"
     fi
   done
 
   unstub buildkite-agent
-}
\ No newline at end of file
+}