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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 6, 2023
+2 −0
Merged

Include Make in builder-target-platform image #192

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Dockerfile
View file
Open in desktop
Copy link
Member

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?

Copy link
Contributor

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).

Copy link
Member Author

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.

Copy link
Member

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!

Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,12 @@ ARG TARGETARCH
# git: for git pip installs
# gcc: for building datrie (for Snakemake)
# libsqlite3-dev, zlib1g-dev: for building pyfastx (for Augur)
# make: for building isal (if necessary, for Augur)
RUN apt-get update && apt-get install -y --no-install-recommends \
curl \
gcc \
git \
make \
jq \
libsqlite3-dev \
zlib1g-dev
Expand Down