-
Notifications
You must be signed in to change notification settings - Fork 18
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
Intstall fails on SmartOS due to pynacl dependency install failure #55
Comments
Workaround for this:
|
It seems this may be more pernicious than originally thought. After successfully installing via
I tried installing pynacl with and without the
|
@rjloura Make sure I understand. Current python-manta is basically broken on SmartOS? |
As I understand it, the build fails on SmartOS (I tried on 17.4.0) when pip installing pynacl, which in term builds libsodium. I see that pkgsrc has libsodium-1.0.16 so presumably the tests passed when pkgsrc built it. If I download the 1.0.16 tarball from https://download.libsodium.org/libsodium/releases/, |
Regarding For the main breakage, recall that pynacl is a wrapper around libsodium. Above I pointed out that if you just download the libsodium it appear to work, so it was unclear what was different when pip did it's thing. I think the answer is https://github.com/pyca/pynacl/blob/master/setup.py#L161: # Run ./configure
subprocess.check_call(
[
configure, "--disable-shared", "--enable-static",
"--disable-debug", "--disable-dependency-tracking",
"--with-pic", "--prefix", os.path.abspath(self.build_clib),
],
cwd=build_temp, If I run that on the same system I was testing with, then
From some trial and error the combination of both The steps to reproduce are along the line of:
I tried it on a few different JPC instances and got:
I'm not sure if this points to a bug in the kernel or something provided by pkgsrc, or what the right place is to look next (cc @jperkin). |
The problem seems to be that thread-local variables from static libraries are not properly initialized in SmartOS.
static TLS Salsa20Random stream = { ... }; (On SmartOS, When the library is statically linked, A workaround is to compile the library without threads |
Just for the record - in case of libsodium-1.0.16 and libsodium-1.0.17 |
The compiler outputs an At first glance, it looks like I think this is because we expect the "main" TLS relocation to have dealt with this for us, as we would in the case of |
If I can trust the Applied to the illumos link-editor, makes things work. What this does is transition the 'call' as we would have with a Running
Looks reasonable for correctness based on my very limited understanding of the code. A more adequate check for correctness -- adding CTF to the salsa20 rdrand object and checking the hrtime at the end of the struct, also looks ok. I don't have a build environment that allows me to build a broken system (I'm using an object from @jasonbking) so I can't reasonably test this further. I'd appreciate if somebody who could, would rebuild |
I was able to recreate the problem (and sent @richlowe the resulting binary). Using an ld with his patch, I do not see the failure anymore. |
What were the steps to test the patch? Is it possible to update ld only in one particular zone without touching GZ? Is there any plan to include the patch in the newest release? Thanks! |
The local dynamic TLS fix was integrated in commit 096c97d62be876a03a0a8cdb0a540e9c84ec509f which was merged into illumos-joyent this morning. If you build your own SmartOS-live image, you can try that. Otherwise the fix should appear in the release that should be out around Feb 13th. |
Just tried to build on: |
Unfortunately, this appears to be a different bug. The earlier bug was something that was just present in 32-bit. |
This is indeed a similar but different issue. The TLS transition related corruption here causes any write to members of |
The amd64 bug is occurring because the link-editor erroneously 0's the addend when transitioning from LD (dtpoff) to LE (tpoff), and thus the structure offsets emitted by the compiler in the form
to get at |
Thanks for your comments and interest. Should I open another issue to track that? |
I filed illumos bug: #10471 "ld(1) amd64 LD->LE TLS transition causes memory corruption" I don't know whether the smartos folks would like a second bug filed or not. |
This bug for 64-bit builds was filed as illumos#10471 |
Since we merge w/ illumos-gate M-F, once it lands in illumos-gate, it should land in illumos-joyent shortly thereafter (and then in the following bi-weekly release). |
Okay. Thanks, will wait patiently. |
I tried the above commands on the latest release (20190328T010321Z), and pynacl as well as ansible in general both build without an error now. I suspect the previous release will also work (I believe it also has the fix), though I didn't have a chance to try to build on that. |
Hi @jasonbking. I confirm. Works like a charm. Thanks a lot! |
Since the issue is resolved, I'm going to go ahead and close this now. |
There is an issue with running the test suite for the libsodium that is bundled with pynacl. So during an install of python-manta on SmartOS you will see a build failure that looks something like this:
The text was updated successfully, but these errors were encountered: