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

remove .so files when copying source into the container #139

Closed
samuelcolvin opened this issue May 25, 2019 · 15 comments · Fixed by #263
Closed

remove .so files when copying source into the container #139

samuelcolvin opened this issue May 25, 2019 · 15 comments · Fixed by #263

Comments

@samuelcolvin
Copy link

Trying to build wheels on travis and I'm getting the following error:

auditwheel: error: cannot repair "/tmp/built_wheel/pydantic-0.27a1-cp37-cp37m-linux_x86_64.whl" to "manylinux1_x86_64" ABI because of the presence of too-recent versioned symbols. You'll need to compile the wheel on an older toolchain.

See this build for details.

I don't get this error when building locally with cibuildwheel --platform linux --output-dir dist.

Any idea what I'm doing wrong?

@samuelcolvin
Copy link
Author

possibly the same as pypa/auditwheel#36 (comment) since the first image build goes fine, it's the second that fails.

@samuelcolvin samuelcolvin changed the title auditwheel fails for 3.6 on travis but not locally auditwheel fails on travis but not locally May 25, 2019
@samuelcolvin
Copy link
Author

sorted this at last, found the answer at pypa/manylinux#214 (comment) - I was building the package on the host before running cibuildwheel.

cibuildwheel is copying the .so files along with the source into the container thus the problem.

cibuildwheel should exclude .so (and perhaps .c?) files when copying the source into the contianer.

@samuelcolvin samuelcolvin changed the title auditwheel fails on travis but not locally remove .so files when copying source into the container May 26, 2019
@joerick
Copy link
Contributor

joerick commented May 26, 2019

Ah, thanks for tracking this down. Maybe a simple way around this would be to delete the 'build' directory inside the docker container before each build? Anything in that folder can be regenerated as part of the build, as I understand it.

@samuelcolvin
Copy link
Author

samuelcolvin commented May 26, 2019

That wouldn't help in my case, during testing I install/build with pip install -e . which means lots of *.so files along side *.py files.

Ideally cibuildwheel would either:

  • delete all .so files in src before copying (with a warning)
  • or better, filter out such files when copying with some kind of glob,
  • or delete .so files inside the container before building
  • or just fail with a clear error message if such files exist in the source directory
  • or document this potential pitfall

Or some combination of the above. I'm afraid I can't be much help on this at the moment - I'm already spending far too much time on open source stuff, but thanks a lot for cibuildwheel, even with this problem it's saved me a lot of time.

@YannickJadoul
Copy link
Member

YannickJadoul commented May 26, 2019

cibuildwheel is copying the .so files along with the source into the container thus the problem.

cibuildwheel should exclude .so (and perhaps .c?) files when copying the source into the contianer.

Maybe I don't fully understand the problem, but this seems like a problem on the user-side of cibuildwheel? cibuildwheel just makes sure the builds inside the container have access to all files in the project folder passed at invocation. It feels rather dangerous to start making assumptions and exclude certain types of files, if you ask me.

Correct me if I'm wrong, but the actual problem seems to be that the project is built twice? Which is in principle not a property of cibuildwheel, but a consequence of how it's used in the CI config?

Is there a possibility to clean up the build? Or maybe to run cibuildwheel in a separate job, such that your builds do not conflict?

@samuelcolvin
Copy link
Author

Is there a possibility to clean up the build? Or maybe to run cibuildwheel in a separate job, such that your builds do not conflict?

Yes that's what I'm doing now.

Maybe just a warning then and a note in docs?

@YannickJadoul
Copy link
Member

Yeah, sure, if this can be confusing, it should be clarified. I'm just not sure where to add it and how general it should be. What kind of note are you thinking about? Something related to this specific problem, or a more general note on cibuildwheel actually making a full independent build?

@samuelcolvin
Copy link
Author

question is what can cause the "presence of too-recent versioned symbols" error? Is it just .so dynamic library files or some other extensions? I'm guessing 99.99% of the time it'll be .so files.

I would say:

  • add to the docs, "make sure your source directory doesn't include any build artifacts like *.c or *.so files before running cibuildwheel"
  • if there are any *.so files in the build directory issue a warning during build saying something like "Build artifacts (*.so files) were found in your source directory "/path/to/src", this may cause cibuildwheel builds to fail. Consider deleting these files before calling cibuildwheel"

@joerick
Copy link
Contributor

joerick commented May 28, 2019

Hm... I'm a little torn here...

I'm hesitant to filter out .so files as part of cibuildwheel, since they only seem to be a problem when built as part of the cython build_ext --inplace command, and (edit, ref #793) I can imagine a situation where a developer might want to include a prebuilt library to be bundled into a wheel, in which case rm **/*.so would be very unexpected behaviour!

So I think my proposal would be:

  • Post a warning if *.so files are found in the project when copied into the container. Something like:

    NOTE: Shared object (.so) files found in this project. 
    
      These files might be built against the wrong OS, causing problems with
      auditwheel.
    
      If you're using Cython and have previously done an in-place build, 
      remove those build files (*.so and *.c) before starting cibuildwheel.
    
  • Also, add a point to cibuildwheel's troubleshooting section, including the auditwheel error text so searches might find it.

How does that sound?

@samuelcolvin
Copy link
Author

yes agreed, I was saying add a warning, not an error.

There are already a number of warnings (eg. "pip is outdated, you should install ...") that people regularly ignore, adding another one wouldn't upset people for whom build succeeds but would save those who do get an error a lot of time.

@YannickJadoul
Copy link
Member

YannickJadoul commented May 28, 2019

since they only seem to be a problem when built as part of the cython build_ext --inplace command

But then this is not really a cibuildwheel problem, I'd argue. We need to make sure we're not going to fix/warn about all issue in the tools that are (potentially) used inside cibuildwheel, because that would swamp the warnings/issues that are actually part of cibuildwheel? Because the exact same problem would happen if you run Cython and auditwheel on an already built project, or not?

EDIT:
So I'm a bit hesitant on whether cibuildwheel should add code searching for *.so files, just to give a warning about a dependency, but maybe it makes sense if cibuildwheel would be the thing users are going to blame. Could we somehow make the steps cibuildwheel takes/dependencies it needs clearer?

@samuelcolvin
Copy link
Author

ok, up to you.

Maybe this issue existing for anyone afflicted with the same problem is enough. Add a comment to the readme or close as you see fit.

@joerick
Copy link
Contributor

joerick commented May 31, 2019

So I'm a bit hesitant on whether cibuildwheel should add code searching for *.so files, just to give a warning about a dependency, but maybe it makes sense if cibuildwheel would be the thing users are going to blame.

If you'll permit, I feel like giving a long answer! Here is a good question - since cibuildwheel abstracts over other build tools, is it responsible when one of those breaks?

My opinion is this: cibuildwheel doesn't really do anything itself - it's always deferring to other tools (pip, wheel, auditwheel, delocate, docker). Without cibuildwheel, the process is really fragmented. Different tools, across different OSs need to be stitched together in just the right way to make it work.

Now clearly we're not responsible for errors in those tools, for fixing errors/crashes there. But I think cibuildwheel's job is providing users with an 'integrated' user experience across those tools. We provide an abstraction. The user says 'build me some wheels', not 'open the docker container, build a wheel with pip, fix up the symbols with auditwheel' etc. However, errors have this habit of breaking abstractions. And this is where users get confused, because the mechanism of cibuildwheel is laid bare, and they must understand a little bit how it works to debug.

So, if we can, I'd like to improve the experience on errors as well. In this case, it takes a bit of knowledge to understand that the linux builds are happening in a totally different OS via docker, that the linked symbols won't match, that auditwheel will fail because of this. A problem with how the tools fit together, instead of the tools themselves.


Also, I've had an idea. Let's do the .so file check and present the warning only if the build fails, instead of at the start. It would appear at the bottom of the log (easier to find) and won't clutter up working builds.

@samuelcolvin
Copy link
Author

Also, I've had an idea. Let's do the .so file check and present the warning only if the build fails, instead of at the start. It would appear at the bottom of the log (easier to find) and won't clutter up working builds.

great idea.

@YannickJadoul
Copy link
Member

In this case, it takes a bit of knowledge to understand that the linux builds are happening in a totally different OS via docker, that the linked symbols won't match, that auditwheel will fail because of this. A problem with how the tools fit together, instead of the tools themselves.

OK, that is a fair point, that I hadn't really considered yet :-) I was (am?) mainly worried to clutter up the cibuildwheel abstractions and simplicity with these kinds of details that are meant to be hidden, but your solution seems like a way of having both! (Thanks for the long answer, by the way; this is an interesting/important thing to remember for future issues.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants