[Mesa-dev] Thou shalt not use shell scripts on Windows (Was: Mesa (master): build systems: uniformize git_sha1.h generation)
Eric Engestrom
eric.engestrom at imgtec.com
Tue Aug 1 13:36:39 UTC 2017
On Tuesday, 2017-08-01 13:04:33 +0000, Jose Fonseca wrote:
> Sorry for being late to the party - I was on PTO when this review
> went out. Only now I was trying to diagnose why some of our builds
> were broken for very long time, and tracked down to this.
>
> Let me start stating what I think it should be obvious for everybody:
> using a shell script on Windows is a awful awful idea. It's not
> admissible. That's the whole reason we use SCons instead of autotools
> and stuff.
>
> The only reason this didn't break things completely for us is that Git
> for windows often bundles sh.exe and some times it's left in the Path.
> But that's a coincidence, which in fact creates problems of its own.
Oops :/
I haven't done any dev on windows in many years, so it didn't occur to
me that it doesn't have shell scripts. Apologies :(
>
> If you truly want to uniformize git_sha1.h generation then one must
> use cross platform scripting language as Python. Like all other
> scripts we have!
Rewriting it to python now, will send a patch asap.
>
> Honestly, I don't know how this pass through peer review
> unchallenged... It should have been stopped before it ever reached
> master.
>
> Jose
>
> ________________________________
> From: mesa-dev <mesa-dev-bounces at lists.freedesktop.org> on behalf of Brian Paul <brianp at vmware.com>
> Sent: Saturday, July 1, 2017 00:19
> To: mesa-dev at lists.freedesktop.org
> Subject: Re: [Mesa-dev] Mesa (master): build systems: uniformize git_sha1.h generation
>
> Hi Eric,
>
> Shouldn't the new script file be put in the bin/ directory with the
> other helper scripts?
>
> -Brian
>
> On 06/29/2017 09:52 AM, Eric Engeström wrote:
> > Module: Mesa
> > Branch: master
> > Commit: 3fd425aed764fb771f2f49ddb6b30b389a114504
> > URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__cgit.freedesktop.org_mesa_mesa_commit_-3Fid-3D3fd425aed764fb771f2f49ddb6b30b389a114504&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=Ie7_encNUsqxbSRbqbNgofw0ITcfE8JKfaUjIQhncGA&m=C2QHFlT8FYNq-A29CoLU1hnnK3jQhP0A-EpQrqbU_GQ&s=ZjxcahOcytLNOgFBZuod2dWN2xvP8Ph32iP3dluOunQ&e=
> >
> > Author: Eric Engestrom <eric.engestrom at imgtec.com>
> > Date: Tue Jun 27 12:08:41 2017 +0100
> >
> > build systems: uniformize git_sha1.h generation
> >
> > Signed-off-by: Eric Engestrom <eric.engestrom at imgtec.com>
> > Reviewed-by: Matt Turner <mattst88 at gmail.com>
> > Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
> >
> > ---
> >
> > git_sha1_gen.sh | 12 ++++++++++++
> > src/Makefile.am | 13 +------------
> > src/SConscript | 28 ++++++++--------------------
> > src/mesa/Android.libmesa_git_sha1.mk | 7 +------
> > 4 files changed, 22 insertions(+), 38 deletions(-)
> >
> > diff --git a/git_sha1_gen.sh b/git_sha1_gen.sh
> > new file mode 100755
> > index 0000000000..20ab8df8ea
> > --- /dev/null
> > +++ b/git_sha1_gen.sh
> > @@ -0,0 +1,12 @@
> > +#!/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 df912c442a..d8a2ee59fc 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
> > + @sh $(top_srcdir)/git_sha1_gen.sh > $@
> >
> > git_sha1.h: git_sha1.h.tmp
> > @echo "updating git_sha1.h"
> > diff --git a/src/SConscript b/src/SConscript
> > index d861af8e4d..5e1171b524 100644
> > --- a/src/SConscript
> > +++ b/src/SConscript
> > @@ -22,27 +22,15 @@ def write_git_sha1_h_file(filename):
> > to retrieve the git hashid and write the header file. An empty file
> > will be created if anything goes wrong."""
> >
> > - args = [ 'git', 'rev-parse', '--short=10', 'HEAD' ]
> > - try:
> > - (commit, foo) = subprocess.Popen(args, stdout=subprocess.PIPE).communicate()
> > - except:
> > - print "Warning: exception in write_git_sha1_h_file()"
> > - # git log command didn't work
> > - if not os.path.exists(filename):
> > - dirname = os.path.dirname(filename)
> > - if dirname and not os.path.exists(dirname):
> > - os.makedirs(dirname)
> > - # create an empty file if none already exists
> > - f = open(filename, "w")
> > - f.close()
> > - return
> > -
> > - # note that commit[:-1] removes the trailing newline character
> > - commit = '#define MESA_GIT_SHA1 "git-%s"\n' % commit[:-1]
> > tempfile = "git_sha1.h.tmp"
> > - f = open(tempfile, "w")
> > - f.write(commit)
> > - f.close()
> > + with open(tempfile, "w") as f:
> > + args = [ 'sh', Dir('#').abspath + '/git_sha1_gen.sh' ]
> > + try:
> > + subprocess.Popen(args, stdout=f)
> > + except:
> > + print "Warning: exception in write_git_sha1_h_file()"
> > + return
> > +
> > if not os.path.exists(filename) or not filecmp.cmp(tempfile, filename):
> > # The filename does not exist or it's different from the new file,
> > # so replace old file with new.
> > diff --git a/src/mesa/Android.libmesa_git_sha1.mk b/src/mesa/Android.libmesa_git_sha1.mk
> > index 0fd176bf7d..a5a1ebb37f 100644
> > --- a/src/mesa/Android.libmesa_git_sha1.mk
> > +++ b/src/mesa/Android.libmesa_git_sha1.mk
> > @@ -46,12 +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) touch $@
> > - $(hide) if which git > /dev/null; then \
> > - git --git-dir $(MESA_TOP)/.git log -n 1 --oneline | \
> > - sed 's/^\([^ ]*\) .*/#define MESA_GIT_SHA1 "git-\1"/' \
> > - > $@; \
> > - fi
> > + $(hide) sh $(MESA_TOP)/git_sha1_gen.sh > $@
> >
> > LOCAL_EXPORT_C_INCLUDE_DIRS := $(intermediates)
> >
> >
> > _______________________________________________
> > mesa-commit mailing list
> > mesa-commit at lists.freedesktop.org
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Dcommit&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=Ie7_encNUsqxbSRbqbNgofw0ITcfE8JKfaUjIQhncGA&m=C2QHFlT8FYNq-A29CoLU1hnnK3jQhP0A-EpQrqbU_GQ&s=W6LuBpaa2ybCgFRk5UR-YrT1lr2rk0XsV8K6c6Ii1AI&e=
> >
>
More information about the mesa-dev
mailing list