Bug 226299

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.

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.

(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.

(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.

We are not going to start adding -devel subpackages to development tool, I assume.

pkgconfig-0.21-4.fc7 addresses the concerns except for the automake dependency

- 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

> - 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.

(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.

(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).

ping?

ping again?

ping 3rd time? Please respond.

Not sure what you want from me

Created attachment 430747 [details]
Patch against rawhide spec

Suggested patch to fix the remaining issues.

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.

Well, I guess this can be closed now...