Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 226299 - Merge Review: pkgconfig
Summary: Merge Review: pkgconfig
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Susi Lehtola
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On: 224148 550470
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 20:40 UTC by Nobody's working on this, feel free to take it
Modified: 2010-08-18 20:27 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-08-18 20:27:01 UTC
susi.lehtola: fedora-review+


Attachments (Terms of Use)
Patch against rawhide spec (deleted)
2010-07-09 19:46 UTC, Susi Lehtola
no flags Details | Diff

Description Nobody's working on this, feel free to take it 2007-01-31 20:40:41 UTC
Fedora Merge Review: pkgconfig

http://cvs.fedora.redhat.com/viewcvs/devel/pkgconfig/
Initial Owner: mclasen@redhat.com

Comment 1 Ralf Corsepius 2007-02-02 06:49:24 UTC
MUSTFIX:

- Invalid debuginfos.
Please remove the strip %{bindir}/* line

- Package ships an aclocal macro
=> Requires: automake

- %makeinstall
Package supports make DESTDIR and therefore should use it.

SHOULD:
- Package wastes time on building shared libs while it only uses the static one
=> Consider %configure ... --disable-shared


Remark:
pkgconfig shipping a local copy of glib and links statically against is
gradually showing its negative sides. The amount of warnings this code emits
proves this code to be showing its age and lets it appear as really error-prone. 
Should pkgconfig support linking against an external glib, this would be preferred.


Comment 2 Matthias Clasen 2007-02-02 07:12:00 UTC
No objections here.

The remark is of course moot, since pkgconfig does not support linking against
an external glib, but I'll point out that you'd have some bootstrapping fun with it,
since glibs configure script uses pkgconfig nowadays.

Comment 3 Ralf Corsepius 2007-02-02 07:29:36 UTC
(In reply to comment #2)
> No objections here.
> 
> The remark is of course moot, since pkgconfig does not support linking against
> an external glib, but I'll point out that you'd have some bootstrapping fun
with it,
> since glibs configure script uses pkgconfig nowadays.
I am well aware about the bootstrapping issue.

But this is nothing which can't be overcome:
1. Build incrementally (this is what glibc, the kernel and GCC do).
2. Make glib a drop in-package to pkgconfig (This is what GCC does with some of
its components)
3. Make building and using pkgconfig's glib an option to pkg-config.



Comment 4 Ville Skyttä 2007-02-02 07:51:00 UTC
(In reply to comment #1)
> - Package ships an aclocal macro
> => Requires: automake

This is strictly speaking correct, but 1) needs to be accompanied by an effort
to make sure that no non-devel packages in the distro have a dependency on
pkgconfig, or 2) the aclocal macro in this package be split to pkgconfig-devel
subpackage.  Otherwise the end result is that a bunch of nontrivial dependencies
will be brought in to regular non-devel setups where they are useless bloat.

Currently examples of non-devel packages requiring pkgconfig include metacity,
gd, gnome-icon-theme and gnome-applets, and I'm sure there's a lot more.

Comment 5 Matthias Clasen 2007-02-02 19:15:58 UTC
We are not going to start adding -devel subpackages to development tool, I assume.

Comment 6 Matthias Clasen 2007-02-02 19:25:08 UTC
pkgconfig-0.21-4.fc7 addresses the concerns except for the automake dependency

Comment 7 Susi Lehtola 2009-05-06 14:09:55 UTC
- Any reason why SMP make is not enabled? (make %{?_smp_mflags})

- Package includes internal glib, which is not allowed. Use BR: glib-devel and --with-installed-glib option to configure.


rpmlint output:
pkgconfig.x86_64: W: devel-file-in-non-devel-package /usr/bin/pkg-config
3 packages and 0 specfiles checked; 0 errors, 1 warnings.

- This is kind of expected.


MUST: The spec file for the package is legible and macros are used consistently. OK
MUST: The package must be named according to the Package Naming Guidelines. OK
MUST: The spec file name must match the base package %{name}. OK
MUST: The package must be licensed with a Fedora approved license and meet the  Licensing Guidelines. OK
MUST: The License field in the package spec file must match the actual license. OK
MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK
MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. N/A

MUST: Optflags are used and time stamps preserved. NEEDSFIX
- Use INSTALL="install -p" as argument to make install.

MUST: Packages containing shared library files must call ldconfig. N/A
MUST: A package must own all directories that it creates or require the package that owns the directory. N/A
MUST: Files only listed once in %files listings. OK
MUST: Debuginfo package is complete. OK

MUST: Permissions on files must be set properly. NEEDSFIX
- %defattr should be (-,root,root,-)

MUST: Clean section exists. OK
MUST: Large documentation files must go in a -doc subpackage. OK

MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. NEEDSFIX
- Add ChangeLog to %doc.

MUST: Header files must be in a -devel package. N/A
MUST: Static libraries must be in a -static package. N/A
MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A
MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. N/A
MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. N/A
MUST: Packages does not contain any .la libtool archives. N/A
MUST: Desktop files are installed properly. N/A
MUST: No file conflicts with other packages and no general names. OK
MUST: Buildroot cleaned before install. OK
SHOULD: %{?dist} tag is used in release. OK
SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK
SHOULD: The package builds in mock. OK

Comment 8 Matthias Clasen 2009-05-06 16:49:53 UTC
> - Package includes internal glib, which is not allowed. Use BR: glib-devel and
> --with-installed-glib option to configure.

Please take a minute to understand why things are the way they are...then you will see that it needs to be this way. Hint: glib is using pkg-config itself.

Comment 9 Ralf Corsepius 2009-05-06 17:47:57 UTC
(In reply to comment #8)
> > - Package includes internal glib, which is not allowed. Use BR: glib-devel and
> > --with-installed-glib option to configure.
> 
> Please take a minute to understand why things are the way they are...then you
> will see that it needs to be this way. Hint: glib is using pkg-config itself.  

Hint: Your argument is a nop.

Comment 10 Susi Lehtola 2009-05-06 19:28:02 UTC
(In reply to comment #8)
> > - Package includes internal glib, which is not allowed. Use BR: glib-devel and
> > --with-installed-glib option to configure.
> 
> Please take a minute to understand why things are the way they are...then you
> will see that it needs to be this way. Hint: glib is using pkg-config itself.  

Once there is a glib package available there's no problem to build against it. This is a cyclic dependency and is nothing new.

Just define a bootstrap variable that chooses between

a) normal case: use glib package from distro

or

b) bootstrap: build with internal glib [then build glib with the newly built pkgconfig and redo build with a)]

You only have to bootstrap once a distribution, and even that you should be able to do with a).

Comment 11 Susi Lehtola 2009-06-14 16:10:40 UTC
ping?

Comment 12 Susi Lehtola 2009-08-05 11:25:35 UTC
ping again?

Comment 13 Susi Lehtola 2010-01-01 22:46:40 UTC
ping 3rd time? Please respond.

Comment 14 Matthias Clasen 2010-07-09 14:58:09 UTC
Not sure what you want from me

Comment 15 Susi Lehtola 2010-07-09 19:46:58 UTC
Created attachment 430747 [details]
Patch against rawhide spec

Suggested patch to fix the remaining issues.

Comment 16 Susi Lehtola 2010-07-09 19:54:04 UTC
Great, so the bundled glib has been removed. rpmlint output now stands at

pkgconfig.src: W: no-cleaning-of-buildroot %clean
pkgconfig.src: W: no-buildroot-tag
pkgconfig.src: W: no-%clean-section
pkgconfig.x86_64: W: incoherent-version-in-changelog 0.25-2 ['1:0.25-2.fc13', '1:0.25-2']
pkgconfig.x86_64: W: devel-file-in-non-devel-package /usr/bin/pkg-config
pkgconfig.x86_64: W: manual-page-warning /usr/share/man/man1/pkg-config.1.gz 58: warning: `I/usr/lib/pkgconfig,' not defined
pkgconfig.x86_64: W: manual-page-warning /usr/share/man/man1/pkg-config.1.gz 60: warning: `I/usr/local/share/pkgconfig' not defined
pkgconfig.x86_64: W: manual-page-warning /usr/share/man/man1/pkg-config.1.gz 154: warning: `I.pc' not defined
pkgconfig.x86_64: W: manual-page-warning /usr/share/man/man1/pkg-config.1.gz 206: warning: `I-Lx:/some/path' not defined
3 packages and 0 specfiles checked; 0 errors, 9 warnings.

Here, the only relevant warning is the incoherent version; the changelog is missing the epoch. You might want to have a look also at the man page stuff.

**

You should still add INSTALL="install -p" to make install, as some non-generated files are losing their time stamps in install.

The defattr stuff was my misunderstanding - the current syntax is also OK.

The ChangeLog reads that it isn't kept up-to-date anymore, so there's no need anymore to add it.

**

The remaining issues are cosmetic, please fix them in the next CVS push and mark this bug as closed. This package has been

APPROVED.

Comment 17 Susi Lehtola 2010-08-18 20:27:01 UTC
Well, I guess this can be closed now...


Note You need to log in before you can comment on or make changes to this bug.