[ Adding alias. ]

* missing "BuildRequires: gettext"

* in -devel package, missing "Requires: gnome-vfs2-devel glib2-devel"
These are "Requires" in the pkgconfig file. glib2-devel is redundant
(due to glibmm24-devel), but a direct requirement.

* Avoid duplicating the licence file "COPYING" in sub-packages.
Here the -devel package requires the main package. 

* GConf schema files are no %config files and should not be
marked as such.

* In the scriptlets, you list the two schema files without wildcards.
You ought to do the same in the %files section, so you will notice
when the set of schema files changes.

* %defattr missing in -devel package

(In reply to comment #2)
> * missing "BuildRequires: gettext"
> * %defattr missing in -devel package
> * in -devel package, missing "Requires: gnome-vfs2-devel glib2-devel"
> These are "Requires" in the pkgconfig file. glib2-devel is redundant
> (due to glibmm24-devel), but a direct requirement.
 
Oopsie. Fixed in release 2.
 

> * Avoid duplicating the licence file "COPYING" in sub-packages.
> Here the -devel package requires the main package. 

I'm of the opinion that such license files should always be included in any
subpackage ot make the licensing explicitly clear. However, since there is not
officially any such guideline, I don't think this a blocker at all.
 

> * GConf schema files are no %config files and should not be
> marked as such.

My understanding is that schemas should be marked %config, as they are the base
configurations for the package which the system administrator can change as he
or she sees fit, etc. Several other of my packages which use gconf schemas have
them marked as %config(noreplace). Why is it invalid? Thanks.

> * In the scriptlets, you list the two schema files without wildcards.
> You ought to do the same in the %files section, so you will notice
> when the set of schema files changes.

Oooooooh. Good catch; thanks. Fixed in release 2.

URLs for release 2:
Spec: http://thecodergeek.com/downloads/fedora/nemiver.spec
SRPM: http://thecodergeek.com/downloads/fedora/nemiver-0.3.0-2.src.rpm

You would not edit installed schema files if you wanted to change
the system-wide GConf defaults. You would edit the key/value
pairs in the database, e.g. with gconf-editor or gconftool-2.
Modifying schema files leads to non-trivial upgrade scenarios,
especially when they are marked as %config(noreplace).

(In reply to comment #4)
> You would not edit installed schema files if you wanted to change
> the system-wide GConf defaults. You would edit the key/value
> pairs in the database, e.g. with gconf-editor or gconftool-2.
> Modifying schema files leads to non-trivial upgrade scenarios,
> especially when they are marked as %config(noreplace).

Aaahh that makes a lot of sense. Thanks for the explanation. I fixed the schemas
stuff in release 3:

Spec: http://thecodergeek.com/download/fedora/nemiver.spec
Spec: http://thecodergeek.com/download/fedora/nemiver-0.3.0-3.src.rpm

However, now rpmlint gives me two messages about that:
  W: nemiver non-conffile-in-etc /etc/gconf/schemas/nemiver-workbench.schemas
  W: nemiver non-conffile-in-etc /etc/gconf/schemas/nemiver-dbgperspective.schemas
I take it there are safe to ignore for GConf schemas then?

Thanks.

(In reply to comment #5)
> 
> However, now rpmlint gives me two messages about that:
>   W: nemiver non-conffile-in-etc /etc/gconf/schemas/nemiver-workbench.schemas
>   W: nemiver non-conffile-in-etc /etc/gconf/schemas/nemiver-dbgperspective.schemas
> I take it there are safe to ignore for GConf schemas then?
>
Yap, that's the usual warnings on about schema files.
If everyone agrees on that, rpmlint could be modified to check the reverse
condition, i.e.: everything under /etc/gconf/schema triggers a warning if it is
actually marked as %config

(In reply to comment #5)
> 
> Spec: http://thecodergeek.com/download/fedora/nemiver.spec
> Spec: http://thecodergeek.com/download/fedora/nemiver-0.3.0-3.src.rpm
> 

Correct links seems to be:
Spec: http://thecodergeek.com/downloads/fedora/nemiver.spec
SRPM: http://thecodergeek.com/downloads/fedora/nemiver-0.3.0-3.src.rpm

(In reply to comment #7)
> 
> Correct links seems to be:
> Spec: http://thecodergeek.com/downloads/fedora/nemiver.spec
> SRPM: http://thecodergeek.com/downloads/fedora/nemiver-0.3.0-3.src.rpm
> 

Err...dang typos. Thanks, Gianluca.

Build successfully in mock for FC5.

rpmlint on src.rpm has only one small complaint

/var/lib/mock/fedora-5-i386-core/result/nemiver-0.3.0-3.fc5.src.rpm
W: nemiver mixed-use-of-spaces-and-tabs (spaces: line 150, tab: line 1)

I'd like to try it out, but there are some issues related to SELinux; this is
what I have in audit.log when running nemiver:

type=AVC msg=audit(1169803885.025:2173): avc:  denied  { execmod } for 
pid=32735 comm="nemiver" name="libdbgperspectiveplugin.so" dev=dm-0 ino=12452120
scontext=user_u:system_r:unconfined_t:s0 tcontext=system_u:object_r:lib_t:s0
tclass=file
type=SYSCALL msg=audit(1169803885.025:2173): arch=40000003 syscall=125
success=no exit=-13 a0=d0c000 a1=20d000 a2=5 a3=bfc34b00 items=0 ppid=1
pid=32735 auid=500 uid=500 gid=500 euid=500 suid=500 fsuid=500 egid=500 sgid=500
fsgid=500 tty=(none) comm="nemiver" exe="/usr/bin/nemiver"
subj=user_u:system_r:unconfined_t:s0 key=(null)
type=AVC_PATH msg=audit(1169803885.025:2173): 
path="/usr/lib/nemiver/plugins/dbgperspective/libdbgperspectiveplugin.so"

Try getting the plugins to compile with -fPIC and see if that helps with the
SELinux issue. The main code uses -fPIC but it isn't propagated down to the plugins.

Paul, how do I go about this? I've tried adding '--with-pic' to the %configure
call, but there is nothing different in the build log about PIC/non-PIC code
generation from not having that. 

Poking through the configure.ac reveals nothing obvious either. :S

Thanks.

Add -fPIC to CFLAGS/CXXFLAGS maybe?

Adding the following three lines to %build before the %configure line certainly
results in all compiles getting -fPIC; hopefully Gianluca can let us know if
that fixes the execmod problem.

# Make sure everything is compiled with -fPIC to avoid execmod problems
export CFLAGS="%{optflags} -fPIC"
export CXXFLAGS="%{optflags} -fPIC"

Created attachment 147096 [details]
mock build log with -fPIC

Still no luck.

[giallu@molzilla SPECS]$ nemiver
|X|virtual GModule*
nemiver::common::DynamicModule::Loader::load_library_from_path(const
nemiver::common::UString&):nmv-dynamic-module.cc:284:raised exception: failed
to load shared library
/usr/lib/nemiver/plugins/dbgperspective/libdbgperspectiveplugin.so:
/usr/lib/nemiver/plugins/dbgperspective/libdbgperspectiveplugin.so: cannot
restore segment prot after reloc: Permission denied

|X|void nemiver::common::Plugin::load_entry_point():nmv-plugin.cc:201:raised
exception: failed to load plugin entry point 'dbgperspective for plugin
'dbgperspective'

|E|nemiver::common::PluginSafePtr
nemiver::common::PluginManager::load_plugin_from_path(const
nemiver::common::UString&,
std::vector<nemiver::common::SafePtr<nemiver::common::Plugin,
nemiver::common::ObjectRef, nemiver::common::ObjectUnref>,
std::allocator<nemiver::common::SafePtr<nemiver::common::Plugin,
nemiver::common::ObjectRef, nemiver::common::ObjectUnref> >
>&):nmv-plugin.cc:562:Failed to load dependant plugin 'dbgperspective

and (in audit.log)

type=AVC msg=audit(1170328411.204:2435): avc:  denied  { execmod } for 
pid=21468 comm="nemiver" name="libdbgperspectiveplugin.so" dev=dm-0
ino=25724931 scontext=user_u:system_r:unconfined_t:s0
tcontext=system_u:object_r:lib_t:s0 tclass=file
type=SYSCALL msg=audit(1170328411.204:2435): arch=40000003 syscall=125
success=no exit=-13 a0=641000 a1=215000 a2=5 a3=bfb3d6e0 items=0 ppid=20331
pid=21468 auid=500 uid=500 gid=500 euid=500 suid=500 fsuid=500 egid=500
sgid=500 fsgid=500 tty=pts4 comm="nemiver" exe="/usr/bin/nemiver"
subj=user_u:system_r:unconfined_t:s0 key=(null)
type=AVC_PATH msg=audit(1170328411.204:2435): 
path="/usr/lib/nemiver/plugins/dbgperspective/libdbgperspectiveplugin.so"

Please also grep for "rpath" in the build log. 
IIRC not having hardcoded rpaths is a MUST item

rpaths are OK as long as they're not for a standard library path such as
/usr/lib or /usr/lib64.

The SELinux issue is probably due to the way that the libdbgperspectiveplugin
object is coded; what does it actually do? It may be worth pointing upstream to
http://people.redhat.com/drepper/selinux-mem.html which may give some clue as to
how to avoid the problem.

In the short term, a fix would probably be to do:

chcon -t textrel_shlib_t
/usr/lib/nemiver/plugins/dbgperspective/libdbgperspectiveplugin.so

Hello; and sorry for not getting back to this bug earlier. I was swamped with
work and class stuff most of last week.  :(

The RPATH in the library is for the %_libdir/nemiver/plugins directory, so this
seems intended and harmless.

In regards to the execmod denials, I've marked it as textrel_shlib_t for the
time being; and once I figure out what it is doing to cause the error (or, more
importantly, how to reproduce it), I'll send a mail to the upstream devs about this.

I've posted updated files to my webspace:
SRPM: http://thecodergeek.com/downloads/fedora/SRPMs/nemiver-0.3.0-4.src.rpm
Spec: http://thecodergeek.com/downloads/fedora/SPECs/nemiver.spec

Using chcon in %install doesn't help because (a) file contexts aren't stored in
RPM packages, and (b) rpm sets the file contexts based on the currently-running
policy at package install time, which would override any file context set
previously.

The quick fix for this is to fix the file context in %post.

The better fix for this is to request that
/usr/lib(64)?/nemiver/plugins/dbgperspective/libdbgperspectiveplugin.so is set
to context type textrel_shlib_t in the main selinux-policy package (request this
on fedora-selinux-list or raise a bug on selinux-policy), so you don't need to
adjust the context type in your own package.

The bext fix is of course to get the underlying memory access fixed upstream as
mentioned before. Hopefully they will have a clue what is going on.

(In reply to comment #18)
> The better fix for this is to request that
> /usr/lib(64)?/nemiver/plugins/dbgperspective/libdbgperspectiveplugin.so is set
> to context type textrel_shlib_t in the main selinux-policy package (request this
> on fedora-selinux-list or raise a bug on selinux-policy), so you don't need to
> adjust the context type in your own package.

+1 
this is always preferable if .so really needs such context

I've posted bug #228628 for the selinux-policy modification, and moved the chcon
call to the %post scriptlet. (Thanks for the information regarding RPM context
management.) Updated files are on my webspace:

Spec: http://thecodergeek.com/downloads/fedora/SPECs/nemiver.spec
SRPM: http://thecodergeek.com/downloads/fedora/SRPMs/nemiver-0.3.0-5.src.rpm

maybe too many % in:

chcon -t textrel_shlib_t	\
	%%{_libdir}/%{name}/plugins/dbgperspective/libdbgperspectiveplugin.so

?

What is necessary to reproduce the selinux error?
I don't get any such output or audit messages when starting nemiver.
The context of the DSO file is the default system_u:object_r:lib_t 

I'd also follow the chcon command with the usual "&> /dev/null || :" to avoid
issues on systems with SELinux not installed or running a policy not containing
the textrel_shlib_ context type.

Hmm. I've been playing with it for a while with all of the memory protection
features of the targeted policy active; and I can't seem to reproduce the
execmod denial you are seeing.

Gianluca: What policy/settings and versions are you using? 

As eu-findtextrel says it doesn't need text relocations either, I'm very tempted
to simply remove that chcon invocation entirely, as it doesn't appear to need it. 

Unfortunately, I can't fix/workaround a bug that I can't find. :o

This(In reply to comment #24)
> 
> Gianluca: What policy/settings and versions are you using?

The problem was seen in a (supposedly up-to-date) FC5 box, running
selinux-policy-2.3.7-2.fc5

[root@molzilla ~]# sestatus
SELinux status:                 enabled
SELinuxfs mount:                /selinux
Current mode:                   enforcing
Mode from config file:          enforcing
Policy version:                 21
Policy from config file:        targeted


> 
> As eu-findtextrel says it doesn't need text relocations either, I'm very tempted
> to simply remove that chcon invocation entirely, as it doesn't appear to need it.

+1. I could always open another BZ if I later find a problem with the published bits

> 
> Unfortunately, I can't fix/workaround a bug that I can't find. :o

sure :)

thanks a lot for your help

Note that the context change went into the Rawhide selinux-policy package
earlier this week (selinux-policy-2.5.3-2.fc7). If the textrel_shlib_t context
isn't needed, this change should be reverted.

(In reply to comment #26)
> Note that the context change went into the Rawhide selinux-policy package
> earlier this week (selinux-policy-2.5.3-2.fc7). If the textrel_shlib_t context
> isn't needed, this change should be reverted.
> 
> 
I've posted a modification to bug 228628, asking that the change be reverted.

I'm going to be gone most of today, so I'll post an updated spec/SRPM first
thing tomorrow. Thanks!

Ok. I edited the spec to not perform the labeling and rebuilt for FC6: I can
confirm the selinux problem is definetely not present here.

Good morning! :]

I've uploaded 0.3.0-6, which removes the chcon invocation.

Spec: http://thecodergeek.com/downloads/fedora/SPECs/nemiver.spec
SRPM: http://thecodergeek.com/downloads/fedora/SRPMs/nemiver-0.3.0-6.src.rpm

Thanks.

ping?

(In reply to comment #30)
> ping?

Pong. :)

I'd really appreciate a review of this, as I'd love to get it into the Package
Collection in time for Fedora 7...

Ok, since I am probably going to be a user of this, I'll try to review the
package. Please note this is my first official review, so anyone watching over
my shoulders will be really welcome...

Here is the review:

MUST ITEMS:
+ Package name is valid
+ spec file name match base package name
+ spec file is written in American English
+ spec file is legible (very!)

+ Sources matches upstream (289d46e97c125b95fdc5de9dd9736d7c  nemiver-0.3.0.tar.bz2)
+ License is acceptable (GPL)
+ License is included in the package
+ Package builds (in mock) for FC6 i386

+ BuildRequires list looks sane
+ Macros are consistently used
+ Use %find_lang macro for locale files
+ use ldconfig for installed .so files
+ package is not relocatable

+ headers and .so files correctly packaged in -devel
+ -devel correctly Requires base package
+ .la files excluded from package

+ .desktop file present and correctly installed

rpmlint output:

/var/lib/mock/fedora-6-i386-core/result/nemiver-0.3.0-6.fc6.i386.rpm
W: nemiver non-conffile-in-etc /etc/gconf/schemas/nemiver-workbench.schemas
W: nemiver non-conffile-in-etc /etc/gconf/schemas/nemiver-dbgperspective.schemas

This are usually to be ignored (and probably rpmlint should as well...)

/var/lib/mock/fedora-6-i386-core/result/nemiver-0.3.0-6.fc6.src.rpm
W: nemiver mixed-use-of-spaces-and-tabs (spaces: line 167, tab: line 1)

line 167 is in the changelog section, so I think we can ignore this


Just a couple questions: 
is the "touch" trick for forcing the icon cache update the usual/preferred/best
way to do that?
are you going to remove COPYING from the -devel subpackage as suggested in
comment #2?

ping.

I am going to approve this as soon as you can provide some info about my remarks

Pong :) 

Hi, Gianluca and Thanks for the review!

>(In reply to comment #33)
> Just a couple questions: 
> is the "touch" trick for forcing the icon cache update the
> usual/preferred/best way to do that?

It is, according to the Packaging/ScriptletSnippets page on the wiki.

> are you going to remove COPYING from the -devel subpackage as suggested in
> comment #2?
No. I'm of the opinon that such license texts should be included in development
subpackages to make the license explicitly clear (and not have to backreference
that of the original SRPM's main package). I know of no guideline for or against
thish, though; and it seems more sensible to me tokeep it in there.

(In reply to comment #35)
> >(In reply to comment #33)
> > Just a couple questions: 
> > is the "touch" trick for forcing the icon cache update the
> > usual/preferred/best way to do that?
> 
> It is, according to the Packaging/ScriptletSnippets page on the wiki.

Ok. seems I missed it until now...

> 
> > are you going to remove COPYING from the -devel subpackage as suggested in
> > comment #2?
> No. I'm of the opinon that such license texts should be included in development
> subpackages to make the license explicitly clear (and not have to backreference
> that of the original SRPM's main package). I know of no guideline for or against
> thish, though; and it seems more sensible to me tokeep it in there.
> 

I have no strong opinion against this either (though I did not notice other
packages doing the same).

In the lack of an official guideline about this and given there was more than
enough time for further comments the package is APPROVED

Great; thanks for your review & comments!

New Package CVS Request
=======================
Package Name: nemiver
Short Description: A C/C++ Debugger for GNOME - point, click, debug!
Owners: peter
Branches: FC-6 devel

Thanks for the branching, Josh.

Built successfully for devel; comps entry done too.

Package Change Request
======================
Package Name: nemiver
New Branches: el6
Owners: dodji

I am requesting an EPEL 6 branch for the Nemiver package. I have also requested an EPEL 6 branch for its dependencies ghex and gtksourceviewmm as they are the ones missing from EPEL 6 at the moment.

Git done (by process-git-requests).