appraisal icon indicating copy to clipboard operation
appraisal copied to clipboard

Allow "ruby file:" syntax in gemfile

Open swelther opened this issue 1 year ago • 9 comments

Given in the gemfile the following ruby syntax is used:

ruby file: "../.ruby-version"

…

Appraisal would fail with wrong number of arguments (given 0, expected 1) (ArgumentError)

…/appraisal-2.5.0/lib/appraisal/bundler_dsl.rb:66:in `ruby': wrong number of arguments (given 0, expected 1) (ArgumentError)
	from …/appraisal-2.5.0/lib/appraisal/gemfile.rb:27:in `run'
	from …/appraisal-2.5.0/lib/appraisal/gemfile.rb:19:in `instance_eval'
	from …/appraisal-2.5.0/lib/appraisal/gemfile.rb:19:in `run'
	from …/appraisal-2.5.0/lib/appraisal/gemfile.rb:25:in `block in dup'
…

This PR fixes this issue by allowing keyword arguments.

There is just one "visual" downsize that the syntax in the appraisal gemfiles is converted to Ruby 1.8 syntax:

-ruby file: ".ruby-version"
+ruby {:file=>".ruby-version"}

Not sure if this is a big issue. Could be probably ignored because it still works and is just some minor visual thing.

swelther avatar Nov 14 '24 11:11 swelther

Hm seems I opened the PR too fast, in a real example the ruby line is omitted. Need to check why or if only my integration is somehow wrong :)

swelther avatar Nov 14 '24 11:11 swelther

I didn't find the already existing spec for this feature, adapted the code to work now. Just wondering why the test does not fail.

swelther avatar Nov 14 '24 13:11 swelther

Hello, we are also looking forward to see this fix merged. Is there anything that blocks this from beeing released?

McRip avatar Mar 04 '25 13:03 McRip

I just rebased to get the tests to run, but the output is a bit unfortunate. To maintain support for older Ruby versions, it might have to stay. Unless anyone can think of another approach?

nickcharlton avatar Mar 25 '25 11:03 nickcharlton

hm looks like the syntax is different depending on the Ruby version. Should I update the spec to make it pass with every version or is this a no-go?

swelther avatar Apr 01 '25 09:04 swelther

Ah yeah, interesting. I think we might just need a different approach.

I still can't think of a nicer way to do it, but we could write out a string with the value?

nickcharlton avatar Apr 01 '25 13:04 nickcharlton

Yeah sure, that could work :) I'll make some changes tomorrow 🤞

swelther avatar Apr 01 '25 14:04 swelther

Just pushed a version that could work. Not sure about the solution, I basically reimplement the ruby_version_entry method. But if I just call it the test would not fail without my fix.

What do you think?

swelther avatar Apr 02 '25 09:04 swelther

At least the test is now green :) Except for "jruby-head", wondering what is happening there 🤔

But spec/acceptance/bundle_without_spec.rb:6 is failing, but the reason does not seem to be my changes, when I comment them out it still fails locally. Any idea what do to about that one?

swelther avatar Apr 02 '25 12:04 swelther