Page MenuHomeFreeBSD

Mk/Uses/gem.mk: Do not throw away .so files in /ext
AbandonedPublic

Authored by swills on Jul 30 2018, 11:41 AM.

Details

Reviewers
greg_unrelenting.technology
Group Reviewers
portmgr
Ruby
Summary

security/rubygem-scrypt builds a native library directly in the 'ext' directory. This is a valid location supported by Ruby.

Currently, the whole 'ext' directory is thrown away, and for this port, the built library is nuked along with the source.

Let's only delete non-*.so files from there.

Diff Detail

Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18426
Build 18134: arc lint + arc unit

Event Timeline

Maybe the problematic ports should be fixed instead.

Yes, I was thinking along the same lines, I currently have 1668 rubygems-* ports, if only one behaves differently, maybe it should be fixed.

Not everything belongs in the framework.

linimon retitled this revision from gem.mk: Do not throw away .so files in /ext to Mk/Uses/gem.mk: Do not throw away .so files in /ext.Aug 2 2018, 9:48 PM

Maybe the problematic ports should be fixed instead.

In D16506#350391, @mat wrote:

Yes, I was thinking along the same lines, I currently have 1668 rubygems-* ports, if only one behaves differently, maybe it should be fixed.

Not everything belongs in the framework.

I spent some time looking at this and it looks like the scrypt gem is correct and we shouldn't be deleting the ext dir completely. This doesn't impact other gems because they put things in a lib dir, because they're all based on cargo-culted docs and use extconf, but scrypt uses ffi-compiler.

The removal of the ext dir was added so that we weren't packaging intermediate output. I think we are safe to continue removing the CACHE_DIR, so I don't think that line should be removed. I think what should happen instead of removing the ext dir is that we should run "find" in the ext dir and delete .o files there.

I can update the review with that change, but the question will be what effect this will have on the other gem packages and if there are other files that we don't want to package. I didn't want this list to turn into a compendium of all the things that can get left over that aren't things we want packaged. But it's impossible to police all the gems and all the bits they leave behind that we don't want to package.

Looks like I have to commandeer the revision in order to update it.

Here's my proposed updated diff. Greg, can you confirm this works for you? I'll try to evaluate if this has a negative impact on other gem packages. I don't think it will need and exp-run because worst case it should just mean we package up extra files that we don't want in a few cases.

Oh, and for reference, this was originally done in ports svn r407788.

Actually, now that I look at it more, I like your solution better. I'll do some testing and commit this.

FYI, I've submitted a patch for exp-run. Please see PR 240425.