Bug 203520 (evolution-brutus)

New SRPM with fixes to all rpmlint warnings:

http://www.omesc.com/content/downloads/dist/Fedora%20Core%205/SRPMS/evolution-brutus-1.1.6-2.src.rpm

-- 
  jules

at first glance:
- change make to: make %{?_smp_mflags}
- pkgconfig is not really needed as a buildrequire, since it is also required by
gnome-common.
- Vendor and Packager tag should be removed
- I guess the autoreq tag can go as well
- replace license with just the text GPL
- --enable-brutus-debug=no should be yes to build with symbols?
- don't use the macro %makeinstall since it is broken
- BuildRequires: gettext is missing (required to build the translations)

I am not a sponsor so I can't approve your package, but I hope my directions help.

Also I get:
checking Evolution Data Server version... 1.7
configure: error: Unsupported version of Evolution Data Server. Please report
this to <bugs>.
It looks like you are also upstream so I suggest very strongly to have this
fixed since it is needed to be build for devel (upcoming FC6)

Thank you for your input! 

I've put updated srpm and spec files on the site that addresses all of your #2
comments (except for your %makeinstall macro comment, what do you mean by
that?). I'll install the newest FC6 beta tomorrow to have a go at your #2 comment.

Spec URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%205/SPECS/evolution-brutus.spec
SRPM URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%205/SRPMS/evolution-brutus-1.1.6-3.src.rpm

Best regards,
  jules

Well, that should be #3 comment to get it compiling with e-d-s CVS HEAD.

-- 
  jules

(In reply to comment #4)
> Thank you for your input! 
> 
> I've put updated srpm and spec files on the site that addresses all of your #2
> comments (except for your %makeinstall macro comment, what do you mean by
> that?).
http://fedoraproject.org/wiki/Packaging/Guidelines#head-fcaf3e6fcbd51194a5d0dbcfbdd2fcb7791dd002

Also why are you building with: --enable-brutus-debug=no ?


> I'll install the newest FC6 beta tomorrow to have a go at your #3 comment.
Great
> 
> Spec URL:
>
http://www.omesc.com/content/downloads/dist/Fedora%20Core%205/SPECS/evolution-brutus.spec
> SRPM URL:
>
http://www.omesc.com/content/downloads/dist/Fedora%20Core%205/SRPMS/evolution-brutus-1.1.6-3.src.rpm
> 
Don't forget to update the changelog each time you make changes.

> > I've put updated srpm and spec files on the site that addresses all of your #2
> > comments (except for your %makeinstall macro comment, what do you mean by
> > that?).
>
http://fedoraproject.org/wiki/Packaging/Guidelines#head-fcaf3e6fcbd51194a5d0dbcfbdd2fcb7791dd002

OK, I've fixed that in the spec.


> Also why are you building with: --enable-brutus-debug=no ?

brutus-debug is an internal configure variable that explicitly appends "-ggdb
-O1" to CFLAGS and defines BRUTUS_DEBUG. A lot of debug output is printed to
stdout when BRUTUS_DEBUG is defined in the source and the environment has
BRUTUS_DEBUG defined to something, e.g. "yes". brutus-debug is therefore
disabled during rpm build. "-g -O2" is appended to CFLAGS during rpm build so
the debuginfo rpms contain the symbols.

This srpm and spec has all the updates:

Spec URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%205/SPECS/evolution-brutus.spec
SRPM URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%205/SRPMS/evolution-brutus-1.1.6-4.src.rpm


> Don't forget to update the changelog each time you make changes.

Yes, I've missed that. The 1.1.6-4 srpm/spec has updated Changelog entries.

Thanks,
  jules

I just checked if this package (-1.1.6-4) can be rebuilt,
however, it failed in mock.

+ ./configure --build=i686-redhat-linux-gnu --host=i686-redhat-linux-gnu
--target=i386-redhat-linux-gnu --program-prefix= --prefix=/usr --exec-pr
efix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
--datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib --libexecdir=/
usr/libexec --localstatedir=/var --sharedstatedir=/usr/com
--mandir=/usr/share/man --infodir=/usr/share/info --enable-brutus-dist=yes
--enable-br
utus-debug=no --enable-brutus-devel=yes --enable-brutus-spy=no
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
./configure: line 2122: evolution: command not found
checking Evolution Data Server version... 1.7
configure: error: Unsupported version of Evolution Data Server. Please report
this to <bugs>.
error: Bad exit status from /var/tmp/rpm-tmp.53931 (%build)
RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.53931 (%build)
Error building package from evolution-brutus-1.1.6-4.src.rpm, See build log

Check the BuildRequires. 
Also, as the comment #3 , if
this package cannot be rebuilt with evolution-data-server-1.7.92 ,
this package cannot be released because Fedora Extras packages has to
be rebuilt under devel environment first.

Also, you have to add %{?dist} to release.

I'm current installing the latest fc6 beta to fix the build issue with eds 1.7.
One thing though,,, now that Evolution 2.8 is out do I have to support the 2.7
development release?

I'll add %{?dist} to release later today.


Thanks,
  jules

(In reply to comment #9)

> One thing though,,, now that Evolution 2.8 is out do I have to support the 2.7
> development release?

Well, Fedora Core devel still has e-d-s 1.7, but since 1.8 is out I suspect it
will be packaged soon. If you want to know for sure you can contact mbarnes, who
is the packager of e-d-s at the moment. But since the changes will be minimal
between 1.7.92 and 1.8 if you support one, you will probably support both. In
short: make sure you support 1.8, then 1.7 support should be trivial and perhaps
not needed if e-d-s is updated soon enough

(In reply to comment #10)
> Well, Fedora Core devel still has e-d-s 1.7, but since 1.8 is out I suspect it
> will be packaged soon. 

Actually, rawhide today has evolution-data-server-1.8.0-1.fc6 .
So at least this package should have the support for evolution-data-server-1.8,
otherwise this package cannot be accepted, I suppose.

Also,  libglade2-devel evolution e2fsprogs-devel is missing for BuildRequires
(checked by mock)

Note: even after I install the missing BR packages above and make some
"version trick" on configure, I still cannot rebuild this package against
evolution-data-server-devel-1.8.0-1.fc6 with following error:

e-cal-backend-brutus.c: In function 'brutus_backend_construct':
e-cal-backend-brutus.c:672: error: too few arguments to function
'e_cal_backend_cache_new'

(In reply to comment #11)
> (In reply to comment #10)
> > Well, Fedora Core devel still has e-d-s 1.7, but since 1.8 is out I suspect it
> > will be packaged soon. 
> 
> Actually, rawhide today has evolution-data-server-1.8.0-1.fc6 .
> So at least this package should have the support for evolution-data-server-1.8,
> otherwise this package cannot be accepted, I suppose.
> 
> Also,  libglade2-devel evolution e2fsprogs-devel is missing for BuildRequires
> (checked by mock)

OK, I'll add those.

> Note: even after I install the missing BR packages above and make some
> "version trick" on configure, I still cannot rebuild this package against
> evolution-data-server-devel-1.8.0-1.fc6 with following error:

That is actually expected. I've only build against the sub-1.7 e-d-s packages so
some API changes for e-d-s 1.7/8 are to be expected. I;ve just finished
installing fc6 test 2 so I expect to fix any issues during today.

Thanks,
  jules

(In reply to comment #11)
> (In reply to comment #10)
> > Well, Fedora Core devel still has e-d-s 1.7, but since 1.8 is out I suspect it
> > will be packaged soon. 
> 
> Actually, rawhide today has evolution-data-server-1.8.0-1.fc6 .
> So at least this package should have the support for evolution-data-server-1.8,
> otherwise this package cannot be accepted, I suppose.
> 
> Also,  libglade2-devel evolution e2fsprogs-devel is missing for BuildRequires
> (checked by mock)

Hmm... strange. Evolution should not be required by evolution-brutus, only e-d-s
and related packages.

-- 
  jules

(In reply to comment #13)
> > Also,  libglade2-devel evolution e2fsprogs-devel is missing for BuildRequires
> > (checked by mock)
> 
> Hmm... strange. Evolution should not be required by evolution-brutus, only e-d-s
> and related packages.
As in the comment #8, configure complains:

/configure: line 2122: evolution: command not found

configure includes the line:
EVOLUTION_VERSION=`evolution --version | cut -b 17,18,19 2>/dev/null`
and this requires evolution.

Note: configure also includes:

cat >>confdefs.h <<_ACEOF
#define EVO_PATHNAME "/usr/bin/evolution-$EVOLUTION_VERSION"
_ACEOF

however, with evolution-2.7.92-7.fc6, EVOLUTION_VERSION is 2.7 while
there exists /usr/bin/evolution-2.8 (not /usr/bin/evolution-2.7), so anyway
this is wrong.

(In reply to comment #14)
> (In reply to comment #13)
> > > Also,  libglade2-devel evolution e2fsprogs-devel is missing for BuildRequires
> > > (checked by mock)
> > 
> > Hmm... strange. Evolution should not be required by evolution-brutus, only e-d-s
> > and related packages.
> As in the comment #8, configure complains:
> 
> /configure: line 2122: evolution: command not found
> 
> configure includes the line:
> EVOLUTION_VERSION=`evolution --version | cut -b 17,18,19 2>/dev/null`
> and this requires evolution.
> 
> Note: configure also includes:
> 
> cat >>confdefs.h <<_ACEOF
> #define EVO_PATHNAME "/usr/bin/evolution-$EVOLUTION_VERSION"
> _ACEOF
> 
> however, with evolution-2.7.92-7.fc6, EVOLUTION_VERSION is 2.7 while
> there exists /usr/bin/evolution-2.8 (not /usr/bin/evolution-2.7), so anyway
> this is wrong.

OK, yes I see.

Thanks,
  jules

OK, I finally managed to build FC6 test 2 rpms. They should address all
outstanding issues and can be downloaded here:

Spec URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SPECS/evolution-brutus.spec
SRPM URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SRPMS/evolution-brutus-1.1.6-4.src.rpm

The FC6 test 2 build box has all yum updates if it matters...

Best regards,
  jules

OK, I made a typo in the URL. Correct URLs here:

Spec URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SPECS/evolution-brutus.spec
SRPM URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SRPMS/evolution-brutus-1.1.6-5.src.rpm


Sorry,
  jules

Just a quick check of 1.1.6-5.

* Well, this package does not aimed for FC5 at all? This package
  can be rebuilt in mock under 20060906 rawhide, however this package
  cannot be rebuilt under FC5 (with 20060906 updates) with following error:

No Package Found for evolution-data-server-devel >= 1.8
No Package Found for ORBit2-devel >= 2.14.1
0:evolution-2.6.3-1.fc5.5.i386
0:gnome-common-2.12.0-2.fc5.noarch
0:e2fsprogs-devel-1.38-12.i386
0:gettext-0.14.5-3.i386
0:libglade2-devel-2.5.1-4.fc5.1.i386
0:intltool-0.34.2-1.1.i386
0:gnome-keyring-devel-0.4.9-1.i386
Cannot find build req  evolution-data-server-devel >= 1.8. Exiting.
ending

   even if I remove version-release related requirement issue, still
   I cannot rebuild this under FC5 with following error:

Requested 'ORBit-2.0 >= 2.14.1' but version of ORBit-2.0 is 2.14.0

Consider adjusting the PKG_CONFIG_PATH environment variable if you
installed software in a non-standard prefix.

Alternatively, you may set the environment variables GNOME_FULL_CFLAGS
and GNOME_FULL_LIBS to avoid the need to call pkg-config.
See the pkg-config man page for more details.

error: Bad exit status from /var/tmp/rpm-tmp.57376 (%build)

* rpmlint for FC6-devel-built rpms complaints:
W: evolution-brutus macro-in-%changelog makeinstall

* Why does this package use Autoreq: no ?
  This description forbids finding libraries requirements, which I think
  is quite unwilling. Even if you want to specify version-related
  requirements, "Autoreq: no" is unnecessary because you can simply add
  the requirements in addition to auto-finding requirements.

(In reply to comment #18)
> Just a quick check of 1.1.6-5.
> 
> * Well, this package does not aimed for FC5 at all? 

It is. I have rpms for FC6, FC5 as well as FC4 on the site below.

>   This package
>   can be rebuilt in mock under 20060906 rawhide, however this package
>   cannot be rebuilt under FC5 (with 20060906 updates) with following error:

This has been fixed in the new SRPM.


> No Package Found for evolution-data-server-devel >= 1.8

Mea culpa. I generated the required e-d-s version during configure from the
version present in the build requirement. This is obviously wrong since a "make
dist" package generated on, say, FC6, then can't build on FC4 or FC5 as they
have different versions of e-d-s installed. This has been fixed.


> No Package Found for ORBit2-devel >= 2.14.1

This is correct. evolution-brutus requires at least ORBit2 2.14.1 due to a
handfull of fixes and features that aren't present in earlier versions. Please
grep for my name in the ORBit2 and libIDL ChangeLogs for the details.

FC4 and FC5 RPMs for the required versions of ORBit2 and libIDL are available in
the FC4 and FC5 directories respectively here:

http://www.omesc.com/content/downloads/dist/

> 
> * rpmlint for FC6-devel-built rpms complaints:
> W: evolution-brutus macro-in-%changelog makeinstall

Fixed in the new package.


> * Why does this package use Autoreq: no ?
>   This description forbids finding libraries requirements, which I think
>   is quite unwilling. Even if you want to specify version-related
>   requirements, "Autoreq: no" is unnecessary because you can simply add
>   the requirements in addition to auto-finding requirements.

Please believe me when I say that I didn't do this lightly. The thing that
forced me to disable Autoreg is that at least one of the libraries (libebook if
I rememver correctly) that are provided internally by e-d-s changed version from
one stable release to another. I observed that when I:

1) Installed evolution-brutus for testing
2) Un-installed evolution-brutus
3) did "yum update"
4) Attempted to install evolution-brutus once more. This was now not possible
due to  Autoreq finding that one of the internal e-d-s libraries had changed
version.

The only way that I could fix this (please correct me if I'm wrong) was to
disable Autoreq.

Well, new packages with all fixes:

Spec URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SPECS/evolution-brutus.spec
SRPM URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SRPMS/evolution-brutus-1.1.6-6.src.rpm


Thanks,
  jules

(In reply to comment #19)
> > * Why does this package use Autoreq: no ?
> >   This description forbids finding libraries requirements, which I think
> >   is quite unwilling. Even if you want to specify version-related
> >   requirements, "Autoreq: no" is unnecessary because you can simply add
> >   the requirements in addition to auto-finding requirements.
> 
> Please believe me when I say that I didn't do this lightly. The thing that
> forced me to disable Autoreg is that at least one of the libraries (libebook if
> I rememver correctly) that are provided internally by e-d-s changed version from
> one stable release to another. I observed that when I:
> 
> 1) Installed evolution-brutus for testing
> 2) Un-installed evolution-brutus
> 3) did "yum update"
> 4) Attempted to install evolution-brutus once more. This was now not possible
> due to  Autoreq finding that one of the internal e-d-s libraries had changed
> version.
> 
> The only way that I could fix this (please correct me if I'm wrong) was to
> disable Autoreq.

This sounds to me like a regular shared library update that would require this
package to be rebuilt against the updated e-d-s? What's different here that
makes this not the case?

(In reply to comment #20)
> (In reply to comment #19)
> > > * Why does this package use Autoreq: no ?
> > >   This description forbids finding libraries requirements, which I think
> > >   is quite unwilling. Even if you want to specify version-related
> > >   requirements, "Autoreq: no" is unnecessary because you can simply add
> > >   the requirements in addition to auto-finding requirements.
> > 
> > Please believe me when I say that I didn't do this lightly. The thing that
> > forced me to disable Autoreg is that at least one of the libraries (libebook if
> > I rememver correctly) that are provided internally by e-d-s changed version from
> > one stable release to another. I observed that when I:
> > 
> > 1) Installed evolution-brutus for testing
> > 2) Un-installed evolution-brutus
> > 3) did "yum update"
> > 4) Attempted to install evolution-brutus once more. This was now not possible
> > due to  Autoreq finding that one of the internal e-d-s libraries had changed
> > version.
> > 
> > The only way that I could fix this (please correct me if I'm wrong) was to
> > disable Autoreq.
> 
> This sounds to me like a regular shared library update that would require this
> package to be rebuilt against the updated e-d-s? What's different here that
> makes this not the case?

That is surely one way to fix it. My gripe with this is that perfectly fine RPMs
that installed with, say, eds-1.4.1 won't install with eds-1.4.1 due to the
changed version of some internal library in eds. I wouldn't mind to just rebuild
the RPMs if the library in question had API changes, but API changes in a stable
eds release serie should'nt happen, right?

Anyway, I won't mind one bit to re-enable Autoreq if that is the right thing to do. 

Thoughts? 
  jules

(In reply to comment #21)
> that installed with, say, eds-1.4.1 won't install with eds-1.4.1 due to the
                                                         ^^^^^^^^^

"eds-1.4.2" not "eds-1.4.1".

sorry,
  jules

Hi,

The only change is that I've enabled Autoreq. All issues raised should be fixed now.

Spec URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SPECS/evolution-brutus.spec
SRPM URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SRPMS/evolution-brutus-1.1.6-7.src.rpm


Thanks,
  jules

Hi.

Well, ORBit2 on FC5 is now 2.14.0-1. So this package still cannot
be rebuilt on FC5 (with full updates). If this package really requires
ORBit2 >= 2.14.1, then you have to ask for FC ORBit2 maintainer to
upgrade FC5 ORBit2 ???

Hi,

Yes, it really requires >=2.14.1. OK, I'll ask the maintainer but I'm really
aiming more at inclusion in FC6 extras. 

BTW, you can get ORBit2-2.14.1 and libIDL-0.8.7 for FC5 here:

http://www.omesc.com/content/downloads/dist/Fedora%20Core%205/SRPMS/ORBit2-2.14.1-omc.1.src.rpm
http://www.omesc.com/content/downloads/dist/Fedora%20Core%205/SRPMS/libIDL-0.8.7-omc.1.src.rpm


Best regards,
  jules

Also you use $RPM_BUILD_ROOT and %{buildroot}
Just pick one of those and use it always for consistency

Done. 

New packages here:

Spec URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SPECS/evolution-brutus.spec
SRPM URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SRPMS/evolution-brutus-1.1.6-8.src.rpm

Thanks,
  jules

--------------------------------------------------
I cannot sponsor you because I am not a member of
sponsors. I can do only pre-review of this package.
--------------------------------------------------

1. From http://fedoraproject.org/wiki/Packaging/Guidelines :

* Timestamps
  - These packages include many text files and preserving timestamps
    is then preferable. Try to keep timestamps (can this package
    accept 'make INSTALL="install -c -p" install'?)

* File and Directory Ownership
  - The following directories are not owned by any packages.
    /usr/include/evolution-data-server-1.8/brutus/
    /usr/share/idl/brutus/

2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
   = Nothing.

3. Other things I have noticed :
   - Well, %{find_lang} %{name}-2.8 perhaps means:
     Conflicts: evolution < 2.7
     Conflicts: evolution >= 2.9

mock build for FE-6 devel i386 is okay.

(In reply to comment #28)
> --------------------------------------------------
> I cannot sponsor you because I am not a member of
> sponsors. I can do only pre-review of this package.
> --------------------------------------------------

Yes, but I want to express my gratitude to you for doing this 
pre-review anyway. Thanks!


> 1. From http://fedoraproject.org/wiki/Packaging/Guidelines :
> 
> * Timestamps
>   - These packages include many text files and preserving timestamps
>     is then preferable. Try to keep timestamps (can this package
>     accept 'make INSTALL="install -c -p" install'?)

Fixed.


> * File and Directory Ownership
>   - The following directories are not owned by any packages.
>     /usr/include/evolution-data-server-1.8/brutus/
>     /usr/share/idl/brutus/

Fixed I hope. I added the directories before the files within 
them and prefixed with the %dir directive. That should fix 
it, right?


> 2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
>    = Nothing.
> 
> 3. Other things I have noticed :
>    - Well, %{find_lang} %{name}-2.8 perhaps means:
>      Conflicts: evolution < 2.7
>      Conflicts: evolution >= 2.9

I am not sure if anything is wrong here. The "2.8" version tag is 
autogenerated during autogen.sh execution from the spec.in file. I can 
see that it would complicate matters if the SRPM that is generated for 
FC6 is used under, say, FC5. Should I completely drop the version tag here?


New release here:

Spec URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SPECS/evolution-brutus.spec
SRPM URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SRPMS/evolution-brutus-1.1.6-9.src.rpm


Thanks,
  jules

--------------------------------------------------
I cannot sponsor you because I am not a member of
sponsors. I can do only pre-review of this package.
--------------------------------------------------

* Well, another unowned directory is found:
  /usr/include/brutusd-1.0/

* Well, "2.8" tag is actually related with evolution version.
  configure says:

GETTEXT_PACKAGE=evolution-brutus-${EVOLUTION_VERSION}

  and po/Makefile.in.in says:

echo "installing $$lang.gmo as $$dir/$(GETTEXT_PACKAGE).mo"

  So, the line %{find_lang} %{name}-2.8 should be changed according
  to the version of evolution. If this src.rpm is supposed to be
  used both on FC6-devel and FC5, then the suffix of "2.8" must be
  automatically changed. One way to do this is:

--------------------------------------------------------
for f in %{_bindir}/evolution-* ; do
   evover=${f#%{_bindir}/evolution-}
done

%{find_lang} %{name}-${evover}
---------------------------------------------------------
  With applying this, the version-specific conflict for evolution can be
  removed.

(In reply to comment #30)
> --------------------------------------------------
> I cannot sponsor you because I am not a member of
> sponsors. I can do only pre-review of this package.
> --------------------------------------------------
> 
> * Well, another unowned directory is found:
>   /usr/include/brutusd-1.0/

OK, fixed.


> * Well, "2.8" tag is actually related with evolution version.
>   configure says:
> 
> GETTEXT_PACKAGE=evolution-brutus-${EVOLUTION_VERSION}
> 
>   and po/Makefile.in.in says:
> 
> echo "installing $$lang.gmo as $$dir/$(GETTEXT_PACKAGE).mo"
> 
>   So, the line %{find_lang} %{name}-2.8 should be changed according
>   to the version of evolution. If this src.rpm is supposed to be
>   used both on FC6-devel and FC5, then the suffix of "2.8" must be
>   automatically changed. One way to do this is:
> 
> --------------------------------------------------------
> for f in %{_bindir}/evolution-* ; do
>    evover=${f#%{_bindir}/evolution-}
> done
> 
> %{find_lang} %{name}-${evover}
> ---------------------------------------------------------
>   With applying this, the version-specific conflict for evolution can be
>   removed.

Well, I really did not intent this SRPM to be used on anything but FC6(-devel),
as FC4/5 SRPMs are automatically generated by my build script, but your solution
is so elegant and clever that I simply *must* use it. Thanks a lot!

New files here:

Spec URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SPECS/evolution-brutus.spec
SRPM URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SRPMS/evolution-brutus-1.1.6-10.src.rpm
 
Thanks,
  jules

Rebuild.

Spec URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SPECS/evolution-brutus.spec
SRPM URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SRPMS/evolution-brutus-1.1.6-11.src.rpm
 

Well, I cannot formally review this package because
I am not a sponsor, so I hope someone who has sponsor
membership would review this.

Yes, please. The spec and srpm files should not technically hinder an adoption
into extras but I really need a formal review as well as a sponsor to make this
happen.

Step forward please.

Hmm... the "osgcal" must be an accident flip of the mouse. 


Sorry.

Rebuild for Brutus Server 0.9.31.

Spec URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SPECS/evolution-brutus.spec
SRPM URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SRPMS/evolution-brutus-1.1.6-12.src.rpm

I've recently worked on getting e-b working on Ubuntu Edgy. This was no trivial
task, so enough changes has accumulated to warrant a new release. 

Spec URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SPECS/evolution-brutus.spec
SRPM URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SRPMS/evolution-brutus-1.1.7-1.src.rpm

I will formally review this package as I became a Fedora
Extras sponsor.

Excellent! I look forward to working with you ;-)

These comments come from a glance at the spec.

You should not add the doc files that are in the main package
in the -devel package (README/COPYING...). 

You have 2 %changelog in the spec file.

The latest changelog entry is not very informative.

%{_datadir}/idl/ seems to be unowned.

AutoReq:         yes is unneeded.

Why is gnome-common needed as builrequires?

in devel the requires for main should be (see guidelines):
Requires:         %name = %{version}-%{release}

I may have misunderstood something, but it seems to me that
this package is a connector for the Brutus server, not 
for microsoft exchange, and it is the Brutus server which does 
the work of connecting to the exchange server. The description
should reflect that (unless I am wrong...).

The url points to the main OMC site, but evolution-brutus is
not easily found from that page since this page is primarily 
about Brutus. The page I found which really describes
e-b is:
http://www.omesc.com/modules/xoopsfaq/index.php?cat_id=6

OK, thanks for the glance. 

I've taken care of everything except for the gnome-common thing. gnome-common is
required for the build, so why shouldn't it be in "BuildRequires"?

New files here:

Spec URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SPECS/evolution-brutus.spec
SRPM URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SRPMS/evolution-brutus-1.1.7-2.src.rpm

Well, first formal review of this.

1. http://fedoraproject.org/wiki/Packaging/Guidelines :

* Use rpmlint
  rpmlint is not silent.

E: evolution-brutus (for source)
   hardcoded-library-path in 
      /usr/lib/evolution-data-server-1.2/camel-providers/*
   - You should use %{_libdir}.

W: evolution-brutus undefined-non-weak-symbol 
     /usr/lib/libBrutusd-1.0.so.1.0.0 TC_BRUTUS_BrutusTC_struct
W: evolution-brutus undefined-non-weak-symbol 
     /usr/lib/libBrutusd-1.0.so.1.0.0 TC_BRUTUS_IMAPISession_struct
   - undefined non-weak symbols are found. For the case of this
     package, as there is a corresponding .pc file in -devel package,
     this should be fixed.

* BuildRequires:
  - gnome-common
    Well I also cannot understand why gnome-common is needed for
    BuildRequires.
    I tried mockbuild without gnome-common, then it succeeded and
    rpmdiff shows no difference.

* Requires:
  - evolution (for -devel)
    This is not needed because main package requires this.
  - gnome-common (for -devel)
    Well, please recheck if gnome-common is really required for
    -devel package.

* Handling Locale Files:
  - Well, even if we write
------------------------------------------------
for f in %{_bindir}/evolution-* ; do
    evolution_version=${f#%{_bindir}/evolution-}
done
------------------------------------------------
    to avoid evolution version handling, there is another
    handling:
------------------------------------------------
%files -f %{name}-2.8.lang
------------------------------------------------
    This means that this package requires evolution-2.8 to
    rebuild. To avoid this, change like:
------------------------------------------------
for f in %{_bindir}/evolution-* ; do
    evolution_version=${f#%{_bindir}/evolution-}
done
%{find_lang} %{name}-${evolution_version}
for f in %{name}-*.lang ; do
   mv $f %{name}.lang
fi

%files -f %{name}.lang
-------------------------------------------------

* File and Directory Ownership
  - /usr/share/idl (for -devel)
    Well, I found that this is not necessary because
    * this is owned by libbonobo-devel
    * evolution-data-server-devel requires libbonobo-devel
    * -devel requires evolution-data-server-devel

2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
   = Nothing.

3. Other things I found:
* All pc files in -devel package are broken.
  For example, /usr/lib/pkgconfig/libBrutus-1.0.pc contains:
-------------------------------------------------
idldir=@idldir@
privincludedir=@eds_privincludedir@
-------------------------------------------------
  This is not correct.
  And 
-------------------------------------------------
IDL_INCLUDES=-I ${idldir} -I/usr/include/orbit-2.0 \
    -I/usr/include/glib-2.0 -I/lib/glib-2.0/include
-------------------------------------------------
  This environment is not used and unneeded as Requires: should
  add correct directories to be included.

I'm deeply sorry for the long answering time, but an embarrassing error forced
me to reinstall FC6. However... all review points above has (hopefully) been
taking care of. New files here:

Spec URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SPECS/evolution-brutus.spec
SRPM URL:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%206/SRPMS/evolution-brutus-1.1.7-3.src.rpm

Thanks,
  jules

A.
Oh, please don't change the original source tarball without changing
its version!! This will confuse people who use evolution-brutus.

md5sum:
a4ad27d8c158dcea301c23e7e75ebdf0 
evolution-brutus-1.1.7-2/evolution-brutus-1.1.7.tar.gz
aa9d8ae81ae87851b74de0d92fa09ab7 
evolution-brutus-1.1.7-3/evolution-brutus-1.1.7.tar.gz

You should either
* make patches against original 1.1.7.tar.gz tarball or
* release new version of evolution-brutus

B. 
I have not yet rebuilt the new srpm you uploaded, however:
I use rawhide, and Fedora Extras packages have to be rebuilt firstly
for rawhide environment as buildsys forces it.

I have to say that current evolution and evolution-data-server in rawhide is:
[tasaka1@localhost ~]$ rpm -q evolution evolution-data-server
evolution-2.9.1-2.fc7
evolution-data-server-1.9.1-2.fc7

Please adjust your spec file for these versions.
Through a glance look at your spec,

%dir %{_includedir}/evolution-data-server-1.8/brutus

This is no longer correct as current (current means rawhide)
evolution-data-server has: /usr/include/evolution-data-server-1.10/ .
Please use  %dir %{_includedir}/evolution-data-server-*/brutus to
avoid version changing (also please check if there is some API change or
so).

(In reply to comment #44)
> A.
> Oh, please don't change the original source tarball without changing
> its version!! This will confuse people who use evolution-brutus.

OK, I'll just release new versions instead.


> B. 
> I have not yet rebuilt the new srpm you uploaded, however:
> I use rawhide, and Fedora Extras packages have to be rebuilt firstly
> for rawhide environment as buildsys forces it.
> 
> I have to say that current evolution and evolution-data-server in rawhide is:
> [tasaka1@localhost ~]$ rpm -q evolution evolution-data-server
> evolution-2.9.1-2.fc7
> evolution-data-server-1.9.1-2.fc7
> 
> Please adjust your spec file for these versions.
> Through a glance look at your spec,
> 
> %dir %{_includedir}/evolution-data-server-1.8/brutus
> 
> This is no longer correct as current (current means rawhide)
> evolution-data-server has: /usr/include/evolution-data-server-1.10/ .
> Please use  %dir %{_includedir}/evolution-data-server-*/brutus to
> avoid version changing (also please check if there is some API change or
> so).

Just one question: All of these version dependancies are detected during a run
of autogen.sh and are then used when the spec is created from the spec.in.
Should I remove all of this "clever" version detection stuff and just use '*'
whereever a version dependent directory shows up?

The same goes for my locale file. It has generally been like this:

evolution-brutus-VERSION.mo

I did this after looking in several spec files, such as the evolution-connector
spec. This spec has currently(*):

%files -f evolution-exchange-%{evo_major}.lang

where evo_major is defined in the top of the spec. There is a lot of hardcoded
(via %define) paths in that spec. This must surely be wrong, yes?

BTW, the evolution-connector spec does not use find-lang. Is that wrong?

Thanks,
  jules

(*)
http://cvs.fedora.redhat.com/viewcvs/devel/evolution-connector/evolution-connector.spec?view=markup

(In reply to comment #45)

> Just one question: All of these version dependancies are detected during a run
> of autogen.sh and are then used when the spec is created from the spec.in.
> Should I remove all of this "clever" version detection stuff and just use '*'
> whereever a version dependent directory shows up?

It is up to you. If hardcoded number is worth writing for you, I prefer that
you write the specific version by using macro as you said 
(e.g. %define evo_ver 2.9.1).

> BTW, the evolution-connector spec does not use find-lang. Is that wrong?
I have not yet checked this, however, perhaps it is wrong if this
package includes gettext mo files.

Ok, this took a while, but I finally gor around all of the autoconf 2.59 => 2.60
issues. I've also changed the URLs below to point to the rawhide packages. 

SRPM:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%20Development/SRPMS/evolution-brutus-1.1.9-1.src.rpm

SPEC:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%20Development/SPECS/evolution-brutus.spec

Best regards,
  jules

2nd review of this package:

A. From http://fedoraproject.org/wiki/Packaging/Guidelines :

* Use rpmlint
  rpmlint is not yet silent.
---------------------------------------------------------
W: evolution-brutus devel-file-in-non-devel-package /usr/lib/libBrutusd-1.0.so
W: evolution-brutus devel-file-in-non-devel-package /usr/lib/libBrutus-1.0.so
W: evolution-brutus devel-file-in-non-devel-package /usr/lib/libBrutusPROXY-1.0.so
W: evolution-brutus devel-file-in-non-devel-package /usr/lib/libBrutusSTUBS-1.0.so
W: evolution-brutus devel-file-in-non-devel-package /usr/lib/libBrutusSKELS-1.0.so
---------------------------------------------------------
  These files should be in -devel package.

* BuildRequires:
  The following BR are redundant.
  - libglade2-devel <- this is required by gtkhtml3-devel
                    <- this is required by evolution-devel

* Requires:
  - For -devel package:
    Well, .pc files only includes:
---------------------------------------------------------
Requires: libIDL-2.0 >= 0.8.5, ORBit-2.0 >= 2.14.1, libBrutusSTUBS-1.0,
libBrutusSKELS-1.0
Requires: libIDL-2.0 >= 0.8.5, ORBit-2.0 >= 2.14.1, libBrutusSTUBS-1.0
Requires: libIDL-2.0 >= 0.8.5, ORBit-2.0 >= 2.14.1, libBrutusSTUBS-1.0
Requires: libIDL-2.0 >= 0.8.5, ORBit-2.0 >= 2.14.1 
Requires: libIDL-2.0 >= 0.8.5, ORBit-2.0 >= 2.14.1, libBrutusSTUBS-1.0
---------------------------------------------------------
    This usually means that -devel package only requies:
    * main package
    * pkgconfig
    * ORBit2-devel (libIDL-devel is required by ORBit2-devel)

    Would you explain why -devel package requires some extra packages?

  - By the way, .pc files includes:
    Cflags: <skip> -I/lib/glib-2.0/include
    This causes no problem (so this is not a blocker), however, this makes
    no sense.

= From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines: None

-------------------------------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. But before I accept
this package, someone (I am a candidate) should sponsor you.

Once you are sponsored, you have the right to formally review other 
submitters' review request and approve the packages. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines".

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" (at the time you are not sponsored, you cannot do
   a formal review) of other person's review request.

Please check the details on
http://fedoraproject.org/wiki/Extras/HowToGetSponsored

(In reply to comment #48)
> 2nd review of this package:
> 
> A. From http://fedoraproject.org/wiki/Packaging/Guidelines :
> 
> * Use rpmlint
>   rpmlint is not yet silent.
> ---------------------------------------------------------
> W: evolution-brutus devel-file-in-non-devel-package /usr/lib/libBrutusd-1.0.so
> W: evolution-brutus devel-file-in-non-devel-package /usr/lib/libBrutus-1.0.so
> W: evolution-brutus devel-file-in-non-devel-package /usr/lib/libBrutusPROXY-1.0.so
> W: evolution-brutus devel-file-in-non-devel-package /usr/lib/libBrutusSTUBS-1.0.so
> W: evolution-brutus devel-file-in-non-devel-package /usr/lib/libBrutusSKELS-1.0.so
> ---------------------------------------------------------
>   These files should be in -devel package.

Yes, I noticed that rpmlint complained about these files too, but I think it is
a faulty error. These libraries are required by evolution-brutus and its helper
applications. evolution-brutus would not function if they were absent. They
really do not belong in the -devel package so I ignored rpmlint on this one. Am
I in error here?

 
> * BuildRequires:
>   The following BR are redundant.
>   - libglade2-devel <- this is required by gtkhtml3-devel
>                     <- this is required by evolution-devel

OK, fixed.


> * Requires:
>   - For -devel package:
>     Well, .pc files only includes:
> ---------------------------------------------------------
> Requires: libIDL-2.0 >= 0.8.5, ORBit-2.0 >= 2.14.1, libBrutusSTUBS-1.0,
> libBrutusSKELS-1.0
> Requires: libIDL-2.0 >= 0.8.5, ORBit-2.0 >= 2.14.1, libBrutusSTUBS-1.0
> Requires: libIDL-2.0 >= 0.8.5, ORBit-2.0 >= 2.14.1, libBrutusSTUBS-1.0
> Requires: libIDL-2.0 >= 0.8.5, ORBit-2.0 >= 2.14.1 
> Requires: libIDL-2.0 >= 0.8.5, ORBit-2.0 >= 2.14.1, libBrutusSTUBS-1.0
> ---------------------------------------------------------
>     This usually means that -devel package only requies:
>     * main package
>     * pkgconfig
>     * ORBit2-devel (libIDL-devel is required by ORBit2-devel)
> 
>     Would you explain why -devel package requires some extra packages?

I would if I could ;-) 

I think that I confused a require statement with a buildrequire statement. Most
of those additional packages are definitely not needed to use an
evolution-brutus-devel installation. I've update libBrutus-1.0.pc as
applications linking against libBrutus-1.0 would require to link with
libecal-1.2 as well. This should otherwise be fixed now I hope. libecal is
provided by evolution-data-server-devel so I've kept evolution-data-server-devel
in the Require. 

>   - By the way, .pc files includes:
>     Cflags: <skip> -I/lib/glib-2.0/include
>     This causes no problem (so this is not a blocker), however, this makes
>     no sense.

Agreed, no sense at all. Fixed.


> = From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines: None
> 
>
-------------------------------------------------------------------------------------
> NOTE: Before being sponsored:
> 
> This package will be accepted with another few work. But before I accept
> this package, someone (I am a candidate) should sponsor you.
> 
> Once you are sponsored, you have the right to formally review other 
> submitters' review request and approve the packages. 
> For this reason, the person who want to be sponsored (like you) 
> are required to "show that you have an understanding 
> of the process and of the packaging guidelines".
> 
> Usually there are two ways to show this.
> A. submit other review requests with enough quality.
> B. Do a "pre-review" (at the time you are not sponsored, you cannot do
>    a formal review) of other person's review request.
> 
> Please check the details on
> http://fedoraproject.org/wiki/Extras/HowToGetSponsored

Yes, I will attempt to do some pre-reviews. I don't have any other packages at
hand so pre-reviews seems the way to go.

New files here:

SRPM:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%20Development/SRPMS/evolution-brutus-1.1.10-1.src.rpm

SPEC:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%20Development/SPECS/evolution-brutus.spec

Thanks,
  jules


 

(In reply to comment #49)
> (In reply to comment #48)
> > ---------------------------------------------------------
> > W: evolution-brutus devel-file-in-non-devel-package /usr/lib/libBrutusd-1.0.so
> > W: evolution-brutus devel-file-in-non-devel-package /usr/lib/libBrutus-1.0.so
> > W: evolution-brutus devel-file-in-non-devel-package
/usr/lib/libBrutusPROXY-1.0.so
> > W: evolution-brutus devel-file-in-non-devel-package
/usr/lib/libBrutusSTUBS-1.0.so
> > W: evolution-brutus devel-file-in-non-devel-package
/usr/lib/libBrutusSKELS-1.0.so
> > ---------------------------------------------------------
> >   These files should be in -devel package.
> 
> Yes, I noticed that rpmlint complained about these files too, but I think it is
> a faulty error. These libraries are required by evolution-brutus and its helper
> applications. 

Really? Applications should require libBrutusPROXY-1.0.so.1, not 
libBrutusPROXY-1.0.so for runtime.
----------------------------------------------------------------
[tasaka1@localhost ~]$ ldd -r /usr/bin/brutusd
        linux-gate.so.1 =>  (0x00ab3000)
        libBrutusPROXY-1.0.so.1 => /usr/lib/libBrutusPROXY-1.0.so.1 (0x00bdf000)
        libBrutusSTUBS-1.0.so.1 => /usr/lib/libBrutusSTUBS-1.0.so.1 (0x05996000)
        libglib-2.0.so.0 => /lib/libglib-2.0.so.0 (0x00453000)
        libORBit-2.so.0 => /usr/lib/libORBit-2.so.0 (0x07d21000)
        libuuid.so.1 => /lib/libuuid.so.1 (0x00101000)
        libgnome-keyring.so.0 => /usr/lib/libgnome-keyring.so.0 (0x07fd2000)
        libpthread.so.0 => /lib/libpthread.so.0 (0x002f7000)
        libc.so.6 => /lib/libc.so.6 (0x00189000)
        librt.so.1 => /lib/librt.so.1 (0x00448000)
        libgmodule-2.0.so.0 => /lib/libgmodule-2.0.so.0 (0x00600000)
        libdl.so.2 => /lib/libdl.so.2 (0x002f1000)
        libgobject-2.0.so.0 => /lib/libgobject-2.0.so.0 (0x004f3000)
        libgthread-2.0.so.0 => /lib/libgthread-2.0.so.0 (0x00df6000)
        /lib/ld-linux.so.2 (0x0016c000)
----------------------------------------------------------------

You are right again. I'll fix this when I get back to my office on Monday.

Sorry,
  jules

Okay, however it is highly possible that I cannot check your package
re-uploaded until next Thursday (in Japan, EST+14h).

OK, here we go again:

SRPM:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%20Development/SRPMS/evolution-brutus-1.1.12-1.src.rpm

SPEC:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%20Development/SPECS/evolution-brutus.spec


Best regards,
  jules

Well, as I said before, I cannot precisely check your srpm now.
however:

A. undefined non-weak symbols reappeared.
----------------------------------------------------------
W: evolution-brutus undefined-non-weak-symbol /usr/lib/libBrutus-1.0.so.1.0.0
camel_object_type
W: evolution-brutus undefined-non-weak-symbol /usr/lib/libBrutus-1.0.so.1.0.0
camel_object_cast
W: evolution-brutus undefined-non-weak-symbol /usr/lib/libBrutus-1.0.so.1.0.0
camel_base64_decode_simple
W: evolution-brutus undefined-non-weak-symbol /usr/lib/libBrutus-1.0.so.1.0.0
camel_multipart_get_number
W: evolution-brutus undefined-non-weak-symbol /usr/lib/libBrutus-1.0.so.1.0.0
camel_stream_mem_new
W: evolution-brutus undefined-non-weak-symbol /usr/lib/libBrutus-1.0.so.1.0.0
camel_internet_address_get
W: evolution-brutus undefined-non-weak-symbol /usr/lib/libBrutus-1.0.so.1.0.0
camel_mime_message_get_recipients
W: evolution-brutus undefined-non-weak-symbol /usr/lib/libBrutus-1.0.so.1.0.0
camel_mime_part_get_content_id
W: evolution-brutus undefined-non-weak-symbol /usr/lib/libBrutus-1.0.so.1.0.0
camel_mime_part_get_description
...............................
----------------------------------------------------------
Through brief check, 
----------------------------------------------------------
diff -uNr evolution-brutus-1.1.9/server/Makefile.am
evolution-brutus-1.1.12/server/Makefile.am
--- evolution-brutus-1.1.9/server/Makefile.am   2006-10-19 17:14:31.000000000 +0900
+++ evolution-brutus-1.1.12/server/Makefile.am  2006-11-04 09:05:28.000000000 +0900
@@ -40,7 +40,7 @@
        brutus_conv.c
 
 libBrutus_1_0_la_LIBADD = \
-       $(CALENDAR_LIBS)                                                       
                    \
+       -lecal-1.2 \
       
$(BRUTUS_top_dir)/idl_output/libBrutusSTUBS-$(LIBBRUTUS_CURRENT).$(LIBBRUTUS_REVISION).la
  \
       
$(BRUTUS_top_dir)/servant_impl/libBrutusSKELS-$(LIBBRUTUS_CURRENT).$(LIBBRUTUS_REVISION).la
--------------------------------------------------------------------------------
Perhaps this is somewhat incorrect.

B. 
 %package devel
Summary:          IDL and header files for evolution-brutus
Group:            Development/Libraries
Requires:         %name = %{version}-%{release}
Requires:         ORBit2-devel >= 2.14.1
Requires:         pkgconfig >= 0.20
Requires:         evolution-data-server >= 1.6

Perhaps the last line should be "evolution-data-server-devel >= 1.6"
Then evolution-data-server-devel requires libbonobo-devel, which requires
ORBit2-devel, then 
"Requires:         ORBit2-devel >= 2.14.1"
is not necessary.
 

OK, all of the non-weak-symbols should be history by now. The devel package
"Requires:" are fixed as well. 

New files here:

SRPM:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%20Development/SRPMS/evolution-brutus-1.1.13-1.src.rpm

SPEC:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%20Development/SPECS/evolution-brutus.spec


BTW, you have an uncanny nack for knowing the complete chain of package
requriements. Just like above when you points out that if I "Requires:"
evolution-data-server-devel then I don't need ORBit2-devel as it is in
"Requires:" for libbonobo-devel way down the Requires chain. How do you do that?
A special spec hierachy browser or just a very good memory??

Thanks,
  jules

(In reply to comment #55)
 
> BTW, you have an uncanny nack for knowing the complete chain of package
> requriements. Just like above when you points out that if I "Requires:"
> evolution-data-server-devel then I don't need ORBit2-devel as it is in
> "Requires:" for libbonobo-devel way down the Requires chain. How do you do that?
> A special spec hierachy browser or just a very good memory??

Usually finding these redundant (Build)Requires needs some work.
One of the effective way to find these is
* Build srpm by mockbuild
* chroot to the top directory of mockbuild
* For each package listed in (Build)Requires, try:
  'rpm -q --whatrequires <package>'
  or 'rpm -e --test <package>'
Note that some rpm packages have chicken‐and‐egg dependencies.

--------------------------------------------------------------------------------------
BTW, I have not yet checked your new package, 

however, will you do a "pre-review" of some package listed in:
https://bugzilla.redhat.com/bugzilla/showdependencytree.cgi?id=FE-NEW&hide_resolved=1
and tell me what bug you (pre-)reviewed so that I can
judge if I can sponsor you ?

* What you should check through (pre-)review process are 
  generally listed in:
  http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
  http://fedoraproject.org/wiki/Packaging/Guidelines .
* You can also comment about issues which you noticed on the package 
  you (pre)reviewed but which is not listed on the URL above (this 
  criterion is somewhat ambiguous).
  If you have any questions, please notice to me.
  

The content of .pc file again changed......
----------------------------------------------------------
diff -uNr evolution-brutus-devel-1.1.12-1.fc7/usr/lib/pkgconfig/libBrutus-1.0.pc
evolution-brutus-devel-1.1.13-1.fc7/usr/lib/
pkgconfig/libBrutus-1.0.pc
--- evolution-brutus-devel-1.1.12-1.fc7/usr/lib/pkgconfig/libBrutus-1.0.pc     
2006-11-09 02:16:37.000000000 +0900
+++ evolution-brutus-devel-1.1.13-1.fc7/usr/lib/pkgconfig/libBrutus-1.0.pc     
2006-11-09 01:04:44.000000000 +0900
@@ -10,7 +10,7 @@
 
 Name: libBrutus
 Description: Client library for accessing Microsoft Exchange through Brutus
interface
-Version: 1.1.12
-Requires: libIDL-2.0 >= 0.8.5, ORBit-2.0 >= 2.14.1, libBrutusSTUBS-1.0,
libBrutusSKELS-1.0, libecal-1.2
+Version: 1.1.13
+Requires: libIDL-2.0 >= 0.8.5, ORBit-2.0 >= 2.14.1, libBrutusSTUBS-1.0,
libBrutusSKELS-1.0, libecal-1.2, libcamel-1.2
 Libs: -L${libdir} -lBrutus-1.0
 Cflags: -I${privincludedir} -I/usr/include/orbit-2.0 -I/usr/include/glib-2.0
----------------------------------------------------------
However, I cannot find libcamel-1.2.pc in /usr/lib/pkgconfig .

No, that was an annoying typo. I was simply used to the .pc file beeing named
lib*.pc. Sorry about that. It will get fixed today. Another thing for this day
is that I am diving into a few of the non-reviewed packages and try to give some
constructive critisism.

OK, the libBrutus.pc.in typo has been fixed as well as an issue with the
handling of passwords with trailing whitespace.

New files here:

SRPM:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%20Development/SRPMS/evolution-brutus-1.1.14-1.src.rpm

SPEC:
http://www.omesc.com/content/downloads/dist/Fedora%20Core%20Development/SPECS/evolution-brutus.spec

Now of to do some pre-reviews...

Best regards,
  jules

I just did a pre-review of this one:

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

-- 
  jules

Well, 

* I re-reviewed evolution-brutus and this time it is okay.
* Well, for bogl review (bug 214124), I have not yet checked
  this concretely, however I have somewhat different opinion from
  you.

Okay, now I trust you and I will sponsor you.

-------------------------------------------------------
Please go forward according to 

http://fedoraproject.org/wiki/Extras/Contributors

to import this package to Fedora Extras. I will sponsor
you when you have taken steps partway written in the page above
(then I should receive the mail that you need a sponsor).

Oh, I forgot to write this......

------------------------------------------------------
  This package (evolution-brutus) is ACCEPTED by me.

What a great way to begin the day ;-)

I'll jump straight to those additional steps.

Thanks a lot!
  jules

Any trouble occurring to you?

( Just writing a note that this review request is _NOT_ stalled.
  Jules has been mailing to me what he is concerned about concretely ).

I'm closing this one for now as I've been unable to get any response at all from
Red Hat regarding my CLA worries. I do this with great regret but in the hope
that someone else will submit evolution-brutus to Fedora. I will certainly
support any would-be maintainer(s) as much as humanly possible.

Many thanks to Mamoru for his great help getting evolution-brutus into a state
technically suitable for inclusion into Fedora!

Best regards,
  jules
 

Well, I think this package is useful, so I wish someone would 
take this package.

Closing for now, marking this as FE-DEADREVIEW, adding
this to WishList.

Mamoru, after talking to Max Spevack and Jules, I've agreed to be the maintainer
for evolution-brutus.  Does your approval of this package still stand?

http://www.omesc.com/content/downloads/dist/Rawhide/SRPMS/evolution-brutus-1.1.14-1.src.rpm

Well, as some long time has already passed, I want to
re-review this before approving.

And currently the newest seems to be 1.1.25.
Would you repackage this tarball?

http://www.omesc.com/content/downloads/dist/SOURCES/

Note:
As I live in Japan and now it is past 2 AM, I will review
the package once after I go to bed if you submit a new srpm.

No worries, I probably won't have time to update to to 1.1.25 until tomorrow anyhow.

I should add that I'm currently struggling to get my build system to cope with
the necessary modifications to the evolution-brutus.spec.in and configure.in
files that is required to transparently support OpenSUSE from the same set of
files. I expect to release 1.1.26 shortly.

It does seem, though, that the impact on the spec file is going to be minimal. I
can hold the release back a little until I've seen your input on 1.1.25 if you want?

I've now successfully been able to build RPMs on OpenSUSE as well as on rawhide
using the usual GNU auto tools to create the spec file automatically. This,
unfortunately results on some whitespace noise in the resulting spec file. A few
additional empty lines to be specific.

Anyway - here are the new files:

http://www.omesc.com/content/downloads/dist/Rawhide/SPECS/evolution-brutus.spec
http://www.omesc.com/content/downloads/dist/Rawhide/SRPMS/evolution-brutus-1.1.25.3-1.src.rpm
http://www.omesc.com/content/downloads/dist/Rawhide/RPMS/i386/evolution-brutus-devel-1.1.25.3-1.i386.rpm
http://www.omesc.com/content/downloads/dist/Rawhide/RPMS/i386/evolution-brutus-debuginfo-1.1.25.3-1.i386.rpm
http://www.omesc.com/content/downloads/dist/Rawhide/RPMS/i386/evolution-brutus-1.1.25.3-1.i386.rpm
http://www.omesc.com/content/downloads/dist/Rawhide/SOURCES/evolution-brutus-1.1.25.3.tar.gz

You can also start with the tar-ball if you like. Just go to the top-level
directory and do:

./autogen.sh --enable-brutus-target=fedora ; make dist-rpm

This will generate the RPMs in ~/rpmbuild. But - be forewarned - the old content
of ~/rpmbuild will be deleted before the build is actually started.

Thanks,
  jules

Brian, would you expect that I check the rpms submitted
by Jules comment 74? Or will you check Brian's rpms and
modify them somewhat?

It would be easier to maintain for all parties, I think, if we could modify my
build process so that the resulting e-b spec file is good enough for approval
instead of maintaining Brian's spec outside of my source tree. 

But it is about making it as easy as possible for Brian to maintain e-b in
Fedora, not about making it easy for me. So I'll just go with whatever Brian
says ;-)

I haven't had a chance to look at your updated spec Jules, but my experience
with other packages is that using a generic spec file from the tarball usually
doesn't work in the long run.

Well, then should I wait until you upload a new spec/srpm,
Brian?

(In reply to comment #78)
> Well, then should I wait until you upload a new spec/srpm,
> Brian?

Probably. after a quick look at Jules spec there's a few things that should
probably be changed (Unnecessary requires, use %doc, etc).  Of course, I've got
to get some new web-space to upload to, since I shutdown my webserver over the
holidays. ;)

Created attachment 151889 [details]
Updated spec file

Here's an updated spec file.  No major change, though I did remove the license
that was in the header, since it's not really necessary IMO. Jules, if you have
a problem with that please tell me, and I can put it back in.	Once I score
some webspace, I'll post the srpm.

Comment on attachment 151889 [details]
Updated spec file

Argh! wrong spec file.

Created attachment 151890 [details]
Update spec file.

Here's an updated spec file.  No major change, though I did remove the license
that was in the header, since it's not really necessary IMO. Jules, if you have

a problem with that please tell me, and I can put it back in.	Once I score
some webspace, I'll post the srpm.

Note: This is targeted for the devel branch.

Well, again I review this package (evolution-brutus)

For 1.1.25.3-2

* Source0
  - By the way, where is 1.1.25.3 tarball?

* compilation flags
  - Why are fedora specific compilation
    flags passed _twice_ ?

-----------------------------------------------------------
make[3]: Entering directory
`/builddir/build/BUILD/evolution-brutus-1.1.25.3/idl_output'
/bin/sh ../libtool --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I. -I..
-I/usr/include/orbit-2.0 -I/usr/include/glib-2.0
-I/builddir/build/BUILD/evolution-brutus-1.1.25.3/idl_output  -DORBIT2=1
-pthread -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include
-I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0
-I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include
-I/usr/include/gnome-keyring-1 -I/usr/include/orbit-2.0
-I/usr/include/libIDL-2.0    -Wall -Wmissing-prototypes -Wnested-externs
-Wpointer-arith -Wno-sign-compare  -Werror -Werror-implicit-function-declaration
-Wundef -Wpointer-arith -Wbad-function-cast -Wcast-align -Wmissing-prototypes
-Wmissing-declarations -Wnested-externs -std=gnu89 -D_REENTRANT -O2 -g -pipe
-Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables  -DG_LOG_DOMAIN=\"libBrutusSTUBS\" -DORBIT2=1
-pthread -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include
-I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0
-I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include
-I/usr/include/gnome-keyring-1 -I/usr/include/orbit-2.0
-I/usr/include/libIDL-2.0    -Wall -Wmissing-prototypes -Wnested-externs
-Wpointer-arith -Wno-sign-compare  -Werror -Werror-implicit-function-declaration
-Wundef -Wpointer-arith -Wbad-function-cast -Wcast-align -Wmissing-prototypes
-Wmissing-declarations -Wnested-externs -std=gnu89 -D_REENTRANT -O2 -g -pipe
-Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables  -MT IDistList-common.lo -MD -MP -MF
.deps/IDistList-common.Tpo -c -o IDistList-common.lo IDistList-common.c
mkdir .libs
-----------------------------------------------------------

* pkgconfig .pc file
  - Well why are the seemingly-extra include options
-----------------------------------------------------------
-I/usr/include/orbit-2.0 -I/usr/include/glib-2.0
-----------------------------------------------------------
    in Cflags in .pc files needed, although ORBit-2.0 (therefore glib-2.0 and
    is required by .pc files?

-----------------------------------------------------------
For upstream:
* Documentation
  - Why are the following documents empty?
    AUTHORS ChangeLog NEWS

Just back from easter holidays. I'll have had a rather busy catching up day
today so I'll look at the review findings tomorrow.

OK, all of the above has been fixed in my tree except for the last review point.
Should I keep ChangeLog, AUTHORS and NEWS updated too? I've been keeping the
ChangeLog entries in the spec updated so it seems rather as duplicated work to
edit the top-level ChangeLog and NEWS files as well... 

Anyway, I'm currently yum upgrading my rawhide installation and with 233
packages to go it will take a while before I can build the RPMs. I'll post the
updated files tomorrow when yum should be done - knock on wood :-)

Well, IMO these files are generally not empty.
You can see gdm, for example.

* At least, AUTHORS is generally not empty.
* And ChangeLog or NEWS is generally not empty.

(In reply to comment #85)
> I've been keeping the
> ChangeLog entries in the spec updated so...
Well, what is listed in %changelog in spec file is that
the changelog for spec file. 

ChangeLog (or NEWS) is meant to show what changed 
on the evolution-brutus itself (i.e. some additional feature 
added, some deprecated feature removed etc...) so this
is different from spec file %changelog

(In reply to comment #85)
> Should I keep ChangeLog, AUTHORS and NEWS updated too? I've been keeping the
> ChangeLog entries in the spec updated so it seems rather as duplicated work to
> edit the top-level ChangeLog and NEWS files as well... 

None of these are blockers, and it's entirely up to upstream if they want to use
them or not.

(In reply to comment #87)
> (In reply to comment #85)

> None of these are blockers, and it's entirely up to 
> upstream if they want to use
> them or not.

So I wrote as "For upstream". Still check my comment 83. 

I've just uploaded the updated files. These should, knock on wood, fix all
review points.

Get them here:

http://www.omesc.com/content/downloads/dist/Rawhide/SPECS/evolution-brutus.spec
http://www.omesc.com/content/downloads/dist/Rawhide/SRPMS/evolution-brutus-1.1.25.9-1.fc7.src.rpm
http://www.omesc.com/content/downloads/dist/Rawhide/RPMS/i386/evolution-brutus-devel-1.1.25.9-1.fc7.i386.rpm
http://www.omesc.com/content/downloads/dist/Rawhide/RPMS/i386/evolution-brutus-debuginfo-1.1.25.9-1.fc7.i386.rpm
http://www.omesc.com/content/downloads/dist/Rawhide/RPMS/i386/evolution-brutus-1.1.25.9-1.fc7.i386.rpm
http://www.omesc.com/content/downloads/dist/Rawhide/SOURCES/evolution-brutus-1.1.25.9.tar.gz

Thanks,
  jules

Well, again, would you modify before I review spec/srpm
if you want, Brian?

ping?

Brian, anything bad happened to you? Anything that I can help with?? 

Sorry, I was out of town this weekend.  I'll have some time this afternoon to
work on this.

Created attachment 153359 [details]
Updated spec file

Well, for 1.1.25.9-2:

* Requires
  - For devel package, it seems that gpgme-devel, e2fsprogs-devel
    are not needed.
    Note: I usually check by:
    A:
-------------------------------------------------------------------
$ LANG=C grep 'include ' `rpm -ql evolution-brutus-devel` | grep -v Binary | sed
-e 's|^.*:||' | sed -e 's|^[ \t][ \t]*||' | sort | uniq
-------------------------------------------------------------------
    B:
-------------------------------------------------------------------
$ rpm -ql evolution-brutus-devel | grep '/usr/lib/pkgconfig/.*.pc' | xargs cat |
grep Requires
-------------------------------------------------------------------

= License is okay for all 263 files

* Removing documentation
-------------------------------------------------------------------
# Don't bother to pack unnecessary docs.
rm -f %{buildroot}%{_datadir}/doc/%{name}-devel-%{version}/INSTALL
rm -f %{buildroot}%{_datadir}/doc/%{name}-devel-%{version}/building_from_source
-------------------------------------------------------------------
  - Just the following?
-------------------------------------------------------------------
rm -rf %{buildroot}%{_datadir}/doc/%{name}-devel-%{version}/
-------------------------------------------------------------------

(In reply to comment #95)
> Well, for 1.1.25.9-2:

Any other blockers?  Otherwise, your suggestions can be fixed when the package
is imported into CVS since they're fairly minor.

(In reply to comment #96)
> (In reply to comment #95)
> > Well, for 1.1.25.9-2:
> 
> Any other blockers?  Otherwise, your suggestions can be fixed when the package
> is imported into CVS since they're fairly minor.

No any others. Then please fix in CVS.
-------------------------------------------------
  This package (evolution-brutus) is APPROVED by me
-------------------------------------------------

Brian, by the way would you have a time to
review migemo review request I submitted (bug 236493)?

(In reply to comment #98)
> Brian, by the way would you have a time to
> review migemo review request I submitted (bug 236493)?

Yeah, I'll give it a look later today.

New Package CVS Request
=======================
Package Name: evolution-brutus
Short Description: Brutus based Exchange connector for Evolution
Owners: bdpepple
Branches: 
InitialCC: colding

Thanks for approving e-b :-) 

I also like to note that I'm releasing 1.1.26 tomorrow.

As promised:

http://www.omesc.com/content/downloads/dist/Rawhide/SPECS/evolution-brutus.spec
http://www.omesc.com/content/downloads/dist/Rawhide/SRPMS/evolution-brutus-1.1.26.0-1.fc7.src.rpm
http://www.omesc.com/content/downloads/dist/Rawhide/RPMS/i386/evolution-brutus-devel-1.1.26.0-1.fc7.i386.rpm
http://www.omesc.com/content/downloads/dist/Rawhide/RPMS/i386/evolution-brutus-debuginfo-1.1.26.0-1.fc7.i386.rpm
http://www.omesc.com/content/downloads/dist/Rawhide/RPMS/i386/evolution-brutus-1.1.26.0-1.fc7.i386.rpm
http://www.omesc.com/content/downloads/dist/Rawhide/SOURCES/evolution-brutus-1.1.26.0.tar.gz

The main change is that I've moved from GPGME to libgcrypt. GPGME did not behave
the way I wanted on OpenSUSE so I had to change to a lower level API if I wanted
identical behavior on all supported platforms.

Have a nice weekend,
  jules

This release is fixing all of the build stoppers that Brain noticed:

http://www.omesc.com/content/downloads/dist/Rawhide/SPECS/evolution-brutus.spec
http://www.omesc.com/content/downloads/dist/Rawhide/SRPMS/evolution-brutus-1.1.26.2-1.fc7.src.rpm
http://www.omesc.com/content/downloads/dist/Rawhide/RPMS/i386/evolution-brutus-devel-1.1.26.2-1.fc7.i386.rpm
http://www.omesc.com/content/downloads/dist/Rawhide/RPMS/i386/evolution-brutus-debuginfo-1.1.26.2-1.fc7.i386.rpm
http://www.omesc.com/content/downloads/dist/Rawhide/RPMS/i386/evolution-brutus-1.1.26.2-1.fc7.i386.rpm
http://www.omesc.com/content/downloads/dist/Rawhide/SOURCES/evolution-brutus-1.1.26.2.tar.gz

Best regards,
  jules

(In reply to comment #103)
> This release is fixing all of the build stoppers that Brain noticed:
                                                        ^^^^^

I'm sure you're clever but that should read "Brian" ;-)

Package built for all arches in Rawhide.