[Bug 179802] Review Request: seamonkey

[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 report.

Summary: Review Request: seamonkey


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





------- Additional Comments From kengert at redhat.com  2006-02-10 15:02 EST -------
> - Is this really distributed under NPL/MPL? 

Yes it is, I explicitly asked on irc.mozilla.org #seamonkey.
In addition, SeaMonkey's about page says
  "Mozilla Public License and Netscape Public License"


> - Lose the Prefix: tag

removed

Not sure which of all the changes were the culprit, but I got build failures, 
I had to replace some %{prefix} statements to make it work again.


> - Don't BuildRequire autoconf213; if you make changes to configure, include that
> part in the patch

I think that request makes package maintenance a bit inconvenient,
but you probably have a good reason to suggest that.
removed


> - Remove the ExclusiveArch.  You are including all of our current platforms, and
> it probably builds on others that we don't.

removed


> - You probably ought to have the FindExternalProvides stuff that the Firefox and
> Thunderbird package does.  Since the libraries provided aren't versioned, this
> can cause problems when two packages provide the same libraries (mozilla also
> provides these for now, and when xulrunner eventually takes over, it will do so).

I'm confused, because you mention FindExternal and Provides in one word,
but I can't find such a thing in the TB and FF spec files.
Are you suggesting to add the following 3 lines?
  AutoProv: 0
  %define _use_internal_dependency_generator 0
  %define __find_requires %{SOURCE100}
That's what I did.

But I ended up with a depency problem, although package "seamonkey" contained
libxpcom_core.so, package "seamonkey-mail" complained that lib can't be found.
Therefore I set AutoProv to on, that made it work, but we again have 
lots of provides. Is that ok, or do you suggest a different way to fix it?


> - I think you can safely remove the conditional for desktop_file, unless you
> really want to push this to really old releases (I think FC1 needed it, newer
> don't).

removed
  %define desktop_file 1
and all conditionals and all "else" portions.


> - Without a GRE, the -devel package should arguably not be built since that is a
> key part of the -devel platform.

Ok, removed for now, as the primary intention is indeed to provide the application.
We can re-add it in a future package version, if required.


> - regxpcom is no longer required.  This (and the entire block surrounding it)
> should go away.
> - seamonkey-rebuild-databases should not be needed

removed, also removed ldconfig commands


> - Your comment about cp -L doesn't seem needed.

It wasn't my comment, it's a leftover from mozilla.spec.
removed


> - Since this is for Fedora Extras, you probably shouldn't name the default
> pref/bookmarks files with redhat  :-) 

Renamed the files from "mozilla-redhat-" to "seamonkey-fedora-".
Also changed the bookmarks file to match your firefox bookmarks file.


> - I don't think selinux/chcon stuff should be in this specfile.  Is there a bug
> you are trying to work around?

My rawhide system runs with selinux enabled.
It failed during build, and using chcon was the only fix I found.

But now that regxpcom is no longer necessary, chcon isn't either.
I have now removed both regxpcom and chcon, and have been able to 
build on the rawhide system.


> - Use a .mozconfig file (see what I do in the firefox package).  This will make
> it easier to do development with the same flags with a different tree (just copy
> the mozconfig over)

Ok, done. Motivated by your proposal, I tried to 
use the same build code as used in the firefox spec file.
But that failed, I got errors in install stage, the pathes
were incorrect. So I decided to continue to use the current build
commands, and only moved the compilation configure options to mozconfig.
I guess that's sufficient to satisfy your request.


> - The following are installed with +x and shouldn't be.  Using a %defattr in
> %files with the appropriate modes will fix this.

The mozilla 1.7.x package does the same thing!
The statement currently in use is %defattr(-,root,root).
I think we must not chance the modes for most of the files,
so your request means, we'd have to filter the *.js files
from the "list of files" and use explicit %defattr(644) statements for them.
Do you want me to hack "sed/grep" code to change the file lists?
Or should we use some "find" statements to change the modes before they 
are copied? Or do you have a better idea?


Other changes:
- capitalize M in SeaMonkey
- use official 1.0 release source tarball


New SPEC file: http://kuix.de/mozilla/seamonkey/1.0-4/src/seamonkey.spec
New SRPM file: http://kuix.de/mozilla/seamonkey/1.0-4/src/seamonkey-1.0-4.src.rpm

Note, if you want to download at the SOURCES without downloading the .src.rpm,
look here: http://kuix.de/mozilla/seamonkey/1.0-4/src/

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.