[release/7.11] Cherry-pick: Handle missing Artifactory during packaging

[raramakr]

Packaging builds were failing when artifactories for certain gfx architectures
were missing. These cases are now handled gracefully, allowing the build to
continue and providing clearer diagnostics. https://amd-hub.atlassian.net/browse/ROCM-1283

A real‑time package summary is now generated based on build status, improving
visibility into which components succeeded, failed, or were skipped.

Debian release metadata has been updated to include required hash fields
(md5, sha256, etc.), ensuring compliance with Debian tooling expectations.
Additionally, the large regenerate_repo_metadata_from_s3 function has been
split into smaller deb/rpm‑specific functions for better maintainability and
clearer separation of responsibilities.

PRs cherry-picked as part of this PR:
#3017 : Fix for packaging build failures due to missing artifactory
#3059 : Prevent the uploading of .txt files to package repository
#3006 : Fix for missing hash values like md5, sha256 in the Release file.

[nunnikri]

Looks good to me.
Started a job to see if it works fine. https://github.com/ROCm/TheRock/actions/runs/21378125093

[marbre]

@raramakr this needs to go to main first. After it was landed we can cherry-pick it afterwards and target the release branch. Furthermore, make sure to assign @HereThereBeDragons and @araravik-psd as reviewers if you have something that must go into the release branch but this is limited to isolated infra related cherry-picks.

[raramakr]

@raramakr this needs to go to main first. After it was landed we can cherry-pick it afterwards and target the release branch. Furthermore, make sure to assign @HereThereBeDragons and @araravik-psd as reviewers if you have something that must go into the release branch but this is limited to isolated infra related cherry-picks.

The commits in the PR are already in the main branch.
Will add the mentioned reviewers

[ScottTodd]

Cherry-picks should be clearly labeled as such, linking to the commits they are cherry-picking with some rationale for why the commits are also needed on the release branch. This PR was a poor use of reviewer time and attention.

[raramakr]

Cherry-picks should be clearly labeled as such, linking to the commits they are cherry-picking with some rationale for why the commits are also needed on the release branch. This PR was a poor use of reviewer time and attention.

Noted. Will take care of this in future PR.
Update the PR description with the details as well

[HereThereBeDragons]

as these are cherry picked prs i guess it is ok to merge as-is
there are some questions that need to be answered.
and quiet a few of quality improvements that should go into main.

especially:
run_command() should behave like subprocess.run() and as such should have a list of strings with each command and param as an entry of the list, and not a single string containing the entire command as whole.

please either create a pr with those fixes, or create an issue for it to be tracked and not get lost.

[HereThereBeDragons]

is here an else missing?

[nunnikri]

@HereThereBeDragons for rpath packages as per requirement only versioned packages are needed. As a follow up patch will add a comment to make it clear.

[HereThereBeDragons]

ok. my question was here if create_versioned_rpm_package would overwrite create_nonversioned_rpm_package because there is no else branch?

[nunnikri]

no it wont. Its a totally different set of packages. nonversioned depends on versioned packages. Here by default we need to create both versioned and nonversioned packages. But if rpath is enabled we need to create only versioned. Hence the code is bit confusing, Will make it more clear with comments

[araravik-psd]

@nunnikri so if by default we are building both it will be nice if this is added as a comment here. As we are not passing enable_rpath here by default now.

[HereThereBeDragons]

good practice would be

if pkg_type == "rpm":
      ...
elif pkg_type == "deb":
      ...
else: 
   <output error>

this is a fail-safe to catch problems if new package types are added

[nunnikri]

The pkg_type check is already done in line #782. Thats why the else for failsafe is not added here. But can see some more options to optimize the code. Will do as a follow up PR.

[HereThereBeDragons]

shouldnt this be skipped_pkg_list or skip_list or skipped_list for consistency? as already used in other functions in sthis file

[nunnikri]

ACK. will do as a follow up PR

[HereThereBeDragons]

is startswith the correct match? what would be the full dir_name? and can you predict the name and with this reduce the loop duration?

[nunnikri]

ACK. will look at optimizing the loop

[HereThereBeDragons]

split into ["createrepo_c", "--no-database", ...]

[nunnikri]

ACK. will do as the follow up PR

[HereThereBeDragons]

i dont understand thoses ifs here. you either have all the hashes or none. the logic above does not allow to have md5 hashes but not sha1 hashes,

[nunnikri]

As per Debian standard, atleast one hash value is needed. Its not mandatory to have all 3.
md5sum and sha1 are added for older systems to work as backward compatibility. Newer systems will be looking for sha256

[HereThereBeDragons]

i understand, but we are calculating always all 3 hashes. as such we will also always add either none or all 3.
i dont see any selection logic where you can say "only sha256". i am just saying: those 3 ifs can be put together in a single one

[nunnikri]

I understand your concern. Since Debian standard allows to continue even if one hash value generation fails did the coding as seperate if. for example even if hashlib.sha1() fails, functionality can continue with md5 and sha256. But will discuss with other experts in this area and take a call here

[HereThereBeDragons]

this needs more explanation

[nunnikri]

ACK. Will add more comments to make the code more readable.

[HereThereBeDragons]

no job_type for rpm?

[nunnikri]

job_type is used to distinguish dev/nightly or prerelease builds

[HereThereBeDragons]

rpms seems not to take job_type into account, while the function for deb does

[nunnikri]

ah.. now i understood your comment. debian repo creation needs to create a Release file. Which is a text file meta data about the repo. We are differentiating the repo metadata bw dev/nightly/prerelease. Hence that parameter is passed. In rpm case no such provision is available, hence its not used

[nunnikri]

as these are cherry picked prs i guess it is ok to merge as-is there are some questions that need to be answered. and quiet a few of quality improvements that should go into main.

especially: run_command() should behave like subprocess.run() and as such should have a list of strings with each command and param as an entry of the list, and not a single string containing the entire command as whole.

please either create a pr with those fixes, or create an issue for it to be tracked and not get lost.

Sure for all the comments will raise a new PR and bring in main and then cherry pick to release if needed later.

[HereThereBeDragons]

Thanks @nunnikri for all the comments and changes.

One last request is fixing the pre-commit complain:
https://github.com/ROCm/TheRock/actions/runs/21487752843/job/61901605662?pr=3100

diff --git a/build_tools/packaging/linux/upload_package_repo.py b/build_tools/packaging/linux/upload_package_repo.py
index d0ec10a..08c36ac 100644
--- a/build_tools/packaging/linux/upload_package_repo.py
+++ b/build_tools/packaging/linux/upload_package_repo.py
@@ -369,7 +369,9 @@ def regenerate_deb_metadata_from_s3(
             with open(existing_packages, "r") as f:
                 content = f.read()
                 pkg_count = content.count("\nPackage: ")
-            print(f"✅ Downloaded existing Packages file containing ({pkg_count} packages)")
+            print(
+                f"✅ Downloaded existing Packages file containing ({pkg_count} packages)"
+            )
         except Exception as e:
             print(f"⚠️  No existing Packages file found (new repo?): {e}")
             existing_packages = None

if you want to automatize it you can install it in your local python env, see
https://github.com/ROCm/TheRock/tree/main/docs/development/style_guides

[raramakr]

Thanks @nunnikri for all the comments and changes.

One last request is fixing the pre-commit complain: https://github.com/ROCm/TheRock/actions/runs/21487752843/job/61901605662?pr=3100

diff --git a/build_tools/packaging/linux/upload_package_repo.py b/build_tools/packaging/linux/upload_package_repo.py
index d0ec10a..08c36ac 100644
--- a/build_tools/packaging/linux/upload_package_repo.py
+++ b/build_tools/packaging/linux/upload_package_repo.py
@@ -369,7 +369,9 @@ def regenerate_deb_metadata_from_s3(
             with open(existing_packages, "r") as f:
                 content = f.read()
                 pkg_count = content.count("\nPackage: ")
-            print(f"✅ Downloaded existing Packages file containing ({pkg_count} packages)")
+            print(
+                f"✅ Downloaded existing Packages file containing ({pkg_count} packages)"
+            )
         except Exception as e:
             print(f"⚠️  No existing Packages file found (new repo?): {e}")
             existing_packages = None

if you want to automatize it you can install it in your local python env, see https://github.com/ROCm/TheRock/tree/main/docs/development/style_guides

Fixed the pre-commit errors