[PATCH 1/3] waf_generator: Copy headers if necessary.

[Christian Mauderer]

Am 10.08.2017 um 07:55 schrieb Chris Johns:
> On 10/08/2017 15:34, Christian Mauderer wrote:
>> Am 10.08.2017 um 01:08 schrieb Chris Johns:
>>> On 10/08/2017 02:29, Sichen Zhao wrote:
>>>> From: Christian Mauderer <christian.mauderer at embedded-brains.de>
>> There was no real reason. I just chose an arbitrary name.
>> 'build-include' is fine for me too. I would avoid just 'include' because
>> it's a very generic name. If it is added somewhere else, that might lead
>> to a conflict.
> 
> Ah yes, 'build-include' is fine.
> 
>>>> +        start_dir = bld.path.find_dir(headers[0])
>>>> +        for header in start_dir.ant_glob("**/" + headers[1]):
>>>
>>> Do we always want to copy all the files?
>>>
>>
>> What do you mean by "all the files"? As far as I understood, only the
>> files that are changed should be copied by waf.
>>
>> Or do you mean the ant_glob? In that case: I modeled that after the
>> install target of the headers. 
> 
> Should this be an ant_glob or a list? I am wondering about needing to copy a
> selected number of files from a group a glob would catch. If this is not
> happening that is fine, it can be added if the situation arises.
> 

Oh, I think I start to understand what you mean. Like already said, this
works around the problem that the headers are not installed when the
sources are built. So it should definitely should copy the same files
like the install part that also uses that array.

It is possible to add a glob which catches only a single file (even
better if we move the "**" into builder.py). You might have noted that
Sichen had to add a lot of such lines for openssl. It would be nice to
have some cleaner method of writing that but I haven't found one yet.
Maybe you have an idea?

>> I already suggested in the first
>> discussion regarding the patch that we move the "**" to builder.py
>> instead. I planed to create a extra patch for that.
> 
> I had forgotten and thanks.
> 
>>
>>>> +            relsourcepath = os.path.relpath(str(header), start=str(start_dir))
>>>
>>> Is 'str(header)' really 'header.abspath()' ? See
>>> https://waf.io/apidocs/Node.html#waflib.Node.Node.__str__.
>>>
>>> I prefer the explicit use of .abspath() than the conversion operator.
>>
>> You are right. I didn't take a thorough look at the documentation. I'll
>> rework that part. Maybe I can also use something like "path_from()" or
>> some other waf Node function.
>>
> 
> Nice.
> 
>>>
>>>> +            targetheader = os.path.join(target, relsourcepath)
>>>> +            bld(features = 'subst',
>>>> +                target = targetheader,
>>>> +                source = header,
>>>> +                is_copy = True)
>>>
>>> Does a clean remove these files?
>>
>> Just checked: Yes it does.
>>
> 
> Excellent.
> 
>>>
>>>> +
>>>>      # KVM Symbols
>>>>      bld(target = "rtemsbsd/rtems/rtems-kernel-kvm-symbols.c",
>>>>          source = "rtemsbsd/rtems/generate_kvm_symbols",
>>>> diff --git a/waf_generator.py b/waf_generator.py
>>>> index 35fe35f..fb52250 100755
>>>> --- a/waf_generator.py
>>>> +++ b/waf_generator.py
>>>> @@ -445,6 +445,29 @@ class ModuleManager(builder.ModuleManager):
>>>>          self.add('')
>>>>  
>>>>          #
>>>> +        # Add a copy rule for all headers where the install path and the source
>>>> +        # path are not the same.
>>>> +        #
>>>> +        self.add('    # copy headers if necessary')
>>>> +        headerPaths = builder.headerPaths()
>>>
>>> Can we remove this line and then ....
>>>
>>>> +        self.add('    header_build_copy_paths = [')
>>>> +        for hp in headerPaths:
>>>
>>> .... use:
>>>
>>>            for hp in builder.headerPaths():
>>> ?
>>
>> Should work. I'll try it. Maybe it should be changed on the install part
>> too?
> 
> Sure.
> 
>>>> +            if hp[2] != '' and not hp[0].endswith(hp[2]):
>>>
>>> I am ok with another boolean field being add to tuples rather than needing to
>>> encode this some how if that is useful.
>>
>> I would disagree here. It would be a redundant information in that tuple
>> and therefore allow a invalid line if that boolean is not correctly set.
>> Copying file is only necessary in exactly that case: If the dest paths
>> name is not the same as the local path.
> 
> OK.
> 
>>> If a new field is not added can you please update builder.py with this rule so
>>> we know what to do when adding headers to builder.headerPaths()?
>>
>> You mean some comment that describes that behavior? I can add that also
>> I don't really think that it is necessary. It's just a workaround so
>> that we can build the binaries without having to install the headers
>> before the build.
> 
> It took me a while to see what was happening. The table in builder.py is a long
> way from here so even a note would help.

OK. I'll add a comment that shortly describes what happens with the list
of header files.

> 
>>> Do any of these files copied into the build tree need to be installed so users
>>> can access then? I cannot see an install component.
>>
>> The install component is unchanged. It should still work exactly like
>> before. So all headers should be installed correctly just like before.
> 
> Great and thanks for this change.
> 
> Chris
> 

-- 
--------------------------------------------
embedded brains GmbH
Christian Mauderer
Dornierstr. 4
D-82178 Puchheim
Germany
email: christian.mauderer at embedded-brains.de
Phone: +49-89-18 94 741 - 18
Fax:   +49-89-18 94 741 - 08
PGP: Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.