Ruby version should be read from Gemfile.lock by default

[benlangfeld]

Similarly to #38, the version of Ruby to be installed can be specified in Gemfile.lock via RUBY VERSION directive. This should be used the same way .ruby-version and .tool-versions are, for applications which lock their Ruby version using the Gemfile.lock, so as to avoid duplication and possible divergence.

Note to self, relevant functions:

• setup-ruby/index.js

  Lines 108 to 141 in 1198b07

  	function parseRubyEngineAndVersion(rubyVersion) {
  	if (rubyVersion === 'default') {
  	if (fs.existsSync('.ruby-version')) {
  	rubyVersion = '.ruby-version'
  	} else if (fs.existsSync('.tool-versions')) {
  	rubyVersion = '.tool-versions'
  	} else {
  	throw new Error('input ruby-version needs to be specified if no .ruby-version or .tool-versions file exists')
  	}
  	}
  	if (rubyVersion === '.ruby-version') { // Read from .ruby-version
  	rubyVersion = fs.readFileSync('.ruby-version', 'utf8').trim()
  	console.log(`Using ${rubyVersion} as input from file .ruby-version`)
  	} else if (rubyVersion === '.tool-versions') { // Read from .tool-versions
  	const toolVersions = fs.readFileSync('.tool-versions', 'utf8').trim()
  	const rubyLine = toolVersions.split(/\r?\n/).filter(e => /^ruby\s/.test(e))[0]
  	rubyVersion = rubyLine.match(/^ruby\s+(.+)$/)[1]
  	console.log(`Using ${rubyVersion} as input from file .tool-versions`)
  	}
  	let engine, version
  	if (/^(\d+)/.test(rubyVersion) || common.isHeadVersion(rubyVersion)) { // X.Y.Z => ruby-X.Y.Z
  	engine = 'ruby'
  	version = rubyVersion
  	} else if (!rubyVersion.includes('-')) { // myruby -> myruby-stableVersion
  	engine = rubyVersion
  	version = '' // Let the logic in validateRubyEngineAndVersion() find the version
  	} else { // engine-X.Y.Z
  	[engine, version] = common.partition(rubyVersion, '-')
  	}
  	return [engine, version]
  	}

• setup-ruby/bundler.js

  Lines 30 to 49 in 1198b07

  	function readBundledWithFromGemfileLock(lockFile) {
  	if (lockFile !== null && fs.existsSync(lockFile)) {
  	const contents = fs.readFileSync(lockFile, 'utf8')
  	const lines = contents.split(/\r?\n/)
  	const bundledWithLine = lines.findIndex(line => /^BUNDLED WITH$/.test(line.trim()))
  	if (bundledWithLine !== -1) {
  	const nextLine = lines[bundledWithLine+1]
  	if (nextLine) {
  	const bundlerVersion = nextLine.trim()
  	if (isValidBundlerVersion(bundlerVersion)) {
  	console.log(`Using Bundler ${bundlerVersion} from ${lockFile} BUNDLED WITH ${bundlerVersion}`)
  	return bundlerVersion
  	} else {
  	console.log(`Could not parse BUNDLED WITH version as a valid Bundler release, ignoring it: ${bundlerVersion}`)
  	}
  	}
  	}
  	}
  	return null
  	}

[eregon]

That sounds reasonable, I think the Gemfile.lock should be checked last, i.e. the other 2 files should have precedence.
PR welcome.

[benlangfeld]

That sounds reasonable, I think the Gemfile.lock should be checked last, i.e. the other 2 files should have precedence.

Sure :)

PR welcome.

Yep, will prep one today or next week :)

[eregon]

TBH considering the time spent to review the PR, I wonder whether the feature is even worth it at all.

Gemfile supports ruby file: ".ruby-version" so this can deduplicate the information: https://bundler.io/v2.5/man/gemfile.5.html#VERSION-required-
And that has the advantage a Ruby switcher can also automatically use the right version, no Ruby switcher looks in Gemfile/Gemfile.lock for the ruby version AFAIK.

[benlangfeld]

TBH considering the time spent to review #592, I wonder whether the feature is even worth it at all.

Great way to motivate contributors.

[eregon]

I am sorry it went this way, to be clear I did not close the PR and I am not against it, but we should discuss the merit of this feature based on the the second paragraph in #590 (comment) first. I didn't realize that before, otherwise of course I would have brought it up before the PR.

I think you should also consider the maintainers point of view and how much time I spent on this.
I do not want endless discussions about preference of whitespace trimming and what not, especially when it is actually intended and significant.
My POV is you act like you know better about a bunch of things but I wrote like 90% of this project, so I think I know better and I have more context about this project, and yet in the review you force me to explain things in lots of details (a big time investment). That is frustrating, and I think contributors should be mindful of the time investment of maintainers and try to minimize it.

[benlangfeld]

I didn't force you to go into great detail on anything, I followed your request to split two different issues with this project out into distinct PRs where you could choose to engage with them or not, and you opted to be rude and dismissive because someone dared to have an opinion that differs from yours.

Thank you for your efforts and have a good day.