1375798 - [cross-osx] ERROR: Could not find the clang shared library in the path /home/worker/workspace/build/src/clang/lib

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

Cross OSX builds end with:

[task 2017-06-23T09:02:55.406134Z] 09:02:55     INFO -  checking for llvm-config... /home/worker/workspace/build/src/clang/bin/llvm-config
[task 2017-06-23T09:02:55.413715Z] 09:02:55     INFO -  ERROR: Could not find the clang shared library in the path /home/worker/workspace/build/src/clang/lib
[task 2017-06-23T09:02:55.413877Z] 09:02:55     INFO -  returned by `llvm-config --libdir` (searched for files [u'libclang.so.1', u'libclang.dylib']).

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1375798 - Add a unit test for library/object prefix/suffixes.

This happen to uncover the fact that mingw clang was not handled
properly after bug 1372987. This will allow us to substantially modify
the function that handles them and avoid regressions.

Review commit: https://reviewboard.mozilla.org/r/152132/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/152132/

Comment 2

[Mike Hommey [:glandium]]

Attachment: Bug 1375798 - Limit the SO_VERSION option to openbsd builds.

SO_VERSION has only an impact on those builds. Also set the default at
the option level, instead of doing that in the library_name_info
function.

Review commit: https://reviewboard.mozilla.org/r/152134/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/152134/

Comment 3

[Mike Hommey [:glandium]]

Attachment: Bug 1375798 - Reorganize the library_name_info function.

The function as it currently is matches how things were done in
old-configure.in. However, that's just confusing and hard to follow. In
fact, the unit test failing numerous times while writing this patch
pretty much highlights the problem.

So instead of a confusing set of overrides to the prefixes and suffixes,
spell out the whole set for each set of platforms. This also happens to
make the function shorter. Win/win.

At the same time, we normalize the function output as a nested
namespace, where we get, for each of dll, lib, import_lib, etc. a
prefix/suffix pair. Further down the road, we can imagine changing those
to class instances with a method allowing to format file names based on
those prefix/suffixes.

Review commit: https://reviewboard.mozilla.org/r/152136/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/152136/

Comment 4

[Mike Hommey [:glandium]]

Attachment: Bug 1375798 - Avoid using target_is_windows and target_is_darwin in library_name_info.

We eventually want to templatize the library_name_info function, in
order to have one for host and one for target. So we don't want 3
different dependencies for the target.

Review commit: https://reviewboard.mozilla.org/r/152138/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/152138/

Comment 5

[Mike Hommey [:glandium]]

Attachment: Bug 1375798 - Figure out host library/object prefix/suffixes and use them for libclang.

The libclang test wants to find a libclang library for use for rust bindgen.
But that's a host process, that needs a host libclang. However, we
currently only have the target library/object prefix/suffixes. This
works fine... except when cross-compiling.

So we need to figure out the proper ones for the host, and use those
instead. For that, we templatize library_name_info in order to get a
separate set of library/object prefix/suffixes for the host and the
target.

And we use the host set for the libclang check.

Ideally, the build system would also use the host set for host tools
builds, but we'll leave that to a followup.

Review commit: https://reviewboard.mozilla.org/r/152140/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/152140/

Comment 6

[Michael Shal [:mshal]]

Comment on attachment 8880760 [details]
Bug 1375798 - Add a unit test for library/object prefix/suffixes.

https://reviewboard.mozilla.org/r/152132/#review157726

Comment 7

[Michael Shal [:mshal]]

Comment on attachment 8880761 [details]
Bug 1375798 - Limit the SO_VERSION option to openbsd builds.

https://reviewboard.mozilla.org/r/152134/#review157878

Comment 8

[Michael Shal [:mshal]]

Comment on attachment 8880762 [details]
Bug 1375798 - Reorganize the library_name_info function.

https://reviewboard.mozilla.org/r/152136/#review157884

Nice! This is much easier to understand.

Comment 9

[Michael Shal [:mshal]]

Comment on attachment 8880763 [details]
Bug 1375798 - Avoid using target_is_windows and target_is_darwin in library_name_info.

https://reviewboard.mozilla.org/r/152138/#review157888

Comment 10

[Michael Shal [:mshal]]

Comment on attachment 8880764 [details]
Bug 1375798 - Figure out host library/object prefix/suffixes and use them for libclang.

https://reviewboard.mozilla.org/r/152140/#review157894

::: moz.configure:147
(Diff revision 1)
>  @depends('SO_VERSION', when=is_openbsd)
>  def so_version(value):
>      return value
>  
> -@depends(target, c_compiler, so_version)
> +@template
> +def library_name_info(host_or_target):

It's a bit confusing to have library_name_info() be:

1) a function
2) a nested function within the first function
3) a variable set to the value of the function.

Can we come up with different names for 1) and 2)?

::: moz.configure:155
(Diff revision 1)
> +        host: host_c_compiler,
> +        target: c_compiler,
> +    }[host_or_target]
> +
> +    @depends(host_or_target, compiler, so_version)
> -def library_name_info(target, c_compiler, so_version):
> +    def library_name_info(target, c_compiler, so_version):

Shouldn't this match the @depends variable names? Eg: 'target' here is really either a host or a target, which makes some of the if statements below read slightly inaccurately (even though it works). So I'd suggest (host_or_target, compiler, so_version)

Comment 11

[Mike Hommey [:glandium]]

Comment on attachment 8880760 [details]
Bug 1375798 - Add a unit test for library/object prefix/suffixes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152132/diff/1-2/

Comment 12

[Mike Hommey [:glandium]]

Comment on attachment 8880761 [details]
Bug 1375798 - Limit the SO_VERSION option to openbsd builds.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152134/diff/1-2/

Comment 13

[Mike Hommey [:glandium]]

Comment on attachment 8880762 [details]
Bug 1375798 - Reorganize the library_name_info function.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152136/diff/1-2/

Comment 14

[Mike Hommey [:glandium]]

Comment on attachment 8880763 [details]
Bug 1375798 - Avoid using target_is_windows and target_is_darwin in library_name_info.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152138/diff/1-2/

Comment 15

[Mike Hommey [:glandium]]

Comment on attachment 8880764 [details]
Bug 1375798 - Figure out host library/object prefix/suffixes and use them for libclang.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152140/diff/1-2/

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0f78019d37dc
Add a unit test for library/object prefix/suffixes. r=mshal
https://hg.mozilla.org/integration/autoland/rev/9e83254e336b
Limit the SO_VERSION option to openbsd builds. r=mshal
https://hg.mozilla.org/integration/autoland/rev/374900b2142a
Reorganize the library_name_info function. r=mshal
https://hg.mozilla.org/integration/autoland/rev/8847981eee44
Avoid using target_is_windows and target_is_darwin in library_name_info. r=mshal
https://hg.mozilla.org/integration/autoland/rev/a07449eb1876
Figure out host library/object prefix/suffixes and use them for libclang. r=mshal

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0f78019d37dc
https://hg.mozilla.org/mozilla-central/rev/9e83254e336b
https://hg.mozilla.org/mozilla-central/rev/374900b2142a
https://hg.mozilla.org/mozilla-central/rev/8847981eee44
https://hg.mozilla.org/mozilla-central/rev/a07449eb1876