Sorry, I forgot to comment the line.

 >>#remove unused files
 >>rm -rf $RPM_BUILD_ROOT%{_mandir}/ja
 >
 >
 > Japanese man pages?  Shouldn't they go into the l10n pkg
 > instead?
 >

My suggestion is to remove the l10n man pages until Solaris man pages works 
with UTF-8 man pages on none UTF-8 locales with symlinks.
Currently Solaris prepares the man pages with each encoding so the community 
man page won't be shown correctly by default.

I'm not sure if we integrate GNU man.

Thanks,
fujiwara

Laszlo (Laca) Peter wrote:
> Hi Halton,
> 
> Some comment below.
> 
> On Wed, 2008-02-20 at 14:51 +0800, Halton Huo wrote:
> 
>>Hi,
>>
>>There are some questions you may ask:
>>Q1: Why so many tracker branding patches?
>>A:  We offer some new features and fixes which are not in 0.6.4 tarball:
>>    * Thunderbird emails indexing (06-thunderbird.diff)
>>    * Web history indexing (07-firefox-history.diff)
>>    * Sync latest trunk code, there are some important fixes in it.
>>(03-r1071-latest.diff), it is zipped for size concern.
>>    Also we customized tracker action like:
>>    * Disable autostart feature (02-disable-autostart.diff)
>>    * Add opensolaris page in man page (-09-man.diff)
>>    And Porblems only on Solaris:
>>    * w3m crash on soalris (01-w3m-crash.diff)
>>
>>Q2: What libgsf-01-uninstalled-pc.diff for?
>>A:  From changelog Brian added this patch in on 2004-04-12, he could be
>>answer this question.
>>   * Mon Apr 12 2004 - <brian.cameron at sun.com
>>   - Added patch 1 to add uninstalled.pc file for Solaris build.
>>
>>
>>Thanks,
>>Halton.
>>
>>
>>
>>
>>
>>
>>
>>rpm spec file attachment (SUNWdesktop-search.spec)
>>
>>#Additional recommended packages
>>Requires:       SUNWgnome-media
>>Requires:       SUNWpng
>>Requires:       SUNWogg-vorbis
>>Requires:       SUNWlibexif
>>Requires:       SUNWgnome-pdf-viewer
>>Requires:       SUNWlxsl
>>Requires:       SUNWdesktop-search-libs
> 
> 
> So are these "recommended" or required?
> 
> 
>>%package extension 
>>Summary:        %{summary} - extension files
> 
> 
> What are they?
> 
> 
>>SUNW_BaseDir:   %{_basedir}
>>%include default-depend.inc
>>Requires: %name
>>
> 
> 
>># Install firefox extension
>>mkdir -p $RPM_BUILD_ROOT%{_libdir}/firefox/extensions
>>cd $RPM_BUILD_ROOT%{_libdir}/firefox/extensions
>>mkdir \{fda00e13-8c62-4f63-9d19-d168115b11ca\}
>>cd \{fda00e13-8c62-4f63-9d19-d168115b11ca\}
>>unzip %SOURCE1
>># FIXME: delete this link when firefox3 is removed or it is default
>>mkdir -p $RPM_BUILD_ROOT%{_libdir}/firefox3/extensions
>>cd $RPM_BUILD_ROOT%{_libdir}/firefox3/extensions
>>ln -s ../../firefox/extensions/\{fda00e13-8c62-4f63-9d19-d168115b11ca\} 
>>\{fda00e13-8c62-4f63-9d19-d168115b11ca\}
> 
> 
> Where is this magic number coming from?
> In any case, I suggest you use a macro or a variable to avoid typos.
> 
> 
>># Install firefox extension
>>mkdir -p $RPM_BUILD_ROOT%{_libdir}/thunderbird/extensions
>>cd $RPM_BUILD_ROOT%{_libdir}/thunderbird/extensions
>>mkdir \{b656ef18-fd76-45e6-95cc-8043f26361e7\}
>>cd \{b656ef18-fd76-45e6-95cc-8043f26361e7\}
>>unzip %SOURCE2
> 
> 
> 
>>%files extension
>>%defattr (-, root, bin)
>>%dir %attr (0755, root, bin) %{_libdir}
>>%{_libdir}/firefox*
>>%{_libdir}/thunderbird
> 
> 
> Okay, so the "extension" package includes ff and tb extensions
> for tracker?  Hmm... I would prefer to have them in 2 separete
> pkgs:  SUNWdesktop-search-firefox and SUNWdesktop-search-thunderbird,
> dependant upon SUNWfirefox and SUNWthunderbird respectively.
> 
> 
>>%files root
>>%defattr (-, root, sys)
>>%dir %attr (0755, root, sys) %{_sysconfdir}
>>%{_sysconfdir}/*
> 
> 
> I'm not sure what's in the root pkg (maybe the %files root section
> should be more explicit?) but SUNWdesktop-search should probably
> depend on it.
> 
> 
>>
>>
>>
>>
>>
>>rpm spec file
>>attachment
>>(SUNWdesktop-search-libs.spec)
>>
> 
> 
> 
>>Requires: SUNWpostrun
> 
> 
> Only the root pkg seems to use postrun so this shouldn't be
> needed.
> 
> 
>>#remove unused files
>>rm -rf $RPM_BUILD_ROOT%{_mandir}/ja
> 
> 
> Japanese man pages?  Shouldn't they go into the l10n pkg
> instead?
> 
> 
> 
>>%post root
>>( echo 'test -x /usr/bin/gconftool-2 || {';
>>  echo '  echo "ERROR: gconftool-2 not found"';
>>  echo '  exit 0';
>>  echo '}';
>>  echo 'umask 0022';
>>  echo 'GCONF_CONFIG_SOURCE=xml:merged:/etc/gconf/gconf.xml.defaults';
>>  echo 'export GCONF_CONFIG_SOURCE';
>>  echo '/usr/bin/gconftool-2 --makefile-install-rule
>>%{_sysconfdir}/gconf/schemas/*.schemas'
>>) | $BASEDIR/var/lib/postrun/postrun -u -c JDS_wait
> 
> 
> Please use %include instead
> 
> 
>>
>>
>>
>>
>>
>>rpm spec file
>>attachment
>>(tracker.spec)
>>
>>#
>># spec file for package tracker
>>#
>># Copyright 2008 Sun Microsystems, Inc.
>># This file and all modifications and additions to the pristine
>># package are under the same license as the package itself.
>>#
>># Owner:halton
>>#
>>
>>Name:           tracker
>>License:        GPL
>>Group:          Applications/System
>>Version:        0.6.4
>>Release:        1
>>Distribution:   Java Desktop System
>>Vendor:         Sun Microsystems, Inc.
>>URL:            http://www.tracker-project.org
>>Summary:        Desktop search tool
>>Source:
>>http://www.gnome.org/~jamiemcc/tracker/tracker-%{version}.tar.bz2
>># date:2008-01-23 owner:jerrytan type:branding
>>Patch1:         %{name}-01-w3m-crash.diff
> 
> 
> Doesn't look like a branding patch to me.
> 
> 
>># date:2008-01-23 owner:halton type:branding
>>Patch2:         %{name}-02-disable-autostart.diff
> 
> 
>># date:2008-01-23 owner:halton type:branding
>>Patch3:         %{name}-03-r1071-latest.diff
> 
> 
> s/branding/feature maybe, since this is code taken from upstream
> You can also add state:upstream.
> 
> 
>># date:2008-01-23 owner:halton type:bug
>>bugzilla:503725,503727,503960,503966,504000
>>Patch4:         %{name}-04-tp-reindex.diff
> 
> 
> I don't see this patch attached to any of these bug reports,
> are these bugids correct?
> 
> 
>># date:2008-01-23 owner:halton type:bug bugzilla:503376
>>Patch5:         %{name}-05-evo-reload.diff
> 
> 
>># date:2008-01-23 owner:migi type:branding
>>Patch6:         %{name}-06-thunderbird.diff
> 
> 
> This one doesn't look like branding either.
> 
> 
>># date:2008-01-23 owner:jerrytan type:branding
>>Patch7:         %{name}-07-firefox-history.diff
> 
> 
> And neither does this.  Are you guys pushing these upstream?
> 
> 
>># date:2008-01-23 owner:rickju type:bug bugzilla:511474
>>Patch8:         %{name}-08-check-remote.diff
>># date:2008-01-23 owner:jerrytan type:branding
>>Patch9:         %{name}-09-man.diff
> 
> 
> This one needs some linguistic review, see below.
> 
> 
>>
>>
>>
>>
>>
>>rpm spec file
>>attachment
>>(w3m.spec)
> 
> 
> looks good
> 
>>
>>
>>
>>
>>
>>rpm spec file
>>attachment
>>(libgsf.spec)
> 
> 
> looks good
> 
>>
>>
>>
>>
>>
>>differences
>>between files
>>attachment
>>(tracker-01-w3m-crash.diff)
>>
>>Index: tracker-trunk/src/trackerd/trackerd.c
>>===================================================================
>>--- tracker-trunk/src/trackerd/trackerd.c       (revision 882)
>>+++ tracker-trunk/src/trackerd/trackerd.c       (working copy)
>>@@ -854,9 +854,9 @@
>>        gboolean pushed_events, first_run;
>> 
>>         /* block all signals in this thread */
>>-        sigfillset (&signal_set);
>>+//        sigfillset (&signal_set);
>> #ifndef OS_WIN32
>>-        pthread_sigmask (SIG_BLOCK, &signal_set, NULL);
>>+//        pthread_sigmask (SIG_BLOCK, &signal_set, NULL);
>> #endif
>> 
>>        g_mutex_lock (tracker->files_signal_mutex);
>>
> 
> 
> Can you explain what this fixes and how?  (and/or submit upstream)
> 
> 
>>
>>
>>
>>
>>
>>differences
>>between files
>>attachment
>>(tracker-09-man.diff)
>>
>>--- docs/trackerd.1     Tue Jan 22 15:27:24 2008
>>+++ docs/trackerd.1.new Tue Jan 22 15:27:04 2008
>>@@ -100,6 +100,10 @@
>> needs a
>> .BR dbus-daemon(1)
>> to be running within your current session.
>>+.PP
>>+Please check tracker releasenotes on solaris platform on
>>+.PP
>>+\fBhttp://opensolaris\&.org/os/project/jds/tasks/MetaTracker/\fR\&;.
>> 
>> .SH SEE ALSO
>> .BR tracker-search-tool (1),
> 
> 
> We need proper wording here.
> 
> Thanks,
> Laca
> 
> 
> 



Reply via email to