search

janet-lang / janet

A dynamic language and bytecode vm
https://janet-lang.org
MIT License
3.51k stars 227 forks source link

Avoid removing too many symbols with strip on macOS #1209

Closed pyrmont closed 1 year ago

pyrmont commented 1 year ago

Janet's Makefile calls strip on make install to remove certain symbols. This causes macOS to be unable to dynamically load shared objects at runtime. This PR adds the flags -x and -S which fixes this problem.

Discussion

As part of #1140, @zevv added a call to strip to remove certain symbols when running make install. This does not cause an issue on Linux (and presumably other POSIX systems) but prevents Janet from loading shared object files dynamically on macOS.

A simple example is Janet's JSON project. When the test suite is run on macOS 13.4.1, the output is:

$ jpm -l test
running test/suite0.janet ...
error: could not load native build/json.so: dlopen(build/json.so, 0x0002): symbol not found in flat namespace '_janet_arity'
  in native [src/core/corelib.c] on line 294
  in native-loader [boot.janet] on line 2966, column 40
  in require-1 [boot.janet] on line 2989, column 18
  in import* [boot.janet] on line 3020, column 15
  in _thunk [test/suite0.janet] (tailcall) on line 1, column 1
non-zero exit code in test/suite0.janet: 1
Failing test scripts: 1

The changes in this PR fix this problem.

Alternatives

The addition of the call to strip was not the primary purpose of #1140 and I have guessed that the objective of calling strip was to remove debug symbols. I confess I am not very familiar with this aspect of compilation and so am not sure if the flags used in this PR are appropriate. More information about the flags is available in Apple's man page (I can't find an up to date web version of the macOS man pages so this is the link to the current version in Apple's open source repository on GitHub). A copy of the Linux man page is also viewable.

If there's a more appropriate combination of flags, please let me know.

zevv commented 1 year ago

right, I admit I'm pretty much Linux-centric and had not considered or (or expected) issues with other OSes. Removing the strip makes sense, maybe we can come up with some tests for CI to make sure this does not break again if someone gets the same idea some time in the future.