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.

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

sugarjar 1.0.1 (new formula) #157910

Merged
merged 2 commits into from
Jan 1, 2024
Merged

sugarjar 1.0.1 (new formula) #157910

merged 2 commits into from
Jan 1, 2024

Conversation

jaymzh
Copy link
Contributor

@jaymzh jaymzh commented Dec 21, 2023

Sugarjar is a utility that wraps git and gh to make working with both easier. It's already distributed in nearly every Linux distribution. I'd like to make it available to mac users via brew.

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

@github-actions github-actions bot added autosquash Automatically squash pull request commits according to Homebrew style. new formula PR adds a new formula to Homebrew/homebrew-core ruby Ruby use is a significant feature of the PR or issue labels Dec 21, 2023
@jaymzh jaymzh force-pushed the sugarjar branch 2 times, most recently from 102dcfd to 7eb73e7 Compare December 21, 2023 00:21
@github-actions github-actions bot removed the autosquash Automatically squash pull request commits according to Homebrew style. label Dec 21, 2023
@jaymzh jaymzh changed the title Add sugarjar (1.0.1) to homebrew sugarjar 1.0.1 (new formula) Dec 21, 2023
Copy link
Contributor

Thanks for contributing to Homebrew! 🎉 It looks like you're having trouble with a CI failure. See our contribution guide for help. You may be most interested in the section on dealing with CI failures. You can find the CI logs in the Checks tab of your pull request.

@jaymzh
Copy link
Contributor Author

jaymzh commented Dec 21, 2023

I believe I qualify for an exception to the notability clause:

  • I am the maintainer of the software
  • it's in every major Linux distro (not packaged by me, except in fedora)
  • it is in fact quite popular even though it's small enough not that many have needed to fork/contribute.

@SMillerDev
Copy link
Member

I am the maintainer of the software

@SMillerDev
Copy link
Member

The README also seems to suggest that the software is already deprecated and users should try Sapling instead.

Formula/s/sugarjar.rb Outdated Show resolved Hide resolved
Formula/s/sugarjar.rb Outdated Show resolved Hide resolved
Formula/s/sugarjar.rb Outdated Show resolved Hide resolved
@jaymzh
Copy link
Contributor Author

jaymzh commented Dec 21, 2023

The README also seems to suggest that the software is already deprecated and users should try Sapling instead.

As it turns out Sapling is a big lift, and lots of people still want this. So I'm actively developing it, but want people to go poke at Sapling if they're new, in case that's a better fit.

And while in Linux land most people just went off and packaged it, apparently my mac users don't particularly want to, but they email me and ask me to. ::shrug::

@jaymzh
Copy link
Contributor Author

jaymzh commented Dec 21, 2023

All updates in, except for the dependencies - once I hear back on how you want those done, I'll do that as well.

Copy link
Member

@SMillerDev SMillerDev left a comment

Choose a reason for hiding this comment

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

The README also seems to suggest that the software is already deprecated and users should try Sapling instead.

@jaymzh
Copy link
Contributor Author

jaymzh commented Dec 29, 2023

The README also seems to suggest that the software is already deprecated and users should try Sapling instead.

Not deprecated. As it says, it's being maintained since Sapling is too big a lift for some (and not allowed in certain organizations). I've done two releases since that was added even.

As I mentioned previously - I get a lot of requests for this in Brew.

@SMillerDev SMillerDev requested a review from a team December 31, 2023 09:48
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

I will happily this this included into Homebrew. It seems useful and there have been requests upstream.

Using bundle install and a Gemfile.lock over resources seems preferable. Sorry for the confusion here; if you can note where in our docs lead you astray here @jaymzh I'll fix them.

Check out swiftgen as an example of a formula using bundle install.

@jaymzh
Copy link
Contributor Author

jaymzh commented Dec 31, 2023

Thanks @MikeMcQuaid!

I can make those changes. I'll get my lockfile checked in upstream and use that.

As to confusion, I wanted to do a bit of spelunking so that I could best understand and recommend useful doc updates. However, what I found is that I don't think any ruby projects do it as described here. Here's a breakdown:

There's only 9 formula that seem to do a bundle install, but 5 of them are Ruby itself which we can ignore, which leaves 4 (licensefinder, mailcatcher, mikutter, and swiftgen).

  • licensefinder only has dependencies for tests, so this only sort-of counts, but it does a bundle install in its test block. It does not have a Gemfile.lock upstream, so none of the installed stuff there is locked to versions.
  • mailcatcher only uses bundle install for some third-party install, and in fact, lists all it's gems as resources and downloads them all manually as this PR originally did.
  • mikutter lists each gem manually as a resource, downloads them all into bunder's cache directory and then does bundle install, which is weird since it both has the problem of having to keep the dependencies in sync across the Formula and the Gemfile, but also uses bundler
  • swiftgen is a special case since it's not really a ruby project (doesn't have it's own gemspec (switfgen.gemspec)) and is really a Swift project.

Here's how I found those:

$ grep '"bundle", "install"' */*
l/licensefinder.rb:      system "bundle", "install"
m/mailcatcher.rb:      system "bundle", "install"
m/mikutter.rb:    system "bundle", "install",
r/ruby.rb:    system bin/"bundle", "install", "--binstubs=#{testpath}/bin"
r/ruby@2.6.rb:    system bin/"bundle", "install", "--binstubs=#{testpath}/bin"
r/ruby@2.7.rb:    system bin/"bundle", "install", "--binstubs=#{testpath}/bin"
r/ruby@3.0.rb:    system bin/"bundle", "install", "--binstubs=#{testpath}/bin"
r/ruby@3.1.rb:    system bin/"bundle", "install", "--binstubs=#{testpath}/bin"
s/swiftgen.rb:    system "bundle", "install", "--without", "development", "release"

Sidenote: mikutter depends on ruby but does not do uses_from_macos 'ruby'

Meanwhile, there's 21 projects that download their dependent gems manually:

$ grep -l 'url.*gem"' */*
a/asciidoctor.rb
b/braid.rb
c/cucumber-ruby.rb
d/dexter.rb
g/ghi.rb
g/gitlab-gem.rb
h/haiti.rb
h/haste-client.rb
l/licensed.rb
m/mailcatcher.rb
m/mikutter.rb
p/pandocomatic.rb
p/pgsync.rb
t/terraform_landscape.rb
t/terraforming.rb
t/tmuxinator.rb
t/travis.rb
u/uffizzi.rb
u/um.rb
w/wpscan.rb
y/youplot.rb

And 41 that declare they need ruby:

$ grep -l 'uses_from_macos "ruby"' */*
a/apibuilder-cli.rb
b/braid.rb
b/brew-gem.rb
c/clojure.rb
e/ec2-ami-tools.rb
f/foreman.rb
g/ghi.rb
g/gist.rb
g/git-game.rb
g/git-tracker.rb
g/gitlab-gem.rb
g/gnu-time.rb
h/haste-client.rb
h/hub.rb
i/imessage-ruby.rb
i/inko.rb
l/logstash.rb
l/lolcat.rb
l/lunchy.rb
m/mkvtoolnix.rb
m/mruby.rb
o/ocl-icd.rb
o/opal.rb
p/passenger.rb
p/pgsync.rb
p/pit.rb
q/qsoas.rb
r/rbenv.rb
r/reposurgeon.rb
r/ronn.rb
r/rubyfmt.rb
s/schema-evolution-manager.rb
s/sourcery.rb
s/style-check.rb
s/subversion.rb
s/sugarjar.rb
s/swiftgen.rb
s/swig@3.rb
t/texlive.rb
u/um.rb
y/youplot.rb

So... best I can tell there's no single ruby project in Homebrew that quite gets this right. You asked for specific documentation to update. I'd start with https://docs.brew.sh/Formula-Cookbook#specifying-gems-python-modules-go-projects-etc-as-dependencies

And I'd add maybe something along the lines of:

## Ruby Gem Dependencies

The preferred mechanism for installing gem dependencies is to use `bundler` with the upstream's Gemfile.lock. This requires the upstream checks in their `Gemfile.lock`, so if they don't, it's a good idea to file a bug and ask them to do so. Assuming they have one, this is as simple as:

    ENV["GEM_HOME"] = libexec
    system "bundle", "install"

From there, you can build and install the project itself:

    system "gem", "build", "<project>.gemspec"
    system "gem", "install", "--ignore-dependencies", "<project>-#{version}.gem"

And install any bins, and munge their shebang lines, with:

    bin.install libexec/"bin/<bin>"
    bin.env_script_all_files(libexec/"bin", GEM_HOME: ENV["GEM_HOME"])

## Python dependencies

<similar instructions here>

@github-actions github-actions bot added the autosquash Automatically squash pull request commits according to Homebrew style. label Dec 31, 2023
@github-actions github-actions bot removed the autosquash Automatically squash pull request commits according to Homebrew style. label Dec 31, 2023
@jaymzh jaymzh force-pushed the sugarjar branch 2 times, most recently from 43e2ebc to a2d4b46 Compare December 31, 2023 21:28
@jaymzh
Copy link
Contributor Author

jaymzh commented Dec 31, 2023

OK @MikeMcQuaid - those updates are in, but it's failing because it seems to be running on the native MacOSX ruby which is not only ancient, but deprecated:

  ==> gem install bundler
  ERROR:  Error installing bundler:
  	The last version of bundler (>= 0) to support your Ruby & RubyGems was 2.4.22. Try installing it with `gem install bundler -v 2.4.22`
  	bundler requires Ruby version >= 3.0.0. The current ruby version is 2.6.10.210.
$ ruby --version
ruby 2.6.10p210 (2022-04-12 revision 67958) [universal.x86_64-darwin23]
$ irb

WARNING: This version of ruby is included in macOS for compatibility with legacy software. 
In future versions of macOS the ruby runtime will not be available by 
default, and may require you to install an additional package.

My understanding was that it'd use the ruby provided by Homebrew (from the confusingly-named uses_from_macos

Should I...

  • Depend on the Homebrew ruby package directly (this seems to be strongly discouraged)
  • Something else?

@p-linnane
Copy link
Member

@jaymzh uses_from_macos will use the system provided version. It's fine to declare a dependency on Ruby to use our packaged version instead.

@github-actions github-actions bot removed the ruby Ruby use is a significant feature of the PR or issue label Dec 31, 2023
@github-actions github-actions bot added the ruby Ruby use is a significant feature of the PR or issue label Dec 31, 2023
@jaymzh
Copy link
Contributor Author

jaymzh commented Dec 31, 2023

@jaymzh uses_from_macos will use the system provided version. It's fine to declare a dependency on Ruby to use our packaged version instead.

@p-linnane, Updated, but then it fails tests with:

    * Dependency 'ruby' is provided by macOS; please replace 'depends_on' with 'uses_from_macos'.

@p-linnane
Copy link
Member

That's expected, but we can override it. @Homebrew/maintainers thoughts on removing this audit now that we no longer use system Ruby ourselves?

@jaymzh
Copy link
Contributor Author

jaymzh commented Dec 31, 2023

Ah, gotcha. In that case, I think this is ready for re-review, the only tests failing are the ruby and the "not notable enough" - both of which folks here have said they're willing to override.

Let me know if there are other changes need. Thanks!

Sugarjar is a utility that wraps git and gh to make working
with both easier. It's already distributed in nearly every
Linux distribution. I'd like to make it available to mac users
via brew.
@p-linnane
Copy link
Member

I've pushed some minor formatting changes and will merge this once CI finishes.

@p-linnane p-linnane added CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. notability Project is not notable enough for inclusion labels Dec 31, 2023
Copy link
Contributor

github-actions bot commented Jan 1, 2024

@github-actions github-actions bot added the CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. label Jan 1, 2024
@BrewTestBot BrewTestBot added this pull request to the merge queue Jan 1, 2024
Merged via the queue into Homebrew:master with commit f107a2b Jan 1, 2024
12 checks passed
@MikeMcQuaid
Copy link
Member

That's expected, but we can override it. @Homebrew/maintainers thoughts on removing this audit now that we no longer use system Ruby ourselves?

@p-linnane Yes, we should do that for Ruby specifically 👍🏻. Thanks for merging!

And I'd add maybe something along the lines of:

@jaymzh This looks great, could you open a PR just with the exact text you'd got there? No worries if not! Thanks for all your patience and work on this PR and contributing to Homebrew: you rock!

@jaymzh jaymzh deleted the sugarjar branch January 1, 2024 22:38
jaymzh added a commit to jaymzh/brew that referenced this pull request Jan 1, 2024
In Homebrew/homebrew-core#157910 we discussed
some improvements to docs on setting up gems. THis is an attempt at some
docs for that.

If someone can help with pip docs for this I'll add it too.

Signed-off-by: Phil Dibowitz <phil@ipom.com>
@jaymzh
Copy link
Contributor Author

jaymzh commented Jan 1, 2024

@MikeMcQuaid - here's a PR for that: Homebrew/brew#16418

jaymzh added a commit to jaymzh/brew that referenced this pull request Jan 1, 2024
In Homebrew/homebrew-core#157910 we discussed
some improvements to docs on setting up gems. THis is an attempt at some
docs for that.

If someone can help with pip docs for this I'll add it too.

Signed-off-by: Phil Dibowitz <phil@ipom.com>
jaymzh added a commit to jaymzh/brew that referenced this pull request Jan 13, 2024
In Homebrew/homebrew-core#157910 we discussed
some improvements to docs on setting up gems. THis is an attempt at some
docs for that.

If someone can help with pip docs for this I'll add it too.

Signed-off-by: Phil Dibowitz <phil@ipom.com>
jaymzh added a commit to jaymzh/brew that referenced this pull request Jan 13, 2024
In Homebrew/homebrew-core#157910 we discussed
some improvements to docs on setting up gems. THis is an attempt at some
docs for that.

If someone can help with pip docs for this I'll add it too.

Signed-off-by: Phil Dibowitz <phil@ipom.com>
@github-actions github-actions bot added the outdated PR was locked due to age label Feb 1, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. new formula PR adds a new formula to Homebrew/homebrew-core notability Project is not notable enough for inclusion outdated PR was locked due to age ruby Ruby use is a significant feature of the PR or issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants