alphagov / tech-docs-gem

Gem to distribute the tech docs project
https://tdt-documentation.london.cloudapps.digital/
MIT License
15 stars 38 forks source link

FFI gem can cause issues for users on Mohave #254

Closed tombye closed 3 years ago

tombye commented 3 years ago

The ffi gem produces the following error for some users on Mohave (OSX 10.14.6):

dyld: lazy symbol binding failed: Symbol not found: _ffi_prep_closure_loc
  Referenced from: /Users/tombyers/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/ffi-1.15.3/lib/ffi_c.bundle
  Expected in: /usr/lib/libffi.dylib

dyld: Symbol not found: _ffi_prep_closure_loc
  Referenced from: /Users/tombyers/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/ffi-1.15.3/lib/ffi_c.bundle
  Expected in: /usr/lib/libffi.dylib

See https://github.com/ffi/ffi/issues/791 for more details.

I have personally seen in using the GDS way and Notify's tech docs. It has also been an issue on Datagovuk's tech docs.

@timblair suggested it would make sense to pin it in here instead of the downstream projects.

I'm happy to raise a PR to pin it if this seems sensible. Alternatively, I could add a note to the docs if that would be better.

m-green commented 3 years ago

The tech writing team has also been experiencing this issue for a while. @PippaClarkGDS I seem to remember the Digital Identity team had a fix, but was it a workaround?

richardTowers commented 3 years ago

For convenience, the options to workaround this from ffi/ffi#791 :

Also - if anyone's wondering what ffi is, it's the "Foreign Function Interface" library, that allows calls from ruby to native code (usually C). Why do we need it? It's a dependency of sassc (which is the C library we use to compile Sass) and rb-inotify (which middleman uses to detect file changes on linux when it's running in serve mode).


I think pinning ffi in this gem would cause more trouble than it's worth. Because this is a gem (not a project in its own right) we'd have to add an unnecessary direct dependency to ffi in the gemspec, and specify ffi < 1.13. This would force all downstream projects to use ffi 1.12, but if they depend on other gems that specify ffi > 1.12 (or similar) we'd cause a version conflict.

I think it should be up to the individual projects (e.g. gds-way, govuk-developer-docs) to decide whether to pin ffi.

timblair commented 3 years ago

I don't think we shouldn't be pinning this:

I agree with Richard that it should be left up to individual projects on whether to pin (which I also don't think they should, but that's another matter), but it's probably worth adding something to the README in this gem with the suggested fixes if someone does come up against the issue.

richardTowers commented 3 years ago

In an extremely roundabout way we could fix this by replacing the dependency on Sprockets, and using a Middleman "external pipeline" to build the JS / CSS using webpack or something. We already depend on node to install govuk-frontend. In for a penny, in for a pound.

This would remove the need for us to call ffi for anything (it would still need to be installed, through rb-inotify, but I don't think installing it is a problem, it's only when you call it it blows up).

tombye commented 3 years ago

I've put up a PR to add something to the README.

Worth noting that the Sass project uses Dart-sass rather than sassc now, so we'll have to move away from sassc at some point anyway but I'd guess the work involved is non-trivial. I'm thinking of things like building asset paths, adding SHA hashes to them and how sprockets integrates into the asset pipeline of the consuming app* The actual building of those assets could just be done with NPM scripts.

*but all this may be wrong as I've not been in rails land for years now.

m-green commented 3 years ago

Documentation has been updated, so closing this issue now. Let me know if I should reopen it.