Bug 225758

MUSTFIX:

- package must not own
/usr/share/locale/*
and
/usr/share/locale/*/LC_MESSAGES

- spec should use %find_lang

Fixed in rawhide.

Btw, rpmlint output:
W: flex devel-file-in-non-devel-package /usr/lib/libfl.a
W: flex devel-file-in-non-devel-package /usr/include/FlexLexer.h

flex is itself a development package, thus FlexLexer.h is OK here. libfl.a has
to be static for the same reason, so that scanners that are linked against it
don't runtime-depend on flex.

(In reply to comment #2)
> Fixed in rawhide.
Hmm? Either you forgot to commit your, or the public cvs server is a different
one the one you are accessing (and has not sync'ed yet).

It is NOT fixed in 
cvs.fedora.redhat.com:/cvs/dist/devel/flex

> Btw, rpmlint output:
> W: flex devel-file-in-non-devel-package /usr/lib/libfl.a
> W: flex devel-file-in-non-devel-package /usr/include/FlexLexer.h
That's OK with me.

Aha, the CVS that's accessible from outside is a read-only copy of Red Hat's
CVS. I don't know how often do they sync.

The above indicates the package passed the review.  Closing.

I don't see an APPROVED or the fedora-review flag set to '+'.  Ralf, were you
actually reviewing this?  If not, I can do a quick runthrough.

(In reply to comment #6)
> I don't see an APPROVED or the fedora-review flag set to '+'.  Ralf, were you
> actually reviewing this?
Technically yes, formally no.

Sorry, my bad.

OK, this package is pretty much OK but I just want to run the static library by
FESCo so that these merge reviews get the same treatment that any other package
would get.  Some other comments:

Source: should be a URL, probably
  http://dl.sf.net/flex/flex-%{version}.tar.bz2

This package has a build-time dependency on "info" but I don't see why a
text-mode info browser would be useful for the build process.

I'll get back as soon as FESCo has a chance to discuss the static library issue.

* source files match upstream:
   53b56a62ea9409b99b7a0ac4a5204fac16ca7eaf39b9374164c346d6badc6914  
   flex-2.5.33.tar.bz2
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
? Not sure what BuildRequires: info is for.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* debuginfo package looks complete.
O rpmlint warnings are OK (static library OK pending FESCo ack).
* final provides and requires are sane:
   flex = 2.5.33-7.fc7
  =
   /bin/sh
   /sbin/install-info
   m4
* %check is present and all tests pass:
   Tests succeeded: 40
   Tests FAILED: 0
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets are OK (install-info)
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* headers present, in base package because this package is only useful for 
  development.
* no pkgconfig files.
? static libraries present; FESCo ack pending.
* no libtool .la droppings.

* compiler flags are appropriate.
gcc ... -I/usr/include ...

This -I/usr/include is a bug.

You hadn't mentioned that before.  Please explain for the record, thanks.

Done so many times before ;)

1. /usr/include the system header directory and is on a compiler's internal
include file search path. It should never be explicitly passed. Packages
requiring -I/usr/include are broken "by definition" and are likely to be badly
designed.

2. In this particular case, the culprit is toplevel Makefile.am appending
@includedir@ to AM_CPPFLAGS:

AM_CPPFLAGS = ... -I@includedir@ 

This is abuse of the autotools - @includedir@ is a destination directory (a
directory where files are supposed to be installed to).

But, pedantry aside, can you point to any problems that this causes which are
worse than the possible fixes (of patching it out of Makefile.am and running the
autotools or patching it out of the generated Makefile)?  Because in the absense
of any actual breakage, I'm inclined to simply suggest that this issue be
reported to upstream for fixing there.

(In reply to comment #13)
> But, pedantry aside, 
Pedantry? This is a stupid upstream bug, this package's maintainer (@RH) should
contact upstream to have it fixed.

> can you point to any problems that this causes which are
> worse than the possible fixes
Install an alternative GCC to /usr/local, cross-build the package and building
this package with go up in smoke.

> (of patching it out of Makefile.am and running the
> autotools or patching it out of the generated Makefile)?  Because in the absense
> of any actual breakage, I'm inclined to simply suggest that this issue be
> reported to upstream for fixing there.
Right, it doesn't affect building the rpm inside of the buildsystem. But it can
affect users rebuilding the package.

(In reply to comment #14)
> Pedantry?

Yes, indeed, pedantry.

> This is a stupid upstream bug, this package's maintainer (@RH) should
> contact upstream to have it fixed.

Gee, that's exactly what I wrote.  Thanks for backing me up!

I'll approve this package once FESCo acks the static library issue.

After some disussion with other FPC folks, I realized that recent changes to the
static library packaging guidelines require only that linking against static
libraries receive a FESCo ack.  Which means we'll have to look at getting some
sort of blanket exception for flex-using applications, but which means that this
package is fine.

One additional thing I noticed which should be fixed: the package isn't built
with parallel make.  Is there a reason why it isn't?  I built on an 8-way
machine with parallel make turned on and it went fine; the build completed and
the test suite passed with no problems.  But if there is a reason why parallel
make isn't supported, it needs to be commented in the spec.

Really, that's all; I'll trust you to fix up the make bit.

APPROVED

(In reply to comment #16)
> After some disussion with other FPC folks, I realized that recent changes to the
> static library packaging guidelines require only that linking against static
> libraries receive a FESCo ack.  Which means we'll have to look at getting some
> sort of blanket exception for flex-using applications,
You should understand that flex package is devel package and must contain a
libfl* to work correctly.

However, in libfl* is such kind of trivial, that it hasn't seen any changes for
years and is very unlikely to see any changes ever. Furthermore, libfl is almost
never used by real world applications, because they typically replace it.

I.e. the correct way to ship flex is to package "a libfl*" into flex. If it's
libfl.a, so be it.

I cleaned up the spec per comments.  New version was built and the push is
pending.  I checked that upstream doesn't have the @include@ problem, perhaps it
was reported by someone already.

flex-2.5.33-9.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.