[Mesa-dev] [PATCH 20/25] anv: minor tweak in the generation script

[Emil Velikov]

On 21 April 2016 at 22:50, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On Thu, Apr 21, 2016 at 6:16 AM, Emil Velikov <emil.l.velikov at gmail.com>
> wrote:
>>
>> From: Emil Velikov <emil.velikov at collabora.com>
>>
>> Rather than parsing through the same files (public headers) twice, tweak
>> the python script to create both files at the same time.
>
>
> Yes, but it takes almost zero time to generate them and it's going to run in
> parallel before anything else gets built.  I don't know that this really
> saves us anything.
>
Are you sure about this one? Based on my brief testing - things were
pretty much stalled until both files were generated. I'll take another
look.

If anything the approach cuts down the bash output redirection and
some nasty handling around it.
Talking about the following $(PYHON) ....  > $@ || ($(RM) $@; false)
and how often we forget to add it.

>>
>> Chances are that if the public headers change, both files will need to
>> be regenerated.
>>
>> Note to the python masters: this patch aims to be the least evasive
>> change. Feel free to change/rewrite things to your liking.
>>
>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>> ---
>>  src/intel/vulkan/Makefile.am            |  8 +++----
>>  src/intel/vulkan/anv_entrypoints_gen.py | 41
>> ++++++++++++++++++++++++---------
>>  2 files changed, 33 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/intel/vulkan/Makefile.am b/src/intel/vulkan/Makefile.am
>> index 360e97f..110961e 100644
>> --- a/src/intel/vulkan/Makefile.am
>> +++ b/src/intel/vulkan/Makefile.am
>> @@ -126,11 +126,9 @@ VULKAN_LIB_DEPS += \
>>
>>  libvulkan_intel_la_SOURCES = $(VULKAN_GEM_FILES)
>>
>> -anv_entrypoints.h : anv_entrypoints_gen.py $(vulkan_include_HEADERS)
>> -       $(AM_V_GEN) cat $(vulkan_include_HEADERS) | $(CPP) $(AM_CPPFLAGS)
>> - | $(PYTHON2) $< header > $@
>> -
>> -anv_entrypoints.c : anv_entrypoints_gen.py $(vulkan_include_HEADERS)
>> -       $(AM_V_GEN) cat $(vulkan_include_HEADERS) | $(CPP) $(AM_CPPFLAGS)
>> - | $(PYTHON2) $< code > $@
>> +anv_entrypoints.c anv_entrypoints.h : anv_entrypoints_gen.py
>> $(vulkan_include_HEADERS)
>> +       $(AM_V_GEN) cat $(vulkan_include_HEADERS) | $(CPP) $(AM_CPPFLAGS)
>> - | \
>> +       $(PYTHON2) $(srcdir)/anv_entrypoints_gen.py $(builddir)
>>
>>  CLEANFILES = $(BUILT_SOURCES)
>>
>> diff --git a/src/intel/vulkan/anv_entrypoints_gen.py
>> b/src/intel/vulkan/anv_entrypoints_gen.py
>> index cedecfe..19bfb93 100644
>> --- a/src/intel/vulkan/anv_entrypoints_gen.py
>> +++ b/src/intel/vulkan/anv_entrypoints_gen.py
>> @@ -22,7 +22,7 @@
>>  # IN THE SOFTWARE.
>>  #
>>
>> -import fileinput, re, sys
>> +import fileinput, re, sys, os
>>
>>  # Each function typedef in the vulkan.h header is all on one line and
>> matches
>>  # this regepx. We hope that won't change.
>> @@ -51,15 +51,21 @@ def hash(name):
>>
>>      return h
>>
>> -opt_header = False
>> -opt_code = False
>> +if len(sys.argv[1:]) != 1:
>> +    print "Usage: %s <output_directory>" % sys.argv[0]
>> +    exit(1)
>> +
>> +output_dir = sys.argv[1]
>> +if not os.path.isdir(output_dir):
>> +    if os.path.exists(output_dir):
>> +        print "ERROR: Invalid output directory: %s" % output_dir
>> +        exit(1)
>> +
>> +sys.argv.pop()
>> +# Output path exists, now just run the template
>> +output_file = os.sep.join([output_dir, 'anv_entrypoints.c'])
>> +output_header = os.sep.join([output_dir, 'anv_entrypoints.h'])
>>
>> -if (sys.argv[1] == "header"):
>> -    opt_header = True
>> -    sys.argv.pop()
>> -elif (sys.argv[1] == "code"):
>> -    opt_code = True
>> -    sys.argv.pop()
>>
>>  # Parse the entry points in the header
>>
>> @@ -77,7 +83,11 @@ for line in fileinput.input():
>>  # For outputting entrypoints.h we generate a anv_EntryPoint() prototype
>>  # per entry point.
>>
>> -if opt_header:
>> +def generate_header(output_header):
>> +    orig_stdout = sys.stdout
>> +    hdr = file(output_header, 'w')
>> +    sys.stdout = hdr
>
>
> This is a bit ugly...
>
s/a bit/a fair bit/

Definitely, but in all honesty the currently the script isn't the
prettiest one either. I'm pretty sure Dylan will end up rewriting the
whole thing if he has the time/chance. Even without this patch ;-)

-Emil