Test cases: Unwanted RPATH overwrite and Failing to patch internal non-py extensions

[thomaslima]

UPDATE: This PR was modified to only include the test cases that @ehashman has requested.

Hi,

Our wheel has a complicated dependency structure. But the gist of it is the following:

dbcore.so (py_ext) -> lib_tl.so (non-py_ext) -> libcurl.so (external library to be grafted)

Because lib_tl is a non-py extension of dbcore, it was not added to the full_external_refs dictionary, which is used by repair.py to patch the sonames. So the final result is that lib_tl was not patched, and the package was broken.

• Change 1: I added this case to the full_external_refs dictionary.

In addition, dbcore already had an RPATH set to $ORIGIN. So because of its dependency to libcurl.so, patchelf_set_rpath replaced it to $ORIGIN/.libs in repair.py, even though it was not necessary. This way, it could no longer find lib_tl.so.

• Change 2: Instead of overwriting RPATH, I concatenate it: $OLD_RPATH:$ORIGIN/.libs

I am attaching logs so you can verify it works for my wheel. The master version, instead of repairing it to manylinux1, breaks it even further. The patched version properly repairs it to manylinux1.

Please find the wheel in this link:
https://www.dropbox.com/s/3muwwe8d92pv78p/klayout-0.26.0.dev8-cp35-cp35m-linux_x86_64.whl?dl=0

I would add it to the test but it has 86MB.
logs-before.zip
logs-after.zip

[ehashman]

At a first quick readthrough, this seems totally plausible to me, but I'd definitely like to see a test case. Can you add a minimal one?

[ehashman]

I went ahead and filed some bugs to correspond to each of the issues and fixes reported here. If it's not too much trouble, I'd really appreciate it if you split the fixes into two separate PRs with targeted tests :) I am happy to help with the tests if that is at all useful.

Thanks so much for your contribution to auditwheel!

[daa]

Should note that old RPATH should be kept with caution - it must not point to locations from which auditwheel gathered libraries and it must not point to any location outside of built wheel files.

[thomaslima]

@ehashman Thank you! I can work on this later this week. I would love help with the tests. Our project is not suitable for making a test because it is too large. I need two artificial extensions that would replace dbcore.so and lib_tl.so. Maybe there should be some already in the test suite.

[thomaslima]

@daa These checks seem much more complicated to implement than what I envisioned. It seems indeed that auditwheel looks into the rpaths of each extension when looking for libraries, but then doesn't keep track of whether it used them or not – it nonetheless overwrites the rpath when there's is a dependency. This change (not pointing to old locations) would require modifying the logic of repair_wheel, and I would probably need help from a core dev.

The second change, however, seems much simpler to me. We can take out anything that is not a subdirectory of root.

Going forward, I will go ahead and try to implement your second suggestion, and reference the first as a comment in my PR, or maybe we can make a new issue for others to look at.

[thomaslima]

Hello @ehashman

I worked on making a minimal wheel called hello, which depends on a non-py ext called testzlib.h, which depends on zlib. Inspired by the test_manylinux.py test, I copied a few lines of code to test issues #136 and #137. Here, the tests are failing, but will succeed if PRs #139 and #140 are accepted. Because of the structure of this wheel, #140 is going to fail unless #139 is also accepted.

For me, these two fixes are connected, but if you can think of a realistic test case which makes them independent, feel free to add it.

[auvipy]

could you push all related commit to this branch?

[lkollar]

This PR creates tests in the wrong directory, directly under tests. We moved all integration tests under tests/integration. The Travis build is failing as well.

Do you want to revert this @auvipy?

[auvipy]

reverted