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

Portable Ruby 3.3.1 #17312

Merged
merged 3 commits into from
May 17, 2024
Merged

Portable Ruby 3.3.1 #17312

merged 3 commits into from
May 17, 2024

Conversation

MikeMcQuaid
Copy link
Member

Use the latest version of Portable Ruby.

I may have missed some stuff here.

May need new checksums once https://github.com/Homebrew/homebrew-portable-ruby/actions/runs/9105311684 completes.

@Bo98
Copy link
Member

Bo98 commented May 16, 2024

If we're deleting 3.1 vendored gems may need to bump HOMEBREW_REQUIRED_RUBY_VERSION, which is technically a breaking change but let's pretend we didn't miss 4.3.0.

.gitignore Show resolved Hide resolved
@MikeMcQuaid
Copy link
Member Author

If we're deleting 3.1 vendored gems may need to bump HOMEBREW_REQUIRED_RUBY_VERSION, which is technically a breaking change but let's pretend we didn't miss 4.3.0.

Yeh, might just retrospectively add to the notes there if I can get this merged 🔜 and note it'll land in 4.3.1.

Use the latest version of Portable Ruby.
p.rmtree
FileUtils.rm_r p
Copy link
Member

@Bo98 Bo98 May 16, 2024

Choose a reason for hiding this comment

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

Pathname#rmtree now never returning errors is a horrifically subtle change that questions the safety of every #rmtree call we have, given the new behaviour now means rmtree can return without actually removing the directory (somewhat of a security risk potentially).

Copy link
Member

Choose a reason for hiding this comment

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

e.g. this error catching now doesn't work:

backup_path(keg).rmtree if backup_path(keg).exist?
rescue Errno::EACCES, Errno::ENOTEMPTY
odie <<~EOS
Could not remove #{backup_path(keg).parent.basename} backup keg! Do so manually:
sudo rm -rf #{backup_path(keg)}
EOS

We could override the behaviour in extend/pathname.rb. Seems hacky but might be the safest solution.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeh, good catch and idea here @Bo98.

@Bo98 Bo98 force-pushed the portable-ruby-3.3.1 branch 2 times, most recently from 7a6246a to ddb0c39 Compare May 17, 2024 00:32
@EricFromCanada
Copy link
Member

Seeing the same test failure in 10.13 and up that was showing in CI earlier:

1) Cask::Artifact::App install_phase when the target already exists given the force option target is user-owned but contains read-only files overwrites the existing app
     Failure/Error: let(:install_phase) { app.install_phase(command:, adopt:, force:) }

     ErrorDuringExecution:
       Failure while executing; `/usr/bin/env touch /private/tmp/homebrew-tests-20240516-52284-zi9hyc/cask-appdir/Caffeine.app/.homebrew-write-test` exited with 1. Here's the output:
       touch: /private/tmp/homebrew-tests-20240516-52284-zi9hyc/cask-appdir/Caffeine.app/.homebrew-write-test: Permission denied

10.11 and 10.12 weren't able to install the required gems for brew tests.

@Bo98
Copy link
Member

Bo98 commented May 17, 2024

sorbet/sorbet#7895

@Bo98
Copy link
Member

Bo98 commented May 17, 2024

10.11 and 10.12 weren't able to install the required gems for brew tests.

I assume this was the case with Ruby 3.1.4 too?

@MikeMcQuaid MikeMcQuaid force-pushed the portable-ruby-3.3.1 branch 2 times, most recently from 6e5af63 to 9576a7b Compare May 17, 2024 03:27
@MikeMcQuaid MikeMcQuaid requested a review from Bo98 May 17, 2024 03:27
@MikeMcQuaid MikeMcQuaid merged commit 23939d6 into master May 17, 2024
27 checks passed
@MikeMcQuaid MikeMcQuaid deleted the portable-ruby-3.3.1 branch May 17, 2024 03:42
@MikeMcQuaid
Copy link
Member Author

Thanks for all the help here @Bo98 🎉

@github-actions github-actions bot added the outdated PR was locked due to age label Jun 17, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants