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: Po-Chuan Hsieh
URL:
Keywords:
Depends on:
Blocks: 201763
  Show dependency tree / graph
 
Reported: 2018-07-12 13:05 UTC by Val Packett
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, Val Packett 		no flags Details | Diff
scrypt.log (2.89 KB, text/plain)
2018-07-28 23:05 UTC, Val Packett 		no flags Details
rubygems-ext-so.patch (834 bytes, patch)
2018-07-28 23:45 UTC, Val Packett 		no flags Details | Diff
scrypt.patch v2 (508 bytes, patch)
2018-07-29 00:15 UTC, Val Packett 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Val Packett 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 Po-Chuan Hsieh freebsd_committer freebsd_triage 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 Val Packett 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 Val Packett 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 Val Packett 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 Val Packett 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 freebsd_triage 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 Po-Chuan Hsieh freebsd_committer freebsd_triage 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 Val Packett 2018-07-30 11:42:04 UTC 
Okay, created https://reviews.freebsd.org/D16506