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

Stop installing apt packages for compiling Python #7943

Merged

Conversation

jeffwidman
Copy link
Member

@jeffwidman jeffwidman commented Aug 31, 2023

Now that we've stopped compiling Python from source and instead download/copy a pre-compiled version, there's no need to install these apt packages.

A few may be needed for users to build python deps written in native C, but those shouldn't be listed in this section, they should be added later in the the Dockerfile where we already list other packages needed for that:

# Install C-libs needed to build users' Python packages. Please document why each package is needed.
USER root
RUN apt-get update \
&& apt-get upgrade -y \
&& apt-get install -y --no-install-recommends \
# Used by pycurl
libcurl4-openssl-dev \
# Used by mysqlclient
libmysqlclient-dev \
# Used by psycopg Postgres Client
libpq-dev \
# Used by python zoneinfo core lib
tzdata \
# Needed to build `gssapi`/`krb5`
libkrb5-dev \
&& rm -rf /var/lib/apt/lists/*

Also, it's unclear which of these would be needed, so let's start by removing these, and then as needed can add back the couple that we actually discover are needed.

@jeffwidman jeffwidman requested a review from a team as a code owner August 31, 2023 17:33
@jeffwidman jeffwidman force-pushed the no-need-to-install-apt-packages-into-python-core-image branch from c27c418 to 4ec70fd Compare August 31, 2023 17:40
Copy link
Member

@Nishnha Nishnha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM I compared this to the suggested build environment list for pyenv https://github.com/pyenv/pyenv/wiki#suggested-build-environment

The only one not listed here was curl but I guess that's installed elsewhere

@jeffwidman
Copy link
Member Author

I'm going to try deploying this in prod and see what Sentry shows for user errors... but again, probably we should move forward with this, and then add back missing packages as needed in a follow-on PR to

# Install C-libs needed to build users' Python packages. Please document why each package is needed.
USER root
RUN apt-get update \
&& apt-get upgrade -y \
&& apt-get install -y --no-install-recommends \
# Used by pycurl
libcurl4-openssl-dev \
# Used by mysqlclient
libmysqlclient-dev \
# Used by psycopg Postgres Client
libpq-dev \
# Used by python zoneinfo core lib
tzdata \
# Needed to build `gssapi`/`krb5`
libkrb5-dev \
&& rm -rf /var/lib/apt/lists/*

Now that we've stopped compiling Python from source and instead
download/copy a pre-compiled version, there's no need to install these
apt packages.

A few may be needed for users to build python deps written in native C,
but those shouldn't be listed in this section, they should be added
later in the the Dockerfile where we already list other packages needed
for that:
https://github.com/dependabot/dependabot-core/blob/51b268a81a983c6547af4d95768218a03bd8e751/python/Dockerfile#L128-L143

Also, it's unclear which of these would be needed, so let's start by
removing these, and then as needed can add back the couple that we
actually discover are needed.
@jeffwidman jeffwidman force-pushed the no-need-to-install-apt-packages-into-python-core-image branch from 4ec70fd to 021fca9 Compare August 31, 2023 18:12
@jeffwidman jeffwidman merged commit c8901df into main Aug 31, 2023
79 checks passed
@jeffwidman jeffwidman deleted the no-need-to-install-apt-packages-into-python-core-image branch August 31, 2023 18:26
brettfo pushed a commit to brettfo/dependabot-core that referenced this pull request Oct 11, 2023
Now that we've stopped compiling Python from source and instead
download/copy a pre-compiled version, there's no need to install these
apt packages.

A few may be needed for users to build python deps written in native C,
but those shouldn't be listed in this section, they should be added
later in the the Dockerfile where we already list other packages needed
for that:
https://github.com/dependabot/dependabot-core/blob/51b268a81a983c6547af4d95768218a03bd8e751/python/Dockerfile#L128-L143

Also, it's unclear which of these would be needed, so let's start by
removing these, and then as needed can add back the couple that we
actually discover are needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants