[Mesa-dev] [PATCH mesa] build systems: uniformize git_sha1.h generation

[Emil Velikov]

On 29 June 2017 at 08:03, Eric Engestrom <eric at engestrom.ch> wrote:
> On 28 June 2017 09:20:27 BST, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 27 June 2017 at 13:47, Eric Engestrom <eric.engestrom at imgtec.com>
>> wrote:
>> > Signed-off-by: Eric Engestrom <eric.engestrom at imgtec.com>
>> > ---
>> > Note: Autotools and SCons are tested, but Android isn't.
>> > ---
>> >  git_sha1_gen.sh                      | 13 +++++++++++++
>> >  src/Makefile.am                      | 13 +------------
>> >  src/SConscript                       | 28
>> ++++++++--------------------
>> >  src/mesa/Android.libmesa_git_sha1.mk |  7 +------
>> >  4 files changed, 23 insertions(+), 38 deletions(-)
>> >  create mode 100755 git_sha1_gen.sh
>> >
>> > diff --git a/git_sha1_gen.sh b/git_sha1_gen.sh
>> > new file mode 100755
>> > index 0000000000..9630067be0
>> > --- /dev/null
>> > +++ b/git_sha1_gen.sh
>> > @@ -0,0 +1,13 @@
>> > +#!/usr/bin/env bash
>> > +set -eu
>> > +
>> > +# run git from the sources directory
>> > +cd "$(dirname "${BASH_SOURCE[0]}")"
>> That's a cool way of avoiding the  --git-dir bits
>>
>> This can become cd "$(dirname "$0")" and thus s/bash/sh/ across the
>> board?
>
> Sure, changed locally.
>
>>
>> > +
>> > +# don't print anything if git fails
>> > +if ! git_sha1=$(git rev-parse --short=10 HEAD)
>> Error messages seems to be printed - see below examples. A simple
>> 2>/dev/null redirect should do it ?
>
> I meant "don't print the git_sha1.h contents"; I think errors are useful here.
> If you feel strongly about it, I can silence them.
>
Current code does not warn on either one. So if we are to introduce
them, please keep it separate change.

>>
>> git: command not found
>>
>> fatal: Not a git repository (or any parent up to mount point /)
>> Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not
>> set).
>>
>> With the latter message in mind, perhaps we should set --git-dir
>> regardless, since git will otherwise traverse all the way to /? Might
>> want to leave that for another day, though.
>
> Sure, adding `--git-dir=.git` since we should already be in
> the right dir anyway, with the above `cd`.
>
Ack. Feel free to squash or leave it as a follow-up.

>>
>> With the bash/sh and error messages silenced
>> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
>>
Still stands.

-Emil