[gentoo-dev] [PATCH] eclass/dune.eclass: fixes

[gentoo-dev] [PATCH] eclass/dune.eclass: fixes

[Maciej Barć]

bump to EAPI 8
drop support for EAPI 5
set DUNE_PKG_NAME to PN by default
move "Move docs to the appropriate place" block to dune-install
to make dune-install now handle a list of subpackages correctly

Signed-off-by: Maciej Barć <xgqt@gentoo.org>
---
 eclass/dune.eclass | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/eclass/dune.eclass b/eclass/dune.eclass
index 02a8a870e..a5de47aca 100644
--- a/eclass/dune.eclass
+++ b/eclass/dune.eclass
@@ -8,7 +8,7 @@
 # ML <ml@gentoo.org>
 # @AUTHOR:
 # Rafael Kitover <rkitover@gmail.com>
-# @SUPPORTED_EAPIS: 5 6 7
+# @SUPPORTED_EAPIS: 6 7 8
 # @BLURB: Provides functions for installing Dune packages.
 # @DESCRIPTION:
 # Provides dependencies on dDne and OCaml and default src_compile, src_test and
@@ -19,9 +19,10 @@
 # @DESCRIPTION:
 # Sets the actual Dune package name, if different from Gentoo package name.
 # Set before inheriting the eclass.
+: ${DUNE_PKG_NAME:-${PN}}
 
 case ${EAPI:-0} in
-	5|6|7) ;;
+	6|7|8) ;;
 	*) die "${ECLASS}: EAPI ${EAPI} not supported" ;;
 esac
 
@@ -32,7 +33,7 @@ EXPORT_FUNCTIONS src_compile src_test src_install
 
 RDEPEND=">=dev-lang/ocaml-4:=[ocamlopt?] dev-ml/dune:="
 case ${EAPI:-0} in
-	5|6)
+	6)
 		DEPEND="${RDEPEND} dev-ml/dune"
 		;;
 	*)
@@ -55,20 +56,22 @@ dune_src_test() {
 # Installs the dune packages given as arguments. For each "${pkg}" element in
 # that list, "${pkg}.install" must be readable from "${PWD}/_build/default"
 dune-install() {
+	local pkgs
+	if [[ -n "${@}" ]] ; then
+		pkgs="${@}"
+	else
+		pkgs=${DUNE_PKG_NAME}
+	fi
+
+	local myduneopts=(
+		--prefix="${ED%/}/usr"
+		--libdir="${D%/}$(ocamlc -where)"
+		--mandir="${ED%/}/usr/share/man"
+	)
 	local pkg
-	for pkg ; do
-		dune install \
-			--prefix="${ED%/}/usr" \
-			--libdir="${D%/}$(ocamlc -where)" \
-			--mandir="${ED%/}/usr/share/man" \
-			"${pkg}" || die
+	for pkg in "${pkgs}" ; do
+		dune install ${myduneopts[@]} ${pkg} || die
 	done
-}
-
-dune_src_install() {
-	local pkg="${1:-${DUNE_PKG_NAME:-${PN}}}"
-
-	dune-install "${pkg}"
 
 	# Move docs to the appropriate place.
 	if [ -d "${ED%/}/usr/doc/${pkg}" ] ; then
@@ -77,3 +80,7 @@ dune_src_install() {
 		rm -rf "${ED%/}/usr/doc" || die
 	fi
 }
+
+dune_src_install() {
+	dune-install ${1:-${DUNE_PKG_NAME}}
+}
-- 
2.32.0

Re: [gentoo-dev] [PATCH] eclass/dune.eclass: fixes

[Ulrich Mueller]

[-- Attachment #1: Type: text/plain, Size: 1182 bytes --]

>>>>> On Thu, 09 Dec 2021, Maciej Barć wrote:

>  dune-install() {
> +	local pkgs
> +	if [[ -n "${@}" ]] ; then
> +		pkgs="${@}"
> +	else
> +		pkgs=${DUNE_PKG_NAME}
> +	fi
> +
> +	local myduneopts=(
> +		--prefix="${ED%/}/usr"
> +		--libdir="${D%/}$(ocamlc -where)"
> +		--mandir="${ED%/}/usr/share/man"
> +	)
>  	local pkg
> -	for pkg ; do
> -		dune install \
> -			--prefix="${ED%/}/usr" \
> -			--libdir="${D%/}$(ocamlc -where)" \
> -			--mandir="${ED%/}/usr/share/man" \
> -			"${pkg}" || die
> +	for pkg in "${pkgs}" ; do
> +		dune install ${myduneopts[@]} ${pkg} || die
>  	done
>  }

Have you tested this?

IIUC, the space separated list of arguments is assigned to pkgs, with
a fallback to ${DUNE_PKG_NAME}. The 'for pkg in "${pkgs}"' loop isn't
actually a loop because ${pkgs} is inside double quotes, so it will be
executed only once with pkg being equal to pkgs.

The previous logic (simple 'for pkg' which will loop over $@) was
correct but of course without the fallback.

> +dune_src_install() {
> +	dune-install ${1:-${DUNE_PKG_NAME}}
> +}

Do you even need the fallback in dune_install() if you have it here too?

Ulrich

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]

Re: [gentoo-dev] [PATCH] eclass/dune.eclass: fixes

[Maciej Barć]

[-- Attachment #1.1.1: Type: text/plain, Size: 1896 bytes --]

 > IIUC, the space separated list of arguments is assigned to pkgs, with
 > a fallback to ${DUNE_PKG_NAME}. The 'for pkg in "${pkgs}"' loop isn't
 > actually a loop because ${pkgs} is inside double quotes, so it will be
 > executed only once with pkg being equal to pkgs.

Yes, you are right, I was wrong to test it with (modified) dev-ml/menhir 
lib which I believe does not need to call dune-install with multiple 
subpkgs now.

 > Do you even need the fallback in dune_install() if you have it here too?

Yes, to keep old compatibility.


On 12/9/21 08:38, Ulrich Mueller wrote:
>>>>>> On Thu, 09 Dec 2021, Maciej Barć wrote:
> 
>>   dune-install() {
>> +	local pkgs
>> +	if [[ -n "${@}" ]] ; then
>> +		pkgs="${@}"
>> +	else
>> +		pkgs=${DUNE_PKG_NAME}
>> +	fi
>> +
>> +	local myduneopts=(
>> +		--prefix="${ED%/}/usr"
>> +		--libdir="${D%/}$(ocamlc -where)"
>> +		--mandir="${ED%/}/usr/share/man"
>> +	)
>>   	local pkg
>> -	for pkg ; do
>> -		dune install \
>> -			--prefix="${ED%/}/usr" \
>> -			--libdir="${D%/}$(ocamlc -where)" \
>> -			--mandir="${ED%/}/usr/share/man" \
>> -			"${pkg}" || die
>> +	for pkg in "${pkgs}" ; do
>> +		dune install ${myduneopts[@]} ${pkg} || die
>>   	done
>>   }
> 
> Have you tested this?
> 
> IIUC, the space separated list of arguments is assigned to pkgs, with
> a fallback to ${DUNE_PKG_NAME}. The 'for pkg in "${pkgs}"' loop isn't
> actually a loop because ${pkgs} is inside double quotes, so it will be
> executed only once with pkg being equal to pkgs.
> 
> The previous logic (simple 'for pkg' which will loop over $@) was
> correct but of course without the fallback.
> 
>> +dune_src_install() {
>> +	dune-install ${1:-${DUNE_PKG_NAME}}
>> +}
> 
> Do you even need the fallback in dune_install() if you have it here too?
> 
> Ulrich
> 

-- 
Have a great day!

~ Maciej XGQT Barć

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 6295 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]