ruby / pathname

Pathname represents the name of a file or directory on the filesystem, but not the file itself.
BSD 2-Clause "Simplified" License
26 stars 16 forks source link

Support Ruby 2.6+ again #3

Closed kachick closed 2 years ago

kachick commented 3 years ago

https://github.com/ruby/pathname/blob/a71f7d8ed28edae0db219dca7f76f6eaa74de41a/.github/workflows/test.yml#L10

This gem is testing only in Ruby 3.0+ since 0d1b754d3a32c2e26e5f9ed21a02b74489c0910a 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)
deivid-rodriguez commented 3 years ago

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 https://github.com/ruby/pathname/commit/1744e33c5df93639d0229b45dfb2e6ec1395fc14. They should be easy to fix by applying the necessary part from https://github.com/ruby/ruby/commit/59c3b1c9c843fcd2d30393791fe224e5789d1677.

Finally, I think https://github.com/ruby/pathname/commit/0d1b754d3a32c2e26e5f9ed21a02b74489c0910a 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 commented 3 years ago

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

kachick commented 3 years ago

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 commented 3 years ago

I have tried it, but hmm... sorry I can not immediately resolve some of issues ๐Ÿ™‡

First of it is https://github.com/ruby/pathname/pull/3/commits/50596427bd452293ada46b9bd9f93e59b06ab46f, 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? ๐Ÿค”

https://github.com/ruby/pathname/blob/50596427bd452293ada46b9bd9f93e59b06ab46f/test/pathname/test_pathname.rb#L1068-L1077

kachick commented 3 years ago

Ah sorry, the CI failure looks come from my changes ๐Ÿ˜… https://github.com/ruby/pathname/pull/4 So avoid as https://github.com/ruby/pathname/pull/3/commits/dd54500bad8cdc4c438bc7eb33f048936fd1cb6c

deivid-rodriguez commented 3 years ago

Thanks for the update @kachick!

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

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 https://github.com/ruby/net-http/pull/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 commented 3 years ago

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 commented 2 years ago

I created https://github.com/ruby/pathname/pull/13 to implement the alternative proposal.

hsbt commented 2 years ago

@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.

kachick commented 2 years ago

Thank you all! ๐Ÿ™‡