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

Freeze more strings in generated gemspecs #6974

Merged

Conversation

segiddins
Copy link
Member

Specifically, this will have frozen string literals for:

  • Gem platform tuple entries
  • Gem::Version strings
  • Gem::Specification#installed_by_version
  • Dependency requirement strings

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

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

Make sure the following tasks are checked

Specifically, this will have frozen string literals for:
- Gem platform tuple entries
- Gem::Version strings
- Gem::Specification#installed_by_version
- Dependency requirement strings
@martinemde
Copy link
Member

Will this change the checksums of the gemspecs or is it outside of that process? I think we call to_ruby when storing the plain text gemspec for gem content storage?

@segiddins
Copy link
Member Author

Will this change the checksums of the gemspecs or is it outside of that process

it's outside of that process, since the gemspec we upload to quick/ is the marshaled gemspec (and also those don't get regenerated, so changing what would be generated is kosher, and we've done that in the past)

@martinemde
Copy link
Member

it's outside of that process, since the gemspec we upload to quick/ is the marshaled gemspec

Ok cool. Now I'm wondering which gemspec version should be uploaded with gem content.

@segiddins
Copy link
Member Author

Ok cool. Now I'm wondering which gemspec version should be uploaded with gem content.

IMO it should be the yaml gemspec that's inside the .gem, but that's mostly since it's what'd be most helpful to me from a rubygems.org security perspective

@martinemde
Copy link
Member

IMO it should be the yaml gemspec that's inside the .gem, but that's mostly since it's what'd be most helpful to me from a rubygems.org security perspective

That seems most reasonable to me. I can't imagine what else we'd want besides the most useful thing 😆

@segiddins segiddins merged commit 4288da7 into master Sep 21, 2023
83 checks passed
@segiddins segiddins deleted the segiddins/freeze-more-strings-in-generated-gemspecs branch September 21, 2023 18:25
deivid-rodriguez pushed a commit that referenced this pull request Oct 13, 2023
…in-generated-gemspecs

Freeze more strings in generated gemspecs

(cherry picked from commit 4288da7)
deivid-rodriguez pushed a commit that referenced this pull request Oct 13, 2023
…in-generated-gemspecs

Freeze more strings in generated gemspecs

(cherry picked from commit 4288da7)
deivid-rodriguez pushed a commit that referenced this pull request Oct 13, 2023
…in-generated-gemspecs

Freeze more strings in generated gemspecs

(cherry picked from commit 4288da7)
deivid-rodriguez pushed a commit that referenced this pull request Oct 16, 2023
…in-generated-gemspecs

Freeze more strings in generated gemspecs

(cherry picked from commit 4288da7)
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

3 participants