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

Enable use of Bundler v2 and Migrate to Licensed v2 #4934

Merged
merged 6 commits into from
Jul 29, 2020

Conversation

sheerun
Copy link
Contributor

@sheerun sheerun commented Jul 26, 2020

This PR updates bundler to latest version (licensed already supports Bundler 2.x)

I, @lildude, have extended on this PR, thanks @sheerun 🙇

The switch to Bundler 2 pulls in the latest version of Licensed which now has support for Git submodules. This means we no longer need to maintain our own method of caching the grammar licenses.

With this update, I've taken this opportunity to migrate our cached licenses to the new yaml format and location natively supported by Licensed v2 and have removed our old script in favour of calling licensed directly.

@lildude
Copy link
Member

lildude commented Jul 27, 2020

Ooo. I missed Licensed now uses Bundler 2. I also see it's come on a long way since we implemented it and I think we can now get rid of our custom license caching etc. We need to make changes to get the test passing any way.

Do you mind if I push the license-related changes to this PR?

@sheerun
Copy link
Contributor Author

sheerun commented Jul 27, 2020

not at all :)

@lildude lildude changed the title Update bundler Update Bundler and Migrate to Licensed v2 Jul 28, 2020
@lildude
Copy link
Member

lildude commented Jul 28, 2020

Thanks @sheerun. I've pushed my changes and updated the title and OP.

@Alhadis & @pchaigno this PR is massive only because of the migration of all the license files from vendor/licenses/grammar/*.txt to vendor/licenses/git_submodule/*.dep.yml. The other changes are quite small.

@lildude lildude requested review from pchaigno and Alhadis July 28, 2020 09:17
@Alhadis
Copy link
Collaborator

Alhadis commented Jul 28, 2020

I'll try it out soon-ish. Could you offer your thoughts on this in the meantime? I'd prefer to know sooner rather than later whether my suggested approach will suffice, or a different one is needed. 😉

Copy link
Collaborator

@Alhadis Alhadis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unable to reliably test this locally (it installed Bundler 1.x anyway), so I'm blindly assuming it works as intended. 😉

@lildude
Copy link
Member

lildude commented Jul 28, 2020

Unable to reliably test this locally (it installed Bundler 1.x anyway), so I'm blindly assuming it works as intended. 😉

It shouldn't be installing Bundler as you can't use Bundler to install Bundler 😉 . It should use the version of Bundler already installed, provided it's any version 1.10 or later - this is the primary change. So if you've got 1.17.2 installed, it'll use that:

$ gem list bundler

*** LOCAL GEMS ***

bundler (default: 1.17.2)
$ script/bootstrap
You are replacing the current local value of path, which is currently nil
Fetching gem metadata from https://rubygems.org/.......
Fetching gem metadata from https://rubygems.org/.
Resolving dependencies...
Fetching rake 13.0.1
Installing rake 13.0.1
Fetching public_suffix 4.0.5
Installing public_suffix 4.0.5
Fetching addressable 2.7.0
Installing addressable 2.7.0
Using bundler 1.17.2   <------ 👀 
Fetching byebug 11.1.3
Installing byebug 11.1.3 with native extensions
[...]

If you've got 2.1.4 or both, it'll use the newer:

$ gem install bundler -v 2.1.4
Fetching bundler-2.1.4.gem
Successfully installed bundler-2.1.4
1 gem installed
$ gem list bundler

*** LOCAL GEMS ***

bundler (2.1.4, default: 1.17.2)
$ script/bootstrap
Fetching gem metadata from https://rubygems.org/.......
Fetching gem metadata from https://rubygems.org/.
Resolving dependencies...
Fetching rake 13.0.1
Installing rake 13.0.1
Fetching public_suffix 4.0.5
Installing public_suffix 4.0.5
Fetching addressable 2.7.0
Installing addressable 2.7.0
Using bundler 2.1.4   <------ 👀 
Fetching byebug 11.1.3
Installing byebug 11.1.3 with native extensions
[...]

If we switch back to master with both versions installed:

$ git checkout master
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
$ gem list bundler

*** LOCAL GEMS ***

bundler (2.1.4, default: 1.17.2)
$ script/bootstrap
Fetching gem metadata from https://rubygems.org/.......
Fetching gem metadata from https://rubygems.org/.
Resolving dependencies...
Bundler could not find compatible versions for gem "bundler":
  In Gemfile:
    bundler (~> 1.10)

  Current Bundler version:
    bundler (2.1.4)
This Gemfile requires a different version of Bundler.
Perhaps you need to update Bundler by running `gem install bundler`?

Could not find gem 'bundler (~> 1.10)' in any of the relevant sources:
  the local ruby installation
$

Ultimately, this is the big change:

-  s.add_development_dependency 'bundler', '~> 1.10'
+  s.add_development_dependency 'bundler', '>= 1.10'

I'll update the title to better reflect this.

@lildude lildude changed the title Update Bundler and Migrate to Licensed v2 Enable use of Bundler v2 and Migrate to Licensed v2 Jul 28, 2020
@lildude lildude merged commit 96ad118 into github-linguist:master Jul 29, 2020
sambacha pushed a commit to freight-trust/linguist that referenced this pull request Sep 4, 2020
…4934)

* Update bundler

* Update licensed config

* Migrate licenses for licensed v2

* Call licensed instead of our script

* Remove licensed script

This isn't needed anymore as licensed now has native support for sub-modules

Co-authored-by: Colin Seymour <[email protected]>
@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants