Skip to content

Clarify comment and remove TODO#5031

Merged
jeffwidman merged 1 commit intodependabot:mainfrom
jeffwidman:patch-5
May 10, 2022
Merged

Clarify comment and remove TODO#5031
jeffwidman merged 1 commit intodependabot:mainfrom
jeffwidman:patch-5

Conversation

@jeffwidman
Copy link
Copy Markdown
Member

@jeffwidman jeffwidman commented Apr 22, 2022

The current version of the warning:

warning: parser/current is loading parser/ruby27, which recognizes2.7.6-compliant syntax, but you are running 2.7.5.
Please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.

So I removed the TODO comment because it's outdated.
But I left the actual exclusion in because it will be a recurring issue from time to time because https://ppa.launchpadcontent.net/brightbox/ruby-ng/ubuntu can be a little slow to update...
for example see various discussions on the mailing list that sometimes get no response from
the maintainers: https://groups.google.com/g/brightbox-ruby-ubuntu-packaging

@jeffwidman jeffwidman requested a review from a team as a code owner April 22, 2022 19:55
The current version of the warning:
```
warning: parser/current is loading parser/ruby27, which recognizes2.7.6-compliant syntax, but you are running 2.7.5.
Please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
```

So I removed the TODO comment because it's outdated.
But I left the actual exclusion in because it will be a recurring issue from time to time.
# This is a recurring issue that occurs whenever the parser gets
# ahead of our installed ruby version.
%r{parser/current is loading parser/ruby27},
/2.7.\d-compliant syntax, but you are running 2.7.\d/,
Copy link
Copy Markdown
Member Author

@jeffwidman jeffwidman Apr 22, 2022

Choose a reason for hiding this comment

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

I am unclear why these first two lines are separate patterns, when the matching error line is a single line:

parser/current is loading parser/ruby27, which recognizes2.7.6-compliant syntax, but you are running 2.7.5.

This code was originally added here: https://github.com/dependabot/dependabot-core/pull/3913/files#diff-ccda7b2806a38f2ed3bdb918dd76c7720fd55e0d5c825f4d961edde095e97af4R3-R9 but that doesn't provide much context... @feelepxyz do you recall why?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just did a quick Google search for "compliant syntax, but you are running " and saw an issue wherein someone had the message displayed as:

warning: parser/current is loading parser/ruby22, which recognizes
warning: 2.2.3-compliant syntax, but you are running 2.2.4.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.

I'd expect ALLOW_PATTERNS.none? { |pattern| pattern =~ message } to nicely handle that case and the case you quoted 🙂

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, that makes perfect sense, thank you @landongrindheim!

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

Looks good to me!

I'm wondering, given how long it takes for brightbox to package ruby, whether it would make sense to move to something else.

@jeffwidman jeffwidman merged commit 7ae6fbb into dependabot:main May 10, 2022
@jeffwidman jeffwidman deleted the patch-5 branch May 10, 2022 18:44
@jeffwidman
Copy link
Copy Markdown
Member Author

I'm wondering, given how long it takes for brightbox to package ruby, whether it would make sense to move to something else.

@deivid-rodriguez I had the same question... but didn't know offhand where else to pick it up from. Beyond speed of updates, from a security perspective, we need to be especially careful where we pick this up from.

@mattt mattt mentioned this pull request May 10, 2022
@deivid-rodriguez
Copy link
Copy Markdown
Contributor

@jeffwidman Absolutely!

If I recall correctly, brightbox Ruby was born at the time because Linux distributions were too slow updating Ruby. However, now the situation seems exactly the opposite, for example, Ruby 3.0 is already available in Ubuntu 22.04.

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.

3 participants