Eiciel

[Andy Burns]

Michael Schwendt wrote:

> Let me point out some things here instead of posting this in bugzilla:

Thanks for taking the time

> The ticket is assigned to you instead of the default dummy owner of
> Package Review tickets. This may be misunderstood by anyone who browses
> the FE-NEW list, looking for "unassigned" package reviews.

My mistake

> Some of the package revisions look half-hearted, though.

I'm a little but stumped by what you mean there, some of them have been 
a little minor, but I thought incremental was good when learning?

> If a reviewer supplies a patch against your .spec, accept it or refuse it
> and discuss the reason.

I know what you mean there, I did rather ignore the patch submitted, but 
only because I'd already found my own way around some of the issues I 
was having.

> Taking a brief look at your latest package:
> 
> $ rpmlint eiciel-0.9.2-4.src.rpm 
> E: eiciel hardcoded-library-path in /usr/lib/debug/%{_bindir}/%{name}.debug
> E: eiciel hardcoded-library-path in /usr/lib/debug/%{_libdir}/nautilus/extensions-1.0/lib%{name}*.debug

Originally I had used macros, but I found that I *HAD* to hardcode that 
path, as it worked on x86 with macros, but failed on x86_64 as the files 
do not go into /usr/lib64/debug/usr/lib64 but into 
/usr/lib/debug/usr/lib64, perhaps I'm  unaware of a macro that expands 
to /usr/lib on all platforms?

Or perhaps eiciel's make install has placed them in the wrong location, 
and i should be correcting for that?

I had been checking with rpmlint on RPMs and SRPM, and had got rid of 
all errors/warnings, obviously I failed to rpmlint again after my final 
change.

> This smells like "packager confusion". Right now I can't take the time to
> read all 17 comments in the review ticket (writing all this probably has
> taken more, but well, ...). There is cleanup in the spec file needed. The
> debug files need not be excluded at all.

I was told (though not by an Extras contributor) that it was best to not 
package them

> They are not included in your
> package unless you include them explicitly or recursively (e.g. by
> including %_libdir which in turn would be a packaging mistake).

I thought I was aiming for the minimum number of includes (and 
optionally excludes) to catch all files that make install processed,

>> BuildRequires: nautilus
> 
> Although this works, you really want "nautilus-devel", the virtual
> package for the Nautilus extension development files.
> 
> "Nautilus" is written with upper-case 'N', so I would do the same
> in the package %description.

ok

>> %{_datadir}/%{name}/*
> 
> With this, the directory %{_datadir}/%{name} would not be included in
> your package. Listing the binary rpm confirms it:
> 
> drwxr-xr-x  /usr/share/eiciel/doc
> drwxr-xr-x  /usr/share/eiciel/doc/C
> -rw-r--r--  /usr/share/eiciel/doc/C/eiciel.xml
> 
> The fix is this %files entry:
> 
> %{_datadir}/%{name}/
> 
> This includes the directory and the entire tree in it. The explicit slash
> at the end makes the .spec file easier to read. You see immediately that
> this entry includes a directory [recursively], instead of a single file.
> The package listing will look like (notice the top dir!):
> 
> drwxr-xr-x  /usr/share/eiciel
> drwxr-xr-x  /usr/share/eiciel/doc
> drwxr-xr-x  /usr/share/eiciel/doc/C
> -rw-r--r--  /usr/share/eiciel/doc/C/eiciel.xml

Hadn't realised the distinction in wildcards

>> Summary:        Eiciel graphical access control list editor
> 
> Better:
> Summary: Graphical access control list (ACL) editor
> 
> In wide parts of the RPM packaging world and for the majority of packages,
> it is considered bad taste to repeat or mention an application's name in
> the Summary. There is the package name "eiciel", which is very likely
> displayed together with the summary. And for anyone who doesn't know that
> "Eiciel" is a name, above summary is confusing. The one important thing to
> mention is the summary is to give a brief description of what this package
> does: it provides a graphical access control list editor. Everything
> beyond that can go into the %description.

ok

>> -rw-r--r--  /usr/share/applications/eiciel.desktop
>> -rw-r--r--  /usr/share/applications/fedora-eiciel.desktop
> 
> Two desktop entries. The one that appeared here in the menu does not work:
> 
>   Could not launch menu item
> 
>   Details: Failed to execute child process
>   "/home/roger/eiciel/TEST/bin/eiciel" (No such file or directory)
> 
> This indicates that you either want to remove "eiciel.desktop" or use it
> as the source for "desktop-file-install", adding vendor and your own
> changes, and using --delete-original to get rid of it.

I wasn't too clear about .desktop files, I should have asked about my 
use of desktop-file-install particularly about "vendor" and 
"add-category", I was just following what I could see elsewhere.

Thanks again, I'll give it a brush up later ...