Re: Git revision in tarballs - Mailing list pgsql-hackers

On Thu, Jul 15, 2021 at 3:53 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Magnus Hagander <magnus@hagander.net> writes:
> > On Thu, Jul 15, 2021 at 1:40 PM Josef Šimánek <josef.simanek@gmail.com> wrote:
> >> The only problem I do see is adding "git" as a new dependency. That
> >> can potentially cause troubles.
>
> > But only for *creating* the tarballs,  and not for using them. I'm not
> > sure what the usecase would be to create a tarball from an environment
> > that doesn't have git?
>
> I agree, this objection seems silly.  If we ever move off of git, the
> process could be adapted at that time.  However, there *is* a reasonable
> question whether this ought to be handled by "make dist" versus the
> tarball-wrapping script.
>
> >> For the file name, I have seen GIT_VERSION or REVISION file names used
> >> before in another projects. Using ".gitrevision" doesn't make sense to
> >> me since it will be hidden on Unix by default and I'm not sure that is
> >> intended.
>
> > It was definitely intended, as I'd assume it's normally a file that
> > most people don't care about, but more something that scripts that
> > verify things would. But I'm more than happy to change it to a
> > different name if that's preferred. I looked around a bit and couldn't
> > find any general consensus for a name for such a file, but I may not
> > have looked carefully enough.
>
> We already have that convention in place:
>
> $ ls -a
> ./                      .gitignore      README.git      contrib/
> ../                     COPYRIGHT       aclocal.m4      doc/
> .dir-locals.el          GNUmakefile     config/         src/
> .editorconfig           GNUmakefile.in  config.log      tmp_install/
> .git/                   HISTORY         config.status*
> .git-blame-ignore-revs  Makefile        configure*
> .gitattributes          README          configure.ac
>
> So ".gitrevision" or the like seems fine to me.
>
> My thoughts about the proposed patch are (1) you'd better have a
> .gitignore entry too, and (2) what is the mechanism that removes
> this file?  It seems weird to have a make rule that makes a
> generated file but none to remove it.  Perhaps maintainer-clean
> should remove it?

maintainer-clean sounds reasonable for that, yes.


> Both of those issues vanish if this is delegated to the tarball
> making script; as does the need to cope with a starting point
> that isn't a specific commit.  So on the whole I'm leaning to
> the idea that it would be better done over there.

I'd be fine with either. The argument for putting it in the makefile
would be, uh, maybe it makes it a tad bit easier to verify builds
because you get it in your local build as well. But it's not like it's
very *hard* to do it...

Putting it in the tarball making script certainly works for me,
though, if that's what people prefer. And that does away with the
"clean" part as that one blows away the whole directory between each
run.

--
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/