Bug 166253 - Review Request: perl-Gtk2-GladeXML
Review Request: perl-Gtk2-GladeXML
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jef Spaleta
Fedora Package Reviews List
http://search.cpan.org/dist/Gtk2-Glad...
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2005-08-18 07:33 EDT by Gavin Henry
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-05-08 00:27:44 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Gavin Henry 2005-08-18 07:33:30 EDT
Spec Url: http://www.perl.me.uk/downloads/modules/perl-Gtk2-GladeXML.spec

SRPM Url: http://www.perl.me.uk/downloads/modules/perl-Gtk2-GladeXML-1.005-2.src.rpm

Description: Glade is a free user interface builder for GTK+ and GNOME. 
After designing a user interface with glade-2 the layout 
and configuration are saved in an XML file. libglade is a 
library which knows how to build and hook up the user interface 
described in the Glade XML file at application run time.
Comment 2 Jef Spaleta 2005-08-19 11:02:35 EDT
perl-Gtk2-GladeXML-1.005-3.src.rpm
builds in mock for fc4

rpmlint perl-Gtk2-GladeXML-1.005-3.fc4.i386.rpm
W: perl-Gtk2-GladeXML devel-file-in-non-devel-package
/usr/lib/perl5/vendor_perl/5.8.6/i386-linux-thread-multi/Gtk2/GladeXML/Install/gladexmlperl.h

Now the review guidelines say that all header files must be in devel subpackage,
but so far I can't find any examples of a contributed perl package that follows
this rule. So i'm pretty sure we can ignore this because its a perl package.
Both perl-Glib and perl-Gtk2 for example have similar files without a devel package.

Good:
meets packaging naming guides for perl packages
spec name matches
licensed GPL and text included in the doc section
matches upstream source md5 097ab67c4f0025b74e721ae835a2d554
owns all created directories
legible spec
check and clean sections look good
no shared objects in linker path to worry about

I'm not seeing any obvious blockers. But I would like to ask why you are doing
  find examples -type f -exec chmod -x {} ';'
to remove the executable bit on the sample scripts?  If you are going to remove
the executable bit should you include a short additional readme file in the
examples directory tell users they need to run the examples under a call to perl
instead of trying to re-enabled the x bit.

-jef
Comment 3 Paul Howarth 2005-08-19 11:08:03 EDT
(In reply to comment #2)
> I'm not seeing any obvious blockers. But I would like to ask why you are doing
>   find examples -type f -exec chmod -x {} ';'
> to remove the executable bit on the sample scripts?  If you are going to remove
> the executable bit should you include a short additional readme file in the
> examples directory tell users they need to run the examples under a call to perl
> instead of trying to re-enabled the x bit.

This is probably to prevent additional dependencies being added to the package
by find_requires as a result of executeable example code. Another way around
that would be to use a custom find_requires script that runs the default script
and then filters out any bogus dependencies.
Comment 4 Jef Spaleta 2005-08-19 12:12:36 EDT
(In reply to comment #3)
> Another way around
> that would be to use a custom find_requires script that runs the default script
> and then filters out any bogus dependencies.

Sounds like that cure is worse than the disease.  I'm not sure how important
leaving the example scripts executable is.  Here's the requirements that get
sucked in if the scripts are left executable

perl(constant)
perl(Data::Dumper)
perl(File::Spec)
perl(Gtk2::Gdk::Keysyms)
perl(Gtk2::GladeXML)
perl(Gtk2::SimpleList)
/usr/bin/perl

All of these are provided by packages which are required already, perl and
perl-Gtk2. So in this case we aren't really filtering anything significant.

I think as a compromise it might be most appropriate to add a README.FEDORA file
or similar which points out which additional packages are needed to run the
examples and that the non executable examples can be run as arguments to the
perl command instead of as stand-alone executables.

example FEDORA.README
To use the the examples in this directory you may need to install these
additional packages if they are not already present on your system:
<list of additional packages here, though in this case the list is empty>

The examples are shipped non-executable. To run the examples call them as
arguments to the perl command. For example:
>cd  /usr/share/doc/perl-Gtk2-GladeXML-1.005/examples
>perl progress.pl


-jef


Comment 5 Paul Howarth 2005-08-19 12:17:42 EDT
(In reply to comment #4)
> (In reply to comment #3)
> > Another way around
> > that would be to use a custom find_requires script that runs the default script
> > and then filters out any bogus dependencies.
> 
> Sounds like that cure is worse than the disease.  I'm not sure how important
> leaving the example scripts executable is.  Here's the requirements that get
> sucked in if the scripts are left executable
> 
> perl(constant)
> perl(Data::Dumper)
> perl(File::Spec)
> perl(Gtk2::Gdk::Keysyms)
> perl(Gtk2::GladeXML)
> perl(Gtk2::SimpleList)
> /usr/bin/perl
> 
> All of these are provided by packages which are required already, perl and
> perl-Gtk2. So in this case we aren't really filtering anything significant.

In that case I don't see any point in turning off the executable bits of the
example files at all.

Comment 6 Jef Spaleta 2005-08-25 22:48:13 EDT
Okay coming back to this sprog dependancy, two small specfile corrections I
think should be made before I do final review approval:

1) remove "find examples -type f -exec chmod -x {} ';'"
As per discussion in comment 3 - comment 5, this isn't really needed, the
examples do not pull in any additional dependancies not already required or assumed.

2) use %{__perl} Makefile.PL INSTALLDIRS=vendor OPTIMIZE="$RPM_OPT_FLAGS"
similar problem as discussed in bug 166252

Just give me one last spec file to review this one will be in the bag i think.

-jef
Comment 7 Gavin Henry 2005-08-26 10:23:12 EDT
Cheers, will do as soon as I can.
Comment 8 Gavin Henry 2005-09-14 04:53:36 EDT
Added OPTIMIZE="$RPM_OPT_FLAGS" and removed "find examples -type f -exec chmod
-x {} ';'", then rebuilt

New version:

http://www.perl.me.uk/downloads/modules/ perl-Gtk2-GladeXML-1.005-4.src.rpm
http://www.perl.me.uk/downloads/modules/perl-Gtk2-GladeXML.spec
http://www.perl.me.uk/downloads/modules/md5sums
Comment 9 Jef Spaleta 2005-09-14 17:50:50 EDT
perl-Gtk2-GladeXML-1.005-4.src.rpm builds in mock for fc4 and devel
OPTMIZE looks good
demo executables work on fc4

Approved.
Comment 10 Jose Pedro Oliveira 2005-09-14 19:37:10 EDT
(In reply to comment #6)

> 1) remove "find examples -type f -exec chmod -x {} ';'"
> As per discussion in comment 3 - comment 5, this isn't really needed, the
> examples do not pull in any additional dependancies not already required or
assumed.

I was the one who asked Gavin to add the above line to the specfile (see
https://www.redhat.com/archives/fedora-extras-list/2005-July/msg00567.html).

Some of of us have been following the policy of disabling the executable bit
bit for every documentation file.  The main reason is that it avoids pulling
new requirments as explained by Paul in comment #2 (rpm still pulls requirements
from perl modules though).  The other reason is more of personnal taste - we
don't like to see runnable files shipped as documentation.

/jpo

PS - The package is still approved.
Comment 11 Jef Spaleta 2005-09-14 19:56:59 EDT
> I was the one who asked Gavin to add the above line to the specfile (see
> https://www.redhat.com/archives/fedora-extras-list/2005-July/msg00567.html).
> 
> Some of of us have been following the policy of disabling the executable bit
> bit for every documentation file.  The main reason is that it avoids pulling
> new requirments as explained by Paul in comment #2 (rpm still pulls requirements
> from perl modules though).  The other reason is more of personnal taste - we
> don't like to see runnable files shipped as documentation.

in this case.. and im sure its very much a special case..there are no extra deps
pulled in. But I'm also not seeing a check and removal of executable bits
documented in the living wiki documentation of the review process. Sorry I
missed the discussion thread in the extras list where concensous was reached.
Can you get this incorporated as a reviewer SHOULD check or maybe a blurb about
non-executable examples in the packaging guidelines for "Documentation."  I
don't have a strong opinion either way, so I just went with my gut.  But now i
know...and knowing is half the battle. Yo Joe!

I still think it might be a good idea to have boilerplate fedora readme saying
that the examples have been delibrately made un-executable since its a change
from upstream source behavior.


-jef
Comment 12 Jose Pedro Oliveira 2005-09-14 20:29:31 EDT
(In reply to comment #11)
> ... But I'm also not seeing a check and removal of executable bits
> documented in the living wiki documentation of the review process. Sorry I
> missed the discussion thread in the extras list where concensous was reached.
> Can you get this incorporated as a reviewer SHOULD check or maybe a blurb about
> non-executable examples in the packaging guidelines for "Documentation."  I
> don't have a strong opinion either way, so I just went with my gut.  But now i
> know...and knowing is half the battle. Yo Joe!

I don't think it has been written in a Wiki page.  It is one those lessons
learned the hard way, i.e, after being bitten by things like:
https://www.redhat.com/archives/fedora-extras-list/2005-July/msg00793.html
;)

Note: We have been using this convention since Fedora.us days (at least some
of the perl packagers).

/jpo
Comment 13 Alex Lancaster 2006-02-25 18:50:12 EST
Built and available in development since September 2005, please CLOSE as
NEXTRELEASE.

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