fix: Remove .dockerignore to give clean working tree during Docker build #1569

matthewfeickert · 2021-08-30T20:43:46Z

Description

Resolves #1500

Remove the .dockerignore file

Lines 1 to 12 in ce2ffab

    
           # Ignore everything 
        
           ** 
        
           # Except the following 
        
           !.git/** 
        
           !docker/** 
        
           !src/** 
        
           !LICENSE 
        
           !pyproject.toml 
        
           !README.rst 
        
           !setup.cfg 
        
           !setup.py

from the project as during the Docker build of the image the COPY step to get the source code in

pyhf/docker/Dockerfile

Line 8 in ce2ffab

COPY . /code

will NOT copy over all files in the repo, which from Git's perspective means the files are missing and puts the Git working tree in a dirty state. setuptools_scm views a dirty working tree as a dev release, so even though pyhf is being installed through pip locally on a release tag

pyhf/docker/Dockerfile

Line 19 in ce2ffab

python -m pip --no-cache-dir install .[xmlio,contrib] && \

setuptools_scm thinks it is a dev release and tags it as a dev release of the next patch release (e.g., 0.6.3.dev0 instead of 0.6.2). Removing the .dockerignore file from the project resolves this.

Removing the file actually has no effect on the built Docker images as the Docker build is multistage. The COPY step happens in the first stage, but the second stage that actually gets published just copies over the Python virtual environment in the Docker image which ignores all non-installable files

pyhf/docker/Dockerfile

Lines 22 to 30 in ce2ffab

    
           FROM base 
        
           ENV PATH=/usr/local/venv/bin:"${PATH}" 
        
           RUN apt-get -qq -y update && \ 
        
               apt-get -qq -y install --no-install-recommends \ 
        
                   curl && \ 
        
               apt-get -y autoclean && \ 
        
               apt-get -y autoremove && \ 
        
               rm -rf /var/lib/apt/lists/* 
        
           COPY --from=builder /usr/local/venv /usr/local/venv

So we can remove the file without repercussions.

Checklist Before Requesting Reviewer

Tests are passing
"WIP" removed from the title of the pull request
Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

Summarize commit messages into a comprehensive review of the PR

* Remove .dockerignore from the project to give a clean git working tree during the build of the Docker image
   - The .dockerignore causes the a dirty working tree as the ignored files are viewed as missing and so setuptools_scm views the repo as being in a dev state and not a clean tag state
* As is, the .dockerignore is not even being used in a useful manner as the COPY command in the Dockerfile is in the first stage of a multistage build that both share the same base image, so the .dockerignore is unused by the final published image

codecov · 2021-08-30T20:50:21Z

Codecov Report

Merging #1569 (d4d09d8) into master (ce2ffab) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1569   +/-   ##
=======================================
  Coverage   97.70%   97.70%           
=======================================
  Files          63       63           
  Lines        4050     4050           
  Branches      576      576           
=======================================
  Hits         3957     3957           
  Misses         54       54           
  Partials       39       39

Flag	Coverage Δ
contrib	`25.40% <ø> (ø)`
unittests	`97.48% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce2ffab...d4d09d8. Read the comment docs.

matthewfeickert added 2 commits August 30, 2021 15:37

placeholder

da4a555

Remove docker ignore for clean installs on tags

d4d09d8

matthewfeickert added fix A bug fix Docker Involving Docker images or builds labels Aug 30, 2021

matthewfeickert self-assigned this Aug 30, 2021

matthewfeickert requested review from kratsg and lukasheinrich August 30, 2021 20:50

lukasheinrich approved these changes Aug 30, 2021

View reviewed changes

matthewfeickert merged commit c8a0e64 into master Aug 30, 2021

matthewfeickert deleted the fix/remove-dockerignore-for-clean-builds branch August 30, 2021 21:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Remove .dockerignore to give clean working tree during Docker build #1569

fix: Remove .dockerignore to give clean working tree during Docker build #1569

matthewfeickert commented Aug 30, 2021 •

edited

Loading

codecov bot commented Aug 30, 2021 •

edited

Loading

	# Ignore everything
	**

	# Except the following
	!.git/**
	!docker/**
	!src/**
	!LICENSE
	!pyproject.toml
	!README.rst
	!setup.cfg
	!setup.py

	FROM base
	ENV PATH=/usr/local/venv/bin:"${PATH}"
	RUN apt-get -qq -y update && \
	apt-get -qq -y install --no-install-recommends \
	curl && \
	apt-get -y autoclean && \
	apt-get -y autoremove && \
	rm -rf /var/lib/apt/lists/*
	COPY --from=builder /usr/local/venv /usr/local/venv

fix: Remove .dockerignore to give clean working tree during Docker build #1569

fix: Remove .dockerignore to give clean working tree during Docker build #1569

Conversation

matthewfeickert commented Aug 30, 2021 • edited Loading

Description

Checklist Before Requesting Reviewer

Before Merging

codecov bot commented Aug 30, 2021 • edited Loading

Codecov Report

matthewfeickert commented Aug 30, 2021 •

edited

Loading

codecov bot commented Aug 30, 2021 •

edited

Loading