[PATCH 1/3] waf_generator: Copy headers if necessary.
Christian Mauderer
christian.mauderer at embedded-brains.de
Thu Aug 10 05:34:35 UTC 2017
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 are some cases, where a header is installed into a directory with
>> a different name then it's source directory. In that case, the build
>> might fail because the header is not found. One example would be the
>> <openssl/opensslv.h>. The source for this file is in
>> freebsd/crypto/openssl/crypto/opensslv.h.
>>
>> To allow the build to work in such cases too, copy such files into a
>> temporary location in the build tree.
>> ---
>> builder.py | 1 +
>> libbsd_waf.py | 15 +++++++++++++++
>> waf_generator.py | 23 +++++++++++++++++++++++
>> 3 files changed, 39 insertions(+)
>>
>> diff --git a/builder.py b/builder.py
>> index bb633cb..53802c7 100755
>> --- a/builder.py
>> +++ b/builder.py
>> @@ -172,6 +172,7 @@ def commonNoWarnings():
>>
>> def includes():
>> return ['-Irtemsbsd/include',
>> + '-Ilibbsd_build/include',
>
> Is this path under the 'build' directory?
Yes.
>
> Why the 2 directories in the path? Could the path simply be 'build-include' or
> even 'include' ? We know the context because we are looking under the build
> directory.
>
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.
> I would prefer:
>
> def buildInclude():
> return 'libbsd_build/include'
>
> The path can then be referenced in waf_generator.py where needed, for example:
>
> for i in builder.includes() + ['-I' + builder.buildInclude()]:
> ....
>
Agreed. I'll change that.
>> '-Ifreebsd/sys',
>> '-Ifreebsd/sys/contrib/pf',
>> '-Ifreebsd/sys/net',
>> diff --git a/libbsd_waf.py b/libbsd_waf.py
>> index aee2e7a..1784f8b 100644
>> --- a/libbsd_waf.py
>> +++ b/libbsd_waf.py
>> @@ -62,6 +62,7 @@ def build(bld):
>> for i in ['-Irtemsbsd/@CPU@/include', '-Ifreebsd/sys/@CPU@/include']:
>> includes += ["%s" % (i[2:].replace("@CPU@", "x86"))]
>> includes += ["rtemsbsd/include"]
>> + includes += ["libbsd_build/include"]
>> includes += ["freebsd/sys"]
>> includes += ["freebsd/sys/contrib/pf"]
>> includes += ["freebsd/sys/net"]
>> @@ -123,6 +124,20 @@ def build(bld):
>> rule = "sed -e 's/@NET_CFG_SELF_IP@/%s/' -e 's/@NET_CFG_NETMASK@/%s/' -e 's/@NET_CFG_PEER_IP@/%s/' -e 's/@NET_CFG_GATEWAY_IP@/%s/' < ${SRC} > ${TGT}" % (net_cfg_self_ip, net_cfg_netmask, net_cfg_peer_ip, net_cfg_gateway_ip),
>> update_outputs = True)
>>
>> + # copy headers if necessary
>> + header_build_copy_paths = [
>> + ]
>> + for headers in header_build_copy_paths:
>> + target = os.path.join("libbsd_build/include", headers[2])
>> + 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. 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.
>> + 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.
>
>> + 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.
>
>> +
>> # 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?
>
>> + 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.
>
> 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.
>
>> + self.add(' %s,' % (str(hp)))
>> + self.add(' ]')
>> + self.add(' for headers in header_build_copy_paths:')
>> + self.add(' target = os.path.join("libbsd_build/include", headers[2])')
>
> Use builder.buildInclude() rather than hard coding the path.
>
OK.
>> + self.add(' start_dir = bld.path.find_dir(headers[0])')
>> + self.add(' for header in start_dir.ant_glob("**/" + headers[1]):')
>> + self.add(' relsourcepath = os.path.relpath(str(header), start=str(start_dir))')
>> + self.add(' targetheader = os.path.join(target, relsourcepath)')
>> + self.add(' bld(features = \'subst\',')
>> + self.add(' target = targetheader,')
>> + self.add(' source = header,')
>> + self.add(' is_copy = True)')
>> + self.add('')
>> +
>> + #
>> # Add the specific rule based builders for generating files.
>> #
>> if 'KVMSymbols' in data:
>>
>
> 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.
Kind regards
Christian
--
--------------------------------------------
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.
More information about the devel
mailing list