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

Support Ruby 2.6+ again #3

Closed
wants to merge 7 commits into from
Closed

Support Ruby 2.6+ again #3

wants to merge 7 commits into from

Conversation

kachick
Copy link
Member

@kachick kachick commented Mar 27, 2021

ruby: [ 3.0, head ]

This gem is testing only in Ruby 3.0+ since 0d1b754
Actually, old rubies can't finish tests.

bin/setup; rake failed as below.

On ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-darwin20]

install -c tmp/x86_64-darwin20/pathname/2.7.2/pathname.bundle lib/pathname.bundle
cp tmp/x86_64-darwin20/pathname/2.7.2/pathname.bundle tmp/x86_64-darwin20/stage/lib/pathname.bundle
Loaded suite /Users/kachick/.gem/ruby/2.7.2/gems/rake-13.0.3/lib/rake/rake_test_loader
Started
...............................................................................
...............................................................................
...............................................................................
..............................................E
===============================================================================
Error: test_ractor_shareable(TestPathnameRactor): NameError: undefined local variable or method `skip' for #<TestPathnameRactor:0x00007ff6c7a2d878>
/Users/kachick/repos/pathname/test/pathname/test_ractor.rb:7:in `setup'
===============================================================================

Finished in 0.196045 seconds.
-------------------------------------------------------------------------------
284 tests, 368 assertions, 0 failures, 1 errors, 0 pendings, 0 omissions, 0 notifications
99.6479% passed
-------------------------------------------------------------------------------
1448.65 tests/s, 1877.12 assertions/s
rake aborted!
Command failed with status (1)
/Users/kachick/.gem/ruby/2.7.2/gems/rake-13.0.3/exe/rake:27:in `<top (required)>'
Tasks: TOP => default => test
(See full trace by running task with --trace)

On ruby 2.3.8p459 (2018-10-18 revision 65136) [x86_64-linux]

mkdir -p tmp/x86_64-linux/pathname/2.3.8
cd tmp/x86_64-linux/pathname/2.3.8
/usr/local/bin/ruby -I. ../../../../ext/pathname/extconf.rb
checking for rb_file_s_birthtime()... no
creating Makefile
cd -
cd tmp/x86_64-linux/pathname/2.3.8
/usr/bin/make
compiling ../../../../ext/pathname/pathname.c
../../../../ext/pathname/pathname.c: In function 'path_each_line':
../../../../ext/pathname/pathname.c:363:16: warning: implicit declaration of function 'rb_block_call_kw'; did you mean 'rb_block_call'? [-Wimplicit-function-declaration]
         return rb_block_call_kw(rb_cFile, id_foreach, 1+n, args, 0, 0, RB_PASS_CALLED_KEYWORDS);
                ^~~~~~~~~~~~~~~~
                rb_block_call
../../../../ext/pathname/pathname.c:363:72: error: 'RB_PASS_CALLED_KEYWORDS' undeclared (first use in this function)
         return rb_block_call_kw(rb_cFile, id_foreach, 1+n, args, 0, 0, RB_PASS_CALLED_KEYWORDS);
                                                                        ^~~~~~~~~~~~~~~~~~~~~~~
../../../../ext/pathname/pathname.c:363:72: note: each undeclared identifier is reported only once for each function it appears in
../../../../ext/pathname/pathname.c:366:16: warning: implicit declaration of function 'rb_funcallv_kw'; did you mean 'rb_funcallv'? [-Wimplicit-function-declaration]
         return rb_funcallv_kw(rb_cFile, id_foreach, 1+n, args, RB_PASS_CALLED_KEYWORDS);
                ^~~~~~~~~~~~~~
                rb_funcallv
../../../../ext/pathname/pathname.c: In function 'path_read':
../../../../ext/pathname/pathname.c:388:57: error: 'RB_PASS_CALLED_KEYWORDS' undeclared (first use in this function)
     return rb_funcallv_kw(rb_cFile, id_read, 1+n, args, RB_PASS_CALLED_KEYWORDS);
                                                         ^~~~~~~~~~~~~~~~~~~~~~~
../../../../ext/pathname/pathname.c: In function 'path_write':
../../../../ext/pathname/pathname.c:429:58: error: 'RB_PASS_CALLED_KEYWORDS' undeclared (first use in this function)
     return rb_funcallv_kw(rb_cFile, id_write, 1+n, args, RB_PASS_CALLED_KEYWORDS);
                                                          ^~~~~~~~~~~~~~~~~~~~~~~
../../../../ext/pathname/pathname.c: In function 'path_binwrite':
../../../../ext/pathname/pathname.c:450:61: error: 'RB_PASS_CALLED_KEYWORDS' undeclared (first use in this function)
     return rb_funcallv_kw(rb_cFile, id_binwrite, 1+n, args, RB_PASS_CALLED_KEYWORDS);
                                                             ^~~~~~~~~~~~~~~~~~~~~~~
../../../../ext/pathname/pathname.c: In function 'path_readlines':
../../../../ext/pathname/pathname.c:472:62: error: 'RB_PASS_CALLED_KEYWORDS' undeclared (first use in this function)
     return rb_funcallv_kw(rb_cFile, id_readlines, 1+n, args, RB_PASS_CALLED_KEYWORDS);
                                                              ^~~~~~~~~~~~~~~~~~~~~~~
../../../../ext/pathname/pathname.c: In function 'path_open':
../../../../ext/pathname/pathname.c:680:69: error: 'RB_PASS_CALLED_KEYWORDS' undeclared (first use in this function)
         return rb_block_call_kw(rb_cFile, id_open, 1+n, args, 0, 0, RB_PASS_CALLED_KEYWORDS);
                                                                     ^~~~~~~~~~~~~~~~~~~~~~~
../../../../ext/pathname/pathname.c: In function 'path_s_glob':
../../../../ext/pathname/pathname.c:1099:77: error: 'RB_PASS_CALLED_KEYWORDS' undeclared (first use in this function)
         return rb_block_call_kw(rb_cDir, id_glob, n, args, s_glob_i, klass, RB_PASS_CALLED_KEYWORDS);
                                                                             ^~~~~~~~~~~~~~~~~~~~~~~
../../../../ext/pathname/pathname.c: In function 'path_glob':
../../../../ext/pathname/pathname.c:1147:74: error: 'RB_PASS_KEYWORDS' undeclared (first use in this function)
         return rb_block_call_kw(rb_cDir, id_glob, n, args, glob_i, self, RB_PASS_KEYWORDS);
                                                                          ^~~~~~~~~~~~~~~~
../../../../ext/pathname/pathname.c:1161:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
../../../../ext/pathname/pathname.c: In function 'path_s_glob':
../../../../ext/pathname/pathname.c:1113:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
../../../../ext/pathname/pathname.c: In function 'path_open':
../../../../ext/pathname/pathname.c:685:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
../../../../ext/pathname/pathname.c: In function 'path_binwrite':
../../../../ext/pathname/pathname.c:451:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
../../../../ext/pathname/pathname.c: In function 'path_write':
../../../../ext/pathname/pathname.c:430:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
../../../../ext/pathname/pathname.c: In function 'path_readlines':
../../../../ext/pathname/pathname.c:473:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
../../../../ext/pathname/pathname.c: In function 'path_read':
../../../../ext/pathname/pathname.c:389:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
../../../../ext/pathname/pathname.c: In function 'path_each_line':
../../../../ext/pathname/pathname.c:368:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
Makefile:239: recipe for target 'pathname.o' failed
make: *** [pathname.o] Error 1
rake aborted!
Command failed with status (2): [/usr/bin/make...]
Tasks: TOP => default => compile => compile:x86_64-linux => compile:pathname:x86_64-linux => copy:pathname:x86_64-linux:2.3.8 => tmp/x86_64-linux/pathname/2.3.8/pathname.so
(See full trace by running task with --trace)

Follow 0d1b754

On ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-darwin20],
`bin/setup; rake` failed as below

```
install -c tmp/x86_64-darwin20/pathname/2.7.2/pathname.bundle lib/pathname.bundle
cp tmp/x86_64-darwin20/pathname/2.7.2/pathname.bundle tmp/x86_64-darwin20/stage/lib/pathname.bundle
Loaded suite /Users/kachick/.gem/ruby/2.7.2/gems/rake-13.0.3/lib/rake/rake_test_loader
Started
...............................................................................
...............................................................................
...............................................................................
..............................................E
===============================================================================
Error: test_ractor_shareable(TestPathnameRactor): NameError: undefined local variable or method `skip' for #<TestPathnameRactor:0x00007ff6c7a2d878>
/Users/kachick/repos/pathname/test/pathname/test_ractor.rb:7:in `setup'
===============================================================================

Finished in 0.196045 seconds.
-------------------------------------------------------------------------------
284 tests, 368 assertions, 0 failures, 1 errors, 0 pendings, 0 omissions, 0 notifications
99.6479% passed
-------------------------------------------------------------------------------
1448.65 tests/s, 1877.12 assertions/s
rake aborted!
Command failed with status (1)
/Users/kachick/.gem/ruby/2.7.2/gems/rake-13.0.3/exe/rake:27:in `<top (required)>'
Tasks: TOP => default => test
(See full trace by running task with --trace)
```
@deivid-rodriguez
Copy link
Contributor

It sounds a bit too much to me to drop support this early.

The failure on ruby 2.7 should be simple to fix by replacing skip with omit. Failures on ruby 2.6 and earlier seem like a real regression caused by 1744e33. They should be easy to fix by applying the necessary part from ruby/ruby@59c3b1c.

Finally, I think 0d1b754 is a wrong motivation to stop testing versions in CI: gemifying a standard library means it can be upgraded from older rubies as long as the gem supports them, so those should still be tested until support is officially dropped.

@hsbt
Copy link
Member

hsbt commented Mar 30, 2021

Agreed. It caused by my laziness. It should be set >= 2.6.

@kachick
Copy link
Member Author

kachick commented Mar 30, 2021

Oh, sorry I didn't know these Gemfied repositories should support which ruby versions... 😮

Hmm... @deivid-rodriguez told me so good references, thank you for the digging 🙏

So I might to try to support >= 2.6 😋
(Then can we drop 2.5? It is not yet EOL. 🤔 )

@kachick kachick changed the title Bump supported ruby version Support Ruby 2.6+ again Mar 30, 2021
@kachick kachick marked this pull request as draft March 30, 2021 01:17
@kachick kachick marked this pull request as ready for review March 30, 2021 03:03
@kachick
Copy link
Member Author

kachick commented Mar 30, 2021

I have tried it, but hmm... sorry I can not immediately resolve some of issues 🙇

First of it is 5059642, I didn't know ruby accepts integer key as a keyword arguments since ruby 2.7...(Or it is my misunderstanding? 😓 )
It used in spawn, but looks can not be used in 2.6. So just omitted..

Second issue looks CI, them I can finish running tests in my local, but CI looks fault https://github.com/ruby/pathname/pull/3/checks?check_run_id=2224125960

The error looks weird.

Error: test_split(TestPathname): TypeError: hash key 6 is not a Symbol
/home/runner/work/pathname/pathname/test/lib/core_assertions.rb:293:in `assert_separately'
/home/runner/work/pathname/pathname/test/pathname/test_pathname.rb:1071:in `test_split'
     1068:   def test_split
     1069:     assert_equal([Pathname("dirname"), Pathname("basename")], Pathname("dirname/basename").split)
     1070: 
  => 1071:     assert_separately([], <<-'end;')
     1072:       require 'pathname'
     1073: 
     1074:       mod = Module.new do
===============================================================================
................O

The alerting code is nothing in this PR... So CI might using ruby bundled one? 🤔

def test_split
assert_equal([Pathname("dirname"), Pathname("basename")], Pathname("dirname/basename").split)
end
def test_blockdev?
with_tmpchdir('rubytest-pathname') {|dir|
open("f", "w") {|f| f.write "abc" }
assert_equal(false, Pathname("f").blockdev?)
}
end

@kachick
Copy link
Member Author

kachick commented Mar 30, 2021

Ah sorry, the CI failure looks come from my changes 😅 #4
So avoid as dd54500

@deivid-rodriguez
Copy link
Contributor

Thanks for the update @kachick!

Then can we drop 2.5? It is not yet EOL. 🤔

In my opinion, if the changes here are enough to support ruby 2.5, I think it's worth keeping it. I'm pushing for the same move for net-http at ruby/net-http#19. The gemification of these libraries is tricky since many other gems already use them, I think we can make the experience a bit smoother by being slightly conservative for the first releases at rubygems.org.

But this is up to @hsbt.

@deivid-rodriguez
Copy link
Contributor

I just run into this.

My preference would be to reintroduce support for old rubies, but if this is not desired by the maintainers of this library, could we bump the required_ruby_version in the gemspec to >= 2.7 and release a new version?

Thanks!

@deivid-rodriguez
Copy link
Contributor

I created #13 to implement the alternative proposal.

@hsbt
Copy link
Member

hsbt commented Dec 23, 2021

@deivid-rodriguez Thanks.

@kachick I want to support only the stable versions. So, Ruby 2.6 will be EOL status at 2022/3. I will close this.

@hsbt hsbt closed this Dec 23, 2021
@kachick
Copy link
Member Author

kachick commented Dec 23, 2021

Thank you all! 🙇

@kachick kachick deleted the update-gemspec branch December 23, 2021 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants