[Privoxy-devel] 0004-Enable-building-Privoxy-with-OpenSSL

[Lee]

On Thursday, August 10th, 2023 at 2:39 PM, Fabian Keil wrote:

> Lee wrote on 2023-08-09 at 07:37:00:
> 
> > On Wednesday, August 9th, 2023 at 5:19 AM, Fabian Keil wrote:
> > 
> > > The patch looks reasonable to me, but please split it
> > > in two using one for the changes to openssl.c and project.h
> > > and one for the change to windows/MYconfigure.
> > 
> > OK.. but are the changes to openssl.c and/or project.h necessary for anything besides Windows?
> > I don't mind splitting it into two patches - I'd just like to understand the reasoning.
> 
> 
> In general, I prefer small commits as I consider them
> easier to review and more convenient when bisecting in
> case of regressions.
> 
> Looking at your commit 96a448cdd70b I now think it could
> even have been split in two again, one commit with the
> changes to project.h which I assume where required
> to get this to build (in which case this could have
> been mentioned in the commit message) and another one
> with the change to openssl.c to prevent the "crashes".

Yes, that one was kind of border-line.  On the one hand, it was what I had to do to get openssl working..  and on the other fixing the crashes could have been a separate commit.  But privoxy not working for more than ~45 minutes at a time is hardly "working" in my book, so one patch seemed more correct.

> I also think the line "windows: Enable building Privoxy with OpenSSL"
> is a bit misleading as the commit does not actually change
> the TLS library (while the commit a87d1d6ab does).

"Enable building with" doesn't mean one is actually building with .. just that it's now possible to build with (and get a working program :).
Maybe I got the "Can I <something>? Yes, you're physically able to <something> but no, you may not." nonsense a few too many times.
 
> I think a line like "windows: Fix the build with OpenSSL"
> would be more appropriate as leading line for commit 96a448cdd.

at least in my mind "fix" means to repair something & I don't think privoxy ever worked with openssl on windows..

> In general I try to produce good commit messages myself

that's a talent.  I have lots of room for improvement there :(

> but of course I'm not perfect either and, for example,
> regret that I didn't add another paragraph or two for
> 53748ca8ca.
> 
> Probably the developer manual should contain a couple
> of example commit messages ...

It would help

> > > One issue I see with the patch is that using an external
> > > Apache2-licensed library like OpenSSL 3.x requires the
> > > Privoxy binary to be distributed under GPLv3 (or later)
> > > instead of GPLv2 or later so the win32_blurb[] in win32.c
> > > may need a modification before the next release so we don't
> > > mislead our users.
> > 
> > Didn't Mbed-TLS require changing the Privoxy license to GPLv2 or 3?
> 
> In e8baaf115bc I changed the license for pcrs.c to GPLv2
> or later after getting permission from Andreas.
> 
> This was necessary to be able to legally distribute a Privoxy
> binary that is linked to an mbedTLS version that is distributed
> under the Apache 2 license.
> 
> > I thought I was safe enabling privoxy to use openssl..
> 
> 
> I'm not saying that it can't be legally done but OpenSSL
> is not a system library on Windows so linking to it isn't
> covered by this "special exception" in the GPLv2:
> 
> | However, as a
> | special exception, the source code distributed need not include
> | anything that is normally distributed (in either source or binary
> | form) with the major components (compiler, kernel, and so on) of the
> | operating system on which the executable runs, unless that component
> | itself accompanies the executable.
> 
> Therefore, in my opinion, a Privoxy binary for Windows that is linked
> to and distributed with an OpenSSL library under the Apache 2 license
> has to be distributed under the GPLv3 or later instead of GPLv2 or
> later.

I was hoping that since I haven't distributed anything with OpenSSL that I'd be OK...
In any case, I'm fine with building Privoxy for distribution with whatever TLS library you prefer.

Lee