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

do not add .git folder to a build layer #269

Merged
merged 1 commit into from
Jan 27, 2019
Merged

do not add .git folder to a build layer #269

merged 1 commit into from
Jan 27, 2019

Conversation

nagimov
Copy link
Contributor

@nagimov nagimov commented Jan 27, 2019

.git folder isn't present in tarballs pulled from releases, e.g. wget https://github.com/umputun/remark/archive/v.1.2.1.tar.gz, and not used during the build anyways

`.git` folder isn't present in tarballs pulled from releases, e.g. `wget https://github.com/umputun/remark/archive/v.1.2.1.tar.gz`, and not used during the build anyways
@umputun
Copy link
Owner

umputun commented Jan 27, 2019

not sure why it has anything to do with tarballs. They supposed to use $DRONE_TAG to form the version, but not .git. This part never worked properly because I'll need some sort of artifact storage on drone side and upload those artifacts to release. I have tried to implement it, but obviously, something is not right here and it doesn't keep/publish them to github properly.

regarding the conclusion - I agree, .git seems to be an artifact of some legacy method to get version number for local builds, but we don't use it anymore (defaulted to "local").

@umputun umputun merged commit 7712f37 into umputun:master Jan 27, 2019
@nagimov nagimov deleted the patch-1 branch January 27, 2019 20:26
@paskal paskal added this to the v1.3 milestone Jan 15, 2022
@nc7s
Copy link

nc7s commented May 31, 2022

It's still here on master, as of now:

ADD .git/ /build/backend/.git/

If it's just for a revision number, could it be done using the git command?

@umputun
Copy link
Owner

umputun commented May 31, 2022

git command won't work without .git folder.

@nc7s
Copy link

nc7s commented May 31, 2022

How about running the scripts locally then copying the results over?

@umputun
Copy link
Owner

umputun commented May 31, 2022

it means more complicated build process. I don't really see a reason to overcomplicate things. Build layer doesn't affect the final size of the container anyway

@nagimov
Copy link
Contributor Author

nagimov commented May 31, 2022

my two cents: having ADD .git breaks this workflow:

VER=1.10.0
mkdir ${VER}
wget https://github.com/umputun/remark42/archive/refs/tags/v${VER}.tar.gz
tar -xvf *.tar.gz -C ${VER}
cd ${VER}/*
docker build -t ${VER} .

since .git folder isn't included in release tarballs:

...
Step 9/53 : ADD .git/ /build/backend/.git/
ADD failed: file not found in build context or excluded by .dockerignore: stat .git/: file does not exist

@umputun
Copy link
Owner

umputun commented May 31, 2022

ah, I see. Have not thought anyone actually using tarballs instead of git checkout, but it is a valid concern for sure. I don't want to create some special make's target for the primary docker build and would rather prefer some special target to build container from the non-git source.

@nagimov
Copy link
Contributor Author

nagimov commented Jun 1, 2022

For reference, .git directory was removed from the Dockerfile by this PR (#269) back in 2019 since it wasn't used in the Dockerfile then. It was re-introduced by aea7724 that started using git-rev.sh to fetch the git version and tag the build with it.

I see two ways of solving this (assuming it actually needs "solving"):

  1. stop using git-rev.sh and always tag builds with version="local" the way it used to work prior to aea7724
  2. make git-rev.sh fall back to version="local" if .git isn't found, and make Dockerfile copy .git conditionally only if it exists (not supported by docker officially although hacky workarounds exist)

I fixed this for myself with sed 's/...' Dockerfile before building it 😄

@nc7s
Copy link

nc7s commented Jun 1, 2022

Yeah, @nagimov made the point. I was trying to deploy it to Railway, and saw the same error, but the case wasn't as persuasive as tarballs.

Anyway, if all you need is branch-hash-timestamp, it could be done locally using $(git branch --show-current)-$(git log -1 --format=%h)-$(date +%Y%m%dT%H:%M:%S). You are sending the same work directory to build after all. This could be run before docker build, setting an env var, then ENV BUILD_TAG in Dockerfile.

@nagimov
Copy link
Contributor Author

nagimov commented Jun 1, 2022

+1 for @bnoctis cleaner solution. Version ENV can be set via git in the Makefile and passed to docker build --build-arg ...
I would also add something like [[ -z "$version" ]] || version="local" to the Dockerfile to allow for unset ENV

umputun added a commit that referenced this pull request Jun 5, 2022
* removes .git from build layer, emulates GH build #269

* rundev target with git version passed in
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.

4 participants