[Bug 757156] New: Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

Summary: Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

https://bugzilla.redhat.com/show_bug.cgi?id=757156

Summary: Review Request: perl-Env-C - Get/Set/Unset Environment
                    Variables on the C level
           Product: Fedora
           Version: rawhide
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: medium
          Priority: medium
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail],
                    [e-mail]
    Classification: Fedora
      Story Points: ---
              Type: ---

Spec URL: http://www.fi.muni.cz/~kas/tmp/perl-Env-C.spec
SRPM URL: http://www.fi.muni.cz/~kas/tmp/perl-Env-C-0.08-1.fc16.src.rpm
Description:
This module provides a Perl API for getenv(3), setenv(3) and unsetenv(3).
It also can return all the environ variables.
Review Description: this is my first CPAN package ported to Fedora RPM, so I
will need a sponsor.

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

Jan "Yenya" Kasprzak  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |177841(FE-NEEDSPONSOR)

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

Willington Vega  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #1 from Willington Vega  2011-12-02 08:50:51 EST ---
Hello Jan,

I'm currently learning about Fedora package guidelines so I can't sponsor you,
but I would like to provide an informal review for your package.

rpmlint return no errors for your SPEC:
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

rpmlint return some warnings for the SRPM you provided and generated RPMs
packages, but I don't think that's a problem:
perl-Env-C.src: W: spelling-error %description -l en_US getenv -> Genet
perl-Env-C.src: W: spelling-error %description -l en_US setenv -> seventeen
perl-Env-C.src: W: spelling-error %description -l en_US unsetenv -> unsweetened
perl-Env-C.src: W: spelling-error %description -l en_US environ -> environs,
environment
1 packages and 0 specfiles checked; 0 errors, 4 warnings.

The package builds also builds in mock.

Also, the spec file includes the following when listing directory ownership:

%{perl_vendorarch}/auto/*
%{perl_vendorarch}/Env*

but the Perl packaging guidelines[1] says it should be:

# For arch-specific packages: vendorarch
%{perl_vendorarch}/*
%exclude %dir %{perl_vendorarch}/auto/

will that work for this package?

Also, I went through the MUST and SHOULD[2] and couldn't find any blockers. One
of the SHOULDs I want to note is that there is no license text as separated
file in the sources.

Hope this helps to get more attention to and improve your package.

1. http://fedoraproject.org/wiki/Packaging:Perl
2. https://fedoraproject.org/wiki/Packaging:ReviewGuidelines

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

--- Comment #2 from Willington Vega  2011-12-02 08:59:34 EST ---
Perhaps is a good idea to write to Fedora Perl SIG mailing list[1] and let them
know about your package. Not sure if you already this.

1. https://lists.fedoraproject.org/mailman/listinfo/Perl-devel

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

--- Comment #3 from Jan "Yenya" Kasprzak  2011-12-02 10:18:00 EST ---
Re: comment #1

Hello Willington, thanks for the review!

I think spelling errors reported by rpmlint are not valid. I have updated the
.spec and src.rpm file to fix the directory ownership problem you have
reported. It would be nice to have cpanspec modified so that it generated a
.spec file which actually follows the guidelines. I will report the issue to
the cpanspec package.

As for the license text: perl modules often refer to the license of Perl itself
(dual GPL+Artistic). This one explicitly states only Artistic license (which
makes sense for smallish modules like this one).

Re: comment #2

I have joined the list and let them know about this package.

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

--- Comment #4 from Willington Vega  2011-12-03 09:12:38 EST ---
Hi Jan,

I agree, those spelling errors are not a problem and reporting the issue with
the directory ownership back to the cpanspec package is a good idea.

I've seen on other reviews that is good to provide the updated spec file and
srpm as a different one, with the modifications, an increased release number
and the corresponding Changelog documentation. It is also helpful when
comparing spec files to debug errors like the one I'm going to describe next:

The package no longer builds in mock from the SRPM:
mock SRPMS/perl-Env-C-0.08-1.fc15.src.rpm

The important part of the build.log file says:

+ make test
PERL_DL_NONLAZY=1 /usr/bin/perl "-Iblib/lib" "-Iblib/arch" test.pl
Can't locate Test/More.pm in @INC (@INC contains: blib/lib blib/arch
/usr/local/lib64/perl5 /usr/local/share/perl5 /usr/lib64/perl5/vendor_perl
/usr/share/perl5/vendor_perl /usr/lib64/perl5 /usr/share/perl5 .) at test.pl
line 4.
BEGIN failed--compilation aborted at test.pl line 4.
make: *** [test_dynamic] Error 2
RPM build errors:
error: Bad exit status from /var/tmp/rpm-tmp.yd2GUq (%check)
    Bad exit status from /var/tmp/rpm-tmp.yd2GUq (%check)
Child returncode was: 1

The problem is your updated spec file no longer BuildRequires perl(Test::More).

Thank you for your quick update.

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

Paul Howarth  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #5 from Paul Howarth  2011-12-06 10:29:57 EST ---
I'm concerned about the license of this package: upstream just refers to
"Artistic" but your spec says "Artistic 2.0". The latter would be acceptable
for Fedora but the former isn't. Could you get upstream to clarify which
version of the Artistic license they mean? Some upstreams have been known to
say "Artistic" when they actually mean "same as Perl", which might possibly be
the case here too.

Have you submitted any other packages for review yet?

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

--- Comment #6 from Jan "Yenya" Kasprzak  2011-12-06 12:06:20 EST ---
Re: comment #4

my fault, I have added dependency on Test::More back. I have also bumped the
release number and added a %changelog entry as you suggest:

http://www.fi.muni.cz/~kas/tmp/perl-Env-C.spec
http://www.fi.muni.cz/~kas/tmp/perl-Env-C-0.08-2.fc16.src.rpm

Re: comment #5

I have asked upstream to clarify the licensing.

No, I have not submitted any other packages. Should I submit them? I wanted to
go through the whole process of making a Fedora package with the single package
first.

If anybody is interested in (pre-)review, I have uploaded them to
http://www.fi.muni.cz/~kas/tmp/fedora-packages/. Some of them cannot be built
in mock as-is, because they depend on other packages from this set, so they
have probably to be reviewed in a single request. Also, they all curretly have
the problem mentioned in comment #1 (directory ownership of
%{perl_vendorarch}... as created by cpanspec).

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

--- Comment #7 from Paul Howarth  2011-12-06 13:58:20 EST ---
(In reply to comment #6)
> Re: comment #5
>
> I have asked upstream to clarify the licensing.

Good.
> No, I have not submitted any other packages. Should I submit them? I wanted to
> go through the whole process of making a Fedora package with the single package
> first.

The whole process can't continue much further with this one until the license
is clarified unfortunately, which is why it would be a good idea to submit a
few others too.

Regarding your plans for participation in Fedora, do you see yourself sticking
to perl modules or might you have other things to bring in in the future?
> If anybody is interested in (pre-)review, I have uploaded them to
> http://www.fi.muni.cz/~kas/tmp/fedora-packages/. Some of them cannot be built
> in mock as-is, because they depend on other packages from this set, so they
> have probably to be reviewed in a single request.

Are there any more "leaf" packages, that don't have any non-Fedora
dependencies?
> Also, they all currently have
> the problem mentioned in comment #1 (directory ownership of
> %{perl_vendorarch}... as created by cpanspec).

This is more a matter of taste. If I was packaging this module, I'd use:

%files
%defattr(-,root,root,-)
%doc Changes README
%{perl_vendorarch}/auto/Env/
%{perl_vendorarch}/Env/
%{_mandir}/man3/Env::C.3pm*

I prefer to keep wildcards to a minimum so that if future versions start
shipping different files, I notice it and can investigate to see if it's a
problem or not. What's important is that your package "owns" the necessary
directories and files. All versions of this package that you have submitted
thus far satisfy that requirement.

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

--- Comment #8 from Jan "Yenya" Kasprzak  2011-12-07 15:34:19 EST ---
OK, here is the quoted discussion with the upstream package author. Lines
prefixed with ">" are mine, and lines prefixed with ":" are written by the
package author:

: > : Personally, I don't really care which exact of the free-as-a-beer
: > : license is used, GPL, Artistic XYZ, or whatever you'd like it to be.
: >
: >       OK. So "the same terms as Perl itself" would be OK with you?
: [...]
: >       So if you say "the same terms as Perl itself" or "Artistic 2.0"
: > or "Artistic clarified" is OK with you, I would be able to package
: > Env::C for Fedora.
: 
: Yes, either of the above works. I doubt I'll release a new version of
: the module just to tweak this, unless it's really important. Perhaps
: you can just quote this communication as a proof.

Is it sufficient to have it this way and keep License: Artistic 2.0
in the spec file?

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

--- Comment #9 from Jan "Yenya" Kasprzak  2011-12-07 15:53:41 EST ---
Re: comment #7
> Regarding your plans for participation in Fedora, do you see yourself sticking
> to perl modules or might you have other things to bring in in the future?

I want to contribute packages which I need myself, but recently everything I
have needed was already packaged except some Perl modules. So for now, I will
stick with Perl modules. (context: I have been building RPM packages for at
least 12 years now, does anybody here remember Red Hat Contrib|Net, the early
community packaging effort for Red Hat Linux?).
> Are there any more "leaf" packages, that don't have any non-Fedora
> dependencies?

E.g. perl-TeX-Encode, perl-Scalar-String, perl-IO-Socket-Multicast,
perl-Data-Integer, perl-Data-Float, or perl-DBD-ODBC.
> > Also, they all currently have
> > the problem mentioned in comment #1 (directory ownership of
> > %{perl_vendorarch}... as created by cpanspec).
>
> This is more a matter of taste.

Well, perhaps the Packaging:Perl wiki page should be modified to not have this
requirements then? (I cannot check the exact wording as Fedora wiki currently
says 503 Service Unavailable for me).

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

--- Comment #10 from Paul Howarth  2011-12-08 06:56:24 EST ---
(In reply to comment #8)
> OK, here is the quoted discussion with the upstream package author. Lines
> prefixed with ">" are mine, and lines prefixed with ":" are written by the
> package author:
>
> : > : Personally, I don't really care which exact of the free-as-a-beer
> : > : license is used, GPL, Artistic XYZ, or whatever you'd like it to be.
> : >
> : >       OK. So "the same terms as Perl itself" would be OK with you?
> : [...]
> : >       So if you say "the same terms as Perl itself" or "Artistic 2.0"
> : > or "Artistic clarified" is OK with you, I would be able to package
> : > Env::C for Fedora.
> : 
> : Yes, either of the above works. I doubt I'll release a new version of
> : the module just to tweak this, unless it's really important. Perhaps
> : you can just quote this communication as a proof.
>
> Is it sufficient to have it this way and keep License: Artistic 2.0
> in the spec file?

Yes, but include the email from the author saying this is OK as one of the %doc
files in the package. That's what happened a while back for perl-NetAddr-IP,
which had the same issue.

http://pkgs.fedoraproject.org/gitweb/?p=perl-NetAddr-IP.git;a=blob;f=License...

(In reply to comment #9)
> Re: comment #7
> > Regarding your plans for participation in Fedora, do you see yourself sticking
> > to perl modules or might you have other things to bring in in the future?
>
> I want to contribute packages which I need myself, but recently everything I
> have needed was already packaged except some Perl modules. So for now, I will
> stick with Perl modules. (context: I have been building RPM packages for at
> least 12 years now, does anybody here remember Red Hat Contrib|Net, the early
> community packaging effort for Red Hat Linux?).

The issue here is that perl module packages tend to be very formulaic and
generally don't need much tweaking from what you get out of cpanspec. As a
result, package submissions of perl modules don't provide much insight into
your understanding of packaging in general or the Fedora guidelines in
particular. The usual approach to demonstrating this understanding is to do
unofficial reviews of other packages like Willington did for this one (though
preferably not just perl modules).
> > Are there any more "leaf" packages, that don't have any non-Fedora
> > dependencies?
>
> E.g. perl-TeX-Encode, perl-Scalar-String, perl-IO-Socket-Multicast,
> perl-Data-Integer, perl-Data-Float, or perl-DBD-ODBC.

If there's any of those with anything "unusual" about them, submit that, else
pick one at random for now.
> > > Also, they all currently have
> > > the problem mentioned in comment #1 (directory ownership of
> > > %{perl_vendorarch}... as created by cpanspec).
> >
> > This is more a matter of taste.
>
> Well, perhaps the Packaging:Perl wiki page should be modified to not have this
> requirements then? (I cannot check the exact wording as Fedora wiki currently
> says 503 Service Unavailable for me).

Maybe the wording could be clearer. The guideline is talking about which
directories must be owned by the package, by way of some sample %files entries.
The important bit is the directory ownership rather than the implementation
though.

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

--- Comment #11 from Jan "Yenya" Kasprzak  2011-12-23 07:58:39 EST ---
OK, I have created a new .spec and .src.rpm file. The only difference is added
e-mail from upstream in %doc:

http://www.fi.muni.cz/~kas/tmp/perl-Env-C-0.08-3.src.rpm
http://www.fi.muni.cz/~kas/tmp/perl-Env-C.spec

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

Jan "Yenya" Kasprzak  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-cvs?

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

--- Comment #12 from Jan "Yenya" Kasprzak  2012-02-17 08:45:16 EST ---
New Package SCM Request
=======================
Package Name: perl-Env-C
Short Description: Get/Set/Unset Environment Variables on the C level
Owners: yenya
Branches: f15 f16 f17
InitialCC:

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

--- Comment #13 from Jon Ciesla  2012-02-17 08:45:30 EST ---
Please include an SCM request.

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

Jan "Yenya" Kasprzak  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-cvs?

--- Comment #14 from Jan "Yenya" Kasprzak  2012-02-17 10:10:13 EST ---
New Package SCM Request
=======================
Package Name: perl-Env-C
Short Description: Get/Set/Unset Environment Variables on the C level
Owners: yenya
Branches: f15 f16 f17
InitialCC: perl-sig

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

--- Comment #15 from Jon Ciesla  2012-02-17 10:30:35 EST ---
In looking closer, it doesn't look like this has been approved.

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

--- Comment #16 from Jan "Yenya" Kasprzak  2012-02-17 11:29:35 EST ---
Oh, well. I have received an e-mail that I have been sponsored, and the
following text:
> Thank you for applying for the packager group.
>
> Welcome to the Fedora packager group. Please continue the process from:
> https://fedoraproject.org/wiki/PackageMaintainers
> /Join#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner

Which is what I have been trying to follow today. If this is not the correct
procedure, anybody care to enlighten me what am I missing? Thanks!

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=757156

Petr Šabata  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #17 from Petr Šabata  2012-02-20 04:18:21 EST ---
It seems like Paul was working on the review here.  He should assign the bug to
himself and when he's happy with your package, set the 'fedora-review+' flag. 
Then you can submit the SCM request with 'fedora-cvs?' again.

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=757156

Paul Howarth  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
             Blocks|177841                      |
           Assignee|[e-mail]    |[e-mail]
              Flags|                            |fedora-review?

--- Comment #18 from Paul Howarth  ---
(In reply to comment #16)
> Oh, well. I have received an e-mail that I have been sponsored, and the
> following text:
> > Thank you for applying for the packager group.
> >
> > Welcome to the Fedora packager group. Please continue the process from:
> > https://fedoraproject.org/wiki/PackageMaintainers
> > /Join#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner
>
> Which is what I have been trying to follow today. If this is not the correct
> procedure, anybody care to enlighten me what am I missing? Thanks!

I didn't know that Radek Vokál had sponsored you, and you didn't remove the
FE-NEEDSPONSOR blocker from this ticket (which I've now done). Given that
you're now sponsored, we should be able to finish this review off quickly.

Your SRPM seems to have disappeared; can you put it back so I can check it out?
You might also want to remove the "find" command that removes empty directories
from the buildroot:

find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null ;

This isn't actually needed as rpm-build ignores empty directories (though not
empty files) in the buildroot.

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=757156

Paul Howarth  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?([e-mail])

--- Comment #19 from Paul Howarth  ---
Jan, are you still interested in this package?

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=757156

--- Comment #20 from Paul Howarth  ---
This review appears to be stalled; please respond soon if you're still
interested in submitting this package.

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=757156

Paul Howarth  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|---                         |INSUFFICIENT_DATA
        Last Closed|                            |2015-02-05 03:25:10

--- Comment #21 from Paul Howarth  ---
Closing due to lack of response. Please re-open if still interested.

[Bug 757156] Review Request: perl-Env-C - Get/Set/Unset Environment Variables on the C level

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=757156

Petr Šabata  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://bugzilla.redhat.com
                   |                            |/show_bug.cgi?id=1213757