I've been asked to use SeaMonkey (capitalized M) in descriptions, will change
that in next version of spec file.

Review comments:
- Is this really distributed under NPL/MPL?  I think it should be MPL/GPL/LGPL
- Lose the Prefix: tag
- Don't BuildRequire autoconf213; if you make changes to configure, include that
part in the patch
- Remove the ExclusiveArch.  You are including all of our current platforms, and
it probably builds on others that we don't.
- 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 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).
- Without a GRE, the -devel package should arguably not be built since that is a
key part of the -devel platform.
- regxpcom is no longer required.  This (and the entire block surrounding it)
should go away.
- Your comment about cp -L doesn't seem needed.
- Since this is for Fedora Extras, you probably shouldn't name the default
pref/bookmarks files with redhat :-)
- seamonkey-rebuild-databases should not be needed
- I don't think selinux/chcon stuff should be in this specfile.  Is there a bug
you are trying to work around?
- The following are installed with +x and shouldn't be.  Using a %defattr in
%files with the appropriate modes will fix this.
++ seamonkey ++
/usr/lib/seamonkey-1.0/components/nsXmlRpcClient.js
/usr/lib/seamonkey-1.0/components/nsComposerCmdLineHandler.js
/usr/lib/seamonkey-1.0/components/nsSidebar.js
/usr/lib/seamonkey-1.0/components/nsProgressDialog.js
/usr/lib/seamonkey-1.0/components/nsCloseAllWindows.js
/usr/lib/seamonkey-1.0/components/nsHelperAppDlg.js
/usr/lib/seamonkey-1.0/components/nsFilePicker.js
/usr/lib/seamonkey-1.0/components/nsDictionary.js
/usr/lib/seamonkey-1.0/components/nsUpdateNotifier.js
/usr/lib/seamonkey-1.0/components/nsDownloadProgressListener.js
/usr/lib/seamonkey-1.0/components/jsconsole-clhandler.js
/usr/lib/seamonkey-1.0/components/nsProxyAutoConfig.js
/usr/lib/seamonkey-1.0/components/nsResetPref.js
/usr/lib/seamonkey-1.0/components/nsInterfaceInfoToIDL.js
++ seamonkey-chat ++
/usr/lib/seamonkey-1.0/components/chatzilla-service.js
++ seamonkey-dom-inspector ++
/usr/lib/seamonkey-1.0/components/inspector-cmdline.js
++ seamonkey-js-debugger ++
/usr/lib/seamonkey-1.0/components/venkman-service.js
++ seamonkey-mail ++
/usr/lib/seamonkey-1.0/components/offlineStartup.js
/usr/lib/seamonkey-1.0/components/nsLDAPPrefsService.js
/usr/lib/seamonkey-1.0/components/nsAbLDAPAttributeMap.js
/usr/lib/seamonkey-1.0/components/smime-service.js
/usr/lib/seamonkey-1.0/components/mdn-service.js
++++


Optional:
- 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)

Fixed up the tracker bugs

> - 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/

(In reply to comment #4)
> > - 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

Mozilla's build inadequacies is their problem.  The only reason we have this
package at all is because mozilla still needs it to generate configure.  There's
no need to force this requirement on people who simply want to build an SRPM.


 
> > - 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?

Does it work if you Require: seamonkey (it should be Require anyway, and please
one Require per line.)



> > - 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,

What specific errors?



> The mozilla 1.7.x package does the same thing!

Yeah, its a bug that we need to fix there, too.  Don't copy over bugs :-)

> 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?

I think either solution is probably sufficient.  Will look at the rest over the
weekend.

> > 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,
> 
> What specific errors?

During install, the RPM build system tries to access a path that does not match
what is produced in /var/tmp

As it requires so much time to build, I was not motivated to make further
attempts, but decided to go with the code that works.


> Does it work if you Require: seamonkey (it should be Require anyway, and
> please one Require per line.)

You mean  RequireS:  instead of  Prereq:  ?
Trying.


> > Or should we use some "find" statements to change the modes before they 
> > are copied? Or do you have a better idea?
>
> I think either solution is probably sufficient.

I found the existing spec has 
  # fix permissions of the chrome directories
  /usr/bin/find . -type d -perm 0700 -exec chmod 755 {} \; || :
I will add
  # We don't want JS files to be executable
  /usr/bin/find . -type f -name \*.js -exec chmod 644 {} \; || :
and see if that works

> > Does it work if you Require: seamonkey (it should be Require anyway, and
> > please one Require per line.)

> You mean  RequireS:  instead of  Prereq:  ?
> Trying.

No, it still does not work.

[root@kaiefast x86_64]# rpm -ivh seamonkey-1.0-5.x86_64.rpm
seamonkey-mail-1.0-5.x86_64.rpm
error: Failed dependencies:
        libxpcom_core.so()(64bit) is needed by seamonkey-mail-1.0-5.x86_64

> > - The following are installed with +x and shouldn't be. 
>
> I will add
>   # We don't want JS files to be executable
>   /usr/bin/find . -type f -name \*.js -exec chmod 644 {} \; || :
> and see if that works

Yes, that worked fine.

Summary of the previous comments.
Chris, from all your comments, only one issue is left:

> > - 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).
> 
> Are you suggesting to add the following 3 lines?
>   AutoProv: 0
>   %define _use_internal_dependency_generator 0
>   %define __find_requires %{SOURCE100}

As said before, that didn't work for me.
You also proposed 

> Does it work if you Require: seamonkey (it should be Require anyway, and please
> one Require per line.)

But that didn't work either.

What should we do?
Is there an easy way to make the dependency/provides system use absolute pathes,
so your predicted ambiguity won't occur, or could we skip this for now?

I discussed with Chris. In order to get rid of the "Provides" problem, we agreed
to simplify, no longer use separate packages for mail etc., but produce a single
seamonkey package. I'll also remove the devel package.

I uploaded a new spec file to
  http://kuix.de/mozilla/seamonkey/1.0-6/src/seamonkey.spec

See also this diff:
  http://kuix.de/mozilla/seamonkey/1.0-6/src/spec.diff

Chris, can you please continue the review?

Chris asked me to run mock and rpmlint on the RPM.


mock does not seem to report anything, it says:

init
clean
prep
setup
build
ending
done


rpmlint gives 

source:

W: seamonkey strange-permission seamonkey-make-package.pl 0775
W: seamonkey strange-permission seamonkey.sh.in 0775
W: seamonkey strange-permission find-external-requires 0775
W: seamonkey prereq-use fileutils perl
W: seamonkey prereq-use /usr/bin/killall
W: seamonkey prereq-use desktop-file-utils >= %{desktop_file_utils_version}

binary:

W: seamonkey invalid-license NPL/MPL
W: seamonkey dangerous-command-in-%preun rm
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/messenger.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/messenger.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/en-mac.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/en-mac.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/en-US.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/en-US.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/toolkit.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/toolkit.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/en-win.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/en-win.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/help.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/help.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/sroaming.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/sroaming.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/content-packs.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/content-packs.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/classic.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/classic.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/US.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/US.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/comm.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/comm.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/modern.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/modern.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/inspector.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/inspector.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/pippki.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/pippki.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/pipnss.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/pipnss.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/venkman.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/venkman.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/en-unix.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/en-unix.jar
W: seamonkey uncompressed-zip /usr/lib/seamonkey-1.0/chrome/chatzilla.jar
E: seamonkey no-jar-manifest /usr/lib/seamonkey-1.0/chrome/chatzilla.jar


Actually I asked the SeaMonkey developers again regarding the license, and the
NPL/MPL was a mistake. I'll change that to be same as firefox: MPL/LGPL

Is action required for the other items?

- No need to Require: nss, nspr.  RPM will auto generate those requires.

- Please use %{?_smp_mflags} instead of -j$CPUS and you can get rid of the
relevant getconf calls.

- It would be good to get those scripts that rpmlint complained about changed to
0755 instead of 0775.

- Also, I think it would be good to try and use %ghost to avoid having to do the
rm stuff in %preun

- %define _unpackaged_files_terminate_build 0
I just noticed this on.  Can you turn that off and make sure it still builds? 
If there are unpackaged files, you should explicitly rm them, or list them in
your %files section with %exclude prepended.

> No need to Require: nss, nspr.  RPM will auto generate those requires.

I can't confirm that. (Remember that we use the find-external-requires tool, if
that has any influence on the auto generate that you expected.).

With the old version of the spec (that explicitly requires nspr/nss):

[kaie@kaiefast x86_64]$ rpm -qp --requires seamonkey-1.0-5.x86_64.rpm | egrep -i
'nspr|nss'
nspr >= 4.6
nss >= 3.11
libnspr4.so()(64bit)
libnss3.so()(64bit)

Using your proposal to remove the Requires I get:
[kaie@kaiefast x86_64]$ rpm -qp --requires seamonkey-1.0-6.x86_64.rpm | egrep -i
'nspr|nss'
libnspr4.so()(64bit)
libnss3.so()(64bit)

Is that what you want?

> Please use %{?_smp_mflags} instead of -j$CPUS and you can get rid of the
> relevant getconf calls.

Ok, changed and removed.

> - %define _unpackaged_files_terminate_build 0
> I just noticed this on.  Can you turn that off and make sure it still builds? 
> If there are unpackaged files, you should explicitly rm them, or list them in
> your %files section with %exclude prepended.

Talked to Chris off-bugzilla. As there are 3500 unpackaged files, it would be a
lot of work to get this cleaned up. As the existing Mozilla package has this
flag turned on, too, we decided to use it for SeaMonkey, too.

> - It would be good to get those scripts that rpmlint complained about 
> changed to 0755 instead of 0775.

Done. Note that rpmlint still complains. But again this warning is produced on
the existing Mozilla packages, too, so that's not new a bug I'm introducing, but
it would be great if we could postpone that.

> - Also, I think it would be good to try and use %ghost to avoid having to do 
> the rm stuff in %preun

I added two more explicitly listed rdf files.
There were no unlisted overlayinfo dirs left, so no need to "rm".
I added the greprefs %dir, so it gets uninstalled properly.
I removed the %preun section.
I did an "install - execute - uninstall" cycle and verified that all files get
removed.

I will upload an updated SPEC when I know your answer to comment 14, thanks.

Yeah, comment 14 is correct.  Since the sonames will be bumped if the ABI
breaks, we should be fine with just generating the requires on the actual
libraries.  This is what we do for other packages.  (This helps for when
libraries change packagenames as things did recently with mozilla-nss -> nss ..
it probably won't change anymore, but the prinicple is still the same)

Chris, can you please look whether you need any additional changes?

http://kuix.de/mozilla/seamonkey/1.0-7/src/seamonkey.spec
http://kuix.de/mozilla/seamonkey/1.0-7/src/spec.diff
http://kuix.de/mozilla/seamonkey/1.0-7/src/seamonkey-1.0-7.src.rpm

> As there are 3500 unpackaged files,

May I ask why 3500 files are not covered by any %files section?

> May I ask why 3500 files are not covered by any %files section?

The mozilla install target installs a lot of various files for development and
testing, some files that seem to belong to calendar, etc.

Whoever wrote the original spec files (probably a long while ago) for the
mozilla package seems to have made the decision that it's too much work to sort
out everything, manually pick the files we need to ship, and ignore the rest.
Pure guessing from my part.

We decided to use MPL as the license tag, because that's the only license that
covers everything.

The license tag is the only difference to the previous spec file.

http://kuix.de/mozilla/seamonkey/1.0-8/src/seamonkey.spec
http://kuix.de/mozilla/seamonkey/1.0-8/src/seamonkey-1.0-8.src.rpm

Chris, is this version acceptable for the initial seamonkey package release?

4 weeks passed since I made the previous comment.

Could you please approve, or do you want me to make further changes?
Thanks.

Okay, looks good enough.  There's a few issues still outstanding, but those are
large hairy issues, and those exist in the mozilla package itself as well.
Package APPROVED

Thanks for approving!!!

Before importing, I will create a new release. The only thing that will be new:
I will increase the release number and add suffix %{?dist} as suggested on
http://fedoraproject.org/wiki/Extras/BuildRequests

Package built into Development and FC-5.

Package Change Request
======================
Package Name: seamonkey
New Branches: el6
Owners: buc

gecko-maint members, any objections?

Owners, any objections of trademark ACL issues here?

No objections.

Thanks.  Dmitry, re-set the cvs flag, then, unless someone objects.

Re-set again.

Git done (by process-git-requests).

Thanks!