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

Properly install and package bundled and default gem bits #6609

Merged
merged 5 commits into from
Mar 11, 2021

Conversation

headius
Copy link
Member

@headius headius commented Mar 11, 2021

The logic to install bundled and default gems was half broken, as described in #6604. This affected both distribution tarballs and HEAD builds since the bin scripts for default gems were either not installed at all or not included in the tarball at release time.

The following changes were made:

  • When installing gems, always install env-shebang binstubs into the main JRuby bin. Previously this was done only for bundled gems.
  • The actual exes for default gems go into specifications/default/bin rather than as partial gems under gems/.
  • All files in bin/ are included in the release distribution, rather than a whitelist.
  • The rake, rdoc, and ri scripts are now removed from bin/ since they are properly installed by the build. This aligns with master and should avoid losing the rake script when switching branches.

With these changes, I can do the following which did not work before:

  • Run the rubocop-ast specs with a clean HEAD build from the jruby-9.2 branch
  • Run the same specs with an unpacked distribution tarball

Fixes #6604.

This should be built from a clean clone, so everything under the
gem dir needs to be included for all preinstalled and default gems
to be fully functional.
* Specify JRuby bin dir and env shebang during install
* Copy default exes to appropriate location
* Include all bin stubs in dist
@headius headius added this to the JRuby 9.2.17.0 milestone Mar 11, 2021
@headius
Copy link
Member Author

headius commented Mar 11, 2021

One failure, which may indicate we need the bin scripts (sans setup and console) to be in the gems dir after all. I was unable to get bundle exec racc to work without the bin dir under specifications/default so this is unexpected to me.

@headius
Copy link
Member Author

headius commented Mar 11, 2021

The failure:

Gem.bin_path finds executables of default gems, which are the only files shipped for default gems FAILED
Expected File.exist? "/home/travis/build/jruby/jruby/lib/ruby/gems/shared/gems/racc-1.5.2-java/bin/racc"
to be truthy but was false
/home/travis/build/jruby/jruby/spec/ruby/library/rubygems/gem/bin_path_spec.rb:29:in `block in <main>'

@headius headius changed the title Properly install and dist bundled and default gem bits Properly install and package bundled and default gem bits Mar 11, 2021
@headius headius force-pushed the fix_dist_gems branch 4 times, most recently from e79285d to bc1f1b6 Compare March 11, 2021 17:34
@headius
Copy link
Member Author

headius commented Mar 11, 2021

Ok, with recent changes we are looking good.

I tested the following scenarios with bundle exec rake spec from rubocop-ast:

  • Clean clone + build of 9.2 HEAD
  • Fresh JRuby 9.2.17.0 distribution

Both succeed and are putting the right files in the right places for default gems with executables.

The problems fixed here:

  • Default gems were not installing bin stubs. This prevented them from being run at a command line or via bundle exec.
  • The dist archives were not including all contents of bin/ and were spotty about including default gem contents in the installed gems dir.

Note that this was actually broken for HEAD builds, but because we frequently install racc for other testing the racc executables and stubs were often in place (just lucky) for local testing. This PR fixes jruby-9.2 HEAD builds, and will fix jruby-head builds once merged to master (resolving #6604 and finishing incomplete work from #6416).

The diff from 9.2.16.0 tar.gz to 9.2.17.0 follows:

diff -r -q jruby-9.2.16.0/ jruby-9.2.17.0/
Only in jruby-9.2.17.0/bin: install_doc
Only in jruby-9.2.17.0/bin: install_doc.bat
Only in jruby-9.2.17.0/bin: lock_jars
Only in jruby-9.2.17.0/bin: lock_jars.bat
Only in jruby-9.2.17.0/bin: racc
Only in jruby-9.2.17.0/bin: racc.bat
Only in jruby-9.2.17.0/bin: ruby
Files jruby-9.2.16.0/lib/jruby.jar and jruby-9.2.17.0/lib/jruby.jar differ
Only in jruby-9.2.16.0/lib/ruby/gems/shared/gems: fileutils-1.1.0
Only in jruby-9.2.17.0/lib/ruby/gems/shared/gems: racc-1.5.2-java
Only in jruby-9.2.16.0/lib/ruby/gems/shared/gems: webrick-1.6.1
Files jruby-9.2.16.0/lib/ruby/stdlib/org/yaml/snakeyaml/1.26/snakeyaml-1.26.jar and jruby-9.2.17.0/lib/ruby/stdlib/org/yaml/snakeyaml/1.26/snakeyaml-1.26.jar differ
Files jruby-9.2.16.0/lib/ruby/stdlib/org/yaml/snakeyaml/maven-metadata-local.xml and jruby-9.2.17.0/lib/ruby/stdlib/org/yaml/snakeyaml/maven-metadata-local.xml differ

The relevant changes seen here:

  • The binstubs are there along with their sibling bat files.
  • The 'ruby' symlink is included. This may need testing in the zip file, since the symlink will obviously not work on Windows.
  • The fileutils and webrick default gem shells have been removed, since we now properly get the list of executables for those gems (they have no exposed executables, but ship the setup and console scripts used for gemless installation and testing in IRB).

@headius headius requested a review from enebo March 11, 2021 18:41
This file was not included in the stdlib artifact before, and by
extension not included in the dist archives build from that
artifact. To avoid trouble with this new executable being in the
tarball, we exclude it here.

[skip ci]
@headius headius merged commit d9f3585 into jruby-9.2 Mar 11, 2021
@headius headius deleted the fix_dist_gems branch March 11, 2021 19:32
@deivid-rodriguez
Copy link
Contributor

@headius Just one thing to note. The fact that webrick and fileutils ship setup and console binstubs is just a packaging mistake. These are generic binstubs provided by bundler to aid developing gems, but are not supposed to be shipped. bundle gem generates them by default in bin/, but also sets the bindir specification field to exe/. It doesn't make sense that gems would ship these since they would conflict all the time. If they were to be run by end users, how would bundle exec console know which script to run (the one from webrick or the one from fileutils, or from other gem)?

@headius
Copy link
Member Author

headius commented Mar 11, 2021

@deivid-rodriguez Ok, thank you for explaining that. I assumed they should not be there, but our old logic unconditionally copied whatever it saw in spec.bindir, which included these files for gems with no other executables. I have fixed the logic to use spec.executables and no longer see setup and console copying over.

Is this a bug in the default bundle gem generation (i.e. it is including these files in the gem when it should not), or are the libraries themselves at fault?

@deivid-rodriguez
Copy link
Contributor

I have fixed the logic to use spec.executables and no longer see setup and console copying over.

That makes sense 👍.

Is this a bug in the default bundle gem generation (i.e. it is including these files in the gem when it should not), or are the libraries themselves at fault?

It's the libraries, they should not include bin/console and bin/setup in their list of files.

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.

2 participants