Bug 444760

First comments

From rpmlint :
obm.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 32)
=> minor and easy to fix

obm.noarch: E: non-standard-uid /var/www/obm apache
obm.noarch: E: non-standard-gid /var/www/obm apache
=> seems ok, but could probably be /var/lib/obm ?

obm.noarch: E: non-standard-uid /usr/share/obm apache
obm.noarch: E: non-standard-gid /usr/share/obm apache
=> really need to be fixed. Apache shouldn't have write acces here

From obm-httpd.conf
ErrorLog & CustomLog & AddDefaultCharset should be in comment (for vhost use only)

From obm documentation, some php_flag/value missing
php_value include_path ".:/usr/share/pear:/usr/share/php:/usr/share/obm"
php_flag session.bug_compat_42  Off
php_flag session.bug_compat_warn Off
php_flag magic_quotes_gpc On

Config should go to /etc/obm (probably an simple link in /usr/share/obm/conf)
A working obm_conf.inc & obm_conf.ini should be provided

Alias /images could potentialy conflict with other apps.

You can change it to obmimages as this can be configured in obm_conf.inc

yeah, 

i actually did a better merge of this webapp, will upload it asap.
I had some talk with upstream to fix some of this issues directly into the source.

well, 

http://laxathom.fedorapeople.org/RPMS/obm/obm.spec
http://laxathom.fedorapeople.org/RPMS/obm/obm-2.1.9-2.fc8.src.rpm

So it looks like rpmlint output isn't that clean.  Most of these would be pretty
simple to fix.  A few of the warnings seem odd to me. Once these items get
cleaned up a little, I can dig into the RPM, install it and offer some more
feedback. The package looks close. 


 rpmlint obm-2.1.9-2.fc9.src.rpm
obm.src:27: W: unversioned-explicit-provides obm-core
obm.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 12)
obm.src: W: strange-permission obm-config.sh 0755

rpmlint  obm*
obm.noarch: E: non-standard-uid /var/lib/obm apache
obm.noarch: E: non-standard-gid /var/lib/obm apache
obm.noarch: W: spurious-executable-perm /usr/share/doc/obm-2.1.9/README.fedora
obm.noarch: W: file-not-utf8
/usr/share/doc/obm-2.1.9/database/support_incident/structure_tables.sql
obm.noarch: E: zero-length /etc/obm/obm-rpm.conf
obm-config.noarch: W: no-documentation
obm-config.noarch: W: devel-file-in-non-devel-package /usr/bin/obm-config
obm-MySQL.noarch: W: spurious-executable-perm
/usr/share/doc/obm-MySQL-2.1.9/mysql.sql
obm-Satellite.noarch: W: no-documentation
obm-services.noarch: W: no-documentation

Michael asked me to take a look.  I find it useful to comment on the found
rpmlint complaints as many of them are bogus, but that's not always easy since
rpmlint doesn't really give any indication of how much import you should give to
its spew.  Here's my take:

  obm.src:27: W: unversioned-explicit-provides obm-core
This is bad since you currently provide every possible version of obm-core,
which makes it impossible for anything to depend on a specific obm-core version.
 You must provide a version.

  obm.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 12)
I simply ignore these annoying warnings.  I mean, it's nice if they're cleaned
up, but come on.

  obm.src: W: strange-permission obm-config.sh 0755
rpmlint complains about any executable in the src.rpm; I've never seen this as
being problematic but I'm not sure you can count on files in src.rpm having any
specific permissions.

Note also that a more recent rpmlint gives:
  obm.src:304: E: files-attr-not-set
The %files list for the -services subpackage is missing the %defattr line.

The non-stanrard-uid complaints are OK.

The README.fedora file really shouldn't be executable.

structure_tables.sql should be passed through iconv; there's an extended ASCII
character in the first line and the file doesn't containing any specific
information about how it should be displayed.

What is the point of the obm-rpm.conf file?  If it needs to be there but
shouldn't initially hold anything, can we at least put a comment in there?

What is obm-config?  I'm guessing it's not a pkgconfig file, and if so then the
rpmlint complaint is bogus.

The mysql.sql file should not be executable.

The no-documentation warnings are OK as long as there isn't some bit of relevant
documentation that's been missed.

I personally would use "obm-satellite" and "obm-mysql" as I dislike mixed-case
package names; qt3-MySQL is the only non-Perl/Python module that uses that case,
for example, while 59 packages downcase it including the mysql package itself.

ho craps, how i could let pass these bogus (shame on me!)

Anyways,

> What is the point of the obm-rpm.conf file?  If it needs to be there but
> shouldn't initially hold anything, can we at least put a comment in there?


Right, it's just a config cache file which's use by upstream for the configuration.

 
> What is obm-config?  I'm guessing it's not a pkgconfig file, and if so then the
> rpmlint complaint is bogus.

definitively not. It's a shell script that help you configure all the stuff
easier. Maybe an "obm-setup" could be better ?
 
  
> I personally would use "obm-satellite" and "obm-mysql" as I dislike mixed-case
> package names; qt3-MySQL is the only non-Perl/Python module that uses that case,
> for example, while 59 packages downcase it including the mysql package itself.

I'm ok with that.

I wouldn't bother with renaming obm-config just to quiet rpmlint; the complaint
is obviously bugus and should just be ignored.

fine, available bogus should now be ignored.

http://laxathom.fedorapeople.org/RPMS/obm/obm.spec
http://laxathom.fedorapeople.org/RPMS/obm/obm-2.1.9-3.fc8.src.rpm

Grrr, see #447921

You should also note than version 2.1.10 is out...

Created attachment 310528 [details]
init script for satellite

Init script fixed to :
- use localized message
- user daemon command to display success/failure status
- user killproc command to display success/failure status
- change restart option to really restart the daemon
- add reload option

Created attachment 310529 [details]
Apache conf file

Apache configuration file fixed to
- change Directory to "/usr/share/obm" (upper)
- move php-value/php-flag in directory
- comment log (should only be uncomment with virtual host)
- add php_flag display_errors  Off  (obm produce a lot of "notice")

Created attachment 310530 [details]
init script for satellite

Missing /var/log/obm used by satellite.

Cycle in link to satellite conf file.

You must change from

pushd $RPM_BUILD_ROOT%{_sysconfdir}/%{name}-satellite/
ln -s ./%{name}Satellite.cf
$RPM_BUILD_ROOT%{_datadir}/obm-satellite/%{name}Satellite.cf

To 
ln -s ../../..%{_sysconfdir}/%{name}-satellite/%{name}Satellite.cf \
  $RPM_BUILD_ROOT%{_datadir}/obm-satellite/

Another bogus assignment  Please stop this.  Unfortunately I do not have the time now to put all of these back the way they were, so I can only ask you, bashton, to undo all of the bogus assignments you have made.

Anything happening here?

yeah.

http://laxathom.fedorapeople.org/RPMS/obm/obm-2.1.9-4.el5.src.rpm
http://laxathom.fedorapeople.org/RPMS/obm/obm.spec

Grr... .src.cpm seems broken :(
And version 2.1.14 available...

Notes :

Typo error in %preun Satellite
obmSatellite -> obm-satellite

Question
Why the mysql subpackage requires obm main package ?
Shouldn't we be able to install the database on a different server than the UI ?

A solution is to make obm  a meta package to install all the component at the same time (including database and SGBD, what most user need), and a subpackage for the UI (for expert users who want to split installation).

It could be a good idea to have a separate package for Artichow lib to avoid multiple installation.

License for Artichow is "Public Domain"

/etc/obm : owned by obm and obm-config
/etc/obm-satellite : not owned

Xavier Lamien, can you continue with this review, or would you like somebody to take over the review request to get obm in?

Sure thing.
I'm working on a new release. Actually a couple a changes have been made around this new release and require to review the packaging of OBM.

Remi let me know if you do still interesting to review this as well.

There seem to be major issues to be resolved first, please remove NotReady from the Whiteboard once this package is ready for a review.

New review request for OBM at https://bugzilla.redhat.com/show_bug.cgi?id=544989

*** Bug 544989 has been marked as a duplicate of this bug. ***

Ping? Any progress here? Or we can close this review?

Stalled Review. Closing per:
https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
If you ever want to continue with this review, please reopen or
submit new review.