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

Remove OpenStruct dependency #18777

Open
1 task done
dduugg opened this issue Nov 15, 2024 · 5 comments · May be fixed by #18847
Open
1 task done

Remove OpenStruct dependency #18777

dduugg opened this issue Nov 15, 2024 · 5 comments · May be fixed by #18847
Assignees
Labels
features New features help wanted We want help addressing this

Comments

@dduugg
Copy link
Member

dduugg commented Nov 15, 2024

Verification

Provide a detailed description of the proposed feature

ostruct will no longer be a default gem as of Ruby 3.5: ruby/ruby#10428
It's already discouraged for "performance, version compatibility, and potential security issues".

We have two instances to clean up:

Library/Homebrew/cli/args.rb
Library/Homebrew/cmd/uses.rb

What is the motivation for the feature?

☝️

How will the feature be relevant to at least 90% of Homebrew users?

resolution of the performance, version compatibility, and potential security issues described above.

What alternatives to the feature have been considered?

None

@dduugg dduugg added the features New features label Nov 15, 2024
@dduugg dduugg self-assigned this Nov 15, 2024
@MikeMcQuaid MikeMcQuaid added the help wanted We want help addressing this label Nov 15, 2024
@MikeMcQuaid
Copy link
Member

Agreed, we should do this.

@apainintheneck
Copy link
Contributor

It'd be simple to create a small class that replicates the OpenStruct usage in Homebrew::Cmd::Uses (I can do this real quick). Removing it from Homebrew::CLI::Args will be a bit more involved.

@dduugg
Copy link
Member Author

dduugg commented Nov 16, 2024

Yep, the class is a single file, and must of the functionality is via an @table attr that is accessed via method_missing: https://github.com/ruby/ostruct/blob/master/lib/ostruct.rb

It still leaves the major concerns intact though (e.g. performance & security), the code remains difficult to follow, etc. Opening this so we have a bit of time to think of something better (but feel free to self-assign if you have strong opinions).

@apainintheneck
Copy link
Contributor

I see your point. The goal here is not just to remove the OpenStruct dependency but also refactor the surrounding code so that it doesn't rely on OpenStruct-like anti-patterns.

@MikeMcQuaid
Copy link
Member

The goal here is not just to remove the OpenStruct dependency but also refactor the surrounding code so that it doesn't rely on OpenStruct-like anti-patterns.

Exactly. OpenStruct is also pretty Sorbet-unfriendly.

@dduugg dduugg linked a pull request Dec 1, 2024 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
features New features help wanted We want help addressing this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants