Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add options to don't use bundled libs #608

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dartmol203
Copy link

@dartmol203 dartmol203 commented Apr 29, 2024

I was maintaining the Debian package of rsync and to be able to remove the bundled libs zlib and popt, I had to add this changes as a patch.

I intend to remove those libs to make the Debian package follow some internal rules that recommends the usage of packages instead of importing the whole lib inside the package.

The proposed changes make it possible to repack rsync removing the said libraries without breaking anything.

This patch makes it possible to avoid using bundled libraries when configuring, by using --with-included-zlib=no

@dartmol203 dartmol203 changed the title add options to don't use bunbled libs add options to don't use bundled libs Apr 29, 2024
@dartmol203 dartmol203 force-pushed the master branch 2 times, most recently from eb14b7b to 01fe769 Compare April 29, 2024 17:02
This changes make it possible to config rsync while not using the included zlib and popt libs

Signed-off-by: Andre Correa <[email protected]>

Co-authored-by: Sergio Durigan Junior <[email protected]>
@tridge
Copy link
Member

tridge commented May 29, 2024

@dartmol203 the problem with not using the bundled zlib is rsync by default uses a modification to zlib that allows for better compression ratios by compressing data that is not sent over the link so that the compression history is better setup for the data.
If you use the non-bundled zlib this this will break compatibility.
Unfortunately I have noticed that distros like debian have gone ahead and made this change anyway, which leaves us with the messy situation of incompatible rsync versions.

@WayneD
Copy link
Member

WayneD commented May 29, 2024

I'm not understanding why the existing configure options aren't working for you. The change you made just seems to prevent the zlib/dummy file & popt/dummy files from being created. Those dummy files won't affect what libraries are configured into rsync. The existing --without-included-popt and --without-included-zlib options already work fine. Are you just trying to neaten up the build dir?

@WayneD
Copy link
Member

WayneD commented May 29, 2024

Some comments on the old zlib issue for Tridge's sake: the modern rsync avoids the implied compression data when using zlib compression in order to be more compatible with a wide range of zlib implementations. I did get support for the rsync kluge into the official zlib, but given that rsync compression seems to be somewhat fragile, the default is to avoid the old-style implied compress data (since it seems like the official library might not be 100% compatible with the included popt in some scenarios). In the most modern rsyncs, a transfer negotiates a compression algorithm from the list of "zstd lz4 zlibx zlib" where zlibx is the newer style of zlib compression. This means that the newest rsync supports both kinds of zlib compression, even with an external zlib, but defaults to the newer style of zlib when zstd or lz4 isn't available on the other side.

@samueloph
Copy link
Contributor

Hello everyone,

@tridge

If you use the non-bundled zlib this this will break compatibility.
Unfortunately I have noticed that distros like debian have gone ahead and made this change anyway, which leaves us with the messy situation of incompatible rsync versions.

It has been a few years since I've switched Debian's rsync to dynamically link to plain zlib, so my memory is not perfect, but I recall that I did extensive testing to ensure there was compatibility by making transfers between differently built rsyncs.

I also have a brief memory of Wayne writing somewhere that it should be fine to make the switch. It was probably what he mentioned in a later comment on this thread: #608 (comment)

All of this being said, I'm happy to take feedback on this and consider reverting the change if you think that's the right approach (again, considering Wayne's comment, sounds like we're fine).

@WayneD

I'm not understanding why the existing configure options aren't working for you.
Are you just trying to neaten up the build dir?

We are looking for support for the following scenario:

  1. Get the rsync tarball, or clones the repo.
  2. Remove the zlib and popt folders from rsync's sources
  3. Build rsync with the flags with_included_popt=no and with_included_zlib=no

The PR adds support for this, without any downsides attached.

Now, the reason for explicitly removing those folders is two-fold:

  1. It makes impossible for one to mistakenly compile with the bundled copy.
  2. It tidies up the distributed source code, less files, less licenses/copyright holders involved.

@WayneD
Copy link
Member

WayneD commented Nov 8, 2024

As I pointed out, the existing configure options work just fine without any patches. The only thing this diff seems to do is to not create "empty" dirs (that contain just a "dummy" file) in the build dir, which might be nice but isn't necessary. Those empty dirs don't contain any built files and are ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants