Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

extract: Leave a hint in place of the removed bottles block #17590

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+11 −1
Open

extract: Leave a hint in place of the removed bottles block #17590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Library/Homebrew/dev-cmd/extract.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def run
result.sub!("class #{class_name} < Formula", "class #{versioned_name} < Formula")

# Remove bottle blocks, as they won't work.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the bottles be used? If not, can you please elaborate: what's the value in noting where one can find them?

Also, brew extract can be used on formulae from taps other than homebrew/core, so this may not be accurate.

Copy link
Contributor Author

@Kentzo Kentzo Jun 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the bottles be used?

As in re-used from the original formula? No, because after the extraction the name of the formula gets changed from <formula> to <formula>@<version> and thus the URL that Homebrew uses internally gets changed from docker://ghcr.io/homebrew/core/<name>:<version> to docker://ghcr.io/homebrew/core/<name>/<version>:<version>.

what's the value in noting where one can find them

When the intent is to pin formula for a reproducible build, it's important to archive the bottles as well, so one could use Homebrew (an old version, if necessary) with a custom tap and a custom HOMEBREW_BOTTLE_DOMAIN.

As it stands it's hard / impossible to entirely automate the process, e.g. via imaginary brew extract --include-bottles because:

  1. Bottles may not be present
  2. Formula may come from a custom tap

This hint is the next best thing to do as it puts the developer right on track of getting it to work. I'd greatly appreciate it as user.

Perhaps a better solution would be to either document the entire workflow of "taking a snapshot" of a given formula or, at least, document how Homebrew stores and retrieves the bottles.

Also, brew extract can be used on formulae from taps other than homebrew/core, so this may not be accurate.

Right, so I deliberately used the word "might" here :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you just rename the formula to not include the @version part?

As it stands it's hard / impossible to entirely automate the process, e.g. via imaginary brew extract --include-bottles because:

  1. Bottles may not be present
  2. Formula may come from a custom tap

We can simply restrict --include-bottles to homebrew/core formulae, and fail if there are no bottles (or, maybe, still extract the formula and just error on the bottle part). I used to have a custom brew command that just downloaded and extracted bottles here. I stopped using it and I don't think it works anymore, but it could maybe serve as a starting point to implementing something like this.

Copy link
Contributor Author

@Kentzo Kentzo Jun 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Rylan12 Still there is a problem that Homebrew, being rolling-release package manager, may decide to abandon old bottles.

Instead of --include-bottles I'd rather prever --extract-bottles so I could store them elsewhere and not depend on benevolence of the original formula maintainer.

That being said, while this is under discussion and development I think a hint is a good enough placeholder.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*please also see this discussion: https://github.com/orgs/Homebrew/discussions/5454

result.sub!(BOTTLE_BLOCK_REGEX, "")
result.sub!(BOTTLE_BLOCK_REGEX, "# The bottles might be available as oci images at docker://ghcr.io/homebrew/core/#{name}:#{version}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if, instead of injecting a comment, we output a warning saying "bottles have been removed because XYZ, but they may still be available at this URL if you'd like to extract them". That way the URL is displayed for easy copy/paste without needing to open the new file to find it, and it also doesn't pollute the new file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This output will be lost with the end of the terminal session while comment stays with the formula forever. And it's easier to script. Perhaps we should do both?


path = destination_tap.path/"Formula/#{name}@#{version_string}.rb"
if path.exist?
Expand Down
10 changes: 10 additions & 0 deletions Library/Homebrew/test/dev-cmd/extract_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,5 +63,15 @@
expect(path).to exist
expect(Formulary.factory(path).version).to eq "0.2"
end

it "notes the url where bottles might be found", :integration_test do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely don't need a test for this.

path = target[:path]/"Formula/[email protected]"
expect { brew "extract", "testball", target[:name] }
.to output(/^#{path}$/).to_stdout
.and not_to_output.to_stderr
.and be_a_success
expect(path).to exist
expect(File.read(path)).to match(%r{docker://ghcr.io/homebrew/core/testball:0.2})
end
end
end
Loading