Skip to content

Commit

Permalink
When this package is installed, clean ports files unnecessary to run.
Browse files Browse the repository at this point in the history
  • Loading branch information
@knu
knu committed Aug 12, 2013
1 parent b4471b1 commit 12759cf
Showing 1 changed file with 40 additions and 0 deletions.
40 changes: 40 additions & 0 deletions ext/nokogiri/extconf.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,34 @@

ROOT = File.expand_path(File.join(File.dirname(__FILE__), '..', '..'))

if arg_config('--clean')
require 'pathname'
require 'fileutils'

root = Pathname(ROOT)
pwd = Pathname(Dir.pwd)

# Skip if this is a development work tree
unless (root + '.git').exist?
message "Cleaning files only used during build.\n"

if pwd.relative_path_from(root).fnmatch?('tmp/*')

This comment has been minimized.

Sign in to view

Copy link
@larskanis

larskanis Oct 13, 2013

Member

Why remove the tmp directory only, when it's inside another tmp directory? While gem install the build takes place within ext/nokogiri, so this deletion step is not executed, then. On the other hand, while rake compile the whole clean step is skipped.

If I comment this condition out, the deletion works as expected - while gem install but not within the development work tree. Tested with static linking on Ubuntu 13.10 with rvm and ruby 2.0.0-p247 and 1.9.3-p448.

This comment has been minimized.

Sign in to view

Copy link
@knu

knu Oct 14, 2013

Author Member

The comment below should explain it all. Note that this --clean part is executed right before install.

This comment has been minimized.

Sign in to view

Copy link
@larskanis

larskanis Oct 14, 2013

Member

Yes the --clean part is executed after the extension build but before the install. So root + 'tmp' can not be removed, but pwd + 'tmp' can be removed, regardless whether static linked or not. Line 22 prohibits deletion of pwd + 'tmp' directory for gem install, there most of the amount of data resides. So the comment above is more a bug report than a question.

This comment has been minimized.

Sign in to view

Copy link
@knu

knu Oct 14, 2013

Author Member

That is because files under "tmp" is not garbage for developers. It's essential for debugging. They also help fix-and-rebuild iteration in that they save building time.

This comment has been minimized.

Sign in to view

Copy link
@larskanis

larskanis Oct 14, 2013

Member

That's true for tmp within the development work tree. But ext/nokogiri/tmp within the gem installation tree can be and should be deleted after gem install. But it is not. That's what we are talking about regarding the installation size, don't we?

This comment has been minimized.

Sign in to view

Copy link
@knu

knu Oct 14, 2013

Author Member

I'm not following you. How can a gem installlation tree possibly have a directory named .git?

The tmp directory persists when building a binary gem built (with rake (cross) native gem), but the build gem itself does not include the tmp directory.

This comment has been minimized.

Sign in to view

Copy link
@larskanis

larskanis Oct 14, 2013

Member

The gem install tree does neither have a tmp nor a .git directory. But gem install creates a directory named "ext/nokogiri/tmp" within the gem install tree, with libxml2 and libxslt builds inside. This directory is not removed, currently.

If you run rake gem && gem inst -l pkg/nokogiri-1.6.0.gem --verbose on this branch, you should see that most of the amount of data the installed gem uses, is inside ext/nokogiri/tmp.

This comment has been minimized.

Sign in to view

Copy link
@knu

knu Oct 14, 2013

Author Member

Gem install is not designed to remove anything after installation in the first place, and there is no reason nokogiri alone should do that.
Our source files and object files are useful when investigating a bug, because most bugs are on our side, not in the upstream.

This comment has been minimized.

Sign in to view

Copy link
@knu

knu Oct 14, 2013

Author Member

Please ignore the last part, I intended to include all the source files used for build.

This comment has been minimized.

Sign in to view

Copy link
@knu

knu Oct 14, 2013

Author Member

Maybe there can be an option to remove them, if needs be.

This comment has been minimized.

Sign in to view

Copy link
@larskanis

larskanis Oct 14, 2013

Member

True, I follow you in these respects. It's only that the issue of the two people in #952 and #977 is not properly addressed, because the build directory of the upstream libraries (ext/nokogiri/tmp) is not deleted.

To be honest, it's not that I'm really interested in this point, but I did the requested testing of this branch, to bring it in a clearer state, with the wish to merge issue #976 someday.

This comment has been minimized.

Sign in to view

Copy link
@knu

knu Oct 22, 2013

Author Member

On second thought, I decided to just remove the build directory unless --disable-clean is given.
Thanks for your feedback.
# (root + 'tmp') cannot be removed at this stage because
# nokogiri.so is yet to be copied to lib.
FileUtils.rm_rf(pwd + 'tmp')
end

if enable_config('static')
# ports installation can be safely removed if statically linked.
FileUtils.rm_rf(root + 'ports')
else
FileUtils.rm_rf(root + 'ports' + 'archives')
end
end

exit
end

if defined?(RUBY_ENGINE) && RUBY_ENGINE == 'macruby'
$LIBRUBYARG_STATIC.gsub!(/-static/, '')
end
Expand Down Expand Up @@ -282,4 +310,16 @@ def process_recipe(name, version)

create_makefile('nokogiri/nokogiri')

if enable_config('clean', true)
# Do not clean if run in a development work tree.
File.open('Makefile', 'at') { |mk|
mk.print <<EOF
all: clean-ports
clean-ports: $(DLLIB)
-$(Q)$(RUBY) $(srcdir)/extconf.rb --clean --#{static_p ? 'enable' : 'disable'}-static
EOF
}
end

# :startdoc:

0 comments on commit 12759cf

Please sign in to comment.