Bug 229731 - security/rubygem-scrypt: remove ONLY_FOR_ARCHS
Summary: security/rubygem-scrypt: remove ONLY_FOR_ARCHS
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: arm64 Any
: --- Affects Only Me
Assignee: Sunpoet Po-Chuan Hsieh
URL:
Keywords:
Depends on:
Blocks: 201763
  Show dependency treegraph
 
Reported: 2018-07-12 13:05 UTC by Greg V
Modified: 2018-07-30 11:42 UTC (History)
1 user (show)

See Also:
bugzilla: maintainer-feedback? (sunpoet)


Attachments
scrypt.patch (478 bytes, patch)
2018-07-12 13:05 UTC, Greg V
no flags Details | Diff
scrypt.log (2.89 KB, text/plain)
2018-07-28 23:05 UTC, Greg V
no flags Details
rubygems-ext-so.patch (834 bytes, patch)
2018-07-28 23:45 UTC, Greg V
no flags Details | Diff
scrypt.patch v2 (508 bytes, patch)
2018-07-29 00:15 UTC, Greg V
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Greg V 2018-07-12 13:05:08 UTC
Created attachment 195082 [details]
scrypt.patch

Does not actually invoke sse flags now, builds just fine on aarch64.
Comment 1 Sunpoet Po-Chuan Hsieh freebsd_committer 2018-07-28 13:22:24 UTC
I do not have the environment to test it. Could you please provide a log file for aarch64? Thanks!
Comment 2 Greg V 2018-07-28 23:05:51 UTC
Created attachment 195559 [details]
scrypt.log

(In reply to Sunpoet Po-Chuan Hsieh from comment #1)
Hmmm, I've tried to also test that the gem actually works — well, gem.mk do-install is *deleting* the extension! Because it ends up in the 'ext' directory together with the source.

So currently this is broken on any platform, the .so is not getting installed

Without deleting 'ext', on aarch64 it also can't resolve type 'size_t'.

Also NO_ARCH shouldn't be set here.

I'll try to fix everything now
Comment 3 Greg V 2018-07-28 23:45:07 UTC
Created attachment 195563 [details]
rubygems-ext-so.patch

So something like 'gems/scrypt-3.0.5/ext/scrypt/aarch64-freebsd/libscrypt_ext.so' is a completely valid location for a native extension. https://github.com/ffi/ffi-compiler is pretty much hardcoded to use that kind of location. 

Let's update gems.mk to keep *.so files in /ext instead of removing everything!
Comment 4 Greg V 2018-07-29 00:15:44 UTC
Created attachment 195567 [details]
scrypt.patch v2

For the scrypt gem itself, patch is pretty much the same as original, plus -NO_ARCH

About the 'size_t' thing: that's in rubygem-ffi. They don't have type definitions for non-x86 FreeBSD. Simple fix:

ln -s x86_64-freebsd /usr/local/lib/ruby/gems/2.4/gems/ffi-1.9.25/lib/ffi/platform/aarch64-freebsd

(I'll try doing that in the ffi port)
Comment 5 Greg V 2018-07-29 13:54:26 UTC
So with both patches uploaded here, plus the one from bug 230147, it works on both armv7 and aarch64.
Comment 6 commit-hook freebsd_committer 2018-07-29 19:29:47 UTC
A commit references this bug:

Author: sunpoet
Date: Sun Jul 29 19:28:29 UTC 2018
New revision: 475848
URL: https://svnweb.freebsd.org/changeset/ports/475848

Log:
  Remove ONLY_FOR_ARCHS

  PR:		229731
  Submitted by:	Greg V <greg@unrelenting.technology>

Changes:
  head/security/rubygem-scrypt/Makefile
Comment 7 Sunpoet Po-Chuan Hsieh freebsd_committer 2018-07-29 20:12:04 UTC
Regarding the gem.mk patch, I would suggest having a review (https://reviews.freebsd.org/) and exp-run before it lands.
Comment 8 Greg V 2018-07-30 11:42:04 UTC
Okay, created https://reviews.freebsd.org/D16506