From nobody Tue Aug 08 21:16:28 2023 X-Original-To: dev-commits-ports-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4RL5bQ6ftXz4q5lJ; Tue, 8 Aug 2023 21:16:34 +0000 (UTC) (envelope-from diizzy@FreeBSD.org) Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4RL5bN4T2Nz4q3b; Tue, 8 Aug 2023 21:16:32 +0000 (UTC) (envelope-from diizzy@FreeBSD.org) Authentication-Results: mx1.freebsd.org; dkim=none; spf=softfail (mx1.freebsd.org: 217.70.183.195 is neither permitted nor denied by domain of diizzy@FreeBSD.org) smtp.mailfrom=diizzy@FreeBSD.org; dmarc=none Received: by mail.gandi.net (Postfix) with ESMTPA id 3A69760003; Tue, 8 Aug 2023 21:16:28 +0000 (UTC) List-Id: Commits to the main branch of the FreeBSD ports repository List-Archive: https://lists.freebsd.org/archives/dev-commits-ports-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-ports-main@freebsd.org X-BeenThere: dev-commits-ports-main@freebsd.org MIME-Version: 1.0 Date: Tue, 08 Aug 2023 23:16:28 +0200 From: Daniel Engberg To: Po-Chuan Hsieh Cc: ports-committers@freebsd.org, dev-commits-ports-all@freebsd.org, dev-commits-ports-main@freebsd.org Subject: Re: git: c25f0c013e88 - main - databases/libmemcached: Skip libcrypto.pc when using SSL from base system In-Reply-To: References: <202307091033.369AXqZ7055562@gitrepo.freebsd.org> Message-ID: X-Sender: diizzy@FreeBSD.org Content-Type: multipart/alternative; boundary="=_b670027eca2e6915f0cbea065c273be1" X-GND-Sasl: daniel.engberg@pyret.net X-Spamd-Result: default: False [0.23 / 15.00]; URI_COUNT_ODD(1.00)[25]; NEURAL_HAM_MEDIUM(-1.00)[-0.999]; NEURAL_SPAM_LONG(0.97)[0.972]; NEURAL_HAM_SHORT(-0.45)[-0.446]; RWL_MAILSPIKE_GOOD(-0.10)[217.70.183.195:from]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; RCVD_IN_DNSWL_LOW(-0.10)[217.70.183.195:from]; TO_DN_SOME(0.00)[]; R_DKIM_NA(0.00)[]; FROM_EQ_ENVFROM(0.00)[]; RCVD_COUNT_ONE(0.00)[1]; MIME_TRACE(0.00)[0:+,1:+,2:~]; RCVD_TLS_LAST(0.00)[]; ASN(0.00)[asn:29169, ipnet:217.70.176.0/20, country:FR]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MLMMJ_DEST(0.00)[dev-commits-ports-all@freebsd.org,dev-commits-ports-main@freebsd.org]; FREEFALL_USER(0.00)[diizzy]; RCPT_COUNT_THREE(0.00)[4]; DMARC_NA(0.00)[freebsd.org]; ARC_NA(0.00)[]; R_SPF_SOFTFAIL(0.00)[~all]; FROM_HAS_DN(0.00)[]; MID_RHS_MATCH_FROM(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[] X-Spamd-Bar: / X-Rspamd-Queue-Id: 4RL5bN4T2Nz4q3b --=_b670027eca2e6915f0cbea065c273be1 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset=UTF-8; format=flowed On 2023-08-06 15:48, Po-Chuan Hsieh wrote: > On Tue, Aug 1, 2023 at 11:08 AM Daniel Engberg > wrote: > >> On 2023-07-09 12:33, Po-Chuan Hsieh wrote: >>> The branch main has been updated by sunpoet: >>> >>> URL: >>> https://cgit.FreeBSD.org/ports/commit/?id=c25f0c013e88d84c620b2bb8c56158e9ca7f8bef >>> >>> commit c25f0c013e88d84c620b2bb8c56158e9ca7f8bef >>> Author: Po-Chuan Hsieh >>> AuthorDate: 2023-07-09 10:17:04 +0000 >>> Commit: Po-Chuan Hsieh >>> CommitDate: 2023-07-09 10:17:04 +0000 >>> >>> databases/libmemcached: Skip libcrypto.pc when using SSL from base >>> system >>> >>> - Bump PORTREVISION for package change >>> --- >>> databases/libmemcached/Makefile | 10 +++++++++- >>> databases/libmemcached/files/extra-patch-openssl | 11 +++++++++++ >>> 2 files changed, 20 insertions(+), 1 deletion(-) >>> >>> diff --git a/databases/libmemcached/Makefile >>> b/databases/libmemcached/Makefile >>> index e044f28499fd..71f031d3014f 100644 >>> --- a/databases/libmemcached/Makefile >>> +++ b/databases/libmemcached/Makefile >>> @@ -1,5 +1,6 @@ >>> PORTNAME= libmemcached >>> PORTVERSION= 1.1.4 >>> +PORTREVISION= 1 >>> CATEGORIES= databases >>> >>> MAINTAINER= sunpoet@FreeBSD.org >>> @@ -44,4 +45,11 @@ MURMUR_CMAKE_BOOL= ENABLE_HASH_MURMUR >>> SASL_CMAKE_BOOL= ENABLE_SASL >>> SASL_LIB_DEPENDS= libsasl2.so:security/cyrus-sasl2 >>> >>> -.include >>> +.include >>> + >>> +# Skip libcrypto.pc while using OpenSSL from base system on older >>> FreeBSD versions which does not skip this file >>> +.if ${SSL_DEFAULT} == base && >>> !exists(/usr/libdata/pkgconfig/libcrypto.pc) >>> +EXTRA_PATCHES+= ${PATCHDIR}/extra-patch-openssl >>> +.endif >>> + >>> +.include >>> diff --git a/databases/libmemcached/files/extra-patch-openssl >>> b/databases/libmemcached/files/extra-patch-openssl >>> new file mode 100644 >>> index 000000000000..bf65fa7f4ece >>> --- /dev/null >>> +++ b/databases/libmemcached/files/extra-patch-openssl >>> @@ -0,0 +1,11 @@ >>> +--- src/libhashkit/CMakeLists.txt.orig 2023-03-06 08:47:30 UTC >>> ++++ src/libhashkit/CMakeLists.txt >>> +@@ -45,7 +45,7 @@ if(ENABLE_OPENSSL_CRYPTO) >>> + if(OPENSSL_CRYPTO_LIBRARY) >>> + target_compile_definitions(libhashkit >>> PRIVATE HAVE_OPENSSL_CRYPTO) >>> + target_link_libraries(libhashkit PUBLIC >>> OpenSSL::Crypto) >>> +- pkgconfig_export(REQUIRES_PRIVATE >>> libcrypto) >>> ++ pkgconfig_export(REQUIRES_PRIVATE "") >>> + else() >>> + message(WARNING "Could not find >>> OpenSSL::Crypto") >>> + endif() >> >> Hi, >> >> Resending as I didn't get a reply last time, >> >> I few things I noticed compared to the PR I submitted about this port. >> >> -std=gnu++17 is set when unit tests are enabled so USES= >> compiler:c++11-lang is incorrect > > USES=compiler:c++11-lang for build is correct. > I know that c++17 is required for the tests. > But USES=compiler:c++11-lang is effectively the same as > USES=compiler:c++17-lang. > >> Is there a reason why default filenames aren't used for patches? > > The default filename comes from the path is OK. But it tells you > nothing but the path which is already in the diff header. > Naming the patch file by its purpose is better here. > >> Why are we using flex from base rather from ports? > > What's wrong with flex from base? If you notice any issue, I'm happy to > fix it. > >> That openssl patch can be improved by doing something like this, >> https://cgit.freebsd.org/ports/tree/archivers/libarchive/Makefile#n124 >> >> Best regards, >> Daniel Hi, While compiler:c++11-lang and compiler:c++17-lang is for now that doesn't mean it won't change in the future, if we can avoid unnecessary breakage I don't see why we shouldn't? I get your argument however I'm not sure if we should deviate from Porters Handbook unless necessary, simply because of overall (treewide) maintenance instead of having ports with their own little quirks left and right which isn't going to benefit the project in the long run. One major downside using libraries from base is that you may end up with different versions between support major versions of FreeBSD which may cause inconsistencies. For example, 12 is still on 2.5.37 while 13+ is on 2.6.4 https://cgit.freebsd.org/src/log/contrib/flex?h=stable/12 https://cgit.freebsd.org/src/log/contrib/flex?h=stable/13 Best regards, Daniel Links: ------ [1] http://bsd.port.mk [2] http://bsd.port.pre.mk [3] http://bsd.port.post.mk --=_b670027eca2e6915f0cbea065c273be1 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=UTF-8

On 2023-08-06 15:48, Po-Chuan Hsieh wrote:

On Tue, Aug 1, 2023 at 11:08=E2=80=AFAM Daniel Engberg <= ;diizzy@freebsd.or= g> wrote:
On 2023-07-09 12:33, Po-Ch= uan Hsieh wrote:
> The branch main has been updated by sunpoet:
>
> URL:
> https://cgit.FreeBSD.org/ports/commit/?id=3Dc25f0= c013e88d84c620b2bb8c56158e9ca7f8bef
>
> commit c25f0c0= 13e88d84c620b2bb8c56158e9ca7f8bef
> Author:     Po-C= huan Hsieh <sunpoet@FreeBSD.org>
> AuthorDate: 2023-07-09 10:= 17:04 +0000
> Commit:     Po-Chuan Hsieh <sunpoet= @FreeBSD.org>
> CommitDate: 2023-07-09 10:17:04 +0000
> =
>     databases/libmemcached: Skip libcrypto.pc whe= n using SSL from base
> system
>
>    &= nbsp;- Bump PORTREVISION for package change
> ---
>  d= atabases/libmemcached/Makefile            &nb= sp;     | 10 +++++++++-
>  databases/libmemcached/fi= les/extra-patch-openssl | 11 +++++++++++
>  2 files changed, 2= 0 insertions(+), 1 deletion(-)
>
> diff --git a/databases/= libmemcached/Makefile
> b/databases/libmemcached/Makefile
>= ; index e044f28499fd..71f031d3014f 100644
> --- a/databases/libmemc= ached/Makefile
> +++ b/databases/libmemcached/Makefile
> @@= -1,5 +1,6 @@
>  PORTNAME=3D    libmemcached
&g= t;  PORTVERSION=3D 1.1.4
> +PORTREVISION=3D     = ;   1
>  CATEGORIES=3D  databases
>
&g= t;  MAINTAINER=3D  sunpoet@FreeBSD.org
> @@ -44,4 +45,11 = @@ MURMUR_CMAKE_BOOL=3D ENABLE_HASH_MURMUR
>  SASL_CMAKE_BOOL= =3D     ENABLE_SASL
>  SASL_LIB_DEPENDS=3D = ;   libsasl2.so:security/cyrus-sasl2
>
> -.include &l= t;bsd.port.mk>
> +.include <bsd.port.pre.mk&g= t;
> +
> +# Skip libcrypto.pc while using OpenSSL from base= system on older
> FreeBSD versions which does not skip this file> +.if ${SSL_DEFAULT} =3D=3D base &&
> !exists(/usr= /libdata/pkgconfig/libcrypto.pc)
> +EXTRA_PATCHES+=3D    =   ${PATCHDIR}/extra-patch-openssl
> +.endif
> +
&= gt; +.include <bsd.port.post.mk>
> diff --git a/da= tabases/libmemcached/files/extra-patch-openssl
> b/databases/libmem= cached/files/extra-patch-openssl
> new file mode 100644
> i= ndex 000000000000..bf65fa7f4ece
> --- /dev/null
> +++ b/dat= abases/libmemcached/files/extra-patch-openssl
> @@ -0,0 +1,11 @@> +--- src/libhashkit/CMakeLists.txt.orig       2= 023-03-06 08:47:30 UTC
> ++++ src/libhashkit/CMakeLists.txt
&g= t; +@@ -45,7 +45,7 @@ if(ENABLE_OPENSSL_CRYPTO)
> +    &n= bsp;            if(OPENSSL_CRYPTO_LIBRARY)> +                  &nb= sp;      target_compile_definitions(libhashkit
> PRI= VATE HAVE_OPENSSL_CRYPTO)
> +          &nb= sp;              target_link_libraries(l= ibhashkit PUBLIC
> OpenSSL::Crypto)
> +-     = ;                   pkgconfig_= export(REQUIRES_PRIVATE libcrypto)
> ++        =                 pkgconfig_export(RE= QUIRES_PRIVATE "")
> +            &nb= sp;    else()
> +           = ;              message(WARNING "Could no= t find
> OpenSSL::Crypto")
> +        =          endif()

Hi,

Resendi= ng as I didn't get a reply last time,

I few things I noticed com= pared to the PR I submitted about this port.

-std=3Dgnu++17 is s= et when unit tests are enabled so USES=3D
compiler:c++11-lang is inco= rrect
 
USES=3Dcompiler:c++11-lang for build is correct.
I know that c++17 is = required for the tests.
But USES=3Dcompiler:c++11-lang is effectively the same as USES=3Dcompi= ler:c++17-lang.
 
Is there a reason why defa= ult filenames aren't used for patches?
 
The default filename comes from the path is OK.
But it tells you nothing but the path which is already in the diff header.
Naming the patch file by its purpose is better here.
 
Why are we using flex from= base rather from ports?
 
What's wrong with flex from base? If you notice any issue, I'm happy t= o fix it.
 
That openssl patch can be = improved by doing something like this,
https://cgit.freebsd.org/ports/tree/archivers/liba= rchive/Makefile#n124

Best regards,
Daniel

Hi,

While compiler:c++11-lang and compiler:c++17-lang is for now that doesn'= t mean it won't change in the future, if we can avoid unnecessary breakage = I don't see why we shouldn't?

I get your argument however I'm not sure if we should deviate from Porte= rs Handbook unless necessary, simply because of overall (treewide) maintena= nce instead of having ports with their own little quirks left and right whi= ch isn't going to benefit the project in the long run.

One major downside using libraries from base is that you may end up with= different versions between support major versions of FreeBSD which may cau= se inconsistencies.

For example, 12 is still on 2.5.37 while 13+ is on 2.6.4

= https://cgit.freebsd.org/src/log/contrib/flex?h=3Dstable/12

= https://cgit.freebsd.org/src/log/contrib/flex?h=3Dstable/13

Best regards,

Daniel

--=_b670027eca2e6915f0cbea065c273be1--