Skip to content

Conversation

@ioquatix
Copy link
Member

@ioquatix ioquatix commented Sep 3, 2020

No description provided.

@ioquatix ioquatix marked this pull request as draft September 3, 2020 11:18
@ioquatix ioquatix marked this pull request as ready for review September 3, 2020 11:25
@eregon
Copy link
Member

eregon commented Sep 3, 2020

For my education, can you mention why you use gems.rb instead of the far more common Gemfile in your case?

It's unclear to me if gems.rb is even supported officially by Bundler, I don't find good docs about it. I would like to limit Bundler support to whatever is needed, not add support for experimental things that might not work well in Bundler itself.
gems.rb is probably well supported in Bundler 2 though? What about Bundler 1?

@ioquatix
Copy link
Member Author

ioquatix commented Sep 3, 2020

@colby-swandale sorry to bother you again, but my understanding is gems.rb is the future? Basically, Gemfile is a specific file which contains Ruby code but doesn't end in a .rb which makes it cumbersome. I guess there are other motivations, but I don't know it. All I know is that about a year ago I was told it was the future way of doing things.

@colby-swandale
Copy link
Member

The Gemfile is still the recommendation and is not going anywhere. gems.rb is simply just an alternative option for anyone to freely use that's also supported in Bundler.

@ioquatix
Copy link
Member Author

ioquatix commented Sep 4, 2020

I was under the impression we should migrate to gems.rb - i.e. that Gemfile was kind of the old way and gems.rb was the new way - that was the advice I got a few years ago, but has that changed?

https://depfu.com/blog/2017/09/06/gemfiles-new-clothes

@colby-swandale
Copy link
Member

Ugghhhhhh this article has come back to bite me so much and I wish they would unpublish/update It. The Gemfile, Gemfile.lock was never deprecated, nor do we have any intention to deprecate it. It was an idea that we were thinking about but decided not to follow on. When I first joined the Bundler core team I accidentally recorded that idea into a draft RFC that I was putting together that documented the proposed Bundler 2 breaking changes, but this blog post took all of it and assumed everything was set in stone. 😒

@ioquatix
Copy link
Member Author

ioquatix commented Sep 4, 2020

Fair enough - where did the idea originate from and what was the original motivation?

@deivid-rodriguez
Copy link
Contributor

We wanted to pursue the gems.rb to Gemfile migration until not very long ago for the next major release. The rationale if I recall correctly was that using gems.rb makes it more clear that the Gemfile is only a file that contains ruby code, and also to remove the redundancy of the -file termination in the naming, since its obvious that a Gemfile is a file.

However, the Gemfile terminology is omnipresent in the ruby world in gems README's, blog posts, documentation, stackoverflow answers, and so on, so we realized that making this migration would only bring confusion for little benefit and decided to call it off.

The gems.rb is still perfectly supported though and I don't think it's going anywhere either.

@ioquatix
Copy link
Member Author

ioquatix commented Sep 4, 2020

Thanks for the clarification!

  • Gemfile -> Gemfile.lock
  • gems.rb -> gems.locked
  • BUNDLE_GEMFILE -> ???

What is the correct way to compute a lock path from the BUNDLE_GEMFILE environment variable?

@ioquatix
Copy link
Member Author

ioquatix commented Sep 4, 2020

I just tried it:

BUNDLE_GEMFILE=rack-v1.rb -> rack-v1.rb.lock

@ioquatix
Copy link
Member Author

ioquatix commented Sep 5, 2020

I've reworked the implementation to extract the detection of gem file and lock into a discrete step before kicking off bundle install, and feeding those paths into those functions so they behave consistently. I'll test it out with my own builds.

@ioquatix
Copy link
Member Author

ioquatix commented Sep 5, 2020

It looks to be working:

https://github.com/socketry/timers/runs/1074367338?check_suite_focus=true

Of course, that's assuming the CI tests here also test sufficiently the normal code path with Gemfile/Gemfile.lock.

@ioquatix
Copy link
Member Author

@eregon can you please review this and/or merge it?

@eregon
Copy link
Member

eregon commented Sep 19, 2020

@ioquatix What did you decide for your gems?
I think Gemfile/Gemfile.lock is used by the large majority of gems, and gems.rb is just confusing.
In fact the documentation seems to never mention gems.rb and the man page is man Gemfile: https://bundler.io/man/gemfile.5.html

What bothers me here is that almost no one uses gems.rb and it's not documented.
And that makes the documentation of ruby/setup-ruby more complicated, how do we refer to "$BUNDLE_GEMFILE or gems.rb or Gemfile" succinctly? And similar for Gemfile.lock?
This PR doesn't change documentation and I guess that's a reasonable way, because complicating the current wording for gems.rb seems really not worth it, and even the official Bundler documentation does not do it. But it's not ideal as then we have an undocumented feature.

Anyway, the diff is nice, detectGemfiles() is much cleaner, and I will merge this, thank you for the PR!

@eregon eregon merged commit 1c172ef into ruby:master Sep 19, 2020
@eregon
Copy link
Member

eregon commented Sep 19, 2020

@ioquatix
Copy link
Member Author

I didn't answer your questions.

I now prefer and use gems.rb everywhere.

I prefer it for the reasons stated - it's clear it's a "Ruby" source code file (don't need to special case it in code which processes source code/refactoring/etc) and I strongly dislike "Xyzfile" as a convention.

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.

4 participants