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

[Emil Velikov]

On 29 June 2017 at 08:03, Eric Engestrom  wrote:
> On 28 June 2017 09:20:27 BST, Emil Velikov  wrote:
>> On 27 June 2017 at 13:47, Eric Engestrom 
>> wrote:
>> > Signed-off-by: Eric Engestrom 
>> > ---
>> > 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 00..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 
>>
Still stands.

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Eric Engestrom]

On 28 June 2017 09:20:27 BST, Emil Velikov  wrote:
> On 27 June 2017 at 13:47, Eric Engestrom 
> wrote:
> > Signed-off-by: Eric Engestrom 
> > ---
> > 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 00..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.

> 
> 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`.

> 
> With the bash/sh and error messages silenced
> Reviewed-by: Emil Velikov 
> 
> Thanks
> Emil

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Emil Velikov]

On 27 June 2017 at 13:47, Eric Engestrom  wrote:
> Signed-off-by: Eric Engestrom 
> ---
> 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 00..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?

> +
> +# 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 ?

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.

With the bash/sh and error messages silenced
Reviewed-by: Emil Velikov 

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Matt Turner]

Right  you are. Thanks.

Reviewed-by: Matt Turner 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Eric Engestrom]

On 27 June 2017 18:11:14 BST, Matt Turner  wrote:
> On Tue, Jun 27, 2017 at 5:47 AM, Eric Engestrom
>  wrote:
> > Signed-off-by: Eric Engestrom 
> > ---
> > 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 00..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]}")"
> > +
> > +# don't print anything if git fails
> > +if ! git_sha1=$(git rev-parse --short=10 HEAD)
> > +then
> > +  exit
> > +fi
> > +
> > +printf '#define MESA_GIT_SHA1 "git-%s"\n' "$git_sha1"
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index df912c442a..e49b3bd9cf 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -21,18 +21,7 @@
> >
> >  .PHONY: git_sha1.h.tmp
> >  git_sha1.h.tmp:
> > -   @# Don't assume that $(top_srcdir)/.git is a directory. It
> may be
> > -   @# a gitlink file if $(top_srcdir) is a submodule checkout
> or a linked
> > -   @# worktree.
> > -   @# If we are building from a release tarball copy the
> bundled header.
> > -   @touch git_sha1.h.tmp
> > -   @if test -e $(top_srcdir)/.git; then \
> > -   if which git > /dev/null; then \
> > -   printf '#define MESA_GIT_SHA1 "git-%s"\n' \
> > -   `git --git-dir=$(top_srcdir)/.git rev-parse
> --short=10 HEAD` \
> > -   > git_sha1.h.tmp ; \
> > -   fi \
> > -   fi
> 
> The purpose of the .tmp file is to prevent unnecessary rebuilds (and
> relinks) when the SHA1 doesn't change.
> 
> It looks like you're removing that?

It's still there, you just snipped the quote one line too soon :)

> > + @bash $(top_srcdir)/git_sha1_gen.sh > $@
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Matt Turner]

On Tue, Jun 27, 2017 at 5:47 AM, Eric Engestrom
 wrote:
> Signed-off-by: Eric Engestrom 
> ---
> 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 00..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]}")"
> +
> +# don't print anything if git fails
> +if ! git_sha1=$(git rev-parse --short=10 HEAD)
> +then
> +  exit
> +fi
> +
> +printf '#define MESA_GIT_SHA1 "git-%s"\n' "$git_sha1"
> diff --git a/src/Makefile.am b/src/Makefile.am
> index df912c442a..e49b3bd9cf 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -21,18 +21,7 @@
>
>  .PHONY: git_sha1.h.tmp
>  git_sha1.h.tmp:
> -   @# Don't assume that $(top_srcdir)/.git is a directory. It may be
> -   @# a gitlink file if $(top_srcdir) is a submodule checkout or a linked
> -   @# worktree.
> -   @# If we are building from a release tarball copy the bundled header.
> -   @touch git_sha1.h.tmp
> -   @if test -e $(top_srcdir)/.git; then \
> -   if which git > /dev/null; then \
> -   printf '#define MESA_GIT_SHA1 "git-%s"\n' \
> -   `git --git-dir=$(top_srcdir)/.git rev-parse 
> --short=10 HEAD` \
> -   > git_sha1.h.tmp ; \
> -   fi \
> -   fi

The purpose of the .tmp file is to prevent unnecessary rebuilds (and
relinks) when the SHA1 doesn't change.

It looks like you're removing that?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev