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

Fix compilation of Ruby 3.2.x on FreeBSD #2187

Merged
merged 7 commits into from
Apr 23, 2023
Merged

Fix compilation of Ruby 3.2.x on FreeBSD #2187

merged 7 commits into from
Apr 23, 2023

Conversation

jarmo
Copy link
Contributor

@jarmo jarmo commented Apr 14, 2023

Related to #2184

Copy link
Member

@mislav mislav left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

How come these checks are scoped only to FreeBSD 11 or later?

bin/ruby-build Outdated
Comment on lines 1050 to 1051
if pkg info -e libyaml > /dev/null; then
package_option ruby configure --with-libyaml-dir="$(pkg info --prefix libyaml | cut -wf2)"
Copy link
Member

Choose a reason for hiding this comment

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

Could we just get away with calling pkg info --prefix libyaml once instead of pkg info -e libyaml && pkg info --prefix?

And what is the full output of pkg info --prefix before it gets cut?

@jarmo
Copy link
Contributor Author

jarmo commented Apr 18, 2023

How come these checks are scoped only to FreeBSD 11 or later?

Not sure. I used the same style as was used in the already committed/merged use_freebsd_readline function. Since I didn't understand the reason behind that check then I went with the safer choice of doing the same and not deviate away from it. Maybe it's time to remove these version checks everywhere since FreeBSD 10 is already EOL-ed?

Could we just get away with calling pkg info --prefix libyaml once instead of pkg info -e libyaml && pkg info --prefix?

It's possible. However I think that then we need to check for empty string after command pkg info -p libyaml 2>/dev/null | cut -wf2. Currently pkg info -e returns exit code 0 or 1 depending on the existance of the package:

root@freebsd:~ pkg info -e libyaml
root@freebsd:~ echo $?
0
root@freebsd:~ pkg info -e foo
root@freebsd:~ echo $?
1
root@freebsd:~ pkg info -p libyaml
libyaml-0.2.5                  /usr/local
root@freebsd:~ pkg info -p foo
pkg: No package(s) matching foo

Not sure what stdout is been redirected to /dev/null in use_freebsd_readline function when doing pkg info -e PKG, but as I wrote already then I followed the same style.

And what is the full output of pkg info --prefix before it gets cut?

Output is seen above.

@mislav
Copy link
Member

mislav commented Apr 19, 2023

However I think that then we need to check for empty string after command pkg info -p libyaml 2>/dev/null | cut -wf2

I'd vote for that approach instead of separately checking pkg info -e.

Maybe it's time to remove these version checks everywhere since FreeBSD 10 is already EOL-ed?

Yep, you're welcome to remove checks for FreeBSD 11! Thanks

@jarmo
Copy link
Contributor Author

jarmo commented Apr 19, 2023

@mislav made all the relevant changes.

Copy link
Member

@mislav mislav left a comment

Choose a reason for hiding this comment

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

Thanks; looking good! Only style suggestions remain

bin/ruby-build Outdated

freebsd_package_prefix() {
local package="$1"
echo "$(pkg info --prefix "$package" 2>/dev/null | cut -wf2)"
Copy link
Member

Choose a reason for hiding this comment

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

How about just unwrapping this instead of capturing the output and echo-ing it?

Suggested change
echo "$(pkg info --prefix "$package" 2>/dev/null | cut -wf2)"
pkg info --prefix "$package" 2>/dev/null | cut -wf2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for noticing this silly code :) Fixed it.

bin/ruby-build Outdated
use_freebsd_yaml() {
if is_freebsd; then
local libyaml_prefix="$(freebsd_package_prefix libyaml)"
if [ "$libyaml_prefix" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

Nit: although it might be functionally equivalent, I do like the explicitness of testing strings for not being blank, i.e.

Suggested change
if [ "$libyaml_prefix" ]; then
if [ -n "$libyaml_prefix" ]; then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Explicit checks added.

@mislav mislav merged commit b80e686 into rbenv:master Apr 23, 2023
halostatue added a commit to halostatue/macports-ports that referenced this pull request Apr 28, 2023
This commit contains two updates: 20230424 and 20230428.

```
- 20230424:
  - Fall back on shasum if sha256sum is unavailable by @jas14 in
    rbenv/ruby-build#2177
  - Mark EOL status to Ruby 2.6 and 2.7 by @hsbt in
    rbenv/ruby-build#2180
  - Fix uploading SARIF reports from Differential Shellcheck by @mislav
    in rbenv/ruby-build#2181
  - Fix compilation of Ruby 3.2.x on FreeBSD by @jarmo in
    rbenv/ruby-build#2187
  - Fix truffleruby+graalvm-dev download URLs by @eregon in
    rbenv/ruby-build#2189

- 20230428:
  - Add TruffleRuby 23.0.0-preview1 by @eregon in rbenv/ruby-build#2190
  - Add TruffleRuby+GraalVM 23.0.0-preview1 by @eregon in
    rbenv/ruby-build#2191
```
mojca pushed a commit to macports/macports-ports that referenced this pull request Apr 28, 2023
This commit contains two updates: 20230424 and 20230428.

```
- 20230424:
  - Fall back on shasum if sha256sum is unavailable by @jas14 in
    rbenv/ruby-build#2177
  - Mark EOL status to Ruby 2.6 and 2.7 by @hsbt in
    rbenv/ruby-build#2180
  - Fix uploading SARIF reports from Differential Shellcheck by @mislav
    in rbenv/ruby-build#2181
  - Fix compilation of Ruby 3.2.x on FreeBSD by @jarmo in
    rbenv/ruby-build#2187
  - Fix truffleruby+graalvm-dev download URLs by @eregon in
    rbenv/ruby-build#2189

- 20230428:
  - Add TruffleRuby 23.0.0-preview1 by @eregon in rbenv/ruby-build#2190
  - Add TruffleRuby+GraalVM 23.0.0-preview1 by @eregon in
    rbenv/ruby-build#2191
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants