Re: [PATCH] Makefile: don't hardcode HEAD in dist target

From: Dennis Kaarsemaker <dennis@kaarsemaker.net>
Date: Jun 2, 2014, 19:34 UTC
Message-ID: <20140602193419.GA10198@spirit>
In-Reply-To: <xmqq4n035ej3.fsf@gitster.dls.corp.google.com>

On Mon, Jun 02, 2014 at 11:53:36AM -0700, Junio C Hamano wrote:

Show 28 quoted lines

> Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
> 
> > Instead of calling git-archive HEAD^{tree}, use $(GIT_VERSION)^{tree}.
> > This makes sure the archive name and contents are consistent, if HEAD
> > has moved, but GIT-VERSION-FILE hasn't been regenerated yet.
> >
> > Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
> > ---
> > I have a somewhat odd setup in which I share a .git between multiple
> > checkouts for automated builds. To minimize locking time for parallel
> > builds with different options, there's only a lock around checkout and
> > running git describe $commit > version, the builds themselves run in
> > parallel.
> >
> > This works just fine except during 'make dist', which is hardcoded to
> > package up HEAD, but that's not always the commit that is actually
> > checked out, another process may have checked out something else.
> >
> > I realize this setup is somewhat strange, but the only change necessary
> > to make this work is this one-line patch, so I hope that's acceptable.
> 
> The "dist" target happens to do a clean checkout to a temporary
> directory with "git archive" and then muck with its contents before
> tarring up the result, so moving HEAD around may appear to work for
> this particular target, but htmldocs/manpages targets use what is in
> the current checkout of Documentation/ area.  Turning the HEAD^{tree}
> into $(GIT_VERSION)^{tree} would make the inconsistency between the
> two worse, wouldn't it?

I'd say it would make the consistency better, because now both look at what is checked out instead of at HEAD. I agree that it's a game of whack-a-mole though and it's really easy to add another dependency on HEAD and GIT-VERSION-FILE agreeing with each other.

Show 18 quoted lines

> If we were to change the "dist" rules, we may want to go in the
> opposite direction.  Instead of using "git archive" to make a
> temporary copy of a directory from a commit, make such a temporary
> copy from the contents of the current working tree (or the index).
> And then tar-up a result after adding configure, version etc. to it.
> That would mean what will be in the tarball can be different from
> even HEAD, which is more consistent with the way how documentation
> tarballs are made.
> 
> Alternatively, you can update the dist-doc rules to make a temporary
> copy of documentation area and generate the documentation tarballs
> out of a pristine source of a known version---which would also make
> these two consistent.  I am not sure which one is more preferrable,
> though.
> 
> Why are you sharing the .git/HEAD across multiple checkouts in the
> first place?  If they are to build all different versions, surely
> these working trees are derived from different commits, no?

I'm sharing all of .git using explicit GIT_DIR and GIT_WORK_TREE environment variables, sharing .git/HEAD comes with that. What I'm actually doing is a continuos integration setup that can run many actions at once in freshly checked-out work trees, but sharing .git to save space.

What it specifically doing is running 'make test' for master, next and pu, and make dist for next. As long as I protect the 'git checkout $sha1' with a lock, that all works just fine.

> Have you considered using contrib/workdir/git-new-workdir, perhaps?

I have not, thanks for the pointer. That approach is definitely cleaner than what I am currently doing.

> I dunno.

With the hint above, I actually don't need this patch anymore. And if you're not convinced it's a good idea, it's probably better to drop it :)

Show 16 quoted lines

> >  Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index a53f3a8..63d9bac 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -2433,7 +2433,7 @@ git.spec: git.spec.in GIT-VERSION-FILE
> >  GIT_TARNAME = git-$(GIT_VERSION)
> >  dist: git.spec git-archive$(X) configure
> >  	./git-archive --format=tar \
> > -		--prefix=$(GIT_TARNAME)/ HEAD^{tree} > $(GIT_TARNAME).tar
> > +		--prefix=$(GIT_TARNAME)/ $(GIT_VERSION)^{tree} > $(GIT_TARNAME).tar
> >  	@mkdir -p $(GIT_TARNAME)
> >  	@cp git.spec configure $(GIT_TARNAME)
> >  	@echo $(GIT_VERSION) > $(GIT_TARNAME)/version

-- 
Dennis Kaarsemaker <dennis@kaarsemaker.net>
http://twitter.com/seveas