Bug 174275 - Review Request: nautilus-actions - Nautilus extension for customizing the context menu
Review Request: nautilus-actions - Nautilus extension for customizing the con...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jef Spaleta
David Lawrence
http://www.grumz.net/node/8
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2005-11-26 22:12 EST by Deji Akingunola
Modified: 2007-12-16 17:03 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-01-06 11:28:49 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Deji Akingunola 2005-11-26 22:12:08 EST
Spec Name or Url: ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions.spec
SRPM Name or Url: <tp://czar.eas.yorku.ca/pub/nact/nautilus-actions-0.99-1.src.rpm
Description: Nautilus actions is an extension for Nautilus, the GNOME file manager. It provides an easy way to configure programs to be launch on files selected in Nautilus interface
Comment 1 Brian Pepple 2005-11-26 22:22:43 EST
Some quick notes:

You should probably use User Interface/Desktops, instead of Applications/System
for the group.  Most of the other nautilus plugins (nautilus-image-converter,
nautilus-open-terminal) use this.

I believe you can also drop the Requires for nautilus, since the BR on this
should automatically pull this in.

If I've got some extra time tomorrow, I'll do a proper review.
Comment 2 Brian Pepple 2005-12-07 15:32:45 EST
NEEDSWORK

MD5Sums:
682d452e4b05c77b5a258c7ec49634bc  nautilus-actions-0.99.tar.gz

Good:
* Source URL is canonical
* Upstream source tarball verified
* Package name conforms to the Fedora Naming Guidelines
* Group Tag is from the official list
* Buildroot has all required elements
* All paths begin with macros
* Desktop entry is fine
* All directories are owned by this or other packages
* No deprecated fields used
* All necessary BuildRequires listed.
* All desired features are enabled
* Package rebuilds as non-root user
* Make succeeds even when %{_smp_mflags} is defined
* Files have appropriate permissions and owners

Needswork:
* In the %pre section, you scriptlet needs to be changed, the [NAME] should be
changed to config_newaction.

* Rpmlint errors:
E: nautilus-actions description-line-too-long
W: nautilus-actions conffile-without-noreplace-flag
/etc/gconf/schemas/config_newaction.schemas
W: nautilus-actions devel-file-in-non-devel-package /usr/bin/nautilus-actions-config
W: nautilus-actions devel-file-in-non-devel-package
/usr/bin/nautilus-actions-new-config

* The line where you remove the fedora*.desktop file can be removed, since it
comes before where the desktop file is created & installed.  The
--delete-original switch handles the .desktop file that comes with the source.

* It seems redundant to include both the AUTHORS & MAINTAINERS files, since the
MAINTAINER file only contains their e-mail addresses, which is also included in
the AUTHORS file.  I would suggest not packing the MAINTAINERS file.


Notes:
I'm not running Rawhide, so I'm not able to verify that this runs correctly.  It
does build fine in Mock, though.
Comment 3 Deji Akingunola 2005-12-07 19:08:37 EST
I've actually corrected most of the things you pointed out (except for removing
the MAINTAINERS file) in my local version, I just forgot to post it here.
As for the rpmlint warnings, I'm not sure if its really necessary to create a
devel subpackage for this nautilus-actions.

ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions.spec
ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions-0.99-2.src.rpm
Comment 4 Brian Pepple 2005-12-07 19:34:41 EST
Oh, I forgot one other thing.  Is e2fsprogs-devel really needed to build this? 
I don't see anything the configure script that would require this, since
e2fsprogs deals with ext2 filesystems.

Also, it looks like you description in your spec is still to long.
Comment 5 Deji Akingunola 2005-12-07 19:49:46 EST
(In reply to comment #4)
> Oh, I forgot one other thing.  Is e2fsprogs-devel really needed to build this?

Yes. I'm quite surprised by it too, building it in a clean mock will fail
without e2fsprogs-devel.
   
> I don't see anything the configure script that would require this, since
> e2fsprogs deals with ext2 filesystems.
> 
> Also, it looks like you description in your spec is still to long.

I'll shorten it in the next release.

Comment 6 Deji Akingunola 2005-12-08 11:24:43 EST
Here are comments by package author, Frederic Ruaudel;

> Concerning the -devel rpmlint warnings, it is definitely not necessary
> as the mentioned files are not at all development files.
> nautilus-actions-config is the GUI config tool and
> nautilus-actions-new-config is a command line tool to create a config
> file that can be imported with the GUI tool or directly with
> gconftool-2.
>
> Concerning the requirement for e2fsprogs-devel, it is needed because I
> use the uuid library provided by this package to generate uniq config
> identifiers.
Comment 7 Jef Spaleta 2005-12-17 14:55:40 EST
Are there any blockers left for this review?

Comment 8 Deji Akingunola 2005-12-17 18:18:11 EST
None (that I can think of), except for one line in the package description
extending too long. That is fixed by the files below.
ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions.spec
ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions-0.99-3.src.rpm
Unless Brian spot anything new, the package should be good enough now.
Comment 9 Brian Pepple 2005-12-17 19:02:37 EST
Since I'm not running Rawhide yet, I didn't take this bug for a formal review
(thats why I left it as FE-NEW).  Another reviewer will need to verify that this
package runs as expected before, and then they can give a final approval.  Of
course, until the mesa-libGL package is fixed in Rawhide, they won't be able to
build this package in Mock.
Comment 10 Jef Spaleta 2005-12-18 14:01:27 EST
Got the build in mock against rawhide today to actually start.
It did not compile however.
Build.log reports:
checking for     glib-2.0 >= 2.4.0                       gthread-2.0 >= 2.4.0
                 gmodule-2.0 >= 2.4.0                    gtk+-2.0    >= 2.4.0
                 libglade-2.0 >= 2.4.0                   libgnome-2.0 >= 2.7.0
         libgnomeui-2.0 >= 2.7.0                 gconf-2.0 >= 2.8.0     
libnautilus-extension >= 2.8.0... Package xdmcp was not found in the pkg-config
search
path. Perhaps you should add the directory containing `xdmcp.pc' to the
PKG_CONFIG_PATH environment variable Package 'xdmcp', required by 'X11', not found
configure: error: Library requirements (         glib-2.0 >= 2.4.0
         gthread-2.0 >= 2.4.0                    gmodule-2.0 >= 2.4.0
         gtk+-2.0    >= 2.4.0                    libglade-2.0 >= 2.4.0
         libgnome-2.0 >= 2.7.0           libgnomeui-2.0 >= 2.7.0
 gconf-2.0 >= 2.8.0      libnautilus-extension >= 2.8.0) not met; consider
adjusting the PKG_CONFIG_PATH environment variable if your libraries are in a
nonstandard prefix so pkg-config can find them.
error: Bad exit status from /var/tmp/rpm-tmp.29345 (%build)

you definitely need libXdmcp-devel as a buildrequires
I've added that and the build completes.
I'll see if the resulting packages work on my rawhide box

-jef
Comment 11 Jef Spaleta 2005-12-18 14:12:12 EST
Okay my nautilus-actions-0.99-4  binary seems to work on my rawhide box just fine.
The preference dialog works.. and my test actions show up in the right click
context menu.

Getting the buildrequires fixed as per the last comment seems to be the only
thing left to do.

-jef
Comment 12 Deji Akingunola 2005-12-19 14:51:42 EST
Jef,

The buildrequires for libXdmcp-devel must have been caused by some recent
changes on rawhide, it build fine without it in mock the last time i tried.
I've change the spec file to now buildrequire libXdmcp-devel and also remove the
buildrequires on libSM-devel as it is now required for libgnomeui-devel which is
already in the list. 
ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions.spec
ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions-0.99-4.src.rpm

Note I've not tested this build in mock (I'm currently getting some yum error
using it).
Comment 13 Jef Spaleta 2005-12-19 18:26:25 EST
Tested the the build in which mock configuration? fc4 or rawhide?  
I did my build on an fc4 box against the rawhide mock root.
mock running from a rawhide will not work... since the bump of yum to 2.5.x.  

But mock -r fedora-development-i386-core on an fc4 box does work to build
packages for a rawhide at the moment. 

I should be able to finish this up tonite. What I'm not clear about is, am I
suppose to do a full formal review or does the review brian did count and I'm
just suppose to confirm the blockers are fixed?

All i know is this is going to be pretty useful little package.

-jef
Comment 14 Brian Pepple 2005-12-19 20:04:11 EST
(In reply to comment #13)
> Tested the the build in which mock configuration? fc4 or rawhide?  
> I did my build on an fc4 box against the rawhide mock root.
> mock running from a rawhide will not work... since the bump of yum to 2.5.x.  
>
> But mock -r fedora-development-i386-core on an fc4 box does work to build
> packages for a rawhide at the moment. 
> 

I tested that this built against Rawhide back in comment #2 (this package will
not build in FC4 because of the modular X BR).  Due to the some of the recent
packages pushed out in the last couple of days in Rawhide, it might not build
right now.  Once Rawhide settles down a bit, it should hopefully build providing
the BR packages haven't changed.  I'm guessing if there is any problems, it will
be the modular X packages since there's a lot of work being done on them.

> I should be able to finish this up tonite. What I'm not clear about is, am I
> suppose to do a full formal review or does the review brian did count and I'm
> just suppose to confirm the blockers are fixed?

You just need to verify that the blockers have been fixed, and that it will
build and run as expected in FC5.
Comment 15 Jef Spaleta 2005-12-19 21:41:08 EST
this is maddening... now I'm getting a new build error from mock using your new
src.rpm

Do you need need libXau-devel  as a buildrequires?
-jef

here's the build.log snippet:
checking for     glib-2.0 >= 2.4.0                       gthread-2.0 >= 2.4.0
                 gmodule-2.0 >= 2.4.0                    gtk+-2.0    >= 2.4.0
                 libglade-2.0 >= 2.4.0                   libgnome-2.0 >= 2.7.0
         libgnomeui-2.0 >= 2.7.0                 gconf-2.0 >= 2.8.0     
libnautilus-extension >= 2.8.0... Package xau was not found in the pkg-config
search path. Perhaps you should add the directory containing `xau.pc' to the
PKG_CONFIG_PATH environment variable Package 'xau', required by 'X11', not found
configure: error: Library requirements (         glib-2.0 >= 2.4.0
         gthread-2.0 >= 2.4.0                    gmodule-2.0 >= 2.4.0
         gtk+-2.0    >= 2.4.0                    libglade-2.0 >= 2.4.0
         libgnome-2.0 >= 2.7.0           libgnomeui-2.0 >= 2.7.0
 gconf-2.0 >= 2.8.0      libnautilus-extension >= 2.8.0) not met; consider
adjusting the PKG_CONFIG_PATH environment variable if your libraries are in a
nonstandard prefix so pkg-config can find them.
error: Bad exit status from /var/tmp/rpm-tmp.47387 (%build)

Comment 16 Brian Pepple 2005-12-19 21:55:54 EST
I would hold off on trying to build this in Mock until Rawhide has settled down
a bit.
Comment 17 Jef Spaleta 2005-12-30 11:03:04 EST
Okay ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions-0.99-4.src.rpm  compiles
in mock for core-development today.

And the resulting binary that i get appears to work on my rawhide system. I did
some shallow testing and created some test actions. 

I'm starting the clock on approval. Unless someone comes up with a blocker I'll
approve this 24 hours from now.

-jef

Comment 18 Brian Pepple 2005-12-30 11:17:57 EST
Has the BR's been looked at since Bug #176313 has been fixed?
Comment 19 Jef Spaleta 2005-12-30 11:22:33 EST
It appears so... i've successfully built several packages today in mock that
were previously blowing up with libXau oddness.

-jef
Comment 20 Brian Pepple 2005-12-30 11:27:01 EST
I'm sure it builds, but I believe it should be using libX11-devel instead of
libXdmcp-devel.
Comment 21 Jef Spaleta 2005-12-30 11:42:40 EST
actually... it might not "need" either at this point. I forgot that the .4 spec
included that extra buildrequires.   Well I'll look at that tonite if you dont
get to it first. 

-jef

Comment 22 Deji Akingunola 2005-12-30 12:18:52 EST
(In reply to comment #20)
> I'm sure it builds, but I believe it should be using libX11-devel instead of
> libXdmcp-devel.

The resolution of Bug #176313 now makes requiring either redundant as nautilus
now depends on libX11-devel, and libX11-devel depends on both libXdmcp-devel and
libXau-devel.

Deji
Comment 23 Jef Spaleta 2005-12-31 22:05:38 EST
Okay so can you spin up a -5 spec file with the now correct buildrequires. I'll
do a quick build of it in mock and then i'll approve based on that.


-jef
Comment 24 Deji Akingunola 2006-01-01 21:19:37 EST
(In reply to comment #23)
> Okay so can you spin up a -5 spec file with the now correct buildrequires.

Done.
ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions.spec
ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions-0.99-5.src.rpm

Deji
Comment 25 Jef Spaleta 2006-01-01 21:53:31 EST
-5 builds in mock for development  resulting binary installs
approved

Comment 26 Deji Akingunola 2006-01-06 11:28:49 EST
I'd forgotten to close this, the package has already been built and released.
Thanks to Jeff and Brian for the reviews.

Deji
Comment 27 Deji Akingunola 2007-12-16 16:24:20 EST
Package Change Request
======================
Package Name: nautilus-actions
New Branches: EL-5
Comment 28 Kevin Fenzi 2007-12-16 17:03:00 EST
cvs done.

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