-
Notifications
You must be signed in to change notification settings - Fork 72
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
Proper Semantic Versioning #131
Comments
This should be prioritized as impacting many. |
Note that the old version of 3.2.6 is subject to a DoS CVE: https://www.ruby-lang.org/en/news/2024/05/16/dos-rexml-cve-2024-35176/. Impact of the vulnerability may be limited, but this may be a reason to favour the "install |
+1, or at least 3.3.0 would be acceptable? Would also be nice to see the In my case, the requiring Gem is ruby-dbus which has the following: I will recommend that ruby-dbus changes their dependency to here to have a version, such as For the sake of those who think they have the same problem (and the web crawlers), here is dependency chain I have that got me here:
|
It's impossible. If we yanked them, it breaks the many of Ruby application. |
It breaks no one. You released two versions this morning which broke many. I highly doubt anyone pinned the versions to this yet. This is 💯 a major breaking change. Major minor patch. This is far from a patch or a minor change. |
Secondary variant with (very old ruby, bundler, probably solvable but surprising):
|
Could you explain why Chef is related to REXML? #131 (comment) ? Could you also explain what is "kitchen" in Chef context? |
@kou the "kitchen" is a test environment for using Chef. It's not really relevant to the problem at hand but it's useful for those with chef/kitchen related problems to see how to install gcc. The solution above is specific to environments using yum but it's enough to get someone going the right direction. Why Chef is related to REXML: see my dependency chain above - where it starts is the systemd cookbook is widely used, and that uses the dbus-systemd gem. While I agree the versioning here was bad, this could be fixed this up the dependency chain - for example the version could be locked here: mvidner/ruby-dbus#143, but I'd have to keep looking up the chain for the same problems I guess. |
Thanks. If this is related to ruby-dbus (via the systemd cookbook) as you explained, the semantic versioning isn't related to this. Because If you don't want to use REXML 3.2.7 or later, you must pin REXML 3.2.6 by yourself. (I'm not sure whether Chef uses (We don't need to change ruby-dbus because there isn't any reason that ruby-dbus needs to pin REXML. ruby-dbus can work with any versions of REXML.) |
Asciidoctor/Antora user here. 3.2.7/3.2.8 broke our CI as well. Adding dependencies (especially native) in patch-release was not a good idea.
|
This part in the message will fix your CI. |
I believe my CI should not have broke in the first place by a patch update of rexml. There's nothing wrong with adding/removing dependencies, but not this way by breaking everyone. I would not say a single word if 3.2.7/3.2.8 was instead called 4.x.x. |
Could you share your repository for the CI log? |
Thumbs downing people with evidence is very immature. I was able to fix my organization but it took "a village" and a monumental effort from my team to come to a resolution. This is a Ruby core library, consumed by a wide spread of tooling. A simple acknowledgement would be the adult thing to do here. |
There also seems to be an API breaking change when running on JRuby. Passing tests with rexml 3.2.6. Failing tests with rexml 3.2.8. The software itself didn't change at all between these two runs.
|
EDIT: As mentioned below, libonig is optional. I had misinterpreted |
As those before me have already stated, this "patch release" shouldn't have been labeled as such, and it has broken community expectations. In general, patch releases should not add dependencies, or define minimum versions / pin them when previously undefined, particularly if they require native compilation for installation. For those advocating In my ideal world:
While the above is not a panacea for all, and things would still break for some, it would at least be excusable and would leave the community-at-large in a (slightly) less grumpy state. |
Just wanted to highlight rexml 3.2.8 is unusable on JRuby 9.3.7.0 installed via rbenv. In other words, this is different than the "you need to install gcc and ruby-devel to compile strscan with native extensions" The following fails as a result of this line https://github.com/ruby/rexml/blob/v3.2.8/lib/rexml/parsers/baseparser.rb#L216
REXML 3.2.6 does not have this issue:
Also MRI Ruby 3.2.2 and REXML 3.2.8 work:
To be clear the bug above is in the strscan gem, but when combined with rexml depending on strscan, then rexml is completely broken on JRuby 9.3. That seems worthy of yanking rexml 3.2.7 and 3.2.8.
JRuby 9.4.5.0 doesn't have this issue:
For posterity, the issue was fixed in JRuby in jruby/jruby@c5783b9#diff-47288559b9fccc3854cf6d41fca775477a0ad0874cefee5dbae926748de4940eR368-R379 |
rexml 3.2.7 added a runtime dependency on strscan, which requires native extensions on MRI Ruby and whose StringScanner.match method behaves differently depending on whether you're using the native or Java extension. See ruby/rexml#131
rexml 3.2.7 added a runtime dependency on strscan, which requires native extensions on MRI Ruby and whose StringScanner.match method behaves differently depending on whether you're using the native or Java extension. See ruby/rexml#131
rexml 3.2.7 added a runtime dependency on strscan, which requires native extensions on MRI Ruby and whose StringScanner.match method behaves differently depending on whether you're using the native or Java extension. See ruby/rexml#131
Most users of REXML are not direct users. Most users simply have the expectation that most Ruby gems will behave in a particular manner. Most Ruby gems use Semantic Versioning, and it is documented as a recommendation. From that page:
Read semver.org (日本語); please try to understand why we believe this versioning method is important, and why it has been adopted by the larger RubyGems community. If REXML has its own versioning policy as to what constitutes a major, minor, or patch release, please elaborate. |
I updated my comment above. The JRuby issue was fixed in 9.3.11.0.
rubocop depends on Separately, isn't the strscan dependency going to be a major issue for Windows users, especially since XML is so common there? I'm talking specifically about cases where ruby is redistributed to non-developer servers, so there's no devkit. |
I would also mention, that there are Windows users of Chef, who require much more complicated solutions than "yum install gcc" or "yum install ruby-devel". I also suspect that installing dev tools on the production server is NOT a good security pattern. |
💯💯💯 |
No further response seems to be forthcoming for what amounts to a major mishap. |
I know the semantic versioning. It seems that you misunderstand that we can solve this case by using only "REXML uses the semantic versioning as its versioning schema". We need at least the followings if we use the semantic versioning based approach:
If we don't have 2., you always use the latest REXML even if we have 1. For this case, do you have a suggestion how to do 2.? FYI: I will not use this to stop this discussion but strscan dependency isn't related to major/minor/patch release: https://semver.org/#what-should-i-do-if-i-update-my-own-dependencies-without-changing-the-public-api
The semantic versioning focuses on only public API. And 3.2.7 doesn't introduce public API change. So we need to use more strict versioning schema than the semantic versioning if we use the semantic versioning based approach. If we can discuss this case in productive way, we can keep this discussion. But if any of us can't do it, I need to stop this discussion. |
Thanks. I don't know the RuboCop case. |
Why do we need to use REXML with Chef on Windows? I think that systemd isn't used on Windows. So ruby-dbus isn't used. |
FYI: I've implemented #132. You can use pre-installed strscan (as a default gem) with REXML. So you don't need to build strscan. |
Agreed. I wasn't looking to for every case to be solved. That is, in part, the responsibility of the downstream gems and their users.
In my opinion, by setting a specific minimum version of a native extension dependency, you are changing the public API, albeit indirectly. This is one of the major issues when using dependencies that are native extensions. Through no fault of their own, the previous release caused some pain when many users unexpectedly found their builds or systems breaking, including those that are "in production". We all have to be careful about how dependencies are used, as this lesson shows.
Thank you. If the dependency issue is fully corrected through the change, and no further breakage occurs, then it arguably should be released as 3.2.9. Thereafter, I hope we all can be civil, as there is no need for hostility directed towards the maintainers. In the future, when planning for a new release, if there are dependency changes (particularly if those changed dependencies will require building native extensions), it would be good to communicate with some of the direct users before such changes occur. |
This also broke iOS builds using the
|
Chef cookbooks does not need to be OS-dependant and could include different recipes and cookbook references for different environment including different operating systems. However, the OS detection is execution-time, not compile-time and as such the gem dependency is built for all the dependant cookbook available. (Last statement is my understanding, could be wrong. I'm the user of chef, not the developer) |
I know almost nothing about chef, but surely you should be installing from a Gemfile.lock to freeze the exact versions of dependencies you want. Then you can run installing unconstrained software from the internet straight onto your servers seems like a bad idea |
In general, we should not customize well-defined definitions without changing their definition names. It makes communication difficult because each of us may think other thing with the same definition name. If the well-defined definition isn't matched for us, we should not use the original defined name. In this case, if we want to use dependency change for versioning schema, we should not refer the semantic versioning. Because the semantic versioning definition says that dependency change isn't related to major/minor/patch release explicitly.
It seems that it's not realistic. You can see all direct users by https://rubygems.org/gems/rexml/reverse_dependencies . We can't choose who should be communicated. Can REXML users who want to improve REXML and REXML users test unreleased version in their CI (or something) instead? |
@paulschreiber Could you open a new issue for it? It may be a regression bug. Could you also provide an XML file that is related to it and related codes that use REXML in the new issue? |
I'm not familiar with Chef but, in general, needless dependencies should not be included for production. |
See issue below: ruby/rexml#131
See issue below: ruby/rexml#131
Just in case it encourages someone else to upgrade .. I had no trouble compiling
|
REXML 3.2.9 doesn't require strscan 3.0.9 or later. It can use strscan installed as a default gem. Semantic versioning is unrelated. I close this. |
I'm getting this error on 3.2.9:
The error goes away when using 3.2.6. |
@fertrig Thanks for your report. Could you open a new issue for it? It's not related to this issue. |
@kou @nobu, the two most recent releases 3.2.7 and 3.2.8 should be major versions as they introduce
strscan
which seems to require some native extensions and compilation on systems. This is breaking my world with folks using Chef. Can you please yank those and make this 4.x.x or something?For Chef folks looking for a fix, add the following to your cookbooks metadata.rb:
You can also install
gcc
in your kitchen using:Thanks
The text was updated successfully, but these errors were encountered: