RFR: 8277089: Use system binutils to build hsdis

Yasumasa Suenaga ysuenaga at openjdk.java.net
Mon Nov 15 13:55:35 UTC 2021


On Mon, 15 Nov 2021 10:23:48 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:

>> hsdis requires binutils source tree for building. Most of Linux distros provide binutils package. (e.g. binutils-devel from Fedora, binutils-dev from Ubuntu)
>> It would be nice to be able to use them like zlib and lcms.
>> 
>> Unfortunately bfdver.h would not be provided because it is not included install files (`make install`) in binutils. So I changed to use `SEC_ELF_OCTETS` macro to detect binutils version because it was introduced at the same time as `bfd_octets_per_byte()`.
>> 
>> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;f=bfd/bfd-in2.h;h=618265039f697eab9e72bb58b95fc2d32925df58
>> 
>> Please see [JDK-8244819](https://bugs.openjdk.java.net/browse/JDK-8244819) why we need version check.
>
> The basic idea is fine. I also think checking for `SEC_ELF_OCTETS` in the source code, instead of the version number, is actually an improvement.
> 
> The one thing that itches me a bit is what happens when you specify `--with-binutils=system` and a dependent library is not found:
> 
> 
>   AC_CHECK_LIB(iberty, xmalloc, [ HSDIS_LIBS="$HSDIS_LIBS -liberty" ], [ AC_MSG_ERROR([libiberty not found]) ])
> 
> 
> Then the build will fail with no clear indication on why. Instead, I'd recommend that you restructure slightly.
> 
> First check if with_binutils is system. If so, run your lib checks but like this:
> 
>   AC_CHECK_LIB(iberty, xmalloc, [ HSDIS_LIBS="$HSDIS_LIBS -liberty" ], [ bintils_system_error="libiberty not found" ])
> 
> 
> Then you go check the value of with_binutils again in the "switch" statement. And you can replace `AC_MSG_CHECKING` outside the switch statement again. If it is system, you check if `bintils_system_error` is non-empty. If so, you fail and explain that this-and-this error prevented system from working.
> 
> What distributions have you tested this on?

@magicus Thanks for your comment! I refactored this change. Could you review again?

> What distributions have you tested this on?

I tested this change on Fedora 35, and WSL Ubuntu 20.04 for Windows.

-------------

PR: https://git.openjdk.java.net/jdk/pull/6378



More information about the build-dev mailing list