On Fri, 29 Sep 2023 10:44:49 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:
> Nevertheless, I believe the .libs issue needs to be addressed as well, > otherwise this will be a regression for older versions of binutils that are > in a non-build installed location. > > That is, you need to break out the check for `libbfd.a`, and look for it > first in `bfd` and secondly in `bfd/.libs`. All version I checked (2.30, 2.34, 2.38, 2.39, 2.40, 2.41, current tip) all libs are in .libs. And make install copy them from the .lib folder. (in older versions also) `libtool: install: /usr/bin/install -c .libs/libbfd.a /home/rehn/source/gdb/binutils-gdb/install/lib/libbfd.a` Arguably we should only use .libs directory, > make/autoconf/lib-hsdis.m4 line 259: > >> 257: # If we have libsframe add it. >> 258: if test -e $BINUTILS_DIR/libsframe/.libs/libsframe.a; then >> 259: HSDIS_LIBS="$BINUTILS_DIR/bfd/.libs/libbfd.a >> $BINUTILS_DIR/opcodes/libopcodes.a $BINUTILS_DIR/libiberty/libiberty.a >> $BINUTILS_DIR/zlib/libz.a $BINUTILS_DIR/libsframe/.libs/libsframe.a" > > Just append the additional lib. > > Suggestion: > > HSDIS_LIBS="$HSDIS_LIBS $BINUTILS_DIR/libsframe/.libs/libsframe.a" Fixed > make/autoconf/lib-hsdis.m4 line 266: > >> 264: AC_MSG_CHECKING([Checking binutils API]) >> 265: if test -n "$BINUTILS_SRC"; then >> 266: include_dis="#include \"$BINUTILS_SRC/include/dis-asm.h\"" > > All this quoting and duplication seems quite messy. Can't you just set e.g. > `binutils_include_dir` to either `%BINUTILS_SRC/include` or > `%BINUTILS_DIR/include`, and then just do > > > AC_LANG_PROGRAM([#include "$binutils_include_dir/dis-asm.h"], ... > > ? > > (Code written in github editor; warning for quoting mistakes etc) I can check. No really my usual place in the world doing autoconf :) ------------- PR Comment: https://git.openjdk.org/jdk/pull/15138#issuecomment-1740757681 PR Review Comment: https://git.openjdk.org/jdk/pull/15138#discussion_r1341302077 PR Review Comment: https://git.openjdk.org/jdk/pull/15138#discussion_r1341272088