pr-upload: Upload bottles to the Internet Archive #10677

sjackman · 2021-02-23T01:26:35Z

--archive-item=… specifies the item identifier.
HOMEBREW_INTERNET_ARCHIVE_KEY=access:secret specifies the S3-like key. See https://archive.org/account/s3.php

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?
Have you successfully run brew man locally and committed any changes?

--archive-item specifies the item identifier. HOMEBREW_ARCHIVE_KEY=access:secret specifies the S3 key.

BrewTestBot · 2021-02-23T01:27:01Z

Review period will end on 2021-02-24 at 01:26:35 UTC.

Add option --archive-item

jonchang · 2021-02-23T02:17:37Z

Multiple cloud storage services support an S3-like API, including Amazon itself. Would it perhaps make more sense to have a utils/s3 that implements the needed APIs, then (if necessary) have utils/s3/archive to subclass/override the bits that are specific to e.g. the internet archive?

sjackman · 2021-02-23T02:33:39Z

This implementation could pretty easily be adapted to a true S3 implementation like Amazon S3, but in its current form will not work out-of-the-box. I personally have no immediate use case for uploading bottles to Amazon S3, so I'm going to pass on doing this implementation myself.

MikeMcQuaid

Looking good so far, nice work!

MikeMcQuaid · 2021-02-23T08:47:02Z

Library/Homebrew/archive.rb

+require "digest/md5"
+require "utils/curl"
+
+# Archive API client.


Suggested change

# Archive API client.

# The Internet Archive API client.

MikeMcQuaid · 2021-02-23T08:47:20Z

Library/Homebrew/archive.rb

+
+    raise UsageError, "Must set the Archive item!" unless @archive_item
+
+    ENV["HOMEBREW_FORCE_HOMEBREW_ON_LINUX"] = "1" if @archive_item == "homebrew" && !OS.mac?


Can you add a comment explaining this?

This line is here on the foundational software engineering principles of cargo cult engineering and Chesterton's Fence, cribbed from

brew/Library/Homebrew/bintray.rb

Line 32 in 3efab3e

ENV["HOMEBREW_FORCE_HOMEBREW_ON_LINUX"] = "1" if @bintray_org == "homebrew" && !OS.mac?

I'm not sure of its purpose, so I sadly cannot comment it. I'm happy deleting this line, and we can add it back if we find that we need it.

If I were to guess, it was a hack for uploading and verifying (more likely the verifying step) macOS bottles running on a Linux CI runner.

I've deleted this line. We can crib it from bintray.rb if needed.

MikeMcQuaid · 2021-02-23T08:47:53Z

Library/Homebrew/archive.rb

+           warn_on_error: T.nilable(T::Boolean)).void
+  }
+  def upload(local_file, directory:, remote_file:, warn_on_error: false)
+    unless File.exist? local_file


Make local_file a Pathname and then you can use local_file.exist? here and local_file.read below.

MikeMcQuaid · 2021-02-23T08:48:09Z

Library/Homebrew/archive.rb

+
+  sig { returns(String) }
+  def inspect
+    "#<Archive: item=#{@archive_item}>"


Would be good to get some tests for this class.

MikeMcQuaid · 2021-02-23T08:48:39Z

Library/Homebrew/archive.rb

+
+  sig { params(url: String).returns(T::Boolean) }
+  def stable_mirrored?(url)
+    headers, = curl_output("--connect-timeout", "15", "--location", "--head", url)


Why 15? Would be good to have a comment or shared constant if this is copied from elsewhere.

🤷‍♂️ Cribbed from bintray.rb

Deleted this method stable_mirrored?

MikeMcQuaid · 2021-02-23T08:50:05Z

Library/Homebrew/archive.rb

+      directory = bottle_hash["bintray"]["repository"]
+      bottle_count = bottle_hash["bottle"]["tags"].length
+
+      bottle_hash["bottle"]["tags"].each do |_tag, tag_hash|


Suggested change

bottle_hash["bottle"]["tags"].each do |_tag, tag_hash|

bottle_hash["bottle"]["tags"].each_value do |tag_hash|

Nice catch. Rubocop should have an audit for this case.

MikeMcQuaid · 2021-02-23T08:50:22Z

Library/Homebrew/dev-cmd/pr-pull.rb

@@ -49,6 +49,8 @@ def pr_pull_args
             description: "Message to include when autosquashing revision bumps, deletions, and rebuilds."
      flag   "--artifact=",
             description: "Download artifacts with the specified name (default: `bottles`)."
+      flag   "--archive-item=",
+             description: "Upload to the specified Archive item (default: `homebrew`)."


Suggested change

description: "Upload to the specified Archive item (default: `homebrew`)."

description: "Upload to the specified Internet Archive item (default: `homebrew`)."

MikeMcQuaid · 2021-02-23T08:50:38Z

Library/Homebrew/dev-cmd/pr-upload.rb

@@ -27,6 +28,8 @@ def pr_upload_args
      switch "--warn-on-upload-failure",
             description: "Warn instead of raising an error if the bottle upload fails. "\
                          "Useful for repairing bottle uploads that previously failed."
+      flag   "--archive-item=",
+             description: "Upload to the specified Archive item (default: `homebrew`)."


Suggested change

description: "Upload to the specified Archive item (default: `homebrew`)."

description: "Upload to the specified Internet Archive item (default: `homebrew`)."

MikeMcQuaid · 2021-02-23T08:51:44Z

Library/Homebrew/env_config.rb

@@ -15,6 +15,10 @@ module EnvConfig
        description: "Linux only: Pass this value to a type name representing the compiler's `-march` option.",
        default:     "native",
      },
+      HOMEBREW_ARCHIVE_KEY:                   {
+        description: "Use this API key when accessing the Archive.org API (where bottles are stored). " \


Suggested change

description: "Use this API key when accessing the Archive.org API (where bottles are stored). " \

description: "Use this API key when accessing the Internet Archive API (where bottles are stored). " \

MikeMcQuaid · 2021-02-23T08:51:50Z

Library/Homebrew/env_config.rb

@@ -15,6 +15,10 @@ module EnvConfig
        description: "Linux only: Pass this value to a type name representing the compiler's `-march` option.",
        default:     "native",
      },
+      HOMEBREW_ARCHIVE_KEY:                   {


Suggested change

HOMEBREW_ARCHIVE_KEY: {

HOMEBREW_INTERNET_ARCHIVE_KEY: {

BrewTestBot · 2021-02-24T03:06:24Z

Review period ended.

sjackman · 2021-02-24T20:16:52Z

BrewTestBot approved my PR, and it automerged. That wasn't exactly intentional, but I'm happy with this PR. Happy to follow up with another PR if tweaks are needed.

MikeMcQuaid · 2021-02-25T11:03:56Z

@sjackman pulling out some constants and adding any tests would be nice if possible!

sjackman added 2 commits February 22, 2021 17:21

pr-upload: Upload bottles to Archive.org

b0bd15f

--archive-item specifies the item identifier. HOMEBREW_ARCHIVE_KEY=access:secret specifies the S3 key.

Update man pages

2794641

sjackman self-assigned this Feb 23, 2021

BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Feb 23, 2021

sjackman requested review from MikeMcQuaid and mistydemeo February 23, 2021 01:27

sjackman mentioned this pull request Feb 23, 2021

CI: Upload bottles to Archive.org using pr-upload brewsci/homebrew-bio#1293

Merged

4 tasks

sjackman added 4 commits February 22, 2021 17:57

pr-pull: Upload bottles to Archive.org

c11692b

Add option --archive-item

Update man pages

7fd6d21

archive.rb: Rename dir to directory

3d715ca

Add test/archive_spec.rb

0f2c47d

sjackman enabled auto-merge February 23, 2021 02:34

MikeMcQuaid reviewed Feb 23, 2021

View reviewed changes

sjackman added 2 commits February 23, 2021 11:31

Apply Mike's suggestions

641f134

Update man pages

be11a22

sjackman requested a review from MikeMcQuaid February 23, 2021 19:32

sjackman changed the title ~~pr-upload: Upload bottles to Archive.org~~ pr-upload: Upload bottles to the Internet Archive Feb 23, 2021

BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Feb 24, 2021

BrewTestBot approved these changes Feb 24, 2021

View reviewed changes

sjackman merged commit bd75cfc into Homebrew:master Feb 24, 2021

sjackman deleted the sj/pr-upload branch February 24, 2021 20:15

BrewTestBot added the outdated PR was locked due to age label Mar 28, 2021

Homebrew locked as resolved and limited conversation to collaborators Mar 28, 2021

MikeMcQuaid removed the outdated PR was locked due to age label Jun 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pr-upload: Upload bottles to the Internet Archive #10677

pr-upload: Upload bottles to the Internet Archive #10677

sjackman commented Feb 23, 2021 •

edited

Loading

BrewTestBot commented Feb 23, 2021

jonchang commented Feb 23, 2021

sjackman commented Feb 23, 2021

MikeMcQuaid left a comment

MikeMcQuaid Feb 23, 2021

MikeMcQuaid Feb 23, 2021

sjackman Feb 23, 2021

sjackman Feb 23, 2021

sjackman Feb 23, 2021

MikeMcQuaid Feb 23, 2021

MikeMcQuaid Feb 23, 2021

MikeMcQuaid Feb 23, 2021

sjackman Feb 23, 2021

sjackman Feb 23, 2021 •

edited

Loading

MikeMcQuaid Feb 23, 2021

sjackman Feb 23, 2021

MikeMcQuaid Feb 23, 2021

MikeMcQuaid Feb 23, 2021

MikeMcQuaid Feb 23, 2021

MikeMcQuaid Feb 23, 2021

BrewTestBot commented Feb 24, 2021

sjackman commented Feb 24, 2021

MikeMcQuaid commented Feb 25, 2021


		raise UsageError, "Must set the Archive item!" unless @archive_item

		ENV["HOMEBREW_FORCE_HOMEBREW_ON_LINUX"] = "1" if @archive_item == "homebrew" && !OS.mac?

	bottle_hash["bottle"]["tags"].each do \|_tag, tag_hash\|
	bottle_hash["bottle"]["tags"].each_value do \|tag_hash\|

	description: "Upload to the specified Archive item (default: `homebrew`)."
	description: "Upload to the specified Internet Archive item (default: `homebrew`)."

	description: "Use this API key when accessing the Archive.org API (where bottles are stored). " \
	description: "Use this API key when accessing the Internet Archive API (where bottles are stored). " \

pr-upload: Upload bottles to the Internet Archive #10677

pr-upload: Upload bottles to the Internet Archive #10677

Conversation

sjackman commented Feb 23, 2021 • edited Loading

BrewTestBot commented Feb 23, 2021

jonchang commented Feb 23, 2021

sjackman commented Feb 23, 2021

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sjackman Feb 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BrewTestBot commented Feb 24, 2021

sjackman commented Feb 24, 2021

MikeMcQuaid commented Feb 25, 2021

sjackman commented Feb 23, 2021 •

edited

Loading

sjackman Feb 23, 2021 •

edited

Loading