Bug 13497 - pci.h and libpci pkg not found in FreeBSD
Summary: pci.h and libpci pkg not found in FreeBSD
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/radeonhd (show other bugs)
Version: git
Hardware: x86-64 (AMD64) FreeBSD
: medium normal
Assignee: Luc Verhaegen
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-12-03 07:44 UTC by Coleman Kane
Modified: 2007-12-11 08:11 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Query pkg-config for "libpci" in configure script (1.03 KB, patch)
2007-12-03 07:46 UTC, Coleman Kane
no flags Details | Splinter Review
Patch to rhd_conntest Makefile.am to use the pkg-config substitutions for libpci (569 bytes, patch)
2007-12-03 07:48 UTC, Coleman Kane
no flags Details | Splinter Review

Description Coleman Kane 2007-12-03 07:44:27 UTC
The rhd_conntest utility requires the "development suite" for pciutils to be installed on the system. Under FreeBSD, this is available under ports as devel/libpci, and installs a pkgconfig entry for the "libpci" package. These naturally are installed into the /usr/local prefix on FreeBSD. Since configure.ac never queries the pkgconfig database for libpci, configure cannot find it under FreeBSD unless the -I/usr/local/include for CFLAGS and -L/usr/local/lib for LDFLAGS are explicitly set on the commandline to configure.
Comment 1 Coleman Kane 2007-12-03 07:46:27 UTC
Created attachment 12914 [details] [review]
Query pkg-config for "libpci" in configure script

Attached a patch that will make configure query pkg-config for the libpci package, and fill the PCIUTILS_* variables with appropriate information.
Comment 2 Coleman Kane 2007-12-03 07:48:13 UTC
Created attachment 12915 [details] [review]
Patch to rhd_conntest Makefile.am to use the pkg-config substitutions for libpci

Attached the necessary code to have the Makefile.am generate a Makefile for rhd_conntest that will properly source in the substitutions to build the program if libpci is registered in pkg-config's database.
Comment 3 Hans Ulrich Niedermann 2007-12-03 09:18:39 UTC
(In reply to comment #2)
> Patch to rhd_conntest Makefile.am to use the pkg-config substitutions for
> libpci
> 
> Attached the necessary code to have the Makefile.am generate a Makefile for
> rhd_conntest that will properly source in the substitutions to build the
> program if libpci is registered in pkg-config's database.

-rhd_conntest_LDADD   = -lz -lpci
+rhd_conntest_LDADD   = -lz @PCIUTILS_LIBS@ -lpci

Shouldn't this read

+rhd_conntest_LDADD   = @PCIUTILS_LIBS@

? It is the .pc file's job to know whether or not -lz is needed, and where to find the libraries.

I'd be a bit hesitant to add this to radeonhd proper as long as the pciutils people actually ship their own .pc file, or the community of package creators for all systems agree on what it is supposed to be called like.

If pciutils shipped the .pc file, it would be a no-brainer.

I mean... the CFLAGS/LDFLAGS workaround does work flawlessly, so it's not as if this actually prevented you from building radeonhd.
Comment 4 Coleman Kane 2007-12-03 09:33:21 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > Patch to rhd_conntest Makefile.am to use the pkg-config substitutions for
> > libpci
> > 
> > Attached the necessary code to have the Makefile.am generate a Makefile for
> > rhd_conntest that will properly source in the substitutions to build the
> > program if libpci is registered in pkg-config's database.
> 
> -rhd_conntest_LDADD   = -lz -lpci
> +rhd_conntest_LDADD   = -lz @PCIUTILS_LIBS@ -lpci
> 
> Shouldn't this read
> 
> +rhd_conntest_LDADD   = @PCIUTILS_LIBS@
> 
> ? It is the .pc file's job to know whether or not -lz is needed, and where to
> find the libraries.
> 
> I'd be a bit hesitant to add this to radeonhd proper as long as the pciutils
> people actually ship their own .pc file, or the community of package creators
> for all systems agree on what it is supposed to be called like.
> 
> If pciutils shipped the .pc file, it would be a no-brainer.
> 
> I mean... the CFLAGS/LDFLAGS workaround does work flawlessly, so it's not as if
> this actually prevented you from building radeonhd.
> 

I agree with you, that is how it *should* read. However, the libpci.pc file was only added as of pciutils 2.2.6, and I was trying to avoid breaking the build for people who didn't have this version installed. You'll notice in the configure.ac that I let the configure fall through with only a warning when the pkg-config lookup for "libpci" fails. It then attempts to still use the same method as before to check for pci/pci.h.

If you want to do it now, then go ahead and change the patch to read the more appropriate: 
+rhd_conntest_LDADD   = @PCIUTILS_LIBS@

When the newest pciutils filters its way down, this should work for everybody. This change has already made its way into Gentoo portage.
Comment 5 Hans Ulrich Niedermann 2007-12-03 13:02:56 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)

> > -rhd_conntest_LDADD   = -lz -lpci
> > +rhd_conntest_LDADD   = -lz @PCIUTILS_LIBS@ -lpci
> > 
> > Shouldn't this read
> > 
> > +rhd_conntest_LDADD   = @PCIUTILS_LIBS@

> I agree with you, that is how it *should* read. However, the libpci.pc file was
> only added as of pciutils 2.2.6, and I was trying to avoid breaking the build
> for people who didn't have this version installed. You'll notice in the
> configure.ac that I let the configure fall through with only a warning when the
> pkg-config lookup for "libpci" fails. It then attempts to still use the same
> method as before to check for pci/pci.h.

OK, if that pkg-config libpci check fails, I now just set PCIUTILS_LIBS="-lpci -lz" (like libpci.pc does) and proceed with the compile/link check. Then I'll never get "-lpci -lpci".

This should use libpci.pc if present, and totally fall back to current behaviour if not.

> If you want to do it now, then go ahead and change the patch to read the more
> appropriate: 
> +rhd_conntest_LDADD   = @PCIUTILS_LIBS@
> 
> When the newest pciutils filters its way down, this should work for everybody.
> This change has already made its way into Gentoo portage.
> 

I have submitted your patch and one with my changes to the radeonhd mailing list.
Comment 6 Coleman Kane 2007-12-03 13:13:20 UTC
> 
> I have submitted your patch and one with my changes to the radeonhd mailing
> list.
> 

I saw the patch in the submission message, it looks pretty good to me. Should we also post the final patch here (and obsolete the above(s))?
Comment 7 Luc Verhaegen 2007-12-11 08:11:35 UTC
Committed and pushed,

Thanks a lot guys :)


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.