github_packages: support --keep-old

[Bo98]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

We need this to support any sort of workflow for adding bottles to an existing tag, including brew dispatch-build-bottle.

[BrewTestBot]

Review period will end on 2021-04-12 at 15:41:15 UTC.

[MikeMcQuaid]

Great work 👏🏻

[BrewTestBot]

Review period skipped due to critical label.

[dawidd6]

Are you sure this won't break workflows in linuxbrew-core?

[Bo98]

What does linuxbrew-core do here? Use --keep-old on a non-existent package tag?

[dawidd6]

Yes, I think that's what happens.

[Bo98]

I could do this so that it ignores --keep-old if a tag doesn't exist?

diff --git a/Library/Homebrew/github_packages.rb b/Library/Homebrew/github_packages.rb
index ad8839ca2..deb576d26 100644
--- a/Library/Homebrew/github_packages.rb
+++ b/Library/Homebrew/github_packages.rb
@@ -226,21 +226,22 @@ class GitHubPackages
     image_tag = GitHubPackages.image_version_rebuild(version_rebuild)
     image_uri = "#{GitHubPackages.root_url(org, repo, DOCKER_PREFIX)}/#{image_name}:#{image_tag}"
 
-    unless keep_old
-      puts
-      inspect_args = ["inspect", image_uri.to_s]
-      if dry_run
-        puts "#{skopeo} #{inspect_args.join(" ")} --creds=#{user}:$HOMEBREW_GITHUB_PACKAGES_TOKEN"
-      else
-        inspect_args << "--creds=#{user}:#{token}"
-        inspect_result = system_command(skopeo, print_stderr: false, args: inspect_args)
-        if inspect_result.status.success?
-          if warn_on_error
-            opoo "#{image_uri} already exists, skipping upload!"
-            return
-          else
-            odie "#{image_uri} already exists!"
-          end
+    puts
+    inspect_args = ["inspect", image_uri.to_s]
+    if dry_run
+      puts "#{skopeo} #{inspect_args.join(" ")} --creds=#{user}:$HOMEBREW_GITHUB_PACKAGES_TOKEN"
+    else
+      inspect_args << "--creds=#{user}:#{token}"
+      inspect_result = system_command(skopeo, print_stderr: false, args: inspect_args)
+      if keep_old
+        # Tag doesn't exist so ignore --keep-old
+        keep_old = false unless inspect_result.status.success?
+      elsif inspect_result.status.success?
+        if warn_on_error
+          opoo "#{image_uri} already exists, skipping upload!"
+          return
+        else
+          odie "#{image_uri} already exists!"
         end
       end
     end

[dawidd6]

Guess that will do.

[MikeMcQuaid]

@Bo98 that works for me too 👍🏻

[MikeMcQuaid]

Not saying we should but: could moving this skopeo with or without dry run logic into a method (as it's used three times now)

[Bo98]

The different args, even ones like print_stderr, make it a bit tricky.

[MikeMcQuaid]

Fine with me 👍🏻

[Bo98]

Added code to odie when a duplicate manifest is detected.