[PATCH] Update ruby_version requirement to allow ruby 3.0

[PATCH] Update ruby_version requirement to allow ruby 3.0

[Jean Boussier]

Ruby just recently bump the master version to 3.0 and this version requirement in unicorn breaks our ruby-head CI.

I don't really see any reason to protect against newer Ruby version.

From d1c43dd70b06543727ac88e3930caa0482558a6b Mon Sep 17 00:00:00 2001
From: Jean Boussier <jean.boussier@gmail.com>
Date: Tue, 1 Sep 2020 14:14:07 +0200
Subject: [PATCH] Update ruby_version requirement to allow ruby 3.0

---
 unicorn.gemspec | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/unicorn.gemspec b/unicorn.gemspec
index cbe855d..9a79055 100644
--- a/unicorn.gemspec
+++ b/unicorn.gemspec
@@ -25,10 +25,7 @@
   s.homepage = 'https://yhbt.net/unicorn/'
   s.test_files = test_files
 
-  # technically we need ">= 1.9.3", too, but avoid the array here since
-  # old rubygems versions (1.8.23.2 at least) do not support multiple
-  # version requirements here.
-  s.required_ruby_version = '< 3.0'
+  s.required_ruby_version = '>= 1.9.3'
 
   # We do not have a hard dependency on rack, it's possible to load
   # things which respond to #call.  HTTP status lines in responses
-- 
2.26.2

Re: [PATCH] Update ruby_version requirement to allow ruby 3.0

[Eric Wong]

Jean Boussier <jean.boussier@shopify.com> wrote:
> Ruby just recently bump the master version to 3.0 and this
> version requirement in unicorn breaks our ruby-head CI.

Thanks for the note.  Something to that effect should be in the
commit message body.

> I don't really see any reason to protect against newer Ruby version.

I do: Ruby does not have a good track record when it comes to
maintaining backwards compatibility.  I no longer start new
projects in Ruby as a result.

If 3.0 seems OK (we won't be certain until December), then
bumping it to "< 4.0" should be acceptable for now.

Hint: you can make maintainers' life slightly easier by adding
a "--8<--\n" line before the patch, so "git am --scissors" works :>
(the "From " line is also unnecessary)

> From d1c43dd70b06543727ac88e3930caa0482558a6b Mon Sep 17 00:00:00 2001
> From: Jean Boussier <jean.boussier@gmail.com>
> Date: Tue, 1 Sep 2020 14:14:07 +0200
> Subject: [PATCH] Update ruby_version requirement to allow ruby 3.0

As noted above, something along the lines of ruby.git being on
3.0 and our current code being compatible with it is necessary,
here.

> --- a/unicorn.gemspec
 
> -  # technically we need ">= 1.9.3", too, but avoid the array here since
> -  # old rubygems versions (1.8.23.2 at least) do not support multiple
> -  # version requirements here.

I prefer we keep that above comment, too, to keep working with
older rubygems.  Thanks.

> -  s.required_ruby_version = '< 3.0'
> +  s.required_ruby_version = '>= 1.9.3'

Re: [PATCH] Update ruby_version requirement to allow ruby 3.0

[Jean Boussier]

>> I don't really see any reason to protect against newer Ruby version.
> 
> I do: Ruby does not have a good track record when it comes to
> maintaining backwards compatibility.

Regardless, preventing the gem installation before the version even exists
cause massive pains to people trying to do the good thing of testing their
app against ruby master.

Until it is known that compatibility is broken, restricting the ruby version
causes more harm than good. Just this morning I had to submit patches
to a dozen gems.

> Hint: you can make maintainers' life slightly easier by adding
> a "--8<--\n" line before the patch, so "git am --scissors" works :>
> (the "From " line is also unnecessary)

Apologies, I forgot that one.


I included a new version of the patch using `< 4.0`.

-- 

From 307ea35d87c6a45d8b4261f03793a7a74d885bb2 Mon Sep 17 00:00:00 2001
From: Jean Boussier <jean.boussier@gmail.com>
Date: Tue, 1 Sep 2020 17:00:12 +0200
Subject: [PATCH] Update ruby_version requirement to allow ruby 3.0

Ruby just recently bump the master version to 3.0.
This requirement bump is necessary to test unicorn
against ruby master.
---
 unicorn.gemspec | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/unicorn.gemspec b/unicorn.gemspec
index cbe855d..f04a11b 100644
--- a/unicorn.gemspec
+++ b/unicorn.gemspec
@@ -28,7 +28,7 @@
   # technically we need ">= 1.9.3", too, but avoid the array here since
   # old rubygems versions (1.8.23.2 at least) do not support multiple
   # version requirements here.
-  s.required_ruby_version = '< 3.0'
+  s.required_ruby_version = '< 4.0'
 
   # We do not have a hard dependency on rack, it's possible to load
   # things which respond to #call.  HTTP status lines in responses
-- 
2.26.2

Re: [PATCH] Update ruby_version requirement to allow ruby 3.0

[Eric Wong]

Jean Boussier <jean.boussier@shopify.com> wrote:
> >> I don't really see any reason to protect against newer Ruby version.
> > 
> > I do: Ruby does not have a good track record when it comes to
> > maintaining backwards compatibility.
> 
> Regardless, preventing the gem installation before the version even exists
> cause massive pains to people trying to do the good thing of testing their
> app against ruby master.
> 
> Until it is known that compatibility is broken, restricting the ruby version
> causes more harm than good. Just this morning I had to submit patches
> to a dozen gems.

That sucks.  Perhaps adding warnings about untested+unsupported
versions to test_helper.rb and extconf.rb is a better way to go?
(nothing annoying at runtime after it's installed, though)

Then, maybe leave the version check out of the gemspec entirely.

Fwiw, the type of breakage from incompatibilities I'm worried
about is subtle things that don't show up immediately
(e.g. encodings, hash ordering, frozen strings, etc...).

Stuff that obviously breaks at startup is preferable
(e.g. I added some 1.9 symbol hash keys to signal the move
away from 1.8).

Thanks.

Re: [PATCH] Update ruby_version requirement to allow ruby 3.0

[Jean Boussier]

> Perhaps adding warnings about untested+unsupported
> versions to test_helper.rb and extconf.rb is a better way to go?

That could work yes. Something akin to:
"This ruby version wasn't tested, blah blah".

> Then, maybe leave the version check out of the gemspec entirely.

The gemspec ruby version is very useful but for minimum requirement
only. e.g. `>= 1.9.3`.

> Fwiw, the type of breakage from incompatibilities I'm worried
> about is subtle things that don't show up immediately
> (e.g. encodings, hash ordering, frozen strings, etc...).

That's understandable, but 3.0 is not any more likely that 2.7
to break any of these, and it's important that gems are testable on
ruby pre-release, otherwise you end up with a chicken and egg
problem of not being able to report compatibility breakages
to ruby-core.

On a totally different note, it seems that unicorn is not compiling
quite properly against Ruby 3.0.0-dev, at least on linux:

   unicorn_http.so: undefined symbol: Init_unicorn_http

I'm trying to figure out why the symbol isn't exported,
I might come back with another patch. But just in case
you might have an idea what's going on.

Re: [PATCH] Update ruby_version requirement to allow ruby 3.0

[Eric Wong]

Jean Boussier <jean.boussier@shopify.com> wrote:
> > Perhaps adding warnings about untested+unsupported
> > versions to test_helper.rb and extconf.rb is a better way to go?
> 
> That could work yes. Something akin to:
> "This ruby version wasn't tested, blah blah".

OK, can you send a patch for that?

> > Then, maybe leave the version check out of the gemspec entirely.
> 
> The gemspec ruby version is very useful but for minimum requirement
> only. e.g. `>= 1.9.3`.

Yes, I suppose; I was kinda interested in using 2.3+ socket
features (replacing kgio) but I might just use io_uring on
Linux, at least.

> > Fwiw, the type of breakage from incompatibilities I'm worried
> > about is subtle things that don't show up immediately
> > (e.g. encodings, hash ordering, frozen strings, etc...).
> 
> That's understandable, but 3.0 is not any more likely that 2.7
> to break any of these, and it's important that gems are testable on
> ruby pre-release, otherwise you end up with a chicken and egg
> problem of not being able to report compatibility breakages
> to ruby-core.

Agreed.

> On a totally different note, it seems that unicorn is not compiling
> quite properly against Ruby 3.0.0-dev, at least on linux:
> 
>    unicorn_http.so: undefined symbol: Init_unicorn_http
> 
> I'm trying to figure out why the symbol isn't exported,
> I might come back with another patch. But just in case
> you might have an idea what's going on.

I haven't built+tested ruby.git in ages (my computers are too slow).
It could be a failure to completely clean out the old 2.8 stuff
(either in the Ruby worktree, install paths, or unicorn worktree).

Re: [PATCH] Update ruby_version requirement to allow ruby 3.0

[Jean Boussier]

>> That could work yes. Something akin to:
>> "This ruby version wasn't tested, blah blah".
> 
> OK, can you send a patch for that?

I will once I have something working on 3.0.

>> I'm trying to figure out why the symbol isn't exported,
>> I might come back with another patch. But just in case
>> you might have an idea what's going on.
> 
> I haven't built+tested ruby.git in ages (my computers are too slow).
> It could be a failure to completely clean out the old 2.8 stuff
> (either in the Ruby worktree, install paths, or unicorn worktree).

I don't think so, I'm building from a fresh docker image + fresh unicorn
clone.

I also tested other similar gems (e.g. puma) and they export the functions
just fine. 

I have to admit that I don't understand enough about the build process
to figure out what's going on, so I'm not very confident I'll find a fix.

Re: [PATCH] Update ruby_version requirement to allow ruby 3.0

[Eric Wong]

Jean Boussier <jean.boussier@shopify.com> wrote:
> >> I'm trying to figure out why the symbol isn't exported,
> >> I might come back with another patch. But just in case
> >> you might have an idea what's going on.
> > 
> > I haven't built+tested ruby.git in ages (my computers are too slow).
> > It could be a failure to completely clean out the old 2.8 stuff
> > (either in the Ruby worktree, install paths, or unicorn worktree).
> 
> I don't think so, I'm building from a fresh docker image + fresh unicorn
> clone.
> 
> I also tested other similar gems (e.g. puma) and they export the functions
> just fine. 

Do the current dependencies (kgio, raindrops) export alright, as well?
Might be worth comparing the generated Makefile with other gems
and see if there's missing flags or such.

> I have to admit that I don't understand enough about the build process
> to figure out what's going on, so I'm not very confident I'll find a fix.

I'm not a build/linker expert, either.  Also, did things work a
few days/weeks ago when ruby.git was still 2.8?  Maybe mkmf.log
has clues, somewhere, too...

*shrug* my brain hurts from other stuff

Re: [PATCH] Update ruby_version requirement to allow ruby 3.0

[Jean Boussier]

> Do the current dependencies (kgio, raindrops) export alright, as well?

Yes just fine.

> Might be worth comparing the generated Makefile with other gems
> and see if there's missing flags or such.

I tried that but couldn't figure anything.

> Also, did things work a few days/weeks ago when ruby.git was still 2.8?

They worked fine yes. But I doubt it's caused by the version number,
the must have removed some deprecated stuff at the same time.

> *shrug* my brain hurts from other stuff

No worries. It was just in case it would ring a bell for you.
I'll keep looking and ask for help from colleagues.

Re: [PATCH] Update ruby_version requirement to allow ruby 3.0

[Jean Boussier]

Ok, so the linking issues were simply me being silly. I forgot
to run `make ragel` and commit the output (to use a git gem).
Unicorn now run just fine under 3.0.0-dev (current master).

So here's the patch that warn users about the Ruby version
in extconf.rb and test_helper.rb.

I'm not too fan of the duplication, but not sure how this could
be shared.

-- 

From 68a862f65d6bc202bc4191a495e87626c2e4bfa3 Mon Sep 17 00:00:00 2001
From: Jean Boussier <jean.boussier@gmail.com>
Date: Tue, 1 Sep 2020 17:00:12 +0200
Subject: [PATCH] Update ruby_version requirement to allow ruby 3.0

Ruby just recently bump the master version to 3.0.
This requirement bump is necessary to test unicorn
against ruby master.
---
 ext/unicorn_http/extconf.rb | 4 ++++
 test/test_helper.rb         | 4 ++++
 unicorn.gemspec             | 9 +++++----
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/ext/unicorn_http/extconf.rb b/ext/unicorn_http/extconf.rb
index d5f81fb..2e9a7f1 100644
--- a/ext/unicorn_http/extconf.rb
+++ b/ext/unicorn_http/extconf.rb
@@ -1,6 +1,10 @@
 # -*- encoding: binary -*-
 require 'mkmf'
 
+unless RUBY_VERSION < '3.1'
+  warn "Unicorn was only tested against MRI up to 3.0. It might not properly work with #{RUBY_VERSION}"
+end
+
 have_macro("SIZEOF_OFF_T", "ruby.h") or check_sizeof("off_t", "sys/types.h")
 have_macro("SIZEOF_SIZE_T", "ruby.h") or check_sizeof("size_t", "sys/types.h")
 have_macro("SIZEOF_LONG", "ruby.h") or check_sizeof("long", "sys/types.h")
diff --git a/test/test_helper.rb b/test/test_helper.rb
index e3c6ad4..19528f4 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -34,6 +34,10 @@
   Debugger.start
 end
 
+unless RUBY_VERSION < '3.1'
+  warn "Unicorn was only tested against MRI up to 3.0. It might not properly work with #{RUBY_VERSION}"
+end
+
 def redirect_test_io
   orig_err = STDERR.dup
   orig_out = STDOUT.dup
diff --git a/unicorn.gemspec b/unicorn.gemspec
index cbe855d..e6af44d 100644
--- a/unicorn.gemspec
+++ b/unicorn.gemspec
@@ -25,10 +25,11 @@
   s.homepage = 'https://yhbt.net/unicorn/'
   s.test_files = test_files
 
-  # technically we need ">= 1.9.3", too, but avoid the array here since
-  # old rubygems versions (1.8.23.2 at least) do not support multiple
-  # version requirements here.
-  s.required_ruby_version = '< 3.0'
+  # 1.9.3 is the minumum supported version. We don't specify
+  # a maximum version to make it easier to test pre-releases,
+  # but we do warn users if they install unicorn on an untested
+  # version in extconf.rb
+  s.required_ruby_version = ">= 1.9.3"
 
   # We do not have a hard dependency on rack, it's possible to load
   # things which respond to #call.  HTTP status lines in responses
-- 
2.26.2

Re: [PATCH] Update ruby_version requirement to allow ruby 3.0

[Eric Wong]

Jean Boussier <jean.boussier@shopify.com> wrote:
> Ok, so the linking issues were simply me being silly. I forgot
> to run `make ragel` and commit the output (to use a git gem).
> Unicorn now run just fine under 3.0.0-dev (current master).

Odd, just "make" (no args) should work.

> So here's the patch that warn users about the Ruby version
> in extconf.rb and test_helper.rb.
> 
> I'm not too fan of the duplication, but not sure how this could
> be shared.

The small amount of duplication is fine, thanks.

Pushed as commit 06e904f1e8d8e554d8992fbb3de993dff45dec4c
I needed to fix column wrapping to be <80 columns to accomodate
those needing giant fonts.

Anything else in the ruby 3.0 pipeline that might introduce
incompatibilities?

Not sure if releasing unicorn 5.7 is worth doing soon/now or
closer to December 25...  Will we even survive until then?
This year keeps getting worse :<

Re: [PATCH] Update ruby_version requirement to allow ruby 3.0

[Jean Boussier]

>> Ok, so the linking issues were simply me being silly. I forgot
>> to run `make ragel` and commit the output (to use a git gem).
>> Unicorn now run just fine under 3.0.0-dev (current master).
> 
> Odd, just "make" (no args) should work.

It does, but the files it generates are in .gitignore, and I was
trying to use the gem through bundler's git gems. Anyway,
now I know the drill.

> Anything else in the ruby 3.0 pipeline that might introduce
> incompatibilities?

Not that I know of. There not any big breaking change
planned for 3.0. 

> Not sure if releasing unicorn 5.7 is worth doing soon/now or
> closer to December 25...

It is up to you. But the sooner the better for me. We run
our master branch CI against ruby master on a nightly
basis. Until this patch is released I'll have to keep a fork
around.

Re: [PATCH] Update ruby_version requirement to allow ruby 3.0

[Eric Wong]

Jean Boussier <jean.boussier@shopify.com> wrote:
> >> Ok, so the linking issues were simply me being silly. I forgot
> >> to run `make ragel` and commit the output (to use a git gem).
> >> Unicorn now run just fine under 3.0.0-dev (current master).
> > 
> > Odd, just "make" (no args) should work.
> 
> It does, but the files it generates are in .gitignore, and I was
> trying to use the gem through bundler's git gems. Anyway,
> now I know the drill.

Ah, perhaps adding some checks to extconf.rb would be useful?
Or moving the ragel invocation into the extconf.rb-generated
Makefile? (not too sure how to do that...).

I'm not remotely knowledgeable when it comes to bundler.

> > Anything else in the ruby 3.0 pipeline that might introduce
> > incompatibilities?
> 
> Not that I know of. There not any big breaking change
> planned for 3.0. 

Good to know.

> > Not sure if releasing unicorn 5.7 is worth doing soon/now or
> > closer to December 25...
> 
> It is up to you. But the sooner the better for me. We run
> our master branch CI against ruby master on a nightly
> basis. Until this patch is released I'll have to keep a fork
> around.

OK, soon.  Maybe with just the aforementioned ragel check above;
or not even...  Electric grid is failing and public libraries
are all closed due to COVID-19; so net access can be scarce.

Re: [PATCH] Update ruby_version requirement to allow ruby 3.0

[Jean Boussier]

> Ah, perhaps adding some checks to extconf.rb would be useful?
> Or moving the ragel invocation into the extconf.rb-generated
> Makefile? (not too sure how to do that...).

Not really no. And it would make ragel a build dependency.

> I'm not remotely knowledgeable when it comes to bundler.

That's fine, it just makes unicorn a bit harder to use outside of
rubygems releases. But long story short bundler git gems is
pretty much:

  - git clone repo
  - gem build repo.gemspec
  - gem install repo-...

So nothing is executed besides the extconf.rb. Which mean
the version file and the generated.c file would need to be
committed in the repository rather than in .gitignore.

But really I don't think so many people need to use unicorn
branches.

> OK, soon.  Maybe with just the aforementioned ragel check above;
> or not even...  

I'd say not even. It's just for people like me trying to run branches.

Re: [PATCH] Update ruby_version requirement to allow ruby 3.0

[Eric Wong]

Jean Boussier <jean.boussier@shopify.com> wrote:
> > Ah, perhaps adding some checks to extconf.rb would be useful?
> > Or moving the ragel invocation into the extconf.rb-generated
> > Makefile? (not too sure how to do that...).
> 
> Not really no. And it would make ragel a build dependency.

No, the generated .c file would still be distributed with the
gem as it is today.  I'm absolutely against introducing new
build requirements (optional dependencies, maybe, but not
requirements).

Changing the .rl file might result in ragel running (or emit
warning if ragel isn't installed).  Not too sure how to do that
via mkmf, though...  Or it would just throw a warning and drop a
hint to run "make ragel" at the top-level if the .c file is
missing...

> But really I don't think so many people need to use unicorn
> branches.

*shrug*  I figure for any one person that posts a comment for
something on any public forum, there's probably 10 that feel the
same way.

> > OK, soon.  Maybe with just the aforementioned ragel check above;
> > or not even...  
> 
> I'd say not even. It's just for people like me trying to run branches.

Alright, we can experiment with more build system changes
another time another (and I zoned out while writing the 5.7
release notes and forgot to note the minor build system changes
I made right after th 5.6.0 release :x)

https://yhbt.net/unicorn-public/20200908084429.GA16521@dcvr/

Re: [PATCH] Update ruby_version requirement to allow ruby 3.0

[Jean Boussier]

>>> Ah, perhaps adding some checks to extconf.rb would be useful?
>>> Or moving the ragel invocation into the extconf.rb-generated
>>> Makefile? (not too sure how to do that...).
>> 
>> Not really no. And it would make ragel a build dependency.
> 
> No, the generated .c file would still be distributed with the
> gem as it is today.

I meant if the gem is used as a "git gem" through bundler.
That's why some other gems like puma commit the generated
.c files.

> Changing the .rl file might result in ragel running (or emit
> warning if ragel isn't installed).  Not too sure how to do that
> via mkmf, though...  Or it would just throw a warning and drop a
> hint to run "make ragel" at the top-level if the .c file is
> missing...

The thing is ragel isn't part of packages such as build-essentials
etc. So really it wouldn't help much.

It just took me a while to figure out because I was tired, but in
the end it wasn't that bad: 
https://github.com/Shopify/unicorn/compare/upstream-master...Shopify:ruby-3.0