[Bug 619928] Review Request: tigase-server - Tigase XMPP Server in Java

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=619928

Bug 619928 depends on bug 583643, which changed state.

Bug 583643 Summary: Review Request: tigase-xmltools - Tigase XML Tools
https://bugzilla.redhat.com/show_bug.cgi?id=583643

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
             Status|ON_QA                       |CLOSED
         Resolution|                            |ERRATA

--- Comment #6 from Matej Cepl <mcepl at redhat.com> 2010-08-07 00:26:36 EDT ---
(In reply to comment #5)
> In current version of package you may safely drop them as they are. 

I did. There is now in %prep

# to make delete verbose don't use -delete
find . -name \*.jar -exec rm -v '{}' \;

and jars are not installed.

> Alternatively, you may choose to package the tools from src/main/groovy if you
> find them useful. If you choose to do so, you can decide whether to build them
> into .jars at package build time (preferred) and add groovy BR, or leave them
> as they are (the source form) and drag groovy interpreter as binary package
> dependency.

The problem I have is that I would like to have tigase-server working on EL5
and there doesn't seem to be groovy on neither EL6 nor EL5 (actually to be sure
it works on EL5, I am doing all packaging work on EL5 virtual machine). For
now, I have installed groovy scripts into %doc and if anybody wants to have
groovy scripts he must to hack groovy support himself. I hope that could be an
acceptable solution.

> 4.) This is not a wise thing to do: "ant -Dlibs=%{_javadir}"
> 
> Javadir can grow too big and apart from java being slow, you easily loose track
> of jars you depend on. A better solution would be to drop the -Dlibs argument
> and prepend the call to ant with something like:
> 
> build-jar-repository libs tigase-utils tigase-xmltools

Wov! That's a great tool ... I haven't knew about it. Fixed.

> 5.) Please comment on why are you missing the %check section with ant
> run-unittest command. (It seems to me that it is missing dependency on wttols
> which provides the unitgen task, right?)

Yes, and I've made a comment to the %check section

> 6.) The startup script does not seem to work
> 
> JAVA_HOME makes little sense in jpackage environment.
> Please drop the check, or hardcode the value to /usr/lib/jvm.

I don't think it has anything to do with JAVA_HOME (which is not set) but with
asking for groovy* jars. I had them installed from my previous experiments, but
they are not packaged anymore. Fixed.

> 7.) /etc/tigase/tigase.conf contains both user modifiable and
> non-user-modifiable settings.
> 
> Therefore, in case the user e.g. tunes the VM for higher heap size, and
> another version of the package adds new jar to the default classpath, an
> update results in non-functional configuration. Please consider splitting
> the file into two.

Created /etc/tigase/tigase.defaults which is sourced by
/etc/tigase/tigase.conf, but it shouldn't be changed by the sysadmin

> 8.) Why does /etc/tigase/database go into /etc?

You are right. I was on the fence about switching it back to
/usr/share/tigase/database, which I did now.

> Hopefully that's all.    

/me hopes too :)

SPEC file is http://mcepl.fedorapeople.org/rpms/tigase-server.spec
src.rpm is
http://mcepl.fedorapeople.org/rpms/tigase-server-5.0.0-0.3.20100527svn.src.rpm

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.