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

ruby 3.2.0 #118926

Closed
wants to merge 21 commits into from
Closed

ruby 3.2.0 #118926

wants to merge 21 commits into from

Conversation

dawidd6
Copy link
Member

@dawidd6 dawidd6 commented Dec 25, 2022

Created with brew bump-formula-pr.

resource blocks may require updates.

@BrewTestBot BrewTestBot added bump-formula-pr PR was created using `brew bump-formula-pr` rust Rust use is a significant feature of the PR or issue automerge-skip `brew pr-automerge` will skip this pull request labels Dec 25, 2022
@cho-m cho-m added the revision bumps needed Reverse dependencies need to have their revision incremented in the same PR label Dec 25, 2022
@p-linnane p-linnane removed the revision bumps needed Reverse dependencies need to have their revision incremented in the same PR label Dec 25, 2022
@dawidd6
Copy link
Member Author

dawidd6 commented Dec 25, 2022

weechat fails to build:

--   Found python3-embed, version 3.10
  -- Checking for one of the modules 'ruby-3.1;ruby-3.0;ruby-2.7;ruby-2.6;ruby-2.5;ruby-2.4;ruby-2.3;ruby-2.2;ruby-2.1;ruby-2.0;ruby-1.9;ruby'
  CMake Error at src/plugins/CMakeLists.txt:116 (message):
    Ruby not found

@faisal
Copy link
Contributor

faisal commented Dec 25, 2022

Can you add a depends_on "rust" => :build for the source build, as is used for HEAD builds? Without using Rust to build, the produced Ruby won't include YJIT support.

@faisal
Copy link
Contributor

faisal commented Dec 25, 2022

N.b. Ruby 3.2 explicitly shifted a number of dependencies -- see "No longer bundle 3rd party sources" in https://www.ruby-lang.org/en/news/2022/12/25/ruby-3-2-0-released/. Have you considered if this formula depends on the right things, with an eye to what's described in https://github.com/ruby/ruby/blob/master/doc/contributing/building_ruby.md?

Also, I am surprised this builds without explicitly depending on bison. The Ruby 3.2 build docs claim bison 3.0 or later is a build requirement, and HEAD won't build without it (so we have it as an explicit dependency for HEAD builds already), but I have confirmed this will build from source without it (on macOS Ventura, with the latest Xcode and Command Line Tools installed).

@p-linnane p-linnane requested a review from Bo98 December 26, 2022 22:11
@faisal
Copy link
Contributor

faisal commented Dec 27, 2022

This changes the PR up to what the Ruby build docs claim is needed (including rust for YJIT support) and in my tests on macOS Ventura (at least) produces a working binary:

diff --git a/Formula/ruby.rb b/Formula/ruby.rb
index d1aa4f3d34e..6c865809b78 100644
--- a/Formula/ruby.rb
+++ b/Formula/ruby.rb
@@ -40,13 +40,15 @@ class Ruby < Formula
 
   keg_only :provided_by_macos
 
-  depends_on "pkg-config" => :build
+  depends_on "autoconf" => :build # Also depended by libyaml, but declaring here for clarity
+  depends_on "bison" => :build
+  depends_on "rust" => :build
   depends_on "libyaml"
-  depends_on "[email protected]"
+  depends_on "openssl"
   depends_on "readline"
 
+  uses_from_macos "gperf"
   uses_from_macos "libffi"
-  uses_from_macos "libxcrypt"
   uses_from_macos "zlib"
 
   def api_version

@dawidd6 dawidd6 added CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. long build Set a long timeout for formula testing CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. and removed CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. long build Set a long timeout for formula testing labels Dec 27, 2022
@dawidd6
Copy link
Member Author

dawidd6 commented Dec 27, 2022

If I put depends_on "rust" => :build to the ruby formula we get a circular dependency to ruby:

rust[build]
  curl
    openldap
      util-linux
        asciidoctor[build]
          ruby

only on Linux though. Not sure what we do in situations like that.

@dawidd6 dawidd6 added the help wanted Task(s) needing PRs from the community or maintainers label Dec 27, 2022
@faisal
Copy link
Contributor

faisal commented Dec 28, 2022

I suspect this works on macOS because Homebrew can rely on the built-in Ruby before it builds its (bottle-only) Ruby.

Is this dependency only an issue if building everything from source? If so, is it reasonable to expect users to bootstrap, e.g. install a bottled Rust, then use that to build Ruby from source, and then if necessary go back and build Rust from source as well? Are there any other good examples of circular dependencies, and how Homebrew handles them or expects users to handle them?

@SMillerDev
Copy link
Member

If building util-linux with meson the asciidoctor dependency isn't required. But I'll wait for other suggestions from @Homebrew/maintainers

@danielnachun
Copy link
Member

I would love to see us drop the asciidoctor build dependency for util-linux. util-linux is a non-relocatable dependency of curl and complicates the Linux bootstrapping process substantially on older systems. And as per our usual policy, documentation is readily available online.

@dawidd6 dawidd6 added the CI-issue Failure due to temporary CI issue label Jan 2, 2023
@dawidd6 dawidd6 removed the request for review from Bo98 January 2, 2023 13:02
@dawidd6 dawidd6 removed CI-issue Failure due to temporary CI issue help wanted Task(s) needing PRs from the community or maintainers labels Jan 2, 2023
@dawidd6
Copy link
Member Author

dawidd6 commented Jan 2, 2023

On Linux:

==> Testing style-check
  ==> /home/linuxbrew/.linuxbrew/Cellar/style-check/0.14/bin/style-check.rb -v paper.tex
  (none):118: warning: mismatched indentations at 'else' with 'if' at 83
  (none):120: warning: mismatched indentations at 'end' with 'else' at 118
  (none):159: warning: assigned but unused variable - r
  (none):161: warning: assigned but unused variable - windows_detect_bug_avoider
  /home/linuxbrew/.linuxbrew/Cellar/style-check/0.14/bin/style-check.rb:166:in `do_cns': undefined method `=~' for false:FalseClass (NoMethodError)
  	from /home/linuxbrew/.linuxbrew/Cellar/style-check/0.14/bin/style-check.rb:216:in `block (2 levels) in <main>'
  	from /home/linuxbrew/.linuxbrew/Cellar/style-check/0.14/bin/style-check.rb:191:in `each'
  	from /home/linuxbrew/.linuxbrew/Cellar/style-check/0.14/bin/style-check.rb:191:in `each_with_index'
  	from /home/linuxbrew/.linuxbrew/Cellar/style-check/0.14/bin/style-check.rb:191:in `block in <main>'
  	from /home/linuxbrew/.linuxbrew/Cellar/style-check/0.14/bin/style-check.rb:184:in `each'
  	from /home/linuxbrew/.linuxbrew/Cellar/style-check/0.14/bin/style-check.rb:184:in `<main>'
  paper.tex:1:19: Today I worked on homebrew (homebrew)

Probably needs this PR as a patch: nspring/style-check#4

@faisal
Copy link
Contributor

faisal commented Jan 3, 2023

And there's a new Rubygems release: https://github.com/rubygems/rubygems/releases/tag/v3.4.2

@faisal
Copy link
Contributor

faisal commented Jan 3, 2023

This patches the rubygems lines to the latest and greatest: https://github.com/dawidd6/homebrew-core/pull/2

@dawidd6 dawidd6 added the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Jan 3, 2023
@dawidd6
Copy link
Member Author

dawidd6 commented Jan 5, 2023

@cho-m done.

@faisal
Copy link
Contributor

faisal commented Jan 5, 2023

The cucumber-cpp project's issues list hints at some general problems with the wire protocol and with Ruby 3.1 and later, e.g. cucumber/cucumber-cpp#200. @jermus67 might have thoughts on what's going on here.

@chenrui333 chenrui333 removed the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Jan 6, 2023
@cho-m cho-m added no long build conflict Do not allow merging other pull requests when files conflict with this one and removed help wanted Task(s) needing PRs from the community or maintainers 13-arm64 Ventura arm64 is specifically affected labels Jan 6, 2023
cho-m
cho-m previously approved these changes Jan 6, 2023
@cho-m
Copy link
Member

cho-m commented Jan 6, 2023

In separate PR, we should try adding a sleep after the fork in cucumber-cpp test.

chenrui333
chenrui333 previously approved these changes Jan 6, 2023
@BrewTestBot
Copy link
Member

:shipit: @chenrui333 has triggered a merge.

@BrewTestBot
Copy link
Member

⚠️ @chenrui333 bottle publish failed.

@BrewTestBot BrewTestBot dismissed stale reviews from chenrui333 and cho-m January 6, 2023 05:15

bottle publish failed

@chenrui333
Copy link
Member

let's merge the PR for now and followup the test updates separately.

@BrewTestBot
Copy link
Member

:shipit: @chenrui333 has triggered a merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge-skip `brew pr-automerge` will skip this pull request bump-formula-pr PR was created using `brew bump-formula-pr` CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. long build Set a long timeout for formula testing no long build conflict Do not allow merging other pull requests when files conflict with this one rust Rust use is a significant feature of the PR or issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants