Bug 997549 - Review Request: perl-Parallel-Runner - An object to manage running things in parallel processes
Review Request: perl-Parallel-Runner - An object to manage running things in ...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ralf Corsepius
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 997554
  Show dependency treegraph
 
Reported: 2013-08-15 10:52 EDT by Paul Howarth
Modified: 2013-09-19 05:19 EDT (History)
4 users (show)

See Also:
Fixed In Version: perl-Parallel-Runner-0.013-4.fc19
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-08-30 18:56:55 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rc040203: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Paul Howarth 2013-08-15 10:52:48 EDT
Spec URL: http://subversion.city-fan.org/repos/cfo-repo/perl-Parallel-Runner/branches/fedora/perl-Parallel-Runner.spec
SRPM URL: http://www.city-fan.org/~paul/extras/perl-Parallel-Runner/perl-Parallel-Runner-0.013-3.fc20.src.rpm

Description:

There are several other modules to do this, you probably want one of them. This
module exists as a super-specialized parallel task manager. You create the
object with a process limit and callbacks for what to do while waiting for a
free process slot, as well as a callback for what a process should do just
before exiting.

You must explicitly call $runner->finish() when you are done. If the runner is
destroyed before its children are finished, a warning will be generated and
your child processes will be killed, by force if necessary.

If you specify a maximum of 1 then no forking will occur, and run() will block
until the coderef returns. You can force a fork by providing a boolean true
value as the second argument to run(), which will force the runner to fork
before running the coderef; however, run() will still block until the child
exits.


Fedora Account System Username: pghmcfc

This is needed for Fennec, a well-regarded test framework.
Comment 1 Christopher Meng 2013-08-15 22:29:13 EDT
I think we should avoid using metacpan, so:

URL: http://search.cpan.org/dist/Parallel-Runner

Source0: http://search.cpan.org/CPAN/authors/id/E/EX/EXODIST/Parallel-Runner-%{version}.tar.gz
Comment 2 Paul Howarth 2013-08-16 03:06:02 EDT
(In reply to Christopher Meng from comment #1)
> I think we should avoid using metacpan

Why should we avoid using metacpan?
Comment 3 Ralf Corsepius 2013-08-20 08:19:48 EDT
(In reply to Paul Howarth from comment #2)
> Why should we avoid using metacpan?
Why should we use it?

I don't see any reason for not using anything else but cpan.org.
Comment 4 Paul Howarth 2013-08-20 08:45:10 EDT
(In reply to Ralf Corsepius from comment #3)
> (In reply to Paul Howarth from comment #2)
> > Why should we avoid using metacpan?
> Why should we use it?
> 
> I don't see any reason for not using anything else but cpan.org.

I've found that I've migrated my workflow from search.cpan.org to metacpan.org over the last few months; I particularly like the tools for comparing dist releases, which are useful for spotting changes that upstreams have neglected to mention in their changelogs, or additional deps not mentioned in their META.* files. So I naturally used the URLs from there in my spec file to save myself an otherwise unnecessary trip over to cpan.org to check those URLs. However, I'm not emotionally attached to metacpan and if a reviewer wants to insist on cpan.org URLs, I'll change them.
Comment 5 Ralf Corsepius 2013-08-21 00:26:54 EDT
(In reply to Paul Howarth from comment #4)
I regret having to say so, but these days, to me it's prohibitive to use any service located in the UK, which as I understand metacpan.org is (bytemark.co.uk).

Technically, I am worried about inconcistencies between cpan.org and metacpan.org.
Comment 6 Paul Howarth 2013-08-21 03:26:20 EDT
(In reply to Ralf Corsepius from comment #5)
> (In reply to Paul Howarth from comment #4)
> I regret having to say so, but these days, to me it's prohibitive to use any
> service located in the UK, which as I understand metacpan.org is
> (bytemark.co.uk).

Well I could equally say that about services located in the US but then that would prohibit just about any Fedora activity. And it's not as if CPAN/metacpan hold sensitive data anyway.

Anyway, I've updated the package to use cpan.org URLs:

Spec URL: http://subversion.city-fan.org/repos/cfo-repo/perl-Parallel-Runner/branches/fedora/perl-Parallel-Runner.spec
SRPM URL: http://www.city-fan.org/~paul/extras/perl-Parallel-Runner/perl-Parallel-Runner-0.013-4.fc20.src.rpm
Comment 7 Ralf Corsepius 2013-08-21 09:03:13 EDT
(In reply to Paul Howarth from comment #6)

> Well I could equally say that about services located in the US but then that
> would prohibit just about any Fedora activity.
Agreed ... I would expect Fedora and RH to experience the effects of this rather soon. OT for a review, however :)

Anyway, ... going for review now.
Comment 8 Ralf Corsepius 2013-08-21 09:28:19 EDT
APPROVED.

2 remarks:

1. I am somewhat astounded to not see the rhel-compatibility stuff in a spec from you ;)

2. The spec is using plain "perl" instead of "%{__perl}".
This causes non-deterministic build results when rebuilding this package outside of chroots with the user having a binary named "perl" in $PATH.
I recommend using %{__perl}.
Comment 9 Paul Howarth 2013-08-21 09:58:49 EDT
(In reply to Ralf Corsepius from comment #8)
> APPROVED.
> 
> 2 remarks:
> 
> 1. I am somewhat astounded to not see the rhel-compatibility stuff in a spec
> from you ;)

That is because the build requirements of perl(Child) and perl(Module::Build) ≥ 0.40 are not even available in EL-6, let alone EL-5, so there's no point having the rhel-compatibility stuff.

> 2. The spec is using plain "perl" instead of "%{__perl}".
> This causes non-deterministic build results when rebuilding this package
> outside of chroots with the user having a binary named "perl" in $PATH.
> I recommend using %{__perl}.

On the other hand, the packaging guidelines now discourage the use of macros for commands (although to be fair they do suggest things like python [and hence perl] could be exceptions), which gives me the impression that reproduce-ability of builds outside the buildsystem is not really a concern - local versions of such things as rm, mv and find could also affect the build. But that's a devil's advocate position from me really because I'm with you generally regarding determinism of builds.

I think there is actually an advantage of using "perl" rather than "%{__perl}" here: sometimes when debugging failed builds it boils down to the build working with some perl versions and not with others. Being able to do quick rebuilds using a variety of perls by simply switching PATH/using perlbrew could then be seen as a feature rather than a problem.
Comment 10 Paul Howarth 2013-08-21 10:00:32 EDT
New Package SCM Request
=======================
Package Name: perl-Parallel-Runner
Short Description: An object to manage running things in parallel processes
Owners: pghmcfc
Branches: F-19 F-20
InitialCC: perl-sig

Thanks for the review Ralf.
Comment 11 Ralf Corsepius 2013-08-21 10:20:40 EDT
(In reply to Paul Howarth from comment #9)
> (In reply to Ralf Corsepius from comment #8)

> > 2. The spec is using plain "perl" instead of "%{__perl}".
> > This causes non-deterministic build results when rebuilding this package
> > outside of chroots with the user having a binary named "perl" in $PATH.
> > I recommend using %{__perl}.
> 
> On the other hand, the packaging guidelines now discourage the use of macros
> for commands
I know - It's what I consider to be an FPC mistake and why I am only "recommending".

Just push a file named "perl" somewhere in $PATH, containing
#/bin/sh
exit 1

then run 
# rpm --rebuild <your-srpm>
and watch your built fail.
Comment 12 Gwyn Ciesla 2013-08-22 08:20:20 EDT
Git done (by process-git-requests).
Comment 13 Fedora Update System 2013-08-22 09:35:35 EDT
perl-Parallel-Runner-0.013-4.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/perl-Parallel-Runner-0.013-4.fc19
Comment 14 Fedora Update System 2013-08-22 20:37:58 EDT
perl-Parallel-Runner-0.013-4.fc19 has been pushed to the Fedora 19 testing repository.
Comment 15 Fedora Update System 2013-08-30 18:56:55 EDT
perl-Parallel-Runner-0.013-4.fc19 has been pushed to the Fedora 19 stable repository.
Comment 16 Paul Howarth 2013-09-19 05:19:17 EDT
(In reply to Ralf Corsepius from comment #5)
> Technically, I am worried about inconcistencies between cpan.org and
> metacpan.org.

One such inconsistency has just shown up:

https://metacpan.org/source/RSAVAGE/Tree-DAG_Node-1.18/Changes

  "Somehow a corrupted version got uploaded to search.cpan.org, so I've
   just changed the version #. The file on MetaCPAN was fine."

An interesting data point!

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