Bug 495310 - rsync contains forked copy of zlib
Summary: rsync contains forked copy of zlib
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: rsync
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Michal Luscon
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: DuplicSysLibsTracker zsync 526432
TreeView+ depends on / blocked
 
Reported: 2009-04-11 16:15 UTC by Toshio Ernie Kuratomi
Modified: 2018-04-11 14:22 UTC (History)
17 users (show)

Fixed In Version:
Doc Type: Enhancement
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-08-07 11:11:41 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
patch for building zlib as a shared lib (2.25 KB, patch)
2009-10-20 08:31 UTC, Dan Horák
no flags Details | Diff
patch to create subpackages (lib + devel) with the forked copy of zlib (2.31 KB, patch)
2009-10-20 08:32 UTC, Dan Horák
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Samba Project 6916 0 None None None Never

Description Toshio Ernie Kuratomi 2009-04-11 16:15:52 UTC
Description of problem:
rsync is carrying its own copy of zlib.  This is not allowed for reasons noted here:

https://fedoraproject.org/wiki/Packaging:Guidelines#Duplication_of_system_libraries

and in the preceding section on static libraries.

Additionally, other programs are appearing which wish to use that code.  zsync makes its own copy of the rsync library and librsync has discussed embedding but appears to have gone with a less efficient method that uses the system zlib for now.

Additional info:

The rsync zlib is a forked copy.  The zlib/README.rsync file has the following details:

"""
Specific changes that have been made to zlib for rsync include:

- add Z_INSERT_ONLY to allow for efficient history updating without
  actually emitting any data. This is used to compress the matched
  blocks that don't cross the wire, which gives better compression
  ratios on the literal data.

- fixed a number of minor compilation issues. (redefinition of MAX and
  other such trivial things)

- include rsync.h to ensure that we get a consistent set of includes
  for all C code in rsync and to take advantage of autoconf
"""

The first is the divergence that causes other programs wanting to speak the rsync protocol to wish to use this version of the library.

Options:

1) Get these changes into upstream zlib.  The README.rsync has this note:

"""
The rsync maintainers hope to fix this problem in the future by either
merging our changes into the upstream version, or backing them out of
rsync in a way that preserves wire compatibility.  But in the meantime
this version must be maintained in parallel.
"""

but this seems to never have happened.  The diff between vanilla zlib and rsync zlib as of 2005:
http://lists.samba.org/archive/rsync/2005-December/014180.html

2) Get upstream rsync to drop its copy and use system zlib.  This is also something upstream has discussed but has either not found a way to do compatibly or hasn't had the will to achieve.

3) Get the forked zlib into its own library.  If we had a library like libz_rsync we'd have the following advantages:

  * Only one copy on the system instead of one for every application.
  * Security and other fixes would flow from upstream to all the applications via normal dynamic library linking instead of a sequence of upstream's having to notice the changes.
  * If we're lucky enough to get upstream rsync maintaing this, perhaps they'll rethink how much work it is vs getting their changes merged to zlib :-)

We'd still be depending on the libz_rsync maintainers tracking zlib in a timely fashion, though.

Comment 1 Simo Sorce 2009-04-11 18:45:52 UTC
I honestly think this is something you have to pursue upstream.
I don't think it would be a good idea to carry patches to do something like that as a private Fedora modification.

Comment 2 Toshio Ernie Kuratomi 2009-04-11 19:10:12 UTC
<nod> I think it belongs upstream as well.  The question is who is responsible for carrying that upstream?  It's usually the package maintainer who is responsible.  However, this usually comes up at review time and it looks like rsync wasn't reviewed by someone who checked this.

Comment 3 Toshio Ernie Kuratomi 2009-04-11 19:19:26 UTC
BTW, this has come up because it's blocking the zsync review:
  https://bugzilla.redhat.com/show_bug.cgi?id=490140

I've mentioned that they should try and carry this work to rsync upstream here:
  https://fedorahosted.org/fesco/ticket/134

Comment 4 Simon 2009-04-26 17:23:15 UTC
Perhaps this idea is good, perhaps this is totally bullshit. If it is totally bullshit please ignore it.

There are so many forked zlib and every upstream has his own changes and this would be impossible to include all changes.

We should make extra pacakges for every zlib with the project that is forked for as suffix or prefix. like rsync-zlib or zlib-rsync with the dependency of this package. Just an idea, please don't stone me to death for this.

Comment 5 Bug Zapper 2009-06-09 13:39:46 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 11 development cycle.
Changing version to '11'.

More information and reason for this action is here:
http://fedoraproject.org/wiki/BugZappers/HouseKeeping

Comment 6 Toshio Ernie Kuratomi 2009-06-30 05:21:12 UTC
Simo, ping

notting reported at the FESCo meeting a couple months ago that you thought we could fix this without too much difficulty downstream.  What's going on with this?

Comment 7 Simo Sorce 2009-06-30 12:56:40 UTC
I don't remember talking with Notting about this.
And I am not comfortable forking off a library downstream and upstream seem uninterested in dealing with this stuff.
If someone wants to do the job and the patching be my guest though.

My preferred course of action though, would be to get whatever modification necessary included in upstream zlib and not in a distribution specific fork, and then ask upstream to use the available library.

Comment 8 Dan Horák 2009-10-20 08:31:27 UTC
Created attachment 365316 [details]
patch for building zlib as a shared lib

Comment 9 Dan Horák 2009-10-20 08:32:19 UTC
Created attachment 365317 [details]
patch to create subpackages (lib + devel) with the forked copy of zlib

Comment 10 Dan Horák 2009-10-20 08:42:15 UTC
Simo, because we should move somewhere I prepared a patch that will build the internal copy of zlib as a separate shared library and link rsync against it. The other patch adds 2 subpackages to rsync to distribute the library and the development files.

The whole work with creating and installing the shared library is put into the Makefile patch and the spec file patch is minimalized, but it could be also done the other way with minimal Makefile patch and more work in the spec file.

One note to original spec file - setting the CFLAGS when "make" is called is here a bad thing, because the right CFLAGS are already set via %configure directly into the generated Makefile.

Comment 11 Robert Scheck 2009-10-20 09:01:39 UTC
Dan, that's not really the expectation. Rsync should link against the system
zlib or an official fork that can be used by rsync and zsync as well. Current
result is not really the solution from my point of view...

Comment 12 Dan Horák 2009-10-20 09:33:28 UTC
Robert, but my opinion is this is a distribution of a fork that can be used by zsync (and eventually others) and building both rsync and the library from one source archive is desirable. Or do you prefer to have a separate source archive and package for the forked zlib? Accepting the fact that using a system zlib is not possible in short or mid term ...

Comment 13 Robert Scheck 2009-10-20 09:46:22 UTC
Well, is your work a rsync-only thing or can it be used by zsync as well? If
it's a rsync-only thing, we unfortunately IMHO have nothing gained. Because if
I now need a zsync-zlib/zsync-zlib-devel as well, we wouldn't have needed a
shared library being anyway the same rsync fork of zlib.

Comment 14 Dan Horák 2009-10-20 10:02:29 UTC
In zsync you will need to have rsync-zlib-devel as BuildRequires: and patch the buildsystem a bit to link with librsync-zlib and use headers from /usr/include/rsync-zlib. I didn't check the sources of zsync, but there shouldn't be an issue. Or do I miss something?

Comment 15 Dan Horák 2009-10-20 13:54:03 UTC
Looks like I missed something :-( While the rsync and zsync applications are wire-compatible, the rsync-zlib and zsync-zlib are not API compatible, OMG ...

The diffs against upstream versions can be seen at http://fedora.danny.cz/rsync Maybe the changes could be merged into one "super-zlib" ...

Comment 16 Robert Scheck 2009-10-20 18:50:27 UTC
gcc -std=gnu99 -o librsync-zlib.so.1 -shared -Wl,--soname,librsync-zlib.so.1 zlib/deflate.o zlib/inffast.o zlib/inflate.o zlib/inftrees.o zlib/trees.o zlib/z
util.o zlib/adler32.o zlib/compress.o zlib/crc32.o
/usr/bin/ld: zlib/deflate.o: relocation R_X86_64_32S against `_length_code' can not be used when making a shared object; recompile with -fPIC
zlib/deflate.o: could not read symbols: Bad value
collect2: ld returned 1 exit status
make: *** [librsync-zlib.so.1] Error 1

Comment 17 Robert Scheck 2009-10-20 19:03:29 UTC
At first, your patch for rsync is unfortunately not usable at x86_64 right now.

Comment 18 Dan Horák 2009-10-20 20:00:37 UTC
(In reply to comment #17)
> At first, your patch for rsync is unfortunately not usable at x86_64 right now.  

Robert, you must remove the CFLAGS override from the make call in %build section of the spec. It not required even in normal rsync builds and creates the link error you see.

Comment 19 Matt McCutchen 2009-11-21 07:46:27 UTC
FWIW, I entered this upstream: https://bugzilla.samba.org/show_bug.cgi?id=6916

Comment 20 Josephine Tannhäuser 2010-07-13 07:49:51 UTC
Hey all..

Jan Zeleny is the new maintainer of rsync.
Jan, please take care of this bug and finally solve it.

Matt, you opened a bug, but it seems that nothing happened. Please ask upstream about the current state. Perhaps they have talked about it and the result of this talk is not noticed in the bug.

the most important of all questions is:
So what is Plan B if upstream won't fix this bug?

patching rsync to build against syslib will imho fail, because you will lose compatibility to rsync itself (for using compression compress). we can't remove it, because rsync is a standard package for each distribution. 

on the other hand. our guidelines are 100% correct in a philosophical way, but sometimes technical inpracticable. It is not our duty to educate upstream projects. We can only give hints, criticize parts of the code and give suggestions for improvement, but we can't set framework conditions for upstream projects. Imho we have to improve our guidelines.

Another thing is that our FESCO decided arbitrary, because they allow that some forked libs can stay (happened in wordpress bundled libs bug) and other forked libs must be replaced (happened in the rsync and zsync bugs). These decisions are not reasonable for me.

Comment 21 Matt McCutchen 2010-07-13 07:52:16 UTC
(In reply to comment #20)
> Matt, you opened a bug, but it seems that nothing happened. Please ask upstream
> about the current state.

Sorry, I didn't intend to accept responsibility for pushing this upstream.

Comment 22 Josephine Tannhäuser 2010-07-13 08:03:55 UTC
(In reply to comment #21)
> Sorry, I didn't intend to accept responsibility for pushing this upstream.    
If you won't accept the responsibility why you opened this bug in upstreams bugzilla? just open a bug and look what will happened won't help to solve the problem.

Comment 23 Matt McCutchen 2010-07-13 08:16:59 UTC
(In reply to comment #22)
> If you won't accept the responsibility why you opened this bug in upstreams
> bugzilla?

Because I thought the complaint was legitimate and it would be helpful to have it recorded in the upstream tracker, nothing more.  My personal interest in working to solve the problem is minimal at this time.

Comment 24 Jan Zeleny 2010-07-13 10:46:17 UTC
(In reply to comment #20)
> Jan Zeleny is the new maintainer of rsync.
> Jan, please take care of this bug and finally solve it.

> Matt, you opened a bug, but it seems that nothing happened. Please ask upstream
> about the current state. Perhaps they have talked about it and the result of
> this talk is not noticed in the bug.

It seemed that we might be able to achieve some progress, but now I think zlib and rsync upstreams didn't actually start to communicate to solve this issue. And to solve it, communication is exactly what is needed.

> the most important of all questions is:
> So what is Plan B if upstream won't fix this bug?

I'm not sure. I think some discussion is really the most important part and the only acceptable solution. We cannot create our version of either component to solve this in Fedora before we know what is right for both packages. And we cannot decide what's best for upstreams.

CC'ing Ivana, she is maintainer of zlib, so she might be interested in this bug as well

Comment 25 Toshio Kuratomi 2010-07-13 14:25:51 UTC
Note: I searched through the zlib mailing list archives and did not see that the rsync changes were ever submitted to zlib.  (You have to apply to the mailing list, then email Mark Adler to get approved.  Instructions on the mailman info page: http://www.zlib.net/mailman/listinfo/zlib-devel_madler.net)

Someone could have submitted it directly to madler or Jean-Loupe but that's not really the same thing.

I think someone needs to pursue rsync upstream to get and understand the patch to the bundled zlib and then submit that to the zlib upstream.

If that fails (or if it looks like a long process) we can also fork off the rsync bundled zlib into it's own library and have rsync and zsync use it in common.  (note that rsync and zsync modify the zlib library in different ways even though both are supposed to have the same output :-(  If we fork, we should get buy in from the zsync upstream and the Debian/other distros.  At least Debian has noted that this is a problem before so we probably wouldn't need to maintain this alone.

Comment 26 Fedora Admin XMLRPC Client 2010-10-05 11:59:54 UTC
This package has changed ownership in the Fedora Package Database.  Reassigning to the new owner of this component.

Comment 27 Simon 2010-12-21 10:52:12 UTC
Vojtěch, you are the owner of this packages for more than two months. what have you done so far to solve this violation of our guidelines?

Comment 28 Vojtech Vitek 2010-12-21 13:51:38 UTC
Simon, I was investigating on this mostly previous month, after that I had some other urgent work to do.

I had a talk with Jan Zeleny about the situation in rsync/zlib upstream and of course I read all relevant mailing lists.

I have also subscribed myself to zlib-devel mailing list today and I am waiting for Mark Adler's approval.
I am also going to talk with Ivana Varekova, fedora zlib maintainer, about our upcoming co-operation.

1) We should probably measure compression efficiency of rsync-zlib Z_INSERT_ONLY to pursue zlib upstream to merge it. This would also require implementing the receiver-side part in zlib inflate functions. This is now hacked in rsync source code only.

2) If the first step fails, we should pursue rsync upstream to get rid of this optimization and start using system's zlib. They were actually talking about this possibility earlier in the above mentioned mailing list.

Rsync is now missing lot of fixes that have been made between zlib 1.2.3 and current 1.2.5. See http://www.zlib.net/ChangeLog.txt

I will inform you about any progress asap.

Comment 29 Thom Carlin 2011-02-22 18:12:02 UTC
Hi Vojtech, what's the status?

Comment 30 Thom Carlin 2011-03-14 19:32:22 UTC
Any word?

Comment 31 Vojtech Vitek 2011-03-15 14:20:31 UTC
Hi Thom, please accept my apologies for my non-responsiveness.

Few weeks ago, I had another speech with Jan. He explained me the previous progress in communication between rsync and zlib upstreams - as far as what he could remember. Unfortunately he no longer has any copy of the old emails.

Finally, I was also talking with Ivana, the Fedora zlib maintainer. She explained to me the situation between zsync and zlib upstreams. She has also showed me some details from old communication between Colin Phipps (zsync) and Mark Adler (zlib).
When talking about the situation between rsync and zlib, she just referred to the mailing list mentioned above in the bz description.

To sum this up, it seems that no one from Fedora folks, who was involved in this issue before, knows any further details about any communication between upstreams themselves.

I am going to initiate a new communication between those and I will also ask the status from the past (if there is any). But before this step I want to come with some ideas / proposed solutions. That's why I spent some time on understanding the rsync rolling checksums algorithm, Z_INSERT_ONLY optimization in the zlib deflate.c, the rsync "hack" in token.c and of course, on lots of debugging.

I will focus on this issue more in depth and I will let you know about further steps.

Comment 32 Vojtech Vitek 2011-04-26 10:42:57 UTC
Update on this issue:

I have initiated a fresh conversation between Wayne Davison (rsync) and Mark Adler (zlib) off list /with few others CC'ed/. They are now discussing possible changes in zlib (deflate/inflate add dictionary context patches) to make rsync functionality inclusion possible.

I will let you know about further progress.

Comment 33 Vojtech Vitek 2011-05-19 08:14:48 UTC
We are now waiting for zlib upstream response to proposed patches..

Comment 34 Thom Carlin 2011-08-01 00:05:12 UTC
Any word from zlib upstream?

Comment 35 Vojtech Vitek 2011-08-08 11:33:02 UTC
> Subject: Re: Extending zlib to make unsent/shared data (ala rsync --compress) official?
> On Wed, May 18, 2011 at 6:41 AM, Vojtech Vitek <vvitek AT redhat DOT com>
> wrote:
> > was there any off list update on the topic, please?
>
> Sadly, I haven't heard anything back yet.
>
> ..wayne..

I've asked wayne again on July 11 - sadly with no response back yet..

I'm going to ping both upstreams again right now..

Comment 36 Vojtech Vitek 2011-08-23 14:09:24 UTC
Still waiting for zlib developers..
Unfortunately, they didn't comment the proposed patches yet, nor they responded to my email from Aug 8 2011.

Comment 37 Christopher Patrick 2011-10-01 04:56:10 UTC
zsync is in the rpmforge repo just go to http://pkgs.repoforge.org/rpmforge-release/ download the latest el6 for your architecture then install it and then you can install zsync via terminal command as root yum install zsync the list of packages they have is http://pkgs.repoforge.org/ and zsync is at http://pkgs.repoforge.org/zsync/ i can confirm it works for 64 bit fedora 16 beta rc4 fully updated

Comment 38 Simon 2011-10-01 05:50:19 UTC
@Christopher
The fact that rpmforge offers zsync wouldn't solve the problem that rsync and zlib ship their own and modified zlib. This isn't compatible with the fedora packaging guidelines...

Comment 39 Thom Carlin 2011-10-01 10:07:43 UTC
Hi Vojtech, please try pinging the zlib developers again.

Comment 40 Vojtech Vitek 2011-10-03 10:33:23 UTC
Zlib upstream pinged on Oct 03 2011.

Comment 41 Vojtech Vitek 2011-10-10 14:02:44 UTC
Zlib upstream responded on Oct 5 2011.
Now waiting for rsync upstream to answer several questions..

Comment 42 Vojtech Vitek 2011-10-29 20:40:23 UTC
Rsync upstream pinged on Oct 29 2011.

Comment 43 Vojtech Vitek 2011-11-22 10:20:56 UTC
The conversation have just rapidly moved on.. seems like /hopefully/ we could have some results soon.

Comment 44 Vojtech Vitek 2012-05-04 12:54:46 UTC
Technical update:

== zlib ==

Zlib now permits use of inflateSetDictionary() and deflateSetDictionary() in the middle of a stream to insert dictionary data. There is also deflateResetKeep() function to retain history data across deflates.  (There was already an inflateResetKeep().

     https://github.com/madler/zlib/commits/develop

Attached is a new version of reconcile.c that makes use of these capabilities, so that it does not need to maintain its own sliding history when compressing and decompressing.  reconcile.c does not use flush operations to end a deflate block, instead using the built-in self-termination of deflate streams.  So full flushes and the insertion and deletion of empty stored block bytes is not needed to bring the compressed data to a byte boundary.

Comment 45 Vojtech Vitek 2012-05-04 13:02:25 UTC
Rsync upstream has been working on --with-included-zlib=no configure option in their development branch. I'll test the changes and provide feedback asap.

Comment 46 Fedora Admin XMLRPC Client 2012-05-07 09:32:00 UTC
This package has changed ownership in the Fedora Package Database.  Reassigning to the new owner of this component.

Comment 47 Robert Scheck 2012-07-26 22:53:44 UTC
Whats the current status? What happened the last 2.5 months? Now, that yet
another Red Hat employee gave up rsync maintainance within these 3 years...

Comment 48 Michal Luscon 2012-09-18 11:22:36 UTC
I will discuss the current state with Vojtech and inform you.

Michal

Comment 49 Michal Luscon 2012-10-03 09:08:49 UTC
I am waiting for upstream response.

Comment 50 Fedora Update System 2012-10-18 10:43:31 UTC
rsync-3.0.9-4.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/rsync-3.0.9-4.fc18

Comment 51 Fedora Update System 2012-10-18 10:45:27 UTC
rsync-3.0.9-3.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/rsync-3.0.9-3.fc17

Comment 52 Fedora Update System 2012-10-18 15:27:06 UTC
Package rsync-3.0.9-4.fc18:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing rsync-3.0.9-4.fc18'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-16398/rsync-3.0.9-4.fc18
then log in and leave karma (feedback).

Comment 53 Kamil Páral 2012-10-22 11:19:35 UTC
Michal, how was this resolved, what about compatibility?

Specifically I would like to know:
1. Is the new rsync fully compatible with older versions of rsync?
2. Zsync package (see bug 490140) used to bundle rsync-specific zlib library to stay compatible with it. Can it now drop the dependency and still be compatible with older rsync versions, or does rsync-3.0.9-4 (or newer) needs to be deployed to the server for unbundled zsync to work?

Thanks!

Comment 54 Michal Luscon 2012-10-23 13:55:51 UTC
Hi Kamil,

I have not found any incompatibility during my internal testing(just basic tests). I admit that this update was probably too hurried and further reconsideration will be needed. Therefore, I am stopping the update process for now.

Comment 55 Michal Luscon 2013-08-07 11:11:41 UTC
The current rawhide version(3.1.0-1pre1) of Rsync has been built with configure option --included-zlib=no and should be fully compatible with previous rsync versions.


Note You need to log in before you can comment on or make changes to this bug.