New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP/RFC] elfutils: install library files for pkg-config #1615

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@guidosarducci
Copy link
Contributor

guidosarducci commented Dec 9, 2018

Maintainer: @luizluca
Compile tested: mips32-be-malta, openwrt-18.06 & master branch
Run tested: mips32-be-malta, openwrt-18.06 & master branch
Dev tested: building iproute2 with libelf detection using pkg-config
Dependent PR: #1627 (iproute2)

Description:
Support other packages using pkg-config to query existence and details of
libelf and libdw libraries at build time. This PR is also needed by #1627.

@dedeckeh dedeckeh added the packages label Dec 9, 2018

@luizluca

This comment has been minimized.

Copy link
Contributor

luizluca commented Dec 10, 2018

LGTM

@guidosarducci

This comment has been minimized.

Copy link
Contributor

guidosarducci commented Dec 11, 2018

Great, thanks for the feedback. As far as merging goes, will you do that or is it something to request of @dedeckeh ?

@jow-

This comment has been minimized.

Copy link
Contributor

jow- commented Dec 11, 2018

@guidosarducci - did you check the *.pc files for hardcoded paths?

@guidosarducci

This comment has been minimized.

Copy link
Contributor

guidosarducci commented Dec 11, 2018

@jow- Not sure what aspect of hardcoding paths proves problematic, but the libelf.pc and libdw.pc files resemble most of the *.pc files for other dev packages. Could you elaborate?

For reference, I'll insert the relevant files below in any case. I suppose one could say the TLD is hardcoded.
libelf.pc:

prefix=/usr
exec_prefix=/usr
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Name: libelf
Description: elfutils libelf library to read and write ELF files
Version: 0.174
URL: http://elfutils.org/

Libs: -L${libdir} -lelf
Cflags: -I${includedir}

Requires.private: zlib

libdw.pc:

prefix=/usr
exec_prefix=/usr
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Name: libdw
Description: elfutils library for DWARF data and ELF file or process inspection
Version: 0.174
URL: http://elfutils.org/

Libs: -L${libdir} -ldw
Cflags: -I${includedir}

# We need the exact matching elfutils libelf version since internal data
# structures are used.
Requires: libelf = 0.174

# We support various compressed ELF images, but don't export any of the
# data structures or functions.  zlib (gz) is always required, bzip2 (bz2)
# and lzma (xz) are optional.  But bzip2 doesn't have a pkg-config file.
Requires.private: zlib
Libs.private: -lbz2

Do those look OK?

@jow-

This comment has been minimized.

Copy link
Contributor

jow- commented Dec 11, 2018

@guidosarducci - yep, looks okay. I mainly wanted to make sure that ${exec_prefix} and ${prefix} are used so that our pkg-config wrapper can override them.

@guidosarducci

This comment has been minimized.

Copy link
Contributor

guidosarducci commented Dec 11, 2018

@jow- OK, thanks, that's good to know and I'll note that in the future. Can I assume there aren't any other issues with merging?

BTW, after looking at my <staging_dir/usr/lib/pkgconfig/ directory, I see several instances of absolute paths used in e.g. the Libs: or Libs.private: fields. For example, in libxml-2.0.pc:

prefix=/usr
exec_prefix=/usr
libdir=${exec_prefix}/lib
includedir=${prefix}/include
modules=1

Name: libXML
Version: 2.9.8
Description: libXML library version2.
Requires:
Libs: -L${libdir} -lxml2
Libs.private:   -L/home/kodidev/openwrt-project/staging_dir/target-mips_24kc_musl/usr/lib -lz   -lm
Cflags: -I${includedir}/libxml2

For future reference, these packages appear to have *.pc files with the similar problem of absolute paths:

  • libnetfilter-conntrack: libnetfilter_conntrack.pc
  • libopenssl: libssl.pc, libcrypto.pc
  • libxml2: libxml-2.0.pc
  • libncurses: formw.pc, menuw.pc, ncursesw.pc, panelw.pc
@ldir-EDB0

This comment has been minimized.

Copy link
Contributor

ldir-EDB0 commented Dec 12, 2018

@guidosarducci Hi Tony, have committed this but not closed the PR so we don't lose sight of the other 'absolute path pc' files, well not yet at least :-)

@ldir-EDB0

This comment has been minimized.

Copy link
Contributor

ldir-EDB0 commented Dec 12, 2018

Raised a flyspray about the possible package issues https://bugs.openwrt.org/index.php?do=details&task_id=1997

Now closing this PR

@ldir-EDB0 ldir-EDB0 closed this Dec 12, 2018

@guidosarducci

This comment has been minimized.

Copy link
Contributor

guidosarducci commented Dec 13, 2018

@ldir-EDB0 Thanks, Kevin. Regarding "absolute path" .pc files, there are surely more issues than what I listed, since I only build a subset of available packages for testing iproute2. Is there some easy way to scan the build artifacts from a buildbot and automatically make a comprehensive list of problematic files?

@ldir-EDB0

This comment has been minimized.

Copy link
Contributor

ldir-EDB0 commented Dec 13, 2018

Is there some easy way to scan the build artifacts from a buildbot and automatically make a comprehensive list of problematic files?

Dunno but I agree the buildbots are the obvious place to look.

@ldir-EDB0 ldir-EDB0 reopened this Dec 16, 2018

@ldir-EDB0

This comment has been minimized.

Copy link
Contributor

ldir-EDB0 commented Dec 16, 2018

@guidosarducci Hi Tony, I'm afraid I had to revert your change because of https://forum.openwrt.org/t/unable-to-install-ip-full-ip-tiny/27172 - it appears your PRs are not as independent as I had hoped.....so I'm running away for the moment ;-)

And technically it really should have bumped the package version to force a rebuild.... I should have caught that..sorry.

@guidosarducci

This comment has been minimized.

Copy link
Contributor

guidosarducci commented Dec 17, 2018

Mea culpa, I've been building and testing with all three of my PRs enabled, so didn't catch it either. I think it best to finalize both this PR and #1627 and then committing them together. To that end I've listed the latter as a dependency for this one and marked both as [WIP/RFC] (I'll remove the tag when things are stable and ready for merging).

I'll bump the package release too.

@guidosarducci guidosarducci changed the title elfutils: install library files for pkg-config [WIP/RFC] elfutils: install library files for pkg-config Dec 17, 2018

elfutils: install library files for pkg-config
Support other packages using pkg-config to query existence and details of
libelf and libdw libraries at build time. Also bump package release.

Note: since upstream iproute2 checks for libelf using pkg-config at build
time, that package needs an update for proper libelf handling (PR #1627).

Signed-off-by: Tony Ambardar <itugrok@yahoo.com>

@guidosarducci guidosarducci force-pushed the guidosarducci:master-elfutils-pkgconfig branch from 7ab57cf to 3d59204 Dec 17, 2018

@ldir-EDB0

This comment has been minimized.

Copy link
Contributor

ldir-EDB0 commented Dec 19, 2018

Closed as it has been merged... again :-)

@ldir-EDB0 ldir-EDB0 closed this Dec 19, 2018

@guidosarducci

This comment has been minimized.

Copy link
Contributor

guidosarducci commented Dec 20, 2018

Closed as it has been merged... again :-)

Hmm. Merged again... after I remarked/described this as a "WIP" and explicitly listed the related PR, specifically to forestall merging while allowing progress and review. Sigh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment