ext
AbandonedPublic
Actions

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

Details

Reviewers

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

Lint Skipped

Unit

Tests Skipped

Event Timeline

val_packett.cool created this revision.Jul 30 2018, 11:41 AM

Herald added a reviewer: portmgr. · View Herald TranscriptJul 30 2018, 11:41 AM

Herald added subscribers: Contributor Reviewers (ports), mat. · View Herald Transcript

Harbormaster completed remote builds in B18426: Diff 46014.Jul 30 2018, 11:41 AM

mat added a reviewer: Ruby.Jul 30 2018, 1:10 PM

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

In D16506#350386, @antoine wrote:

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.

swills abandoned this revision.Sep 10 2019, 11:33 AM