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.maude...@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.
> 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.
>> 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
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
