Don't install non-default gems to stdlib #4083
Conversation
lib/pom.rb
Outdated
| @@ -73,8 +73,8 @@ def initialize( name, version, default_spec = true ) | |||
| # :id => 'gem-staging' ) | |||
|
|
|||
| plugin( :clean, | |||
| :filesets => [ { :directory => '${basedir}/ruby/gems/shared/specifications/default', | |||
| :includes => [ '*' ] }, | |||
| :filesets => [ { :directory => '${basedir}/ruby/gems', | |||
There was a problem hiding this comment.
Is it safe to drop the whole dir?
There was a problem hiding this comment.
it is safe, but people expect their gems to stayed installed even after a clean build. at least some people do so :)
so I would keep it as it currently is
There was a problem hiding this comment.
It couldn't be left as it is, because bundled gems will be installed to ruby/gems/shared/gems and theirs specifications will be installed to ruby/gems/shared/specifications.
Second problem with this approach is that all bundled gems need to be added here and this is easy to forget to update this list.
And I didn't understand how to handle SNAPSHOT versions in this list, because ${jruby-readline.version} is equal to 1.1.dev-SNAPSHOT, but gem path should be jruby-readline-1.1.dev-java.
Also please see #4084 so more state makes build process more unpredictable.
There was a problem hiding this comment.
if we move this extra clean into 'dist' profile, then a mvn clean install -Pdist is doing the right thing.
there is no need to add new gems in this line https://github.com/jruby/jruby/pull/4083/files#diff-a5692734fb1e233c63e88629e6f7bd12L290 - the bin files can be probably omitted here.
please see also my comments below
There was a problem hiding this comment.
If I don't add gems dir will look like:
/tmp/0 ⚡ ls jruby-9.1.3.0-SNAPSHOT/lib/ruby/gems/shared/gems/**
jruby-9.1.3.0-SNAPSHOT/lib/ruby/gems/shared/gems/rake-10.4.2:
bin/
jruby-9.1.3.0-SNAPSHOT/lib/ruby/gems/shared/gems/rdoc-4.2.0:
bin/
etehtsea
force-pushed
the
fix-stdlib-bundling
branch
from
20ad5e6
to
e1f50dc
Compare
.gitignore
Outdated
| lib/ruby/stdlib/hoe* | ||
| lib/ruby/stdlib/minitest* | ||
| lib/ruby/stdlib/power_assert* |
There was a problem hiding this comment.
don't we want to ignore all those whether they are default gems or auxiliary gems - they both will get installed during the build ?
There was a problem hiding this comment.
bundled gems will be installed to lib/ruby/gems/shared/gems/
|
Where do we stand on this? @mkristian does this seem ok? I don't think this is a release-specific thing, so I'm removing the 9.1.3.0 tag. |
|
Er, I'm adding the non-release milestone. It was not set to a milestone before. |
etehtsea
force-pushed
the
fix-stdlib-bundling
branch
from
c02fc95
to
8f4e5bb
Compare
|
@headius if we want from my side this PR is ok but it does change the |
etehtsea
force-pushed
the
fix-stdlib-bundling
branch
from
8f4e5bb
to
5e40e22
Compare
|
@mkristian updated and left clean phase intact. |
|
+1 here ... on the related note - would love a |
|
still need to look at travis as there are a few failing tests. @kares is |
|
@mkristian no as I want to be able to undo the stdlib installed files but its a specific requirement thus no blocker here. just mentioning @etehtsea's original (reverted) clean logic might be useful (in corner cases). |
|
@kares I understood that this is an extra feature. but deleting installed gems is probably not the solution, as the installed files in lib/ruby/stdlib remains. the problem here is that after switching the branch the pom.rb has no more info about the files installed there. such a feature needs to go via |
|
this has diverged quite esp. now with default and pre-installed gems in 9.2 ... so this would need a rework. |
See #4019 (comment)