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

[monitoring] install libxml2-dev libxslt-dev before pip install #394

Merged
merged 1 commit into from
Dec 8, 2023

Conversation

Shastick
Copy link
Contributor

@Shastick Shastick commented Dec 8, 2023

Without these, building the Dockerfile locally fails with the error below when running make format, for example.

I assume it is a peculiarity of my ARM environment, or related to OS X. I seem to have this issue since the update to Python 3.11.

2.745 Collecting lxml==4.9.1 (from -r /app/monitoring/requirements.txt (line 26))
2.763   Downloading lxml-4.9.1.tar.gz (3.4 MB)
2.837      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 3.4/3.4 MB 46.2 MB/s eta 0:00:00
2.963   Preparing metadata (setup.py): started
3.067   Preparing metadata (setup.py): finished with status 'error'
3.069   error: subprocess-exited-with-error
3.069   
3.069   × python setup.py egg_info did not run successfully.
3.069   │ exit code: 1
3.069   ╰─> [3 lines of output]
3.069       Building lxml version 4.9.1.
3.069       Building without Cython.
3.069       Error: Please make sure the libxml2 and libxslt development packages are installed.
3.069       [end of output]
3.069   
3.069   note: This error originates from a subprocess, and is likely not a problem with pip.
3.069 error: metadata-generation-failed
3.069 
3.069 × Encountered error while generating package metadata.
3.069 ╰─> See above for output.
3.069 
3.069 note: This is an issue with the package mentioned above, not pip.
3.069 hint: See above for details.
3.157 
3.157 [notice] A new release of pip is available: 23.2.1 -> 23.3.1
3.157 [notice] To update, run: pip install --upgrade pip
------
Dockerfile:22
--------------------
  20 |     RUN mkdir -p /app/monitoring
  21 |     COPY ./requirements.txt /app/monitoring/requirements.txt
  22 | >>> RUN pip install -r /app/monitoring/requirements.txt
  23 |     
  24 |     RUN rm -rf __pycache__
--------------------
ERROR: failed to solve: process "/bin/sh -c pip install -r /app/monitoring/requirements.txt" did not complete successfully: exit code: 1

@@ -17,6 +17,9 @@ RUN apt-get update && apt-get install -y openssl curl libgeos-dev gcc && apt-get
# required for gevent to build without error in an ARM environment
RUN apt-get update && apt-get install -y libffi-dev libssl-dev python3-dev build-essential

# required for lxml to install successfully with pip (at least on an ARM environment)
RUN apt-get update && apt-get install -y libxml2-dev libxslt-dev
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that we could wrap this in a check for the architecture we are building on, if we don't want to install these for everyone in every situation, but I guess keeping this Dockerfile simple also has benefits.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think installing a bit of extra stuff is fine in order to keep things consistent across platforms. Sorry for the issue as the point of docker is supposed to be to avoid problems like that, but thanks for identifying and fixing.

@Shastick Shastick marked this pull request as ready for review December 8, 2023 08:16
@Shastick Shastick marked this pull request as draft December 8, 2023 08:17
@Shastick Shastick marked this pull request as ready for review December 8, 2023 08:22
@BenjaminPelletier BenjaminPelletier merged commit 5b4bd9e into interuss:main Dec 8, 2023
9 checks passed
github-actions bot added a commit that referenced this pull request Dec 8, 2023
Install missing dependencies via apt 5b4bd9e
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 this pull request may close these issues.

None yet

2 participants