Bug 979677 - Review Request: perl-CPANPLUS-Dist-Fedora - CPANPLUS backend to build Fedora/RedHat RPMs
Review Request: perl-CPANPLUS-Dist-Fedora - CPANPLUS backend to build Fedora/...
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Extras Quality Assurance
:
Depends On: 979676
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-29 06:26 EDT by Christopher Meng
Modified: 2013-09-16 08:44 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-09-16 06:03:14 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Christopher Meng 2013-06-29 06:26:00 EDT
Spec URL: http://cicku.me/perl-CPANPLUS-Dist-Fedora.spec
SRPM URL: http://cicku.me/perl-CPANPLUS-Dist-Fedora-0.0.4-1.fc20.src.rpm
Description: This is a distribution class to create Fedora packages from CPAN modules, and all its dependencies. This allows you to have the most recent copies of CPAN modules installed, using your package manager of choice, but without 
having to wait for central repositories to be updated.
Fedora Account System Username: cicku
Comment 1 Parag AN(पराग) 2013-07-22 00:48:52 EDT
unable to download srpm
Comment 2 Christopher Meng 2013-07-22 01:04:38 EDT
You can try again now....
Comment 3 Parag AN(पराग) 2013-07-22 01:51:21 EDT
Review:

+ mock build for f20 is successful

+ rpmlint on rpms gave
perl-CPANPLUS-Dist-Fedora.noarch: W: spelling-error Summary(en_US) backend -> backed, back end, back-end
perl-CPANPLUS-Dist-Fedora.src: W: spelling-error Summary(en_US) backend -> backed, back end, back-end
2 packages and 0 specfiles checked; 0 errors, 2 warnings.
=> Looks good

+ Source verified with upstream as (sha256sum)
srpm tarball: 18dce03a3a49ba34db87b519d0d3d5b4229a01d0ff8e170bd04505ca883c7442
upstream tarball: 18dce03a3a49ba34db87b519d0d3d5b4229a01d0ff8e170bd04505ca883c7442

+ license is valid "GPL+ or Artistic"

+ make test gave
All tests successful.
Files=4, Tests=4,  0 wallclock secs ( 0.03 usr  0.00 sys +  0.31 cusr  0.03 csys =  0.37 CPU)

+ Package perl-CPANPLUS-Dist-Fedora-0.0.4-1.fc20.noarch
Provides: perl(CPANPLUS::Dist::Fedora) = 0.0.4 perl-CPANPLUS-Dist-Fedora = 0.0.4-1.fc20

Requires: perl(CPANPLUS::Dist::Base) perl(CPANPLUS::Error) perl(Cwd) perl(File::Basename) perl(File::Copy) perl(IPC::Cmd) perl(List::Util) perl(POSIX) perl(Pod::POM) perl(Pod::POM::View::Text) perl(Template) perl(Text::Wrap) perl(base) perl(strict) perl(warnings)


Suggestions:
1) BR: perl not needed
2) Let the rpm generate Requires: automatically for binary package. Remove manually written Requires: in spec file.


APPROVED.
Comment 4 Petr Šabata 2013-07-22 04:23:35 EDT
(In reply to Parag AN(पराग) from comment #3)
> 1) BR: perl not needed

Why do you think so?  perl is not guaranteed to be in the minimal buildroot and is called in the spec directly.
Comment 5 Parag AN(पराग) 2013-07-22 05:18:21 EDT
I know its not in minimal buildroot anymore and since long time got habit of asking people to remove BR: perl from spec files if I see them but also at the same time mock build always succeed without BR: perl for such packages.

Should we have something written about this in Perl packaging guidelines?
Comment 6 Petr Pisar 2013-07-22 07:02:06 EDT
Parag, global Fedora guidelines state all dependencies expect a short list have to be specified. There is no need to duplicate the same statement in Perl guidelines.
Comment 7 Petr Šabata 2013-07-22 07:08:17 EDT
(In reply to Parag AN(पराग) from comment #5)
> I know its not in minimal buildroot anymore and since long time got habit of
> asking people to remove BR: perl from spec files if I see them but also at
> the same time mock build always succeed without BR: perl for such packages.
>
> Should we have something written about this in Perl packaging guidelines?

This is nothing Perl specific and I believe the generic Guidelines cover it all:
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#BuildRequires

Anyhow, I believe Perl guidelines could use a few updates here and there, however discussing anything (let along pushing changes) in Perl SIG is quite difficult.
Comment 8 Parag AN(पराग) 2013-07-23 00:48:07 EDT
I can take a reference from above link (given below) which clearly says that if mock succeeds you have all the required BR: in spec to build the package.

"If the build fails or is missing certain features due to missing build dependencies, then the missing dependency needs to be found and added"

So, this falls back to Perl Guidelines to specify that one should add BR: perl
Comment 9 Petr Šabata 2013-07-23 03:49:59 EDT
How about the first paragraph?

"Having proper build requirements saves the time of all developers and testers as well as autobuild systems because they will not need to search for missing build requirements manually. It is also a safety feature that prevents builds with that would not otherwise fail, but would be missing crucial features."

What if I change something in your dependency chain and your build breaks?
Do you prefer to wait until you get a FTBFS to listing things your package actually uses?

I still fail to see how this is perl specific.

This attitude causes so much slowdown every rebuild it's ridiculous.
Comment 10 Parag AN(पराग) 2013-07-23 05:02:04 EDT
(In reply to Petr Šabata from comment #9)
> How about the first paragraph?
> 
> "Having proper build requirements saves the time of all developers and
> testers as well as autobuild systems because they will not need to search
> for missing build requirements manually. It is also a safety feature that
> prevents builds with that would not otherwise fail, but would be missing
> crucial features."

This stands correct if mock build will fail and we add that missing BR: and build will be successful.

> What if I change something in your dependency chain and your build breaks?

In that case we can add manually required missing BR:

> Do you prefer to wait until you get a FTBFS to listing things your package
> actually uses?

We can work on this when there will be FTBFS reported.

 
> I still fail to see how this is perl specific.

I will re-iterate mock is not failing so its with perl guidelines to specify that though a single dependent perl module BR: will bring perl package in buildroot, we need to write it BR: perl in spec files.

> This attitude causes so much slowdown every rebuild it's ridiculous.

Again, Its something that perl guidelines are missing....
Comment 11 Marcela Mašláňová 2013-07-23 07:12:49 EDT
Hello Parag,
the reason why some members of Perl SIG are asking for it is, those members of SIG are running annual rebuild. It would take less than a month if all dependencies were added correctly before the rebuild starts. Packages from main perl come and leave, it's not sure if they will be there in next rebuild and it would be huge help to remove at least these problems.

Yes, you are right, current guidelines are not specifying all dependencies, neither BR: perl. The proposal for updated guidelines [1] was sent weeks ago and maybe it's time to propose it to FPC, because no comments were sent in last week or so.

[1] https://lists.fedoraproject.org/pipermail/perl-devel/2013-July/068827.html

Marcela
Comment 12 Parag AN(पराग) 2013-07-23 09:03:17 EDT
Hi Marcela,
   I understand perl module problems and need to specify all the dependencies in spec file. But all these are still not documented as guidelines. 

I have seen that proposal on perl-devel list and waiting for it to come FPC for discussion.
Comment 13 Petr Šabata 2013-07-23 09:25:10 EDT
(In reply to Parag AN(पराग) from comment #10)
> (In reply to Petr Šabata from comment #9)
> > How about the first paragraph?
> > 
> > "Having proper build requirements saves the time of all developers and
> > testers as well as autobuild systems because they will not need to search
> > for missing build requirements manually. It is also a safety feature that
> > prevents builds with that would not otherwise fail, but would be missing
> > crucial features."
> 
> This stands correct if mock build will fail and we add that missing BR: and
> build will be successful.
> 
> > What if I change something in your dependency chain and your build breaks?
> 
> In that case we can add manually required missing BR:
> 
> > Do you prefer to wait until you get a FTBFS to listing things your package
> > actually uses?
> 
> We can work on this when there will be FTBFS reported.

Why do you refuse to prevent the FTBFS in the first place?

> > I still fail to see how this is perl specific.
> 
> I will re-iterate mock is not failing so its with perl guidelines to specify
> that though a single dependent perl module BR: will bring perl package in
> buildroot, we need to write it BR: perl in spec files.
> 
> > This attitude causes so much slowdown every rebuild it's ridiculous.
> 
> Again, Its something that perl guidelines are missing....

As the way how we treat dependencies doesn't affect just Perl but other package group rebuilds too (including Fedora mass rebuilds), I'd prefer rewording the general Guidelines to be more enforcing on this matter.  Treating each package group differently and possible duplication of Guidelines isn't the best option.

(Still, having this in Perl SIG guidelines would be better than nothing...)
Comment 14 Parag AN(पराग) 2013-07-23 09:37:01 EDT
Python package group needs BR: python2-devel, written in its own guidelines.
Comment 15 Christopher Meng 2013-07-23 21:38:08 EDT
Hi Parag, Petr, Marcela and potential guys who are following this,

Do you have a decision now?

I think discussing such things is not only difficult in perl-devel, but also in here.

Please give me an right answer of the correct solution ASAP. I need to submit the SCM request.
Comment 16 Parag AN(पराग) 2013-07-23 23:59:15 EDT
ok, can Perl SIG people elaborate on this?
I see perl 5.18 rebuild has started and I see some of them just rebuilt with no BR: perl. Wouldn't it be easy to also add BR: perl in spec while rebuilding perl packages in rawhide?
Comment 17 Parag AN(पराग) 2013-07-24 00:07:25 EDT
No one other than few perl SIG people replied either here or on packaging list.
Filed -> https://fedorahosted.org/fpc/ticket/321
Comment 18 Petr Pisar 2013-07-24 01:52:38 EDT
(In reply to Christopher Meng from comment #15)
> 
> Please give me an right answer of the correct solution ASAP. I need to
> submit the SCM request.

Declaring all dependencies does not violate any rules and does not harm anything. Contrary, not declaring all dependencies can cause troubles.

As you can see, Parag did not forbid you to declare them. He has just different opinion on their necessity. I'm sorry he cannot see the benefits of specifying all dependencies, or he explains Fedora guidelines in different way.
Comment 19 Petr Pisar 2013-07-24 01:53:59 EDT
(In reply to Parag AN(पराग) from comment #16)
> I see perl 5.18 rebuild has started and I see some of them just rebuilt with
> no BR: perl. Wouldn't it be easy to also add BR: perl in spec while
> rebuilding perl packages in rawhide?

I do not change others packages just because I can. That would be rude.
Comment 20 Toshio Ernie Kuratomi 2013-07-24 20:12:51 EDT
I'm a bit confused by some of the comments made in this bugzilla report.  I asked some questions in the FPC ticket that was opened about this: https://fedorahosted.org/fpc/ticket/321
Comment 21 Parag AN(पराग) 2013-08-26 05:22:17 EDT
No one is (or actually Perl people are not) talking on above ticket. Let's take that advantage.

Christopher,
  Please finish this review by importing the approved srpm.
Comment 22 Christopher Meng 2013-08-28 04:51:12 EDT
Thanks!


New Package SCM Request
=======================
Package Name: perl-CPANPLUS-Dist-Fedora
Short Description: CPANPLUS backend to build Fedora/RedHat RPMs
Owners: cicku
Branches: f20
InitialCC:
Comment 23 Gwyn Ciesla 2013-08-28 07:57:20 EDT
Git done (by process-git-requests).
Comment 24 Ralf Corsepius 2013-08-28 08:45:20 EDT
Please add perl-sig to the CC: as it's common for Fedora perl packages.
Comment 25 Parag AN(पराग) 2013-09-16 02:12:53 EDT
any updates? is this build on all requested branches?

Also, good to add perl-sig as initialCC to all perl packages in SCM request.
Comment 26 Christopher Meng 2013-09-16 05:58:16 EDT
Package Change Request
======================
Package Name: perl-CPANPLUS-Dist-Fedora
InitialCC: perl-sig
Comment 27 Christopher Meng 2013-09-16 06:01:29 EDT
Already imported, I thought it should be closed by bodhi, seems the time I imported f20 didn't use bodhi ;) Thanks for the notification.
Comment 28 Gwyn Ciesla 2013-09-16 08:43:26 EDT
Clearing flag.

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