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

Avoid giving Pathname#relative_path_from a String for Ruby < 2.6 compat #228

Conversation

amatsuda
Copy link
Contributor

@amatsuda amatsuda commented Aug 2, 2023

rake-compiler started to fail on Ruby 2.5 since its version 1.2.4. Here's an actual failure that I saw on haml project, https://github.com/haml/haml/actions/runs/5740494510/job/15558470282#step:5:26
which fails with NoMethodError: undefined method `cleanpath' for "tmp/x86_64-linux/haml/2.5.9":String.

The error seems to be raised from this line

relative_lib_path = Pathname(lib_path).relative_path_from(tmp_path)

and it can be blamed to 0d93b08.

This happens because Pathname#relative_path_from used not to accept a String parameter until ruby/ruby#2049 (ruby/ruby@4cf8286 and ruby/ruby@78dc3da).

The CI on this repo is passing on Ruby 2.5 although it's not working, because there are no test that tries to run copy:* task, whereas there exist some tests that test the definition or dependency to that task. I'm not sure if I'd better add such a test, but for now, let me just post a minimum patch that should fix the error.

@kou
Copy link
Member

kou commented Aug 2, 2023

Thanks!
I've merged #226 instead of this because #226 submitted before this. Sorry.

@kou kou closed this Aug 2, 2023
@amatsuda amatsuda deleted the pathname_relativepath_from_string branch August 3, 2023 00:32
@amatsuda
Copy link
Contributor Author

amatsuda commented Aug 3, 2023

Oh, thank you @ivoanjo and @flavorjones! I should have checked the existing issue reports before investigating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants