-
Notifications
You must be signed in to change notification settings - Fork 6
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
Include Make in builder-target-platform image #192
Conversation
Required for building the Python isal package (for Augur) when there aren't wheels available (such as with recent version 1.5.2).
Merging since this resolves build failures. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into this! Where is this isal dependency even from? I don't get it when installing Augur with pip locally.
This fragility is unfortunate, and I wouldn't be surprised if other indirect dependencies cause breakage like this in the future. Would it be worthwhile to provide a standalone installer for Augur for cases like this, where flexibility in dependency versions defined by setup.py is not necessary and even undesirable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a requirement of xopen for Linux systems only. (I was curious so I tracked it down with pipdeptree in a nextstrain shell).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think a standalone installer for Augur is necessary for this Docker image. It'd be a bit of work for too narrow of a gain, I think, and there are other ways about it. For example, we could take additional measures in the Dockerfile to reduce this sort of breakage by using --prefer-binary
to the pip install
of Augur. That would have gotten us the isal 1.5.1 wheels instead of trying to build isal 1.5.2 from source. We could also maintain a constraints file and pass that in with --constraint
if we wanted to pin things more tightly for this Docker image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tsibley that makes sense. Good to know those alternatives!
Two things to follow up on given discussion in pycompression/python-isal#158 (comment).
|
Required for building the Python isal package (for Augur) when there aren't wheels available (such as with recent version 1.5.2).
Checklist