Re: [Evolution-hackers] Backporting EWS GAL changes to 3.8

2013-05-30 Thread David Woodhouse
On Thu, 2013-05-30 at 20:03 +0200, Milan Crha wrote:
> 
> I expect, personally, if .2 release was building without any issue, that
> I just fire the build and it'll build the same flawlessly also in .3+.
> Thus I suggested on IRC to do the check on the stable version opposite,
> add an option to actually use the stock libmspack, instead of forcing
> not using it. Distro packagers sort of expect such (and bigger) changes
> when packaging new major version, thus that's all fine on master.

Yeah, I understand that and completely agree in the general case.

However, in this case the old build was *broken*. We really *should* be
using the proper libmspack. In this case I *want* to make people jump
through extra hoops if they want to continue to build this the broken
way, after I've gone to the trouble of fixing it :)

It's not as if adding --with-internal-lzx is *much* extra work, if
that's really what they decide to do.

> If you feel like me doing things unnecessary overcomplicated, then feel
> free to commit it as is. After all, it's only my opinion (in a good
> meaning of the words).

If it makes you feel better about the packaging work, I can go and file
a Fedora bug for the violation of the packaging guidelines; shipping a
hacked-up copy of parts of libmspack instead of using the real one? :)

When you package 3.8.3 for Fedora 19, you will *definitely* need to add
libmspack-devel as a BuildRequires, as part of the fix for that. If I
commit things as-is, that's *all* you'll have to do.

If I change things around as you suggest, you'll actually have to do
*more* work because you'll have to add the --without-internal-lzx option
to the configure line too :)

-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature
___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
https://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Backporting EWS GAL changes to 3.8

2013-05-30 Thread Milan Crha
On Thu, 2013-05-30 at 13:03 +0100, David Woodhouse wrote:
> Thanks. I'll wait to see if I've won Milan round, before doing so...

Hi,
my concern was more from packager's point of view, than anything else.
It's a nightmare to chase for dependencies when doing bleeding-edge
packaging, and it hurts the more when packaging stable release, which is
not called 'stable' only as a lack of a better word :)

I expect, personally, if .2 release was building without any issue, that
I just fire the build and it'll build the same flawlessly also in .3+.
Thus I suggested on IRC to do the check on the stable version opposite,
add an option to actually use the stock libmspack, instead of forcing
not using it. Distro packagers sort of expect such (and bigger) changes
when packaging new major version, thus that's all fine on master.

If you feel like me doing things unnecessary overcomplicated, then feel
free to commit it as is. After all, it's only my opinion (in a good
meaning of the words).
Bye,
Milan

___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
https://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Backporting EWS GAL changes to 3.8

2013-05-30 Thread David Woodhouse
On Thu, 2013-05-30 at 07:41 -0400, Matthew Barnes wrote:
> 
> That's the right way.
> 
> A two week-old release is far too new for a stable branch dependency,
> especially one we're introducing mid-stream.  It needs to be optional
> for now, but at the same time we do want packagers to be aware of it. 
> Aborting the configure script by default with a workaround message if
> libmspack 0.4 is not present accomplishes that.

(I know we care about more than just Fedora, but FWIW libmspack 0.4 is
in updates-testing for Fedora 19 and the Fedora packages of evo-ews
3.8.3 should be built to use it, if we do go ahead and push this.)

> We're giving special attention to the stable release this cycle, so I'm
> willing to bend the rules a bit.
> 
> +1 from me.

Thanks. I'll wait to see if I've won Milan round, before doing so...

> I think we could remove the internal lzx copy as early as 3.11.

Note that I'm actually thinking of *disabling* the incremental-update
feature in the case where we're using the internal LZX code. It's all
very well not bothering to check CRCs when we're just doing a simple
decompression of the full file, but when we are applying multiple binary
deltas starting from a file we found in our local cache from last time,
we *really* ought to be checking the output more carefully.

I'm not really sure that affects the question I actually asked about
backporting to 3.8, either way; just reinforces the "you *really* ought
to be using the proper library" assertion. And it means I'd prefer to
remove the fallback before 3.10, rather than after it.

-- 
dwmw2


smime.p7s
Description: S/MIME cryptographic signature
___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
https://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Backporting EWS GAL changes to 3.8

2013-05-30 Thread Matthew Barnes
On Thu, 2013-05-30 at 11:56 +0100, David Woodhouse wrote:
> We had a local copy of the LZX decompression code, taken from libmspack
> and then hacked up somewhat to support the variant that the EWS GAL
> uses. I cleaned up that code and got it accepted into upstream
> libmspack, and libmspack 0.4 has been released a week or two ago.
> 
> As part of that process, the libmspack maintainer spotted a number of
> bugs in our changes. These have all been fixed in the 0.4 release.
> 
> I've fixed the build in master so that we either need to build with
> libmspack 0.4 as an additional dependency (recommended), or use a
> configure option to build with our internal version of the decompression
> code instead. If you configure without an appropriate libmspack, the
> resulting error will tell you about the --with-internal-lzx option.

That's the right way.

A two week-old release is far too new for a stable branch dependency,
especially one we're introducing mid-stream.  It needs to be optional
for now, but at the same time we do want packagers to be aware of it. 
Aborting the configure script by default with a workaround message if
libmspack 0.4 is not present accomplishes that.

We're giving special attention to the stable release this cycle, so I'm
willing to bend the rules a bit.

+1 from me.

I think we could remove the internal lzx copy as early as 3.11.

Matt


___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
https://mail.gnome.org/mailman/listinfo/evolution-hackers


[Evolution-hackers] Backporting EWS GAL changes to 3.8

2013-05-30 Thread David Woodhouse

I've implemented a bunch of fixes to the offline addressbook handling in
evo-ews master, and I'd like to backport them to 3.8.

In fact I've done *all* my testing on the gnome-3-8 branch and haven't
tested master at all as I've been working. (Naughty dwmw2).

The code I'd like to push is at git:// or
http://git.infradead.org/users/dwmw2/evolution-ews.git

We had a local copy of the LZX decompression code, taken from libmspack
and then hacked up somewhat to support the variant that the EWS GAL
uses. I cleaned up that code and got it accepted into upstream
libmspack, and libmspack 0.4 has been released a week or two ago.

As part of that process, the libmspack maintainer spotted a number of
bugs in our changes. These have all been fixed in the 0.4 release.

I've fixed the build in master so that we either need to build with
libmspack 0.4 as an additional dependency (recommended), or use a
configure option to build with our internal version of the decompression
code instead. If you configure without an appropriate libmspack, the
resulting error will tell you about the --with-internal-lzx option.

I've fixed the most egregious of the bugs in our internal copy, but we'd
still be better off using the real libmspack. Our internal version
doesn't check CRC on its output, for one thing. And it's non-trivial to
add, because of the differences between the two code bases.

As well as the LZX fixes, I've also implemented incremental downloads.
My own company's addressbook was a 30M download for the full database,
and it changes every 12 hours. Now it's only a few hundred KiB each
time. As I have an an ADSL line where I pay for daytime bandwidth, I
consider this an important bug fix rather than purely an enhancement :)

So... I understand that we don't like forcing people to add new
dependencies or even to change their ./configure flags in a stable
release, but I think that backporting all this as-is is actually the
right thing to do. We should never have been shipping a hacked-up copy
of a third-party shared library *anyway*. I know it violates the Fedora
packaging guidelines, and probably for other distributions too?

Opinions? I know Milan wasn't happy about even requiring the
--with-internal-lzx option to preserve the (broken) status quo in 3.8,
but then disappeared from IRC before I could try to justify its
necessity. We *could* make it automatically do the "right" thing, using
libmspack if it's available or silently falling back to the internal
version. But some people hate configure scripts doing that, and I've
already changed master *not* to do it that way...

-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature
___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
https://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Placing of goto labels

2013-05-30 Thread David Woodhouse
On Thu, 2013-03-28 at 09:30 +0100, Milan Crha wrote:
> What's the usual way for goto labels in other projects?
> Would it make sense to write labels 'one-space indented'?

FWIW one-space indented is what emacs does by default, and is thus what
you'll usually see in code that I've written.

-- 
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com  Intel Corporation





smime.p7s
Description: S/MIME cryptographic signature
___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
https://mail.gnome.org/mailman/listinfo/evolution-hackers