Bug 177580

Summary: Review Request: lat (LDAP Administration Tool)
Product: [Fedora] Fedora Reporter: Dave Malcolm <dmalcolm>
Component: Package ReviewAssignee: Hans de Goede <hdegoede>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: hdegoede, mschick, paul, paul, pcfe
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-06-17 12:45:57 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 163779    
Attachments:
Description Flags
Rewritten spec for lat 1.0.4
none
diff file for spec none

Description Dave Malcolm 2006-01-11 21:49:39 UTC
Spec Name or Url: http://people.redhat.com/dmalcolm/lat/lat.spec
SRPM Name or Url: http://people.redhat.com/dmalcolm/lat/lat-0.8.2-1.src.rpm
Description: LDAP Administration Tool

LAT is a GNOME-based GUI tool for working with LDAP, written in gtk-sharp/mono

Comment 1 Wart 2006-01-30 00:56:10 UTC
This isn't a formal review because I can't sponsor you, but just a few comments
from browsing the spec file:

* In %install and %clean, don't add the check for "/".  The use of BuildRoot:
ensures that it won't delete your entire hard drive.

* Add a period at the end of the %description.

* Include the full URL to the Source: archive, not just the filename.  The Url:
tag should be the Url to the package's home page, not the source tarball.

* Consider using %{?dist} in the Release: tag.  Many packagers find it useful.

* A couple of warnings appeared during the build that caused some missing file
errors; it looks like something wasn't obeying $(DESTDIR) in the Makefile.
scrollkeeper-update -p /var/tmp/lat-0.8.2-root/var/scrollkeeper -o
/var/tmp/lat-0.8.2-root/usr/share/omf/lat
Cannot stat file: /usr/share/gnome/help/lat/C/lat.xml : No such file or directory
Cannot write to log file: /var/log/scrollkeeper.log : Permission denied
Cannot write to log file: /var/log/scrollkeeper.log : Permission denied
...
error: File not found: /var/tmp/lat-0.8.2-root/var/scrollkeeper/index/0
error: File not found: /var/tmp/lat-0.8.2-root/var/scrollkeeper/TOC/0



Comment 2 Paul Howarth 2006-01-31 11:12:19 UTC
I'm quite interested in this package and am also a potential sponsor, but
unfortunately I don't have a rawhide system (needed for the mono deps) to try
this on. Perhaps after FC5 is released if nobody else steps up before then.

I concur with the remarks on comment #1 though, particularly the first three.

Comment 3 John Mahowald 2006-02-26 16:44:42 UTC
Didn't build on rawhide.

checking for GTKSHARP... configure: error: Package requirements (gtk-sharp-2.0
>= 2.4             gnome-sharp-2.0 >= 2.4                  gconf-sharp-2.0 >=
2.4                  glade-sharp-2.0 >= 2.4) were not met:

No package 'gtk-sharp-2.0' found
No package 'gnome-sharp-2.0' found
No package 'gconf-sharp-2.0' found
No package 'glade-sharp-2.0' found


Seems like you really mean gtk-sharp2 for the (Build)Requires.

Comment 4 Paul Howarth 2006-04-20 15:32:59 UTC
Created attachment 128043 [details]
Rewritten spec for lat 1.0.4

Comment 5 Paul Howarth 2006-04-20 15:36:49 UTC
The attached spec file in Comment #4 addresses all of the issues raised so far,
plus a shedload of others. However, since the initial submitter has not yet
responded to any comment in this bug, I wonder if he's still interested in
maintaining this package?

Comment 6 Dave Malcolm 2006-04-20 15:43:21 UTC
Re comment #4: I'd be very happy if someone else volunteered to maintain it; I
only occasionally use this tool, and haven't touched the package in a while, I'm
afraid.

Comment 7 Joost Soeterbroek 2006-04-22 16:18:05 UTC
does not build on FC5:

find: /var/tmp/lat-0.8.2-root/usr/lib/debug: No such file or directory
+ /usr/lib/rpm/check-rpaths /usr/lib/rpm/check-buildroot
+ /usr/lib/rpm/redhat/brp-compress
+ /usr/lib/rpm/redhat/brp-strip-static-archive /usr/bin/strip
+ /usr/lib/rpm/redhat/brp-strip-comment-note /usr/bin/strip /usr/bin/objdump
+ /usr/lib/rpm/brp-python-bytecompile
Processing files: lat-0.8.2-1
error: File not found: /var/tmp/lat-0.8.2-root/var/scrollkeeper/index/0
error: File not found: /var/tmp/lat-0.8.2-root/var/scrollkeeper/TOC/0
Processing files: lat-debuginfo-0.8.2-1


RPM build errors:
    File not found: /var/tmp/lat-0.8.2-root/var/scrollkeeper/index/0
    File not found: /var/tmp/lat-0.8.2-root/var/scrollkeeper/TOC/

Comment 8 Paul Howarth 2006-04-22 17:12:13 UTC
(In reply to comment #7)
> does not build on FC5:
> 
> find: /var/tmp/lat-0.8.2-root/usr/lib/debug: No such file or directory
> + /usr/lib/rpm/check-rpaths /usr/lib/rpm/check-buildroot
> + /usr/lib/rpm/redhat/brp-compress
> + /usr/lib/rpm/redhat/brp-strip-static-archive /usr/bin/strip
> + /usr/lib/rpm/redhat/brp-strip-comment-note /usr/bin/strip /usr/bin/objdump
> + /usr/lib/rpm/brp-python-bytecompile
> Processing files: lat-0.8.2-1
> error: File not found: /var/tmp/lat-0.8.2-root/var/scrollkeeper/index/0
> error: File not found: /var/tmp/lat-0.8.2-root/var/scrollkeeper/TOC/0
> Processing files: lat-debuginfo-0.8.2-1
> 
> 
> RPM build errors:
>     File not found: /var/tmp/lat-0.8.2-root/var/scrollkeeper/index/0
>     File not found: /var/tmp/lat-0.8.2-root/var/scrollkeeper/TOC/


We knew that from Comment #1. There's an updated spec for lat 1.0.4 in Comment
#4 but it seems from Comment #6 that Dave doesn't really want to maintain this
package anyway (and neither do I as it's not something I use except on a very
occasional basis).


Comment 9 Paul Howarth 2006-05-23 10:07:40 UTC
I have an updated lat 1.0.5 package that any potential maintainer of this
package might like to start with:

http://www.city-fan.org/~paul/extras/lat/

It builds fine in mock, though I've only tried i386.

Comment 10 Hans de Goede 2006-06-08 09:12:57 UTC
Is anyone going to pick this up? If not I suggest we close this as wontfix


Comment 11 Paul Howarth 2006-06-08 09:32:09 UTC
OK OK, I'll offer to maintain it, but if anyone else more interested comes
along, they're welcome to it.

Here are my packages:
SPEC: http://www.city-fan.org/~paul/extras/lat/lat.spec
SRPM: http://www.city-fan.org/~paul/extras/lat/lat-1.0.5-1.src.rpm


Comment 12 Hans de Goede 2006-06-08 10:26:36 UTC
Ok,

Removing the FE_NEEDSPONSOR blocker then. I guess you kinda expect todo the
review now? :)



Comment 13 Paul Howarth 2006-06-08 10:31:01 UTC
You're welcome to review it if you feel the urge, but I'm in no big hurry :-)

Comment 14 Paul Howarth 2006-06-14 13:28:08 UTC
(recovering from bugzilla crash)

New spec, SRPM, and FC5 i386 RPM available here:
http://www.city-fan.org/~paul/extras/lat/

* Mon Jun 12 2006 Paul Howarth <paul> - 1.0.5-2
- Spec file cleanups (#177580)
-   No need to use update-desktop-database (no MIME type in desktop file)
-   No need to remove .la files (artefact from old package)
-   Own directories %{_datadir}/gnome/ %{_datadir}/gnome/help/ %{_datadir}/omf/
-   Put icon in %{_datadir}/icons/hicolor/22x22/apps directory rather than
    %{_datadir}/pixmaps, and update icon cache post-install/removal
-   Add doc files AUTHORS ChangeLog COPYING* README TODO
- Add missing buildreq gettext
- Remove redundant MONO_SHARED_DIR assignments in %%build and %%install
- Don't redefine %%{_libdir}, it's not needed
- Prevent creation of debuginfo package, which would be empty for a mono app

I've added a dependency for this bug of Bug #193957 (nant), where mono packaging
guidelines are being discussed.


Comment 15 Paul F. Johnson 2006-06-14 23:22:41 UTC
The packaging guidelines are nearing completion now....

In short...

- if the final code doesn't include any .so files, you have to make it noarch -
this removes the libdir problems (if it has a configure script, either %define
the target as sparc86x or add --target=sparc86x after the configure call)

- if the final code includes .so files, you have to make it architecture specific

- Don't change things to %datadir unless the makefile puts it there

As soon as spot can have the final things signed off (I'm guessing at the next
FESCO), the wheels can be set in motion for approval.

Comment 16 Hans de Goede 2006-06-15 06:32:34 UTC
(In reply to comment #15)
> The packaging guidelines are nearing completion now....
> 
> In short...
> 
> - if the final code doesn't include any .so files, you have to make it noarch -
> this removes the libdir problems (if it has a configure script, either %define
> the target as sparc86x or add --target=sparc86x after the configure call)
> 
Is this el weirdo --target stuff to remove the problem with %configure
complaining about noarch-redhat being an unknown target? And is this considered
a better fix then the fix posted to f-e-l for this (which patches configure and
the runs autoreconf)?



Comment 17 Paul F. Johnson 2006-06-15 11:36:00 UTC
Yes.

This is what came from discussions on IRC last night/this morning (UK time).
There are two ways to do the target

1. There is a %define which does the trick
2. %configure --target=sparc86x

This method doesn't require any fixes to the configure script. The only
advantage (1) has over (2) is that for (2) you specify target twice.

If when you're building the application only mcs is called, then the final
application should be noarch and be placed in %_libdir (which should be /usr/lib
for noarch if I've understood things correctly).

The reason for libdir rather than datadir is that it is "less wrong" to put
things in there than datadir (this was found by comparing what happens upstream
and how SuSE, Ubuntu/Debian are doing things - spot considered them both to be
wrong, but libdir to be less wrong).

Obviously, everything needs to be ratified, but it looks like noarch for purely
C# apps will be the way to go.

Comment 18 Paul Howarth 2006-06-15 12:44:53 UTC
lat-1.0.5-3 available here:
http://www.city-fan.org/~paul/extras/lat/

* Thu Jun 15 2006 Paul Howarth <paul> - 1.0.5-3
- Pure C# apps should be noarch
- Swap one set of horrible mono hacks for another


Comment 19 Paul F. Johnson 2006-06-15 13:02:17 UTC
Does the app install natively to %_datadir or is this something that has been
added in as discussion was surrounding if mono app should be in %_libdir or
%_datadir?

Comment 20 Paul Howarth 2006-06-15 13:13:24 UTC
(In reply to comment #19)
> Does the app install natively to %_datadir or is this something that has been
> added in as discussion was surrounding if mono app should be in %_libdir or
> %_datadir?

It installs natively to /usr/lib, which from my reading of Comment #17 was the
least-wrong place for it to be, so I haven't tried forcing it to go anywhere
else. I don't have an x86_64 box to test this on though, so I'm not sure what
will happen there.

Comment 21 Paul F. Johnson 2006-06-15 13:33:03 UTC
Fails to build on x86_64

File not found: /var/tmp/lat-1.0.5-3-root-paul/usr/lib64/lat

It looks like in files mono apps will need /usr/lib to be explicitly stated.

You also need to add in

%define debug_package %{nil} 

To remove the debug package


Comment 22 Paul Howarth 2006-06-15 13:36:44 UTC
(In reply to comment #21)
> Fails to build on x86_64
> 
> File not found: /var/tmp/lat-1.0.5-3-root-paul/usr/lib64/lat
> 
> It looks like in files mono apps will need /usr/lib to be explicitly stated.

Yes, I'll change that.
> You also need to add in
> 
> %define debug_package %{nil} 
> 
> To remove the debug package

You got a debuginfo package? Building on i386 I didn't get one after changing
BuildArch: to noarch. That's why I removed the "%define debug_package %{nil}"
that was there previously.

Comment 23 Paul F. Johnson 2006-06-15 13:37:49 UTC
Created attachment 130974 [details]
diff file for spec

Diff file - this now builds happily under x86_64 as well as x86

Comment 24 Hans de Goede 2006-06-15 13:53:55 UTC
Hmm,

This is getting really ugly, so even though the package is:
BuildArch: noarch

When building with rpmbuild on x86_64 %libdir still is /usr/lib64 ?? I just
confirmed this on my own x86_64 system.
So that means that %libdir as passed to configure will also be /usr/lib64,
appearantly lat doesn't honor this bot others do, so it looks like that for pure
mono apps we need:
BuildArch: noarch
+libdir hack
+target hack

Nice :|

Also notice that all the hammer buildsys systems are x86_64 machines!


Comment 25 Paul F. Johnson 2006-06-15 14:06:10 UTC
Surely though the noarch should behave the same irrespective of the package
being built (/usr/lib for all platforms). If it's not behaving the same then
that indicates it being a bug elsewhere.

Comment 26 Hans de Goede 2006-06-15 14:11:45 UTC
Agreed, so someone should file a bug against rpmbuild on this. In the mean time
I suggest we use the %libdir hack to work around this for now.


Comment 27 Paul Howarth 2006-06-15 14:13:52 UTC
Can one of you guys confirm whether or not you're getting a debuginfo package
when building the noarch package on x86_64 (or anywhere else really)? I'm not
getting one on i386.

Comment 28 Paul F. Johnson 2006-06-15 14:15:35 UTC
#27 - my mistake, mock was being silly at this end

#26 - I'll file one now

Comment 29 Paul F. Johnson 2006-06-15 14:24:48 UTC
#28 - the BZ reference for the rpmbuild bug is #195487

Comment 30 Hans de Goede 2006-06-15 14:28:50 UTC
FYI I'm not getting a debuginfo package when building the noarch version of lat
on x86_64.



Comment 31 Paul Howarth 2006-06-15 14:36:06 UTC
lat-1.0.5-4 available here:
http://www.city-fan.org/~paul/extras/lat/

* Thu Jun 15 2006 Paul Howarth <paul> - 1.0.5-4
- Use %%{_prefix}/lib rather than %%{_libdir}, needed for 64-bit builds


Comment 32 Hans de Goede 2006-06-16 17:48:41 UTC
Paul Howarth,

I'm not sure that:
- Use %%{_prefix}/lib rather than %%{_libdir}, needed for 64-bit builds

Is the proper fix, this might work for lat, because its ./configure appearantly
ignores %libdir, but it won't work for other noarch mono packages, I believe the
libdir hack is the "correct" solution for this. Yes in combination with noarch
and the %target hack, ain't mono fun to package?

Paul F. Johnson,

Any opinions on this and has anything been decided on this on last Thursday's
FESco meeting?


Comment 33 Paul F. Johnson 2006-06-16 20:25:44 UTC
I've not seen the reports from last Thursday's FESco meeting and they're not on
the wiki yet (latest one on there is the 1st June).

As to this, you *must* statically define libdir - currently rpmbuild is broken
(quite badly by the looks of it for noarch), so this is the only way to ensure
the build works on all architectures.

By the looks of it as well .pc files on noarch aren't working either (well, not
properly)

Comment 34 Hans de Goede 2006-06-16 20:39:39 UTC
(In reply to comment #33)
> As to this, you *must* statically define libdir - currently rpmbuild is broken
> (quite badly by the looks of it for noarch), so this is the only way to ensure
> the build works on all architectures.
> 

What do you mean with staticly define, I assume you mean the libdir hack aka:
%define _libdir /usr/lib at the top of the spec, right?


Comment 35 Paul Howarth 2006-06-16 20:47:12 UTC
(In reply to comment #32)
> I'm not sure that:
> - Use %%{_prefix}/lib rather than %%{_libdir}, needed for 64-bit builds
> 
> Is the proper fix, this might work for lat, because its ./configure appearantly
> ignores %libdir, but it won't work for other noarch mono packages, I believe the
> libdir hack is the "correct" solution for this. Yes in combination with noarch
> and the %target hack, ain't mono fun to package?

I suspect that many mono packages will completely ignore libdir. To test this
hypothesis, I tried making these changes to PFJ's gtksourceview-sharp package
(Bug #178901):

$ diff -u gtksourceview-sharp.spec.orig gtksourceview-sharp.spec
--- gtksourceview-sharp.spec.orig       2006-06-16 21:28:20.000000000 +0100
+++ gtksourceview-sharp.spec    2006-06-16 21:38:07.000000000 +0100
@@ -1,5 +1,5 @@
 %define extra 0.10
-%define debug_package %{nil}
+#define debug_package %{nil}

 Summary: A C sharp binder for gtksourceview
 Name: gtksourceview-sharp
@@ -11,9 +11,16 @@
 URL: http://go-mono.com/sources/%{name}-%{version}/
 Buildroot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildRequires: mono-data, mono-devel, gtk-sharp2, gtksourceview-devel,
gtk-sharp2-gapi, gecko-sharp2, pkgconfig
+BuildRequires: which
 Requires: mono-core, gtksourceview
 BuildArch: noarch

+# Horrible, horrible, mono hacks
+%define _target_platform sparc86x-%{_vendor}-%{_target_os}%{?_gnu}
+%define monolibdir %{_prefix}/lib
+# To see if any attention is paid to %_libdir
+%define _libdir /opt/lib
+
 %description
 gtksourceview-sharp is a C sharp binder for gtksourceview

@@ -31,7 +38,7 @@

 %build
 export MONO_SHARED_DIR=%{_builddir}/%{?buildsubdir}
-%configure --target=sparc86x
+%configure
 make %{?_smp_mflags}

 %install
@@ -45,12 +52,12 @@
 %doc AUTHORS COPYING INSTALL NEWS README
 %{_datadir}/gapi-2.0/gtksourceview-api.xml
 %{_datadir}/gtksourceview-1.0/language-specs/nemerle.lang
-%{_libdir}/mono/gac/gtksourceview-sharp/*
-%{_libdir}/mono/gtksourceview-sharp-2.0/gtksourceview-sharp.dll
+%{monolibdir}/mono/gac/gtksourceview-sharp/*
+%{monolibdir}/mono/gtksourceview-sharp-2.0/gtksourceview-sharp.dll

 %files devel
 %defattr (-,root,root,-)
-%{_libdir}/pkgconfig/gtksourceview-sharp-2.0.pc
+%{monolibdir}/pkgconfig/gtksourceview-sharp-2.0.pc

 %clean
 %{__rm} -rf %{buildroot}


It built OK for me on i386, and since it ignored my change of libdir to
/opt/lib, I believe it would build on x86_64 too. Obviously this is too small a
sample to use as a basis for setting general guidelines, but it does indicate
that lat is not alone in ignoring libdir.

I think that changing libdir is a more horrible hack that what I've done, and
I'd prefer to keep the horribleness level down as low as possible :-)

Comment 36 Paul F. Johnson 2006-06-16 20:57:31 UTC
#34, yes, the libdir  {_exec_prefix}/lib hack

#35 That should work fine under x86_64 - I'd need to test it though.

Comment 37 Hans de Goede 2006-06-16 21:11:48 UTC
#35 Yes that may work for other mono packages too, and afaik it doesn't work for
ALL mono packages, thats why PFJ came up with the libdir hack, I agree your
version is better, but if the uglier version works in all cases then I vote to
use the uglier version in all cases for consistency. That will also make it
easier to write up some mono packaging guidelines (which we badly need).


Comment 38 Paul F. Johnson 2006-06-16 21:30:11 UTC
#37 - the packaging guidelines are being discussed on IRC quite a bit. spot and
I gave them a good thrashing the other night and more or less, the things up on
the wiki are fine (for now).

The only bit missing is the info I've added on this bug for noarch

That said, it is supposed to be going before the ratification people for
packaging sometime soon, so something should be coming out nicely from that.

For now, keep with the libdir hack (until rpmbuild is fixed), change to noarch
and I can't see any real reason why this can't be approved - it certainly fits
the mono criteria as it stands (for what it's worth - I'm just some guy you know
;-p)

Comment 39 Hans de Goede 2006-06-17 05:04:13 UTC
Ok,

Paul Howarth, can you do one more iteration using the _libdir thingie for
(hopefully) future consistency with other mono packages? Then I'll take a second
look and approve it assuming nothing is wrong.


Comment 40 Paul Howarth 2006-06-17 08:43:42 UTC
I've reinstated the _libdir hack but I really think it's the wrong thing to do.

I looked at the Makefile for this package and it uses a macro called
ASSEMBLYlibdir, which is assigned the value $(prefix)/lib/$(ASSEMBLY), where
$(ASSEMBLY) is the application name. The regular libdir never comes into it. It
would be interesting to see what other applications do, particularly any that do
things differently (any for which the libdir hack is actually necessary?).

1.0.5-5 available at http://www.city-fan.org/~paul/extras/lat/

Comment 41 Hans de Goede 2006-06-17 12:22:42 UTC
(In reply to comment #40)
> I've reinstated the _libdir hack but I really think it's the wrong thing to do.
> 

I dunno it hs the advantage of always working even for mono apps with broken
upstream makefiles.

Anyways lets wait for some kinda final mono packaging guidelines, if these don't
mandate the use of the libdir hack feel free to revert.

In the mean time all MUST items are fixed and it looks like this is packaged
according to the final mono packaging guidelines, so: Approved!

For people missing a Full review one was done but got nuked due to the BZ crash,
comment #14 which is the reply to the full review gives a nice overview of all
must fix items listed in the destroyed full review.


Comment 42 Paul Howarth 2006-06-17 12:45:57 UTC
Imported, tagged, built.

 11138 (lat): Build on target fedora-development-extras succeeded.
     Build logs may be found at
http://buildsys.fedoraproject.org/logs/fedora-development-extras/11138-lat-1.0.5-5.fc6/

owners.list updated

FC-5 branch requested.

Thanks for all the help everyone.

Comment 43 Brian Pepple 2006-06-17 14:03:23 UTC
Ummmm, where's your BR on desktop-file-install?
http://fedoraproject.org/wiki/Packaging/Guidelines?highlight=%28desktop-file-install%29#head-254ddf07aae20a23ced8cecc219d8f73926e9755

Also, your creatation of the directories for '%{_datadir}/gnome/help/' &
'%{_datadir}/omf/' is incorrect, and needs to be fixed.  You are taking
ownership of the directories.  Run 'rpm -qf /usr/share/mime' & 
rpm -qf /usr/share/omf' to verify this yourself.  I've noticed that you seem to
have problems with directories ownership, and would suggest working with your
sponsor or a mentor to prevent this from happening on your future packages.

-1 APPROVAL

Comment 44 Paul Howarth 2006-06-17 14:58:38 UTC
(In reply to comment #43)
> Ummmm, where's your BR on desktop-file-install?

Not necessary (which should be evident from the fact that the package built
successfully on the buildsystem). The dep chain is:

gtk-sharp2 -> gnome-panel -> gnome-menus -> redhat-menus -> desktop-file-utils

> Also, your creatation of the directories for '%{_datadir}/gnome/help/' &
> '%{_datadir}/omf/' is incorrect, and needs to be fixed.  You are taking
> ownership of the directories.  Run 'rpm -qf /usr/share/mime' & 
> rpm -qf /usr/share/omf' to verify this yourself.

This is deliberate.
http://www.redhat.com/archives/fedora-package-review/2006-June/msg01049.html

> I've noticed that you seem to
> have problems with directories ownership, and would suggest working with your
> sponsor or a mentor to prevent this from happening on your future packages.

What other directory ownership problems have you noticed?


Comment 45 Brian Pepple 2006-06-17 15:13:27 UTC
(In reply to comment #44)
> Not necessary (which should be evident from the fact that the package built
> successfully on the buildsystem). The dep chain is:
> 
> gtk-sharp2 -> gnome-panel -> gnome-menus -> redhat-menus -> desktop-file-utils

The wiki should be updated then, since this is clearly stated on the Package
Guidelines as a BR.
 
> > Also, your creatation of the directories for '%{_datadir}/gnome/help/' &
> > '%{_datadir}/omf/' is incorrect, and needs to be fixed.  You are taking
> > ownership of the directories.  Run 'rpm -qf /usr/share/mime' & 
> > rpm -qf /usr/share/omf' to verify this yourself.
> 
> This is deliberate.
> http://www.redhat.com/archives/fedora-package-review/2006-June/msg01049.html

I disagree with this suggestion.  This hasn't been common practice, and should
be forwarded to FESCO or whoever is in charge of the Packaging Guidelines (spot,
I believe) before implementing.
 
> > I've noticed that you seem to
> > have problems with directories ownership, and would suggest working with your
> > sponsor or a mentor to prevent this from happening on your future packages.
> 
> What other directory ownership problems have you noticed?
>

Monodoc, where your taking ownership of directories (/usr/lib/mono &
/usr/lib/mono/gac) which are owned by mono-core.

Comment 46 Paul Howarth 2006-06-17 15:28:41 UTC
(In reply to comment #45)
> (In reply to comment #44)
> > Not necessary (which should be evident from the fact that the package built
> > successfully on the buildsystem). The dep chain is:
> > 
> > gtk-sharp2 -> gnome-panel -> gnome-menus -> redhat-menus -> desktop-file-utils
> 
> The wiki should be updated then, since this is clearly stated on the Package
> Guidelines as a BR.

It *is* a BR. Not directly, but pulled in implicitly by the BR: gtk-sharp2

> > > Also, your creatation of the directories for '%{_datadir}/gnome/help/' &
> > > '%{_datadir}/omf/' is incorrect, and needs to be fixed.  You are taking
> > > ownership of the directories.  Run 'rpm -qf /usr/share/mime' & 
> > > rpm -qf /usr/share/omf' to verify this yourself.
> > 
> > This is deliberate.
> > http://www.redhat.com/archives/fedora-package-review/2006-June/msg01049.html
> 
> I disagree with this suggestion.  This hasn't been common practice, and should
> be forwarded to FESCO or whoever is in charge of the Packaging Guidelines (spot,
> I believe) before implementing.

Not owning the directory can result in the directory being left behind at
package erase time, since none of the dependencies of this package own it.

On the other hand, having multiple packages owning directories can cause issues
with path-based dependencies.

So there's no clear "winner" about which is the correct approach. As for
precedents, a majority of (for example) perl module packages in Extras share
ownership of directories under /usr/lib/perl5/vendor_perl/5.8.8 (FC5)

> > > I've noticed that you seem to
> > > have problems with directories ownership, and would suggest working with your
> > > sponsor or a mentor to prevent this from happening on your future packages.
> > 
> > What other directory ownership problems have you noticed?
> >
> 
> Monodoc, where your taking ownership of directories (/usr/lib/mono &
> /usr/lib/mono/gac) which are owned by mono-core.

monodoc is not one of my packages.


Comment 47 Brian Pepple 2006-06-17 16:06:45 UTC
(In reply to comment #46)
> (In reply to comment #45)
>http://www.redhat.com/archives/fedora-package-review/2006-June/msg01049.html
> > 
> > I disagree with this suggestion.  This hasn't been common practice, and should
> > be forwarded to FESCO or whoever is in charge of the Packaging Guidelines (spot,
> > I believe) before implementing.
> 
> Not owning the directory can result in the directory being left behind at
> package erase time, since none of the dependencies of this package own it.
> 
> On the other hand, having multiple packages owning directories can cause issues
> with path-based dependencies.
> 
> So there's no clear "winner" about which is the correct approach. As for
> precedents, a majority of (for example) perl module packages in Extras share
> ownership of directories under /usr/lib/perl5/vendor_perl/5.8.8 (FC5)
> 

Regardless, this should be to be discussed by FESCO or the Packaging Guideline
group before implementing, since this has not been the common practice.  Was
this something that the Perl SIG came up with?

> > > What other directory ownership problems have you noticed?
> > >
> > 
> > Monodoc, where your taking ownership of directories (/usr/lib/mono &
> > /usr/lib/mono/gac) which are owned by mono-core.
> 
> monodoc is not one of my packages.
> 

Your right, wrong Paul.  Since this was a mono app, I assumed you were Paul
Johnson.  My bad.

Comment 48 Paul Howarth 2006-06-17 16:48:39 UTC
(In reply to comment #47)
> (In reply to comment #46)
> > (In reply to comment #45)
> >http://www.redhat.com/archives/fedora-package-review/2006-June/msg01049.html
> > > 
> > > I disagree with this suggestion.  This hasn't been common practice, and should
> > > be forwarded to FESCO or whoever is in charge of the Packaging Guidelines
(spot,
> > > I believe) before implementing.
> > 
> > Not owning the directory can result in the directory being left behind at
> > package erase time, since none of the dependencies of this package own it.
> > 
> > On the other hand, having multiple packages owning directories can cause issues
> > with path-based dependencies.
> > 
> > So there's no clear "winner" about which is the correct approach. As for
> > precedents, a majority of (for example) perl module packages in Extras share
> > ownership of directories under /usr/lib/perl5/vendor_perl/5.8.8 (FC5)
> > 
> 
> Regardless, this should be to be discussed by FESCO or the Packaging Guideline
> group before implementing, since this has not been the common practice.  Was
> this something that the Perl SIG came up with?

It's been common practice for perl modules for a long time, and it's not really
perl-specific (perl modules just provide a good set of examples). Another good
example is to look at the ownership of /usr/share/pixmaps.

What the guidelines say is:

  Packages must not own files or directories already owned by other
  packages. The rule of thumb here is that the first package to be
  installed should own the files or directories that other packages may
  rely upon. This means, for example, that no package in Fedora should
  ever share ownership with any of the files or directories owned by
  the filesystem or man package. If you feel that you have a good
  reason to own a file or directory that another package owns, then
  please present that at package review time.

Clearly this package is violating the first sentence there. However, following
the rule of thumb, it is quite possible for this package to be the first one to
own the directories you have an issue with, if it is installed on a gnome-free
system for example. The guideline goes on to say that the issue can be discussed
at package review time, and in fact in this case the request to add ownership of
those directories came from the reviewer rather than from me (though I agreed
that it was the right thing to do).


Comment 49 Brian Pepple 2006-06-17 17:02:54 UTC
(In reply to comment #48)
> 
> Clearly this package is violating the first sentence there. However, following
> the rule of thumb, it is quite possible for this package to be the first one to
> own the directories you have an issue with, if it is installed on a gnome-free
> system for example. The guideline goes on to say that the issue can be discussed
> at package review time, and in fact in this case the request to add ownership of
> those directories came from the reviewer rather than from me (though I agreed
> that it was the right thing to do).
> 

This package does depend on Gnome, since it depends on gtk-sharp2 (which
requires libgnomeui, libgnomecanvas, gnome-vfs2, etc.  This sorta defeats your
reasoning for owning the directories.


Comment 50 Paul Howarth 2006-06-17 18:40:00 UTC
(In reply to comment #49)
> This package does depend on Gnome, since it depends on gtk-sharp2 (which
> requires libgnomeui, libgnomecanvas, gnome-vfs2, etc.  This sorta defeats your
> reasoning for owning the directories.

You might think so, but...

$ rpm -qf /usr/share/omf
gnome-doc-utils-0.6.0-1
bug-buddy-2.14.0-1
glade2-2.12.1-2
gnome-user-docs-2.14.2-1.fc5.1
gnome-system-monitor-2.14.1-1.fc5.1
gnome-utils-2.14.0-4.fc5.2
gnome-media-2.14.2-1
gthumb-2.7.7-1
eog-2.14.2-1.fc5.1
gedit-2.14.3-1
gnome-terminal-2.14.2-1
file-roller-2.14.3-1
$ rpm -qf /usr/share/gnome
gnome-doc-utils-0.6.0-1
bug-buddy-2.14.0-1
glade2-2.12.1-2
gnome-user-docs-2.14.2-1.fc5.1
gnome-system-monitor-2.14.1-1.fc5.1
gnome-utils-2.14.0-4.fc5.2
gnome-applets-2.14.2-1.fc5.1
gnome-media-2.14.2-1
gthumb-2.7.7-1
eog-2.14.2-1.fc5.1
gedit-2.14.3-1
gnome-terminal-2.14.2-1
file-roller-2.14.3-1
gnome-games-2.14.2-1
dia-0.95-4.fc5
gnome-session-2.14.2-1.fc5.1
$ rpm -qf /usr/share/gnome/help
gnome-doc-utils-0.6.0-1
bug-buddy-2.14.0-1
glade2-2.12.1-2
gnome-user-docs-2.14.2-1.fc5.1
gnome-system-monitor-2.14.1-1.fc5.1
control-center-2.14.2-1
gnome-utils-2.14.0-4.fc5.2
gnome-applets-2.14.2-1.fc5.1
gnome-media-2.14.2-1
gthumb-2.7.7-1
eog-2.14.2-1.fc5.1
gedit-2.14.3-1
gnome-terminal-2.14.2-1
file-roller-2.14.3-1
gnome-games-2.14.2-1
dia-0.95-4.fc5

None of these packages are dependencies, direct or indirect, of lat.


Comment 51 Hans de Goede 2006-06-17 18:57:00 UTC
Brian,

As shown by the long rpm -qf list posted by Paul, there are many packages who
are owning these dirs, simply because having multiple owned dirs is concidered
less bad then having unowned dirs. Actually many of those packages are owned by me.

The real problem is that there should be some kinda base package, be it
gnome-libs, or maybe gnome-filesystem, which should provide these dirs, but
there isn't.

Further discussion really doesnot belong here, as this is not specific to this
package / review. Please take this discussion to f-e-l, where it has been done
once already with no outcome, or directly to FESco. While you are at it also
include the /use/share/icons/hicolor hierachy in the discussion because that
whole hierarchy has directory ownership issues too.