-
Notifications
You must be signed in to change notification settings - Fork 243
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
fix: Modernize CI #2487
fix: Modernize CI #2487
Conversation
This allows caching if requirements.txt is unchanged
✅ Deploy Preview for specter-desktop-docs canceled.
|
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.
Short explanations of the main changes
COPY . . | ||
COPY requirements.txt . | ||
|
||
RUN pip3 install --upgrade pip | ||
RUN pip3 install babel cryptography | ||
RUN pip3 install . | ||
RUN pip3 install -r requirements.txt | ||
|
||
COPY . . | ||
|
||
RUN pip3 install . --no-deps |
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.
This is the main part of this fix, which is a change in the order deps are installed.
It also makes builds faster as a side effect.
FROM python:3.10-slim-bullseye AS builder | ||
FROM python:3.10-bookworm AS builder | ||
|
||
ARG VERSION | ||
ARG REPO | ||
|
||
RUN apt update && apt install -y git build-essential libusb-1.0-0-dev libudev-dev libffi-dev libssl-dev rustc cargo libpq-dev | ||
RUN apt update && apt install -y git libusb-1.0-0-dev libudev-dev libffi-dev libssl-dev rustc cargo libpq-dev |
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.
This is also just a build speed optimization, instead of using the slim image, just use the non-slim image during build, which has most packages we need for building preinstalled.
--cache-from type=gha \ | ||
--cache-to type=gha,mode=max \ |
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.
This enables caching.
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 so much for this! Tested locally as well.
This