Bug 226276

Created attachment 147312 [details]
New perl spec

This is a new spec for perl with lots and lots of cleanups. The review below is
against this spec.

Created attachment 147314 [details]
Fixed spec

No, look at this spec. Made one minor fix.

Created attachment 147701 [details]
Reviewed perl spec file

This spec file is the one I am doing the review against.

Review:

This review is against the attached spec file (id=147701). I rewrote the spec
file, cleaning up lots of ancient cruft.

Good:

- rpmlint checks return:
W: perl devel-file-in-non-devel-package
/usr/lib/perl5/5.8.8/i386-linux-thread-multi/auto/DynaLoader/DynaLoader.a

(should be safe to ignore, I'm pretty sure the base perl needs this)

W: perl siteperl-in-perl-module
/usr/lib/perl5/site_perl/5.8.6/i386-linux-thread-multi/auto
W: perl siteperl-in-perl-module
/usr/lib/perl5/site_perl/5.8.6/i386-linux-thread-multi
W: perl siteperl-in-perl-module
/usr/lib/perl5/site_perl/5.8.8/i386-linux-thread-multi
W: perl siteperl-in-perl-module
/usr/lib/perl5/site_perl/5.8.7/i386-linux-thread-multi/auto
W: perl siteperl-in-perl-module
/usr/lib/perl5/site_perl/5.8.5/i386-linux-thread-multi
W: perl siteperl-in-perl-module
/usr/lib/perl5/site_perl/5.8.8/i386-linux-thread-multi/auto
W: perl siteperl-in-perl-module
/usr/lib/perl5/site_perl/5.8.5/i386-linux-thread-multi/auto
W: perl siteperl-in-perl-module
/usr/lib/perl5/site_perl/5.8.7/i386-linux-thread-multi
W: perl siteperl-in-perl-module /usr/lib/perl5/site_perl/5.8.6
W: perl siteperl-in-perl-module /usr/lib/perl5/site_perl/5.8.7
W: perl siteperl-in-perl-module /usr/lib/perl5/site_perl/5.8.5
W: perl siteperl-in-perl-module /usr/lib/perl5/site_perl/5.8.8

Since this is perl, not a perl-module, these are all safe to ignore.

W: perl-devel no-documentation
W: perl-suidperl no-documentation

Safe to ignore.

E: perl-suidperl setuid-binary /usr/bin/sperl5.8.8 root 04711
E: perl-suidperl non-standard-executable-perm /usr/bin/sperl5.8.8 04711
E: perl-suidperl non-standard-executable-perm /usr/bin/sperl5.8.8 04711

Since this is suidperl, it will be setuid and have non-standard permissions.
Safe to ignore.

W: perl strange-permission filter-depends.sh 0775

Safe to ignore.

W: perl unversioned-explicit-provides perl(VMS::Filespec)
W: perl unversioned-explicit-provides perl(VMS::Stdio)
W: perl unversioned-explicit-provides perl(:MODULE_COMPAT_5.8.5)
W: perl unversioned-explicit-provides perl(:MODULE_COMPAT_5.8.6)
W: perl unversioned-explicit-provides perl(:MODULE_COMPAT_5.8.7)
W: perl unversioned-explicit-provides perl(:MODULE_COMPAT_5.8.8)
W: perl unversioned-explicit-provides perl(:WITH_ITHREADS)
W: perl unversioned-explicit-provides perl(:WITH_THREADS)
W: perl unversioned-explicit-provides perl(:WITH_LARGEFILES)
W: perl unversioned-explicit-provides perl(:WITH_PERLIO)

These provides don't merit versions, IMHO. Safe to ignore.

W: perl unversioned-explicit-provides perl(abbrev.pl)
W: perl unversioned-explicit-provides perl(assert.pl)
W: perl unversioned-explicit-provides perl(bigfloat.pl)
W: perl unversioned-explicit-provides perl(bigint.pl)
W: perl unversioned-explicit-provides perl(bigrat.pl)
W: perl unversioned-explicit-provides perl(bytes_heavy.pl)
W: perl unversioned-explicit-provides perl(cacheout.pl)
W: perl unversioned-explicit-provides perl(complete.pl)
W: perl unversioned-explicit-provides perl(ctime.pl)
W: perl unversioned-explicit-provides perl(dotsh.pl)
W: perl unversioned-explicit-provides perl(dumpvar.pl)
W: perl unversioned-explicit-provides perl(exceptions.pl)
W: perl unversioned-explicit-provides perl(fastcwd.pl)
W: perl unversioned-explicit-provides perl(find.pl)
W: perl unversioned-explicit-provides perl(finddepth.pl)
W: perl unversioned-explicit-provides perl(flush.pl)
W: perl unversioned-explicit-provides perl(ftp.pl)
W: perl unversioned-explicit-provides perl(getcwd.pl)
W: perl unversioned-explicit-provides perl(getopt.pl)
W: perl unversioned-explicit-provides perl(getopts.pl)
W: perl unversioned-explicit-provides perl(hostname.pl)
W: perl unversioned-explicit-provides perl(importenv.pl)
W: perl unversioned-explicit-provides perl(look.pl)
W: perl unversioned-explicit-provides perl(newgetopt.pl)
W: perl unversioned-explicit-provides perl(open2.pl)
W: perl unversioned-explicit-provides perl(open3.pl)
W: perl unversioned-explicit-provides perl(perl5db.pl)
W: perl unversioned-explicit-provides perl(pwd.pl)
W: perl unversioned-explicit-provides perl(shellwords.pl)
W: perl unversioned-explicit-provides perl(stat.pl)
W: perl unversioned-explicit-provides perl(syslog.pl)
W: perl unversioned-explicit-provides perl(tainted.pl)
W: perl unversioned-explicit-provides perl(termcap.pl)
W: perl unversioned-explicit-provides perl(timelocal.pl)
W: perl unversioned-explicit-provides perl(utf8_heavy.pl)
W: perl unversioned-explicit-provides perl(validate.pl)

These are all "file" provides. They don't have real versions, per se. IMHO, safe
to ignore.

W: perl unversioned-explicit-provides perl(Carp::Heavy)

Same as above, no real versioning here. Safe to ignore

W: perl unversioned-explicit-obsoletes perl-Filter-Simple
W: perl unversioned-explicit-obsoletes perl-Time-HiRes

These obsoletes were last seen in FC-4. The perl package has versioned provides.
Safe to ignore, IMHO.

W: perl patch-not-applied Patch39: perl-5.8.8-bz204679.patch

This patch isn't done yet. That's why its not applied. The spec file is marked
to reflect this. Safe to ignore.

- package meets naming guidelines
- package meets packaging guidelines
- license (Artistic or GPL) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- %check ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file
- devel package ok
- no .la files 
- devel requires base package n-v-r
- debuginfo package has valid content

Please make sure you're comfortable with this new packaging spec file, and if
so, commit it, build it in rawhide. Once that's done, I'll approve this package.

perl-5.8.8-14.fc7 built - this is with your specfile, plus a couple of changes
to fix paths on 64-bit builds, and comments for the various patches reflecting
your work moving them upstream.

I'll attach a diff of the changes I had to make to get it to build on 64bit -
several files end up in /usr/lib/<foo> instead of %{_libdir}/<foo> when
'_libdir' means /usr/lib64 - essentially 'noarch' stuff.  It's a big pain, I
wish I knew a better way to do that.

Created attachment 148936 [details]
diff between spot's spec file and the one that builds on 64bit

Oh, and today I'm going to look at the patches that couldn't be sent upstream
and the one that wasn't done yet.

Moving *only* the headers into a -devel subpackage seems half baked, it breaks a
bunch of builds, yet the perl package itself still contains bags of stuff only
useful for building Perl C extensions: c2ph, half of MakeMaker, ... I'm sure
there's more.

The *-devel split-out seems to break building of all perl-modules.

IMO, it's too late in F7's release cycle for such a massive change.

I'd recommend you to reconsider your decision.

If it's really only a mass rebuild which includes the perl-devel to _all_ perl 
modules, this should be handable like in the past. Announce to the maintainers 
to fix their packages until a specified date, otherwise one mass-fixing and 
rebuild will be done. IMHO this should be possible for the Core packages, too. 
But from what I read now, we're not sure what has to be moved to -devel, or did 
I miss something?

(In reply to comment #10)
> If it's really only a mass rebuild which includes the perl-devel to _all_ perl 
> modules,
I am not sure if it's all, but all of my today's perl-module builds fail.

> this should be handable like in the past.
We are talking about 100s of packages ...

> Announce to the maintainers 
> to fix their packages until a specified date, otherwise one mass-fixing and 
> rebuild will be done. IMHO this should be possible for the Core packages, too. 
> But from what I read now, we're not sure what has to be moved to -devel,
> or did 
>
> I miss something?
This at least matchew with how I understand the current situation.

To me, the question boils down to: Is the decision on to have perl-devel
firm/fix wrt. FC7 or not.

AFAIU, it isn't - It's an accident.

(In reply to comment #9)
> The *-devel split-out seems to break building of all perl-modules.
This happened not only to you, but also to me..
> 
> IMO, it's too late in F7's release cycle for such a massive change.
+1

No strong opinions on whether to roll the -devel split back now or not, but
there are some other rebuild-requiring changes that would be nice to have in the
future - move architecture independent lib dirs (core, vendor) to /usr/share and
all of site stuff to /usr/local/(lib(64)|share|man|bin).  Perhaps save the
-devel split and all of the additional changes to until after F7 so it all could
be taken care of in one swoop?

Alright, let's discuss this change on fedora-perl-devel and make the call by Monday.

See:
https://www.redhat.com/archives/fedora-perl-devel-list/2007-March/msg00009.html
and replies.

Just talked with Robin.  Proposed to make this this transition smoother in the
future is to add "Provides: perl-devel" to the older versions of the perl
package when perl is updated in those distros.

(In reply to comment #15)
> Just talked with Robin.  Proposed to make this this transition smoother in the
> future is to add "Provides: perl-devel" to the older versions of the perl
> package when perl is updated in those distros.

This only makes sense, if the current perl-devel is "correct".

AFAICT so far, this is not the case, the perl/perl-devel split is broken,
because is contains a circular dependency between perl/perl-devel through
ExtUtils::MakeMaker

I agree with Ralf.  A -devel split makes sense iff someone can come up with a
sensible definition for what goes into -devel, but as-is it just look like a
knee-jerk reaction to an rpmlint warning.

As the person who wrote the new spec file, I can assure you that it was not a
knee-jerk reaction to an rpmlint warning. The packaging guidelines are rather
clear about when a package needs a -devel, and perl needed one.

Now, in the same breath, I'm more than willing to cede that there are some bits
missing in -devel. All of ExtUtils::MakeMaker needs to move over, for example
(I'm not sure we can put it in a separate package, not tested whether perl can
build without a local copy).

A knee-jerk reaction to a packaging guideline has the same effect :)

CPAN depends on MakeMaker.  Will CPAN be moved to -devel too?

AND... for the record, it was not a knee-jerk reaction to a packaging guideline
either:

https://www.redhat.com/archives/fedora-packaging/2007-February/msg00025.html

I proposed it and no one else on the Packaging Committee was on board with
permitting perl to be an exception case. So, here we are. Are you done with
"knee-jerking"?

(In reply to comment #20)
> https://www.redhat.com/archives/fedora-packaging/2007-February/msg00025.html
> 
> I proposed it and no one else on the Packaging Committee was on board with
> permitting perl to be an exception case.

I read the replies to the above message pretty much exactly the opposite - I
don't see anyone being explicitly against it.  rdieter and tibbs were explicitly
on board, and myself, f13 and thimm more or less without an opinion formed at
that point (FWIW, I still haven't, but the clock is ticking).

https://www.redhat.com/archives/fedora-packaging/2007-February/msg00027.html
https://www.redhat.com/archives/fedora-packaging/2007-February/msg00029.html
https://www.redhat.com/archives/fedora-packaging/2007-February/msg00038.html
https://www.redhat.com/archives/fedora-packaging/2007-February/msg00039.html
https://www.redhat.com/archives/fedora-packaging/2007-February/msg00047.html

There were also a few non-FPC member comments to the suggestion, slightly
leaning towards being leaving things as is.

Looking at 5.8.8-16 in CVS, although not required in the strictest sense, I
don't think it would be a bad idea to add a manual "Requires:
perl(Test::Harness)" to perl-ExtUtils-MakeMaker.

That dependency is not automatically found by rpmbuild (see test_harness() in
ExtUtils/Command/MM.pm), but is a feature that is indirectly used by a lot of
module packages - with "make test", ExtUtils::MakeMaker runs t/*.t tests using
Test::Harness which is prominently documented in the ExtUtils::MakeMaker man page.

MUSTFIX: The *-16 in CVS still suffers from the "broken epoch" bug.

ok, added the Requires that Ville recommends in comment #22, and (I believe)
fixed the epoch issue.  The version is now 16.2.

Robin: The rationale behind the "Provides: cpan" escapes me.

Packages wanting to use perl(CPAN) should require "perl(CPAN)", 
packages wanting to use /usr/bin/cpan should directly depend on /usr/bin/cpan
(which would be the only correct solution) or can "Requires: perl-CPAN".

If your intention is to provide a "virtual cpan package", then it should be a
versioned "Provides".

I would not add "Provides: cpan".

x86_64 build of 16.2 fails, looks like a %{_prefix}/lib vs %{_libdir} mixup:

RPM build errors:
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/Embed.pm
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/Command
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/Install.pm
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/Installed.pm
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/Liblist
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/Liblist.pm
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/MakeMaker
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/MakeMaker.pm
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/MANIFEST.SKIP
    File not found by glob:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/MM*.pm
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/MY.pm
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/Manifest.pm
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/Mkbootstrap.pm
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/Mksymlists.pm
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/NOTES
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/Packlist.pm
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/PATCHING
    File not found:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/ExtUtils/testlib.pm
    File not found by glob:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/Test/Harness*
    File not found by glob:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/Test/More*
    File not found by glob:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/Test/Builder*
    File not found by glob:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/Test/Simple*
    File not found by glob:
/var/tmp/perl-5.8.8-16.2.cmn6-root-machbuild/usr/lib64/perl5/5.8.8/Test/Tutorial*

The latest (5.8.8-16.3) fixes the issues described in comment #25 and comment #26:

Now the perl-CPAN package Provides: cpan-<version>, and the %{libdir} issues
should be fixed.

Created attachment 153388 [details]
x86_64 build fix for 16.3

The %{_libdir} issues actually became worse - the %exclude's were correct in
16.2 and the inclusions incorrect, but 16.3 made the %excludes incorrect too. 
The attached patch fixes the build for me on FC6 x86_64, resulting packages
untested.

"Provides: cpan-%{version}" looks odd, was it meant to be "Provides: cpan =
%{version}"?

argh, yes, I managed to get both 'fixes' exactly wrong, somehow.  Thanks.

*** Bug 237564 has been marked as a duplicate of this bug. ***

IMO, this perl-libs package (the libperl.so split out) is completely useless and
doesn't solve anything, because the main perl package is i386 arch'ed and filled
with i386 deps.

I strongly recommend to revert this change.

If you want real progress, split out %{_bindir}/* and move everything else into
perl-libs.

Ralf,

The release team disagrees, I'm afraid, and thinks this is the way to go for F7.
 They do, however, 'promise', to fix multilib for F8, and then we can de-hack perl:

https://www.redhat.com/archives/fedora-maintainers/2007-April/msg00462.html

(In reply to comment #32)
> Ralf,
> 
> The release team disagrees, I'm afraid, 
Well, ... the release team doesn't want to know what I think about this and them.

> and thinks this is the way to go for F7.

"perl" still contains TONS of i386 binaries, TONS of i386-paths.

=> You aren't fixing anything by splitting out libperl ... but probably to work
around one of the many bugs inside of rpm and yum.

While we're at it: FC6 yum still suffers from the "not being able to resolve 
perl(ExtUtils::MakeMaker) bug when an old monolytic "perl" is in one of the repos.

Yes, I agree this is certainly a workaround, not a 'fix' in any way.  No
argument there, at all.  But it's the path the release team has picked so far. 
Honestly, I don't understand multilib deeply enough by any means to debate
meaningfully.  If you can change the release team's mind, I'll be more than
happy to revert the change.

(In reply to comment #34)

> Honestly, I don't understand multilib
I implemented and am maintaining many of them in GCC ;)

> deeply enough by any means to debate
> meaningfully. 
I don't think they fixed multilibs (The arch dependent perl paths do not
intersect), but work around a the bug in rpm which causes it to insert bogus
Provides/Requires: libperl.so(XXXX)
into the rpm.

(In reply to comment #31)
> IMO, this perl-libs package (the libperl.so split out) is completely useless
> and doesn't solve anything, because the main perl package is i386 arch'ed and
> filled with i386 deps.
> 
> I strongly recommend to revert this change.

Since the debate above is still unclear:

Some of my packages can't be rpmbuilt properly till the end. At %files they
fails with 
/usr/bin/perl: error while loading shared libraries: libperl.so: cannot open
shared object file: No such file or directory
getOutputFrom(): Broken pipe

So since libperl.so is in perl-libs (on moonshine), will perl-libs be a
dependency on perl or will it be strictly independent of perl ?

(In reply to comment #36)
> So since libperl.so is in perl-libs (on moonshine), will perl-libs be a
> dependency on perl or will it be strictly independent of perl?

perl requires perl-libs.
Or, strictly speaking, perl requires libperl.so which is provided by perl-libs.

If your system contains perl.rpm but nor perl-libs.rpm, your dependencies are
broken.  It might be beacuse of a bug in yum, see bug #240540.

This one has been done for some time now. Setting the review flag to + and closing this bug out.

Package Change Request
======================
Package Name: perl
New Branches: F-12

cvs done.

Package Change Request
======================
Package Name: perl
New Branches: 
Owners: 
InitialCC: perl-sig

Please add perl-sig with watch* permissions only to all Fedora branches.

Done.