Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot use R or Python in release builds due to libsodium (macOS only) #154

Closed
jmcphers opened this issue Feb 9, 2023 · 8 comments · Fixed by #155
Closed

Cannot use R or Python in release builds due to libsodium (macOS only) #154

jmcphers opened this issue Feb 9, 2023 · 8 comments · Fixed by #155
Labels
bug Something isn't working
Milestone

Comments

@jmcphers
Copy link
Collaborator

jmcphers commented Feb 9, 2023

Currently the Jupyter Adapter cannot start in release builds because it includes a hardcoded reference to libsodium at the path used in the build machine.

'/Users/user229818/homebrew/opt/libsodium/lib/libsodium.23.dylib' (no such file),
'/System/Volumes/Preboot/Cryptexes/OS/Users/user229818/homebrew/opt/libsodium/lib/libsodium.23.dylib' (no such file), 
'/Users/user229818/homebrew/opt/libsodium/lib/libsodium.23.dylib' (no such file), 
'/usr/local/lib/libsodium.23.dylib' (no such file), 
'/usr/lib/libsodium.23.dylib' (no such file, not in dyld cache).

Possible solutions:

  • figure out why we suddenly depend on libsodium (this wasn't the case even a few weeks ago) and see if we can avoid the dependency
  • provide a way to bind to the system version of libsodium at runtime -- probably not great unless we can convince ourselves almost all users will already have it
  • bundle libsodium and rewrite the rpath to link to the bundled copy (RStudio does this with libssl)
@jmcphers jmcphers added the bug Something isn't working label Feb 9, 2023
@softwarenerd
Copy link
Contributor

softwarenerd commented Feb 9, 2023

I can start investigating this later.

@kevinushey
Copy link
Contributor

That's presumedly coming from here, in zeromq:

https://github.com/zeromq/zeromq.js/blob/47bb35c1941cb4fa8b510fb7da4d40b37cfa2e5f/binding.gyp#L82-L87

It's not clear to me why we're trying to link to both a static and dynamic copy of libzmq? That shouldn't be necessary?

@kevinushey
Copy link
Contributor

Ah, I think I know what's going on...

$ pkg-config libsodium --libs
-L/opt/homebrew/Cellar/libsodium/1.0.18_1/lib -lsodium

But that folder has:

$ ll /opt/homebrew/Cellar/libsodium/1.0.18_1/lib
total 1176
drwxr-xr-x   6 kevin  admin   192B Feb  9 15:47 ./
drwxr-xr-x  10 kevin  admin   320B Feb  6 16:15 ../
-r--r--r--   1 kevin  admin   247K Feb  6 16:15 libsodium.23.dylib
-r--r--r--   1 kevin  admin   340K May 30  2019 libsodium.a
lrwxr-xr-x   1 kevin  admin    18B May 30  2019 libsodium.dylib@ -> libsodium.23.dylib
drwxr-xr-x   3 kevin  admin    96B Feb  6 16:15 pkgconfig/

And when linking, because both dynamic and static libraries are available, the linker will prefer the dynamic library.

If you really want to link with a static library, it's better to just provide the path to the static library directly.

@kevinushey
Copy link
Contributor

I've filed an issue upstream at zeromq/zeromq.js#557.

@jmcphers
Copy link
Collaborator Author

image

Here's a screenshot if it's helpful.

@jmcphers
Copy link
Collaborator Author

Also, here's the ugly install_name_tool incantation we use in RStudio to rewire libssl to a bundled copy on macOS, in case we need to use a similar technique to circumvent the upstream issue.

https://github.com/rstudio/rstudio/blob/4f7063b13bb0bae8377639a36d3ff4b7626de6e3/src/cpp/desktop/CMakeLists.txt#L542-L548

@kevinushey
Copy link
Contributor

I think I have something figured out:

kevin@MBP-P2MQ:~/positron/extensions/jupyter-adapter [feature/zeromq-local-module]
$ otool -L node_modules/zeromq/build/Release/zeromq.node
node_modules/zeromq/build/Release/zeromq.node:
        /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1300.36.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1319.0.0)
kevin@MBP-P2MQ:~/positron/extensions/jupyter-adapter [feature/zeromq-local-module]
$ nm node_modules/zeromq/build/Release/zeromq.node | grep sodium
0000000000082944 t __sodium_alloc_init
00000000000827e4 t __sodium_dummy_symbol_to_prevent_compare_lto
0000000000082770 t __sodium_dummy_symbol_to_prevent_memcmp_lto
00000000000a89d0 d __sodium_lock
0000000000082678 t __sodium_runtime_get_cpu_features
00000000000828ec T _sodium_add
0000000000082b20 T _sodium_allocarray
00000000000827e8 T _sodium_compare
00000000000825a8 t _sodium_crit_enter
0000000000083e34 t _sodium_crit_enter.cold.1
0000000000082b5c T _sodium_free
00000000000828c8 T _sodium_increment
00000000000824f8 T _sodium_init
0000000000082890 T _sodium_is_zero
00000000000829c4 T _sodium_malloc
0000000000083e5c t _sodium_malloc.cold.1
0000000000082774 T _sodium_memcmp
00000000000826d8 T _sodium_memzero
00000000000825e0 T _sodium_misuse
0000000000082994 T _sodium_mlock
0000000000082c28 T _sodium_mprotect_noaccess
0000000000082c60 T _sodium_mprotect_readonly
0000000000082c98 T _sodium_mprotect_readwrite
0000000000082998 T _sodium_munlock
0000000000082cd0 T _sodium_pad
00000000000826c8 T _sodium_runtime_has_aesni
00000000000826a8 T _sodium_runtime_has_avx
00000000000826b0 T _sodium_runtime_has_avx2
00000000000826b8 T _sodium_runtime_has_avx512f
0000000000082680 T _sodium_runtime_has_neon
00000000000826c0 T _sodium_runtime_has_pclmul
00000000000826d0 T _sodium_runtime_has_rdrand
0000000000082688 T _sodium_runtime_has_sse2
0000000000082690 T _sodium_runtime_has_sse3
00000000000826a0 T _sodium_runtime_has_sse41
0000000000082698 T _sodium_runtime_has_ssse3
0000000000082620 T _sodium_set_misuse_handler
0000000000082700 T _sodium_stackzero
0000000000082918 T _sodium_sub
0000000000082d94 T _sodium_unpad

@kevinushey
Copy link
Contributor

Well, I thought I did! I had an idea -- let's use git submodules, with some local patches! And then:

$ git commit
Found '/Users/kevin/positron/.nvmrc' with version <16.14>
Now using node v16.14.2 (npm v8.5.0)

> husky - npm run -s precommit
> husky - node v16.14.2

Reading git index versions...

Error: Command failed: git show ':extensions/jupyter-adapter/modules/zeromq.js'
fatal: bad object :extensions/jupyter-adapter/modules/zeromq.js

    at ChildProcess.exithandler (node:child_process:399:12)
    at ChildProcess.emit (node:events:526:28)
    at maybeClose (node:internal/child_process:1092:16)
    at Socket.<anonymous> (node:internal/child_process:451:11)
    at Socket.emit (node:events:526:28)
    at Pipe.<anonymous> (node:net:687:12) {
  killed: false,
  code: 128,
  signal: null,
  cmd: "git show ':extensions/jupyter-adapter/modules/zeromq.js'"
}

This is after already running into other random pieces of submodule friction as well. 😵

@petetronic petetronic added this to the Private Alpha milestone Aug 26, 2023
wesm pushed a commit that referenced this issue Mar 28, 2024
Merge pull request #154 from posit-dev/shift-enter


--------------------
Commit message for posit-dev/positron-python@577d59d:

remove shift+enter keybinding


Authored-by: Isabel Zimmerman <[email protected]>
Signed-off-by: Isabel Zimmerman <[email protected]>
wesm pushed a commit that referenced this issue Mar 28, 2024
Merge pull request #154 from posit-dev/shift-enter


--------------------
Commit message for posit-dev/positron-python@577d59d:

remove shift+enter keybinding


Authored-by: Isabel Zimmerman <[email protected]>
Signed-off-by: Isabel Zimmerman <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants