Fix bug 48763270 Mariner's go downloader clobbers prev logs, making it hard to diagnose toolchain download errors

[Xiaohong-Deng]

Merge Checklist

All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)

• [x] The toolchain has been rebuilt successfully (or no changes were made to it)
• [x] The toolchain/worker package manifests are up-to-date
• [x] Any updated packages successfully build (or no packages were changed)
• [x] Packages depending on static components modified in this PR (Golang, *-static subpackages, etc.) have had their Release tag incremented.
• [x] Package tests (%check section) have been verified with RUN_CHECK=y for existing SPEC files, or added to new SPEC files
• [x] All package sources are available
• [x] cgmanifest files are up-to-date and sorted (./cgmanifest.json, ./toolkit/scripts/toolchain/cgmanifest.json, .github/workflows/cgmanifest.json)
• [x] LICENSE-MAP files are up-to-date (./SPECS/LICENSES-AND-NOTICES/data/licenses.json, ./SPECS/LICENSES-AND-NOTICES/LICENSES-MAP.md, ./SPECS/LICENSES-AND-NOTICES/LICENSE-EXCEPTIONS.PHOTON)
• [x] All source files have up-to-date hashes in the *.signatures.json files
• [x] sudo make go-tidy-all and sudo make go-test-coverage pass
• [x] Documentation has been updated to match any changes to the build system
• [x] Ready to merge

---

Summary

The for loop at line 187 of toolchain.mk is meant to write and keep the logs from all the iterations but the logs from the current iteration overwrites the previous one. For more detail please refer to work item BUG 48763270

Change Log

• Every invocation of the target toolchain_rpms_rehydrated flushes the log file.
• Move the for loop from the toolchain.mk to the downloader.go, passing url list to the downloader.

Does this affect the toolchain?

YES

Associated issues

• BUG 48763270 is an internal work item.

Links to CVEs

• None

Test Methodology

• This affects QUICK_REBUILD_TOOLCHAIN=y in local build

[dmcilvaney]

I believe I sent this to m40 back in Jan as an alternative approach:

diff --git a/toolkit/scripts/toolchain.mk b/toolkit/scripts/toolchain.mk
index 15b6c8e7a..168bc3f63 100644
--- a/toolkit/scripts/toolchain.mk
+++ b/toolkit/scripts/toolchain.mk
@@ -191,7 +191,10 @@ $(toolchain_rpms_rehydrated): $(TOOLCHAIN_MANIFEST) $(go-downloader)
  echo "Attempting to download toolchain RPM: $$rpm_filename" | tee -a "$$log_file" && \
  mkdir -p $$rpm_dir && \
  cd $$rpm_dir && \
+  url_num=0 && \
  for url in $(PACKAGE_URL_LIST); do \
+    log_file="$(toolchain_downloads_logs_dir)/$${rpm_filename}.url$${url_num}.log" && \
+    url_num=$$(($$url_num + 1)) && \
    $(go-downloader) --no-verbose --no-clobber $$url/$$rpm_filename \
      $(if $(TLS_CERT),--certificate=$(TLS_CERT)) \
      $(if $(TLS_KEY),--private-key=$(TLS_KEY)) \
@@ -310,7 +313,10 @@ $(toolchain_rpms): $(TOOLCHAIN_MANIFEST) $(depend_REBUILD_TOOLCHAIN) $(go-downlo
  echo "Downloading toolchain RPM: $$rpm_filename" | tee -a "$$log_file" && \
  mkdir -p $$rpm_dir && \
  cd $$rpm_dir && \
+  url_num=0 && \
  for url in $(PACKAGE_URL_LIST); do \
+    log_file="$(toolchain_downloads_logs_dir)/$${rpm_filename}.url$${url_num}.log" && \
+    url_num=$$(($$url_num + 1)) && \
    $(go-downloader) --no-verbose --no-clobber $$url/$$rpm_filename \
      $(if $(TLS_CERT),--certificate=$(TLS_CERT)) \
      $(if $(TLS_KEY),--private-key=$(TLS_KEY)) \

Its a bit cumbersome but will avoid stomping logs while also not requiring changes to the logging library.

[reubeno]

@dmcilvaney @Xiaohong-Deng @PawelWMS -- great to see the good discussion! I see the change is currently targeting main. What do y'all think of retargeting this change against 3.0-dev? That should give you breathing room to make the change the way y'all think it should look without any risk to the 2.0 LTS.

[PawelWMS]

@dmcilvaney @Xiaohong-Deng @PawelWMS -- great to see the good discussion! I see the change is currently targeting main. What do y'all think of retargeting this change against 3.0-dev? That should give you breathing room to make the change the way y'all think it should look without any risk to the 2.0 LTS.

+1. We definitely want to reflect all 2.0 changes in 3.0. Depending on how invasive or not this one is going to be, it may still meet the bar to merge it to main as well. Having said that, I've got nothing against making this change 3.0-dev-only.