Conversation
|
Why would Ruby 2.7 be broken? Seems strange 2.7 would break but 2.6 would be fine. We use 2.7, 3.1, and working on getting 3.4 supported (I have it working on master). Master branch has 2.7 working still. I think it should be supportable here as well? |
more than welcome to take a look. Rest of the versions work fine, 2.7 gets stuck in a bundler loop. Once merged, this will work for Ruby 3.4 too. edit: should say, this is a GA issue, locally 2.7 should work just fine |
|
Also remember this PR is trying to be backwards compatible with all ruby versions. I would focus on that rather than enabling specific ruby versions in master. I would like for us to move to Ruby 4.0.1 asap internally, so this PR has to merge once 2.7 is confirmed to be a GA only issue. |
|
This sounds similar to what we saw in 3.1 at first - a bad requirement gets into a spin resolving the dependencies. I believe the solution was to just blast away the Gemfile.lock and let it rebuild itself. Can you add 2.7 back into the regressions so the logs are generated? I don't see any logs containing 2.7 to see even the problem. |
Hypothesis: Proposal line: It's a CI-only problem. Real users on 2.7 have stable gem environments where gem install origen installs compatible deps system-wide. Keep 2.7 excluded from CI |
|
I disagree - and we saw this with 3.1 originally. Particularly, new users with Ruby 3 would see an infinite bundler loop. The CI problem means something was introduced to affect 2.7. If we intend to support 2.7, the regressions should reflect it. As far as fixing, this may just be putting a conditional statement in the Gemfile/.gemspec so the proper dependencies are pulled. @pderouen, do you recall this from Ruby 3 debug originally? |
I finished isolating the issue and validating this end-to-end on Ruby 2.7.8. This is not an Origen issue. Ruby 2.7 ships with RubyGems 3.1.6, which contains a known dependency resolution bug in the resolver. link Ruby 2.7 requires a combination of:
So I stand by my original position: Ruby 2.7 support requires firm boundaries, and I’m not going to keep revisiting this every time the resolver misbehaves. The following are required: irb 1.8+ pulls in rdoc -> erb -> cgi, which conflicts with Ruby 2.7’s defaults. Without the cap, RubyGems can legally resolve to a broken dependency tree even with a newer resolver. The nanoc constraint reflects the range Origen is actually compatible with. On GA CI, I’ve updated RubyGems to 3.4.22 for Ruby 2.7, and with these bounded constraints in place, CI passes cleanly. I’ve also verified this locally on Ruby 2.7.8. To be clear:
I’ve pushed the changes and enabled all Ruby versions. Ruby 2.7 is supported with the explicit requirement that RubyGems is updated (GA CI is running RubyGems 3.4.22). CI is now passing on both Ubuntu and Windows across Ruby versions [2.6, 2.7, 3.0.4, 3.1, 3.3.1, 3.4.2, 4.0.0]. |
|
Thanks! I do appreciate the effort to keep Ruby 2.7 going. As this shows though, there're differences in the versions to be contested with. While I do not intend to block forward progress, I still do not agree that just removing previously passing tests is the proper route to take. Can you expand on this point specifically:
I think that comes with territory of managing anything across different versions. The gemspec update shows this is a real problem, and a install, for example for a Windows user that doesn't use a managed environment in 2.7 like we have here, may not get a working resolution. In the case of dependency specifics, that can be conditionally done in the gemspec with a A separate discussion worth having at some point is do we consider dropping 2.6 and 2.7 later down the line? I am hopeful that we're at the tail-end of 2.7 here, as I'm pushing for at least 3.1 with 3.4 coming soon (FWIW, the reason I didn't release master for 3.4 support is I couldn't get an environment working quickly enough to run the release that didn't break 2.6 and I've had other priorities, unfortunately). Ruby 4 I have not looked at myself yet. |
|
I agree this is a real problem space when supporting multiple Ruby versions, and I appreciate the concern about downstream users. On the specific point about conditionally capping dependencies via RUBY_VERSION: my understanding (and experience) is that this is not something we can reliably do for runtime dependencies in a gemspec. The gemspec is evaluated at build time, not at install/resolve time, and RubyGems does not support selecting different runtime dependency graphs based on the consumer’s Ruby version in a way that is portable or reliable. In practice, this leads to non-deterministic resolution and makes the problem harder to reason about, not easier. That’s why I’ve intentionally taken a different approach here:
This keeps installs deterministic for users (including unmanaged Windows environments), which is exactly the class of failure we’re trying to avoid. As for this PR specifically: all previous functionality is retained, CI is passing on Ubuntu and Windows across all supported Ruby versions, and Ruby 2.7 is explicitly supported with clear guardrails. From my perspective, this PR is ready for approval and merge. I agree that a broader discussion about eventually dropping 2.6/2.7 is worth having, but I see that as a separate policy conversation. This PR simply puts Ruby 2.7 on stable footing without blocking forward progress. |
|
@priyavadan, thanks for this update. Maybe we should sort out how to handle dropping support for older versions in a call or over a pint :) I completely agree that we need to keep marching forward with supporting newer Ruby versions. As I recall the biggest pain point going to Ruby 3 was (by far) the lint rule changes when the forced update of RuboCop was done. It really was pretty painless to update existing apps and plugins. Also understand the desire to maintain compatibility with older versions to minimize maintenance of legacy apps. I wonder whether we should just have a documented plan for when to drop CI checks for older versions. That way there would be plenty of time for legacy app owners to plan for the inevitable. @coreyeng, the issue I ran into was trying to go from 3.1 to 3.3. A new dependency was needed for the emailer feature (which I don't think has any CI checks) to avoid run time errors on gem releases. After adding the dependency the 2.7 checks for 1 of the operating systems (Windows I think?) would get stuck in an infinite loop trying to boot Origen. I was never able to reproduce it and just dropped it. |
pderouen
left a comment
There was a problem hiding this comment.
I just have the 1 question about the branch reference in the Gemfile. Looks good to me and thanks for the update.
pushed an update to point to https://github.com/Origen-SDK/origen_testers/tree/feature/ruby_4_0
CI results for |
lib/origen/chip_mode.rb
Outdated
| fail '@data_rate_unit must be set with @data_rate, exiting...' if @data_rate.nil? | ||
| end | ||
| unless @data_rate.nil? || @data_rate =~ /n\/a/i || @data_rate =~ /na/i | ||
| unless @data_rate.to_s.nil? || @data_rate.to_s =~ /n\/a/i || @data_rate.to_s =~ /na/i |
There was a problem hiding this comment.
I'm pretty sure @data_rate.to_s.nil? will always return false, as nil.to_s returns ""
There was a problem hiding this comment.
you are right, not sure if this was carry over from the feature/ruby_3_3_1 that i used, will update to @data_rate.nil? like it was originally.
|
going to set |
I don't have any |
|
Thanks! Looks good to me too. Just a quick thing:
That's interesting as I've had the opposite experience. I do agree to use it sparingly, as you said it can get messy, but when I've had to use it, it's worked well - one of the advantages of the Gemfiles and gemspec just being core-Ruby code. It's a greatly missed feature I've found in working with Poetry for O2. |
I need this released before I can fix origen_testers (without complicating the CI workflow)
Add Ruby 4.0 support with backwards compatibility to Ruby 2.6 (minus Ruby 2.7)
Ruby 2.7 is broken, I wont touch it
require 'set'for Ruby 2.6 compatibilityrequire 'nokogiri', conditional byebug loadingNewCops: disablefor stable lint behavior2.6.0, 3.0.4, 3.1.0, 3.3.1, 3.4.2, 4.0.0