diff mbox series

[Openvpn-devel,1/5] MSVC: Disable LZ4

Message ID 20210321144627.1621-1-simon@rozman.si
State Rejected
Headers
Series [Openvpn-devel,1/5] MSVC: Disable LZ4 |

Commit Message

haixiao.yan.cn--- via Openvpn-devel March 21, 2021, 3:46 a.m. UTC 
  Commit 24596b25 ("build: Remove compat-lz4") removed lz4 compat layer,
but openvpn-build\msvc doesn't provide LZ4 library either.

Signed-off-by: Simon Rozman <simon@rozman.si>
---
 config-msvc.h | 1 -
 1 file changed, 1 deletion(-)

Comments

Arne Schwabe March 21, 2021, 5:25 a.m. UTC | #1 
Am 21.03.21 um 15:46 schrieb Simon Rozman via Openvpn-devel:
> Commit 24596b25 ("build: Remove compat-lz4") removed lz4 compat layer,
> but openvpn-build\msvc doesn't provide LZ4 library either.

We should either add lz4 to openvpn-build or change the default of lz4
to disabled in all variant. I don't like the idea that windows has a
different default than the other variants.

Arne
Gert Doering March 21, 2021, 5:50 a.m. UTC | #2 
Hi,

On Sun, Mar 21, 2021 at 03:46:23PM +0100, Simon Rozman via Openvpn-devel wrote:
> Commit 24596b25 ("build: Remove compat-lz4") removed lz4 compat layer,
> but openvpn-build\msvc doesn't provide LZ4 library either.

What would be needed to actually *build* with LZ4 on MSVC?  That is,
build it as prerequisite as LZO is built?

The idea wasn't to remove LZ4 from builds, just to remove the bundled
LZ4 "because all platforms have it now, so we do not need to maintain 
our own copy".  But it seems that was a bit shortsighted wrt windows
building...

gert
haixiao.yan.cn--- via Openvpn-devel March 21, 2021, 8:29 p.m. UTC | #3 
Hi,

> > Commit 24596b25 ("build: Remove compat-lz4") removed lz4 compat layer,
> > but openvpn-build\msvc doesn't provide LZ4 library either.
> 
> What would be needed to actually *build* with LZ4 on MSVC?  That is,
> build it as prerequisite as LZO is built?
> 
> The idea wasn't to remove LZ4 from builds, just to remove the bundled
> LZ4 "because all platforms have it now, so we do not need to maintain
> our own copy".  But it seems that was a bit shortsighted wrt windows
> building...

Thank you and Arne for explaining this. I should have followed the discussion on the OpenVPN meetings. Unfortunately, my workload doesn't allow me to follow on anything these days. So, I am not in condition to prepare LZ4 building in openvpn-build/msvc either.

I can live with LZ4 disabled in my sandbox only.

Shall we drop this patch for now?

Regards, Simon
Gert Doering March 21, 2021, 9:54 p.m. UTC | #4 
Hi,

On Mon, Mar 22, 2021 at 07:29:30AM +0000, Simon Rozman wrote:
> Thank you and Arne for explaining this. I should have followed
> the discussion on the OpenVPN meetings. Unfortunately, my workload
> doesn't allow me to follow on anything these days. So, I am not in
> condition to prepare LZ4 building in openvpn-build/msvc either.

Understood.  Thanks for sending these patches anyway :-)

> I can live with LZ4 disabled in my sandbox only.
> 
> Shall we drop this patch for now?

I have set it to "Rejected" (and the discussion is visible in patchwork).

"It should not be so hard", so I'll try to talk Heiko / Arne / Lev into 
it :-) - might actually be easier than getting generic/build right for
MinGW.

gert
Lev Stipakov March 21, 2021, 10:51 p.m. UTC | #5 
For 2.6, I think we should drop openvpn-build for Windows (VS)
building and switch to vcpkg for dependencies (openssl, lz4 etc) and
cmake as a project file (also supported by VS).
Gert Doering March 21, 2021, 10:55 p.m. UTC | #6 
Hi,

(I have changed the Subject: line to make clear that this is a bigger
topic now)

On Mon, Mar 22, 2021 at 11:51:46AM +0200, Lev Stipakov wrote:
> For 2.6, I think we should drop openvpn-build for Windows (VS)
> building and switch to vcpkg for dependencies (openssl, lz4 etc) and
> cmake as a project file (also supported by VS).

I have no opinion there whatsoever...  please send patches & trac 
documentation :-)

gert
Arne Schwabe March 22, 2021, 1:34 a.m. UTC | #7 
Am 22.03.21 um 10:55 schrieb Gert Doering:
> Hi,
> 
> (I have changed the Subject: line to make clear that this is a bigger
> topic now)
> 
> On Mon, Mar 22, 2021 at 11:51:46AM +0200, Lev Stipakov wrote:
>> For 2.6, I think we should drop openvpn-build for Windows (VS)
>> building and switch to vcpkg for dependencies (openssl, lz4 etc) and
>> cmake as a project file (also supported by VS).
> 
> I have no opinion there whatsoever...  please send patches & trac 
> documentation :-)

I have a very hacky CMakeLists.txt that works well enough to compile
with with Clion and MSVC. But the biggest hurdle is that I would need to
add pkcs11 to vckpg (and also clean up the cmakelists.txt) file. Also
that brings then two ways of building openvpn under Linux/mac/other
Unices, one with cmake and one with autoconf/automake and whole
discussion about build systems.

Arne
Samuli Seppänen March 23, 2021, 3:14 a.m. UTC | #8 
Il 22/03/21 11:55, Gert Doering ha scritto:
> Hi,
> 
> (I have changed the Subject: line to make clear that this is a bigger
> topic now)
> 
> On Mon, Mar 22, 2021 at 11:51:46AM +0200, Lev Stipakov wrote:
>> For 2.6, I think we should drop openvpn-build for Windows (VS)
>> building and switch to vcpkg for dependencies (openssl, lz4 etc) and
>> cmake as a project file (also supported by VS).

I'm not opposed as this sounds more standard than what we have now 
(opevnpn-build/msvc). We can still cross-compile using 
openvpn-build/generic if we wish.

This could potentially simplify the automated Windows MSI build process. 
Right now with Linux + Windows in the mix things are very confusing and 
fragile.

> I have no opinion there whatsoever...  please send patches & trac
> documentation :-)
> 
> gert
> 
> 
> 
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
diff mbox series

Patch

diff --git a/config-msvc.h b/config-msvc.h
index e430ca96..53d97902 100644
--- a/config-msvc.h
+++ b/config-msvc.h
@@ -9,7 +9,6 @@ 
 #define ENABLE_FRAGMENT 1
 #define ENABLE_HTTP_PROXY 1
 #define ENABLE_LZO 1
-#define ENABLE_LZ4 1
 #define ENABLE_MANAGEMENT 1
 #define ENABLE_MULTIHOME 1
 #define ENABLE_PKCS11 1