-
Notifications
You must be signed in to change notification settings - Fork 208
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
Support running all docker images locally #2698
Changes from all commits
d134fa9
ebb21cd
3659766
4c81d14
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
#!/bin/bash | ||
|
||
USER_ID=${HOST_UID:-1000} | ||
GROUP_ID=${HOST_GID:-1000} | ||
|
||
echo "Starting with UID: $USER_ID, GID: $GROUP_ID" | ||
usermod -u $USER_ID builder | ||
groupmod -g $GROUP_ID builder | ||
|
||
exec /usr/sbin/gosu builder "$@" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,7 @@ | |
# docker build \ | ||
# --tag=newsboat-build-tools \ | ||
# --file=docker/ubuntu_18.04-build-tools.dockerfile \ | ||
# docker | ||
# . | ||
# | ||
# Build with non-default compiler and Rust version: | ||
# | ||
|
@@ -26,7 +26,7 @@ | |
# --build-arg cc=clang-7 \ | ||
# --build-arg cxx=clang++-7 \ | ||
# --build-arg rust_version=1.26.1 \ | ||
# docker | ||
# . | ||
# | ||
# Before building in a container, run this to remove any binaries that you | ||
# might've compiled on your host system (or in another container): | ||
|
@@ -37,8 +37,10 @@ | |
# | ||
# docker run \ | ||
# --rm \ | ||
# --mount type=bind,source=$(pwd),target=/home/builder/src \ | ||
# --user $(id -u):$(id -g) \ | ||
# --mount type=bind,source=$(pwd),target=/src \ | ||
# --user root | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we have to specify the user here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The approach taken in this PR requires the entrypoint script to update the user/group id of the The main issue is that cirrus does not expose a way to specify a user, so it will use the user as specified by the last There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Anyway, this solution feels far too convoluted, so I plan to try a different approach |
||
# -e HOST_UID=$(id -u) \ | ||
# -e HOST_GID=$(id -g) \ | ||
# newsboat-build-tools \ | ||
# make | ||
# | ||
|
@@ -69,14 +71,14 @@ RUN apt-get update \ | |
&& apt-get install --assume-yes --no-install-recommends \ | ||
build-essential $cxx_package libsqlite3-dev libcurl4-openssl-dev libssl-dev \ | ||
libxml2-dev libstfl-dev libjson-c-dev libncursesw5-dev gettext git \ | ||
asciidoctor wget \ | ||
asciidoctor wget gosu \ | ||
&& apt-get autoremove \ | ||
&& apt-get clean | ||
|
||
RUN addgroup --gid 1000 builder \ | ||
&& adduser --home /home/builder --uid 1000 --ingroup builder \ | ||
--disabled-password --shell /bin/bash builder \ | ||
&& mkdir -p /home/builder/src \ | ||
&& mkdir -p /src \ | ||
&& chown -R builder:builder /home/builder | ||
|
||
RUN apt-get install locales \ | ||
|
@@ -89,7 +91,7 @@ ENV LANGUAGE en_US:en | |
ENV LC_ALL en_US.UTF-8 | ||
|
||
USER builder | ||
WORKDIR /home/builder/src | ||
WORKDIR /src | ||
|
||
ARG rust_version=1.76.0 | ||
|
||
|
@@ -107,3 +109,11 @@ ARG cxx=g++-8 | |
|
||
ENV CC=$cc | ||
ENV CXX=$cxx | ||
|
||
USER root | ||
COPY docker/entrypoint.sh /usr/local/bin/entrypoint.sh | ||
RUN chmod +x /usr/local/bin/entrypoint.sh | ||
ENTRYPOINT ["/usr/local/bin/entrypoint.sh"] | ||
|
||
# Cirrus doesn't run the entrypoint script so we switch directly to the target user. | ||
USER builder |
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.
Why is this needed? It doesn't look like Dockerfiles use anything outside the docker directory.
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 not strictly necessary, however, I prefer to keep these consistent between all of our docker files.
In the buildtools docker files, I've used a
COPY
command to pull in the newentrypoint.sh
so there it is was necessary to update the path (cirrus seems to always specify the root directory of the git checkout, without option to override it so I did have to include thedocker/
part in theCOPY
command)