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

[Experimental] Add gem exec command to run executables from gems that may or may not be installed #6309

Merged
merged 15 commits into from Feb 22, 2023

Conversation

segiddins
Copy link
Member

What was the end-user or developer problem that led to this PR?

Lack of an easy way to run executables from gems that may or may not be installed.

What is your fix for the problem, implemented in this PR?

See rubygems/rfcs#45

Make sure the following tasks are checked

@segiddins segiddins marked this pull request as ready for review February 6, 2023 06:09
Copy link
Member

@indirect indirect left a comment

Choose a reason for hiding this comment

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

This is so much faster than I expected. Nice work.

One open question: when you run gem exec rails or gem exec rspec, the actual command is coming from a dependency of the gem with the name (railties and rspec-core, respectively). When I tested, it seemed like gem exec rspec did not fetch the newest version of rspec-core, and I had to run gem exec -g rspec-core rspec to get the newest version installed and then run. Should we expect the latest version to be installed? I think maybe we should?

@segiddins
Copy link
Member Author

@indirect try again ;)

@indirect
Copy link
Member

It works! Nice job. :)

While testing it out, I noticed this:

❯ gem uninstall rspec-core
Gem 'rspec-core' is not installed
❯ gem exec -g rspec-core rspec
No examples found.
❯ gem exec -g rspec-core --conservative rspec
No examples found.
  1. So with exec gems installing to a different location that isn't managed by gem install and gem uninstall... how does the end user handle issues installing gems?

While testing out issues installing gems, I ran into this:

❯ gem exec -g tailwindcss-rails tailwindcss
[success output]

❯ gem exec -g tailwindcss-rails --platform x86_64-darwin tailwindcss
[building native extensions blah blah blah]
/Users/andre/.rubies/ruby-3.2.0/lib/ruby/site_ruby/3.2.0/rubygems/dependency.rb:314:in `to_specs': Could not find 'mini_portile2' (~> 2.8.0) among 252 total gem(s) (Gem::MissingSpecError)
Checked in 'GEM_PATH=/Users/andre/.gem/ruby/3.2.0:/Users/andre/.rubies/ruby-3.2.0/lib/ruby/gems/3.2.0' , execute `gem env` for more information
        from /Users/andre/.rubies/ruby-3.2.0/lib/ruby/site_ruby/3.2.0/rubygems/dependency.rb:326:in `to_spec'
        from /Users/andre/.rubies/ruby-3.2.0/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_gem.rb:57:in `gem'
        from extconf.rb:430:in `process_recipe'
        from extconf.rb:838:in `<main>'
  1. I think that's a bug in the --platform switch? Maybe? Bundler does install mini_portile2 as a dependency of nokogiri on x86_64-darwin.

@segiddins
Copy link
Member Author

Should be fixed now! (and the --platform option removed, because it breaks caching and honestly is a bad experience)

@indirect
Copy link
Member

Cool, thank you!

@indirect
Copy link
Member

Oh right, looking back over my previous post, I still have a question: with exec gems installing to a different location that isn't managed by gem install and gem uninstall...

  1. how does the user uninstall a gem installed with exec? is that possible?
  2. how does the end user handle gem installations that fail? do they just get the same error messages as for gem install?

@indirect
Copy link
Member

and I guess also
3. is there any html or other written documentation that needs to be updated for this change?

@segiddins
Copy link
Member Author

  1. how does the user uninstall a gem installed with exec? is that possible?
    it would be possible by setting GEM_HOME to the gem_exec gem home and then running gem uninstall
  2. yeah, should be the same error messages since we're using the same installation code as gem install is
  3. I don't think so? the command has a --help output

@indirect
Copy link
Member

I didn’t think of this until just now, but does gem exec gem uninstall foo work to remove a gem after installing it via gem exec foo? if so I think that’s a good solution to any possible questions raised by the alternate install directory.

@segiddins
Copy link
Member Author

It does not work, since gem isn't provided by any gems

❯ ruby -Ilib -S gem exec --verbose gem
running gem exec with:
           version: >= 0
        executable: gem
          gem_name: gem

HEAD https://index.rubygems.org/
200 OK
GET https://index.rubygems.org/info/gem
200 OK
ERROR:  Could not find a valid gem 'gem' (>= 0) in any repository
GET https://rubygems.org/latest_specs.4.8.gz
304 Not Modified
ERROR:  Possible alternatives: zxygemm, 42_gem, AABeginnerTestGem, AccountGem, Alonso_gem, AndyFirstGem, Backup_GEM_2, BeverageMaker, BidManagerLoggerGem, BoardGameGem

@indirect
Copy link
Member

Ok, I think I’m going to request to special case gem exec gem (sorry), because then no one has to manually set GEM_HOME to work with the exec install dir using the commands they already know.

@segiddins
Copy link
Member Author

OK, gem exec gem now works!

@indirect
Copy link
Member

Okay, this looks good to me! Going to restart the jobs that aren't passing.

@simi
Copy link
Member

simi commented Feb 14, 2023

🤔 Sadly there's gem called gem already pushed to rubygems.org (https://rubygems.org/gems/gem). That's going to conflict with gem exec gem if I understand it well.

@simi simi self-requested a review February 14, 2023 08:46
@indirect
Copy link
Member

I have good news, better news, and bad news.

The good news: the gem gem doesn't have a gemspec in it, so rubygems will never find a gem command to conflict.

The better news is that the gem was handed over to the RubyGems team (by adding Evan Phoenix as an owner). If needed, we can always yank it, but since it has no executable I don't think it matters.

The bad news is that the gem gem is a prerelease, and that made me realize that gem exec needs to support --pre to run commands from prerelease versions if those versions are newer/higher than the newest non-pre-release. So this isn't quite done. Sorry. 😬

@segiddins
Copy link
Member Author

@indirect --pre already was in there & supported! But I added some tests to cover it.

Will save on an error if it would be nil from an exception happening during that line
@segiddins
Copy link
Member Author

All builds are finally green!!

@indirect
Copy link
Member

🎉

@indirect indirect added this pull request to the merge queue Feb 22, 2023
Merged via the queue into master with commit 451ac56 Feb 22, 2023
@indirect indirect deleted the segiddins/gem-exec branch February 22, 2023 08:42
@deivid-rodriguez deivid-rodriguez changed the title Add gem exec command [Experimental] Add gem exec command to run executables from gems that may or may not be installed Mar 8, 2023
deivid-rodriguez pushed a commit that referenced this pull request Mar 8, 2023
Add gem exec command

(cherry picked from commit 451ac56)
@johnnyshields
Copy link
Contributor

Should yank/delete the gem called "gem". That seems like a (minor) security risk to me.

@martinemde
Copy link
Member

I could see yanking the gem gem or at least blocking it from download or new version updates.

@indirect
Copy link
Member

The gem called gem has been handed over to the RubyGems team. If we were to yank it, the name would immediately open up for anyone else to take over, which would create a completely new security risk that does not exist today. We're not going to yank it.

@martinemde
Copy link
Member

I submitted rubygems/rubygems.org#3569 to address this concern.

@jjb jjb mentioned this pull request May 7, 2023
3 tasks
@Mark24Code
Copy link

very helpful! 🎉

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

Successfully merging this pull request may close these issues.

None yet

7 participants