[Mesa-dev] [PATCH] Convert git_sha1_gen script to Python.

[Jose Fonseca]

On 01/08/17 15:32, Emil Velikov wrote:
> On 1 August 2017 at 14:38, Jose Fonseca <jfonseca at vmware.com> wrote:
>> Python is the scripting language we've been using for scripts that need
>> to run across all supported platforms.
>>
>> Shell is *not* a portable language for scripts.
> 
> Guessing that you've hit some issues on Windows?

Yep.  While it's possible to find  sh implementations on Windows all 
suffer from many issues.  It's better to stay away from them.

> All the other platforms should be fine, afaict.

> 
>> ---
>>   Makefile.am                          |  2 +-
>>   bin/git_sha1_gen.py                  | 20 ++++++++++++++++++++
>>   bin/git_sha1_gen.sh                  | 12 ------------
>>   src/Makefile.am                      |  2 +-
>>   src/SConscript                       |  3 ++-
>>   src/mesa/Android.libmesa_git_sha1.mk |  2 +-
>>   6 files changed, 25 insertions(+), 16 deletions(-)
>>   create mode 100755 bin/git_sha1_gen.py
>>   delete mode 100755 bin/git_sha1_gen.sh
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index cf52c834aa..538c38ddeb 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -53,7 +53,7 @@ EXTRA_DIST = \
>>          common.py \
>>          docs \
>>          doxygen \
>> -       bin/git_sha1_gen.sh \
>> +       bin/git_sha1_gen.py \
>>          scons \
>>          SConstruct
>>
>> diff --git a/bin/git_sha1_gen.py b/bin/git_sha1_gen.py
>> new file mode 100755
>> index 0000000000..6d13db1e16
>> --- /dev/null
>> +++ b/bin/git_sha1_gen.py
>> @@ -0,0 +1,20 @@
>> +#!/usr/bin/env python
> Script is already invoked manually via $python.
> Please drop the execute bit alongside the shbang line.

The shell script was also invoked by `sh` and still had the shbang line.

I don't see the harm of the shbang line.  By the contrary, it's convenient.

> 
>> +
>> +import os.path
>> +import subprocess
>> +import sys
>> +
>> +git_dir = os.path.join(os.path.dirname(sys.argv[0]), '..', '.git')
>> +try:
>> +    git_sha1 = subprocess.check_output([
>> +        'git',
>> +        '--git-dir=' + git_dir,
>> +        'rev-parse',
>> +        '--short=10',
>> +        'HEAD',
>> +    ], stderr=open(os.devnull, 'w'))
>> +except subprocess.CalledProcessError as e:
>> +    # don't print anything if git fails
>> +    pass
>> +else:
>> +    sys.stdout.write('#define MESA_GIT_SHA1 "git-%s"\n' % git_sha1.rstrip())
>> diff --git a/bin/git_sha1_gen.sh b/bin/git_sha1_gen.sh
>> deleted file mode 100755
>> index 898e590758..0000000000
>> --- a/bin/git_sha1_gen.sh
>> +++ /dev/null
>> @@ -1,12 +0,0 @@
>> -#!/bin/sh
>> -
>> -# run git from the sources directory
>> -cd "$(dirname "$0")"
>> -
>> -# don't print anything if git fails
>> -if ! git_sha1=$(git --git-dir=../.git rev-parse --short=10 HEAD 2>/dev/null)
>> -then
>> -  exit
>> -fi
>> -
>> -printf '#define MESA_GIT_SHA1 "git-%s"\n' "$git_sha1"
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 5aee6b0141..8d7483fc75 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -21,7 +21,7 @@
>>
>>   .PHONY: git_sha1.h.tmp
>>   git_sha1.h.tmp:
>> -       @sh $(top_srcdir)/bin/git_sha1_gen.sh > $@
>> +       @python $(top_srcdir)/bin/git_sha1_gen.py > $@
>>
> Swap "python" with $(PYTHON2) ...
> 
>> --- a/src/mesa/Android.libmesa_git_sha1.mk
>> +++ b/src/mesa/Android.libmesa_git_sha1.mk
>> @@ -46,7 +46,7 @@ LOCAL_GENERATED_SOURCES += $(addprefix $(intermediates)/, git_sha1.h)
>>   $(intermediates)/git_sha1.h: $(wildcard $(MESA_TOP)/.git/logs/HEAD)
>>          @mkdir -p $(dir $@)
>>          @echo "GIT-SHA1: $(PRIVATE_MODULE) <= git"
>> -       $(hide) sh $(MESA_TOP)/bin/git_sha1_gen.sh > $@
>> +       $(hide) python $(MESA_TOP)/bin/git_sha1_gen.py > $@
> ... and $(MESA_PYTHON2)

OK. I'll fix these.

Jose

> 
> With the above
> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
> 
> Thanks
> Emil
>