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

Deep copy requirements in Gem::Specification and Gem::Requirement #7439

Conversation

flavorjones
Copy link
Contributor

What was the end-user or developer problem that led to this PR?

While drafting rake-compiler/rake-compiler#236, I needed to modify the required_rubygems_version of a Gem::Specification that was a copy of the original.

While doing so, I discovered two things:

  • modifying the required_rubygems_version of a Gem::Specification copy also mutates the original's
  • modifying the requirements of a Gem::Requirements copy also mutates the original's

That is:

requirement2 = requirement.dup

# this modifies original `requirement`
requirement2.concat([">= 3.3.22"])

and

spec2 = spec.dup

# this modifies original `spec`'s `required_rubygems_version`
spec2.required_rubygems_version.concat([">= 3.3.22"])

This was confusing and led to more complex code and tests.

What is your fix for the problem, implemented in this PR?

This PR implements what I believe to be the common-sense behavior of "copy" for these classes to avoid accidental mutation of the originals:

  • deep-copies the Gem::Requirement attributes of a Gem::Specification
  • deep-copies the @requirements attribute of a Gem::Requirement

With this new behavior, my use case described above can be solved by:

spec2 = spec.dup
spec2.required_rubygems_version.concat([">= 3.3.22"])

Make sure the following tasks are checked

Copy link
Member

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

Looks good to me. Definitely more common sense! Just left a small comment.

lib/rubygems/specification.rb Show resolved Hide resolved
to avoid accidentally mutating the original's state when doing:

```ruby
req2 = req.dup
req2.concat([">= 3.3.22"])
```

see rake-compiler/rake-compiler#236 for a
real-world use case that would be made simpler with this behavior.
to avoid accidentally mutating the original's state when doing:

```ruby
spec2 = spec.dup
spec2.required_rubygems_version.concat([">= 3.3.22"])
```

see rake-compiler/rake-compiler#236 for a
real-world use case that would be made simpler with this behavior.
Copy link
Member

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

Thanks!

@deivid-rodriguez deivid-rodriguez merged commit 5712b52 into rubygems:master Feb 2, 2024
75 checks passed
@flavorjones flavorjones deleted the flavorjones-deep-copy-gem-requirements branch February 2, 2024 21:38
deivid-rodriguez added a commit that referenced this pull request Feb 5, 2024
…equirements

feature: deep copy requirements in Gem::Specification and Gem::Requirement
(cherry picked from commit 5712b52)
@deivid-rodriguez deivid-rodriguez changed the title feature: deep copy requirements in Gem::Specification and Gem::Requirement Deep copy requirements in Gem::Specification and `Gem::Requirement Feb 5, 2024
@deivid-rodriguez deivid-rodriguez changed the title Deep copy requirements in Gem::Specification and `Gem::Requirement Deep copy requirements in Gem::Specification and Gem::Requirement Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants