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

Dockerfile: use bats-core #2307

Closed
wants to merge 1 commit into from
Closed

Dockerfile: use bats-core #2307

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
10 changes: 2 additions & 8 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
ARG GO_VERSION=1.13
ARG BATS_VERSION=03608115df2071fff4eaaff1605768c275e5f81f
ARG CRIU_VERSION=v3.13

FROM golang:${GO_VERSION}-buster
Expand Down Expand Up @@ -33,6 +32,7 @@ RUN dpkg --add-architecture armel \
libseccomp-dev:armhf \
libseccomp-dev:ppc64el \
libseccomp2 \
npm \
pkg-config \
protobuf-c-compiler \
protobuf-compiler \
Expand All @@ -49,13 +49,7 @@ RUN dpkg --add-architecture armel \
RUN useradd -u1000 -m -d/home/rootless -s/bin/bash rootless

# install bats
ARG BATS_VERSION
RUN cd /tmp \
&& git clone https://github.com/sstephenson/bats.git \
&& cd bats \
&& git reset --hard "${BATS_VERSION}" \
&& ./install.sh /usr/local \
&& rm -rf /tmp/bats
RUN npm install -g bats
Copy link
Member

Choose a reason for hiding this comment

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

Was there a reason for using npm for installing, and not from GitHub? the old method (with updated repo) should still work; https://github.com/bats-core/bats-core#installing-bats-from-source

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just because it's simpler and someone (hopefully) maintains the package.

Copy link
Member

Choose a reason for hiding this comment

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

Not a big fan; not only would we then depend on the npm package maintainer to maintain the package (in addition to the new bats-core org), it also adds a whole bunch of new dependencies just to install bats;

docker build -t foo -<<EOF
FROM golang:1.13.10-buster
RUN apt-get update && apt-get install -y --no-install-recommends npm && apt-get clean && rm -rf /var/cache/apt /var/lib/apt/lists/*
EOF

docker history foo
IMAGE               CREATED             CREATED BY                                      SIZE                COMMENT
947f3c9961e5        5 seconds ago       RUN /bin/sh -c apt-get update && apt-get ins…   82.6MB              buildkit.dockerfile.v0
<missing>           5 days ago          /bin/sh -c #(nop) WORKDIR /go                   0B
...

That's 83MB to install npm, which we only need to install bats

Copy link
Member

Choose a reason for hiding this comment

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

I recently setup bats for a project, installing from the github repo was very simple. I would rather us do that instead of using npm


# install criu
ARG CRIU_VERSION
Expand Down