[gentoo-dev] [PATCH] fortran-2.eclass: support EAPI 7

[gentoo-dev] [PATCH] fortran-2.eclass: support EAPI 7

[Andrew Savchenko]

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

Hi all!

The only blocker for EAPI 7 update is eutils inheritance, but it
seems to be not used within the current eclass code, probably a
remnant from older days. So it is removed.

Looks like no other EAPI 7 specific changes needed.

diff --git a/eclass/fortran-2.eclass b/eclass/fortran-2.eclass
index 820cbbcb49bd..1baf776e368a 100644
--- a/eclass/fortran-2.eclass
+++ b/eclass/fortran-2.eclass
@@ -1,4 +1,4 @@
-# Copyright 1999-2017 Gentoo Foundation
+# Copyright 1999-2018 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 # @ECLASS: fortran-2.eclass
@@ -8,7 +8,7 @@
 # @AUTHOR:
 # Author Justin Lecher <jlec@gentoo.org>
 # Test functions provided by Sebastien Fabbro and Kacper Kowalik
-# @SUPPORTED_EAPIS: 4 5 6
+# @SUPPORTED_EAPIS: 4 5 6 7
 # @BLURB: Simplify fortran compiler management
 # @DESCRIPTION:
 # If you need a fortran compiler, then you should be inheriting
this eclass. @@ -27,10 +27,10 @@
 #
 # FORTRAN_NEED_OPENMP=1
 
-inherit eutils toolchain-funcs
+inherit toolchain-funcs
 
 case ${EAPI:-0} in
-	4|5|6) EXPORT_FUNCTIONS pkg_setup ;;
+	4|5|6|7) EXPORT_FUNCTIONS pkg_setup ;;
 	*) die "EAPI=${EAPI} is not supported" ;;
 esac
 

Best regards,
Andrew Savchenko

[-- Attachment #2: Type: application/pgp-signature, Size: 833 bytes --]

Re: [gentoo-dev] [PATCH] fortran-2.eclass: support EAPI 7

[Michał Górny]

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

On Sun, 2018-10-28 at 01:38 +0300, Andrew Savchenko wrote:
> Hi all!
> 
> The only blocker for EAPI 7 update is eutils inheritance, but it
> seems to be not used within the current eclass code, probably a
> remnant from older days. So it is removed.
> 
> Looks like no other EAPI 7 specific changes needed.
> 

Please use -U99999 to include more context to the patches.

-- 
Best regards,
Michał Górny

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

Re: [gentoo-dev] [PATCH] fortran-2.eclass: support EAPI 7

[Andrew Savchenko]

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

On Sun, 28 Oct 2018 19:29:28 +0100 Michał Górny wrote:
> On Sun, 2018-10-28 at 01:38 +0300, Andrew Savchenko wrote:
> > Hi all!
> > 
> > The only blocker for EAPI 7 update is eutils inheritance, but it
> > seems to be not used within the current eclass code, probably a
> > remnant from older days. So it is removed.
> > 
> > Looks like no other EAPI 7 specific changes needed.
> > 
> 
> Please use -U99999 to include more context to the patches.
 
diff --git a/eclass/fortran-2.eclass b/eclass/fortran-2.eclass
index 820cbbcb49bd..1baf776e368a 100644
--- a/eclass/fortran-2.eclass
+++ b/eclass/fortran-2.eclass
@@ -1,285 +1,285 @@
-# Copyright 1999-2017 Gentoo Foundation
+# Copyright 1999-2018 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 # @ECLASS: fortran-2.eclass
 # @MAINTAINER:
 # jlec@gentoo.org
 # sci@gentoo.org
 # @AUTHOR:
 # Author Justin Lecher <jlec@gentoo.org>
 # Test functions provided by Sebastien Fabbro and Kacper Kowalik
-# @SUPPORTED_EAPIS: 4 5 6
+# @SUPPORTED_EAPIS: 4 5 6 7
 # @BLURB: Simplify fortran compiler management
 # @DESCRIPTION:
 # If you need a fortran compiler, then you should be inheriting this eclass.
 # In case you only need optional support, please export FORTRAN_NEEDED before
 # inheriting the eclass.
 #
 # The eclass tests for working fortran compilers
 # and exports the variables FC and F77.
 # Optionally, it checks for extended capabilities based on
 # the variable options selected in the ebuild
 # The only phase function exported is fortran-2_pkg_setup.
 # @EXAMPLE:
 # FORTRAN_NEEDED="lapack fortran"
 #
 # inherit fortran-2
 #
 # FORTRAN_NEED_OPENMP=1
 
-inherit eutils toolchain-funcs
+inherit toolchain-funcs
 
 case ${EAPI:-0} in
-	4|5|6) EXPORT_FUNCTIONS pkg_setup ;;
+	4|5|6|7) EXPORT_FUNCTIONS pkg_setup ;;
 	*) die "EAPI=${EAPI} is not supported" ;;
 esac
 
 if [[ ! ${_FORTRAN_2_CLASS} ]]; then
 
 # @ECLASS-VARIABLE: FORTRAN_NEED_OPENMP
 # @DESCRIPTION:
 # Set to "1" in order to automatically have the eclass abort if the fortran
 # compiler lacks openmp support.
 : ${FORTRAN_NEED_OPENMP:=0}
 
 # @ECLASS-VARIABLE: FORTRAN_STANDARD
 # @DESCRIPTION:
 # Set this, if a special dialect needs to be supported.
 # Generally not needed as default is sufficient.
 #
 # Valid settings are any combination of: 77 90 95 2003
 : ${FORTRAN_STANDARD:=77}
 
 # @ECLASS-VARIABLE: FORTRAN_NEEDED
 # @DESCRIPTION:
 # If your package has an optional fortran support, set this variable
 # to the space separated list of USE triggering the fortran
 # dependency.
 #
 # e.g. FORTRAN_NEEDED=lapack would result in
 #
 # DEPEND="lapack? ( virtual/fortran )"
 #
 # If unset, we always depend on virtual/fortran.
 : ${FORTRAN_NEEDED:=always}
 
 for _f_use in ${FORTRAN_NEEDED}; do
 	case ${_f_use} in
 		always)
 			DEPEND+=" virtual/fortran"
 			RDEPEND+=" virtual/fortran"
 			break
 			;;
 		no)
 			break
 			;;
 		test)
 			DEPEND+=" ${_f_use}? ( virtual/fortran )"
 			;;
 		*)
 			DEPEND+=" ${_f_use}? ( virtual/fortran )"
 			RDEPEND+=" ${_f_use}? ( virtual/fortran )"
 			;;
 	esac
 done
 unset _f_use
 
 # @FUNCTION: fortran_int64_abi_fflags
 # @DESCRIPTION:
 # Return the Fortran compiler flag to enable 64 bit integers for
 # array indices
 # @CODE
 fortran_int64_abi_fflags() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	_FC=$(tc-getFC)
 	if [[ ${_FC} == *gfortran* ]]; then
 		echo "-fdefault-integer-8"
 	elif [[ ${_FC} == ifort ]]; then
 		echo "-integer-size 64"
 	else
 		die "Compiler flag for 64bit interger for ${_FC} unknown"
 	fi
 }
 
 # @FUNCTION: _fortran_write_testsuite
 # @INTERNAL
 # @DESCRIPTION:
 # writes fortran test code
 _fortran_write_testsuite() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	local filebase=${T}/test-fortran
 
 	# f77 code
 	cat <<- EOF > "${filebase}.f"
 	       end
 	EOF
 
 	# f90/95 code
 	cat <<- EOF > "${filebase}.f90"
 	end
 	EOF
 
 	# f2003 code
 	cat <<- EOF > "${filebase}.f03"
 	       procedure(), pointer :: p
 	       end
 	EOF
 }
 
 # @FUNCTION: _fortran_compile_test
 # @USAGE: <compiler> [dialect]
 # @INTERNAL
 # @DESCRIPTION:
 # Takes fortran compiler as first argument and dialect as second.
 # Checks whether the passed fortran compiler speaks the fortran dialect
 _fortran_compile_test() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	local filebase=${T}/test-fortran
 	local fcomp=${1}
 	local fdia=${2}
 	local fcode=${filebase}.f${fdia}
 	local ret
 
 	[[ $# -lt 1 ]] && \
 		die "_fortran_compile_test() needs at least one argument"
 
 	[[ -f ${fcode} ]] || _fortran_write_testsuite
 
 	${fcomp} "${fcode}" -o "${fcode}.x" \
 		>> "${T}"/_fortran_compile_test.log 2>&1
 	ret=$?
 
 	rm -f "${fcode}.x"
 	return ${ret}
 }
 
 # @FUNCTION: _fortran-has-openmp
 # @RETURN: return code of the compiler
 # @INTERNAL
 # @DESCRIPTION:
 # See if the fortran supports OpenMP.
 _fortran-has-openmp() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	local flag
 	local filebase=${T}/test-fc-openmp
 	local fcode=${filebase}.f
 	local ret
 	local _fc=$(tc-getFC)
 
 	cat <<- EOF > "${fcode}"
 	       call omp_get_num_threads
 	       end
 	EOF
 
 	for flag in -fopenmp -xopenmp -openmp -mp -omp -qsmp=omp; do
 		${_fc} ${flag} "${fcode}" -o "${fcode}.x" \
 			&>> "${T}"/_fortran_compile_test.log
 		ret=$?
 		(( ${ret} )) || break
 	done
 
 	rm -f "${fcode}.x"
 	return ${ret}
 }
 
 # @FUNCTION: _fortran_die_msg
 # @INTERNAL
 # @DESCRIPTION:
 # Detailed description how to handle fortran support
 _fortran_die_msg() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	echo
 	eerror "Please install currently selected gcc version with USE=fortran."
 	eerror "If you intend to use a different compiler then gfortran, please"
 	eerror "set FC variable accordingly and take care that the necessary"
 	eerror "fortran dialects are supported."
 	echo
 	die "Currently no working fortran compiler is available (see ${T}/_fortran_compile_test.log for information)"
 }
 
 # @FUNCTION: _fortran_test_function
 # @INTERNAL
 # @DESCRIPTION:
 # Internal test function for working fortran compiler.
 # It is called in fortran-2_pkg_setup.
 _fortran_test_function() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	local dialect
 
 	: ${F77:=$(tc-getFC)}
 
 	: ${FORTRAN_STANDARD:=77}
 	for dialect in ${FORTRAN_STANDARD}; do
 		case ${dialect} in
 			77) _fortran_compile_test $(tc-getF77) || \
 				_fortran_die_msg ;;
 			90|95) _fortran_compile_test $(tc-getFC) 90 || \
 				_fortran_die_msg ;;
 			2003) _fortran_compile_test $(tc-getFC) 03 || \
 				_fortran_die_msg ;;
 			2008) die "Future" ;;
 			*) die "${dialect} is not a Fortran dialect." ;;
 		esac
 	done
 
 	tc-export F77 FC
 	einfo "Using following Fortran compiler:"
 	einfo "  F77: ${F77}"
 	einfo "  FC:  ${FC}"
 
 	if [[ ${FORTRAN_NEED_OPENMP} == 1 ]]; then
 		if _fortran-has-openmp; then
 			einfo "${FC} has OPENMP support"
 		else
 			die "Please install current gcc with USE=openmp or set the FC variable to a compiler that supports OpenMP"
 		fi
 	fi
 }
 
 # @FUNCTION: _fortran-2_pkg_setup
 # @INTERNAL
 # @DESCRIPTION:
 # _The_ fortran-2_pkg_setup() code
 _fortran-2_pkg_setup() {
 	for _f_use in ${FORTRAN_NEEDED}; do
 	case ${_f_use} in
 		always)
 			_fortran_test_function && break
 			;;
 		no)
 			einfo "Forcing fortran support off"
 			break
 			;;
 		*)
 			if use ${_f_use}; then
 				_fortran_test_function && break
 			else
 				unset FC
 				unset F77
 			fi
 			;;
 		esac
 	done
 }
 
 
 # @FUNCTION: fortran-2_pkg_setup
 # @DESCRIPTION:
 # Setup functionality,
 # checks for a valid fortran compiler and optionally for its openmp support.
 fortran-2_pkg_setup() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	if [[ ${MERGE_TYPE} != binary ]]; then
 		_fortran-2_pkg_setup
 	fi
 }
 
 _FORTRAN_2_ECLASS=1
 fi


Best regards,
Andrew Savchenko

[-- Attachment #2: Type: application/pgp-signature, Size: 833 bytes --]

Re: [gentoo-dev] [PATCH] fortran-2.eclass: support EAPI 7

[Andreas K. Huettel]

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

> 
> -inherit eutils toolchain-funcs
> +inherit toolchain-funcs
> 
>  case ${EAPI:-0} in
> -	4|5|6) EXPORT_FUNCTIONS pkg_setup ;;
> +	4|5|6|7) EXPORT_FUNCTIONS pkg_setup ;;
>  	*) die "EAPI=${EAPI} is not supported" ;;
>  esac
> 

^ The disadvantage of this is that eutils inheritance then suddenly disappears 
for all ebuilds. And you don't know who assumed implicitly somewhere that 
inheriting fortran-2 makes epatch available...

So how about still inheriting eutils for EAPI 4,5,6 and only dropping it for 
EAPI 7 ?!


-- 
Andreas K. Hüttel
dilfridge@gentoo.org
Gentoo Linux developer
(council, toolchain, perl, libreoffice, comrel)

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 981 bytes --]

Re: [gentoo-dev] [PATCH] fortran-2.eclass: support EAPI 7

[Michał Górny]

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

On Mon, 2018-10-29 at 03:57 +0300, Andrew Savchenko wrote:
> On Sun, 28 Oct 2018 19:29:28 +0100 Michał Górny wrote:
> > On Sun, 2018-10-28 at 01:38 +0300, Andrew Savchenko wrote:
> > > Hi all!
> > > 
> > > The only blocker for EAPI 7 update is eutils inheritance, but it
> > > seems to be not used within the current eclass code, probably a
> > > remnant from older days. So it is removed.
> > > 
> > > Looks like no other EAPI 7 specific changes needed.
> > > 
> > 
> > Please use -U99999 to include more context to the patches.

I'm going to include a few 'easy cleanup' comments since EAPI 7
is a good opportunity to improve the eclass.  I'm going to skip horribly
bad design decisions since I suppose nobody cares.

> diff --git a/eclass/fortran-2.eclass b/eclass/fortran-2.eclass
> index 820cbbcb49bd..1baf776e368a 100644
> --- a/eclass/fortran-2.eclass
> +++ b/eclass/fortran-2.eclass
> @@ -1,285 +1,285 @@
> -# Copyright 1999-2017 Gentoo Foundation
> +# Copyright 1999-2018 Gentoo Authors
>  # Distributed under the terms of the GNU General Public License v2
>  
>  # @ECLASS: fortran-2.eclass
>  # @MAINTAINER:
>  # jlec@gentoo.org
>  # sci@gentoo.org
>  # @AUTHOR:
>  # Author Justin Lecher <jlec@gentoo.org>
>  # Test functions provided by Sebastien Fabbro and Kacper Kowalik
> -# @SUPPORTED_EAPIS: 4 5 6
> +# @SUPPORTED_EAPIS: 4 5 6 7
>  # @BLURB: Simplify fortran compiler management
>  # @DESCRIPTION:
>  # If you need a fortran compiler, then you should be inheriting this eclass.
>  # In case you only need optional support, please export FORTRAN_NEEDED before
>  # inheriting the eclass.
>  #
>  # The eclass tests for working fortran compilers
>  # and exports the variables FC and F77.
>  # Optionally, it checks for extended capabilities based on
>  # the variable options selected in the ebuild
>  # The only phase function exported is fortran-2_pkg_setup.
>  # @EXAMPLE:
>  # FORTRAN_NEEDED="lapack fortran"
>  #
>  # inherit fortran-2
>  #
>  # FORTRAN_NEED_OPENMP=1
>  
> -inherit eutils toolchain-funcs
> +inherit toolchain-funcs
>  
>  case ${EAPI:-0} in
> -	4|5|6) EXPORT_FUNCTIONS pkg_setup ;;
> +	4|5|6|7) EXPORT_FUNCTIONS pkg_setup ;;
>  	*) die "EAPI=${EAPI} is not supported" ;;
>  esac
>  
>  if [[ ! ${_FORTRAN_2_CLASS} ]]; then
>  
>  # @ECLASS-VARIABLE: FORTRAN_NEED_OPENMP
>  # @DESCRIPTION:
>  # Set to "1" in order to automatically have the eclass abort if the fortran
>  # compiler lacks openmp support.
>  : ${FORTRAN_NEED_OPENMP:=0}
>  
>  # @ECLASS-VARIABLE: FORTRAN_STANDARD
>  # @DESCRIPTION:
>  # Set this, if a special dialect needs to be supported.
>  # Generally not needed as default is sufficient.
>  #
>  # Valid settings are any combination of: 77 90 95 2003
>  : ${FORTRAN_STANDARD:=77}
>  
>  # @ECLASS-VARIABLE: FORTRAN_NEEDED
>  # @DESCRIPTION:
>  # If your package has an optional fortran support, set this variable
>  # to the space separated list of USE triggering the fortran
>  # dependency.
>  #
>  # e.g. FORTRAN_NEEDED=lapack would result in
>  #
>  # DEPEND="lapack? ( virtual/fortran )"
>  #
>  # If unset, we always depend on virtual/fortran.
>  : ${FORTRAN_NEEDED:=always}
>  
>  for _f_use in ${FORTRAN_NEEDED}; do
>  	case ${_f_use} in
>  		always)
>  			DEPEND+=" virtual/fortran"
>  			RDEPEND+=" virtual/fortran"
>  			break
>  			;;
>  		no)
>  			break
>  			;;
>  		test)
>  			DEPEND+=" ${_f_use}? ( virtual/fortran )"
>  			;;
>  		*)
>  			DEPEND+=" ${_f_use}? ( virtual/fortran )"
>  			RDEPEND+=" ${_f_use}? ( virtual/fortran )"
>  			;;
>  	esac
>  done
>  unset _f_use
>  
>  # @FUNCTION: fortran_int64_abi_fflags
>  # @DESCRIPTION:
>  # Return the Fortran compiler flag to enable 64 bit integers for
>  # array indices
>  # @CODE
>  fortran_int64_abi_fflags() {
>  	debug-print-function ${FUNCNAME} "${@}"
>  
>  	_FC=$(tc-getFC)

Any reason not to make it local?

>  	if [[ ${_FC} == *gfortran* ]]; then
>  		echo "-fdefault-integer-8"
>  	elif [[ ${_FC} == ifort ]]; then
>  		echo "-integer-size 64"
>  	else
>  		die "Compiler flag for 64bit interger for ${_FC} unknown"
>  	fi
>  }
>  
>  # @FUNCTION: _fortran_write_testsuite
>  # @INTERNAL
>  # @DESCRIPTION:
>  # writes fortran test code
>  _fortran_write_testsuite() {
>  	debug-print-function ${FUNCNAME} "${@}"
>  
>  	local filebase=${T}/test-fortran
>  
>  	# f77 code
>  	cat <<- EOF > "${filebase}.f"

|| die

>  	       end
>  	EOF
>  
>  	# f90/95 code
>  	cat <<- EOF > "${filebase}.f90"

|| die

>  	end

Also, why different indentation?

>  	EOF
>  
>  	# f2003 code
>  	cat <<- EOF > "${filebase}.f03"

|| die

>  	       procedure(), pointer :: p
>  	       end
>  	EOF
>  }
>  
>  # @FUNCTION: _fortran_compile_test
>  # @USAGE: <compiler> [dialect]
>  # @INTERNAL
>  # @DESCRIPTION:
>  # Takes fortran compiler as first argument and dialect as second.
>  # Checks whether the passed fortran compiler speaks the fortran dialect
>  _fortran_compile_test() {
>  	debug-print-function ${FUNCNAME} "${@}"
>  
>  	local filebase=${T}/test-fortran
>  	local fcomp=${1}
>  	local fdia=${2}
>  	local fcode=${filebase}.f${fdia}
>  	local ret
>  
>  	[[ $# -lt 1 ]] && \
>  		die "_fortran_compile_test() needs at least one argument"
>  
>  	[[ -f ${fcode} ]] || _fortran_write_testsuite
>  
>  	${fcomp} "${fcode}" -o "${fcode}.x" \
>  		>> "${T}"/_fortran_compile_test.log 2>&1
>  	ret=$?
>  
>  	rm -f "${fcode}.x"
>  	return ${ret}
>  }
>  
>  # @FUNCTION: _fortran-has-openmp
>  # @RETURN: return code of the compiler
>  # @INTERNAL
>  # @DESCRIPTION:
>  # See if the fortran supports OpenMP.
>  _fortran-has-openmp() {
>  	debug-print-function ${FUNCNAME} "${@}"
>  
>  	local flag
>  	local filebase=${T}/test-fc-openmp
>  	local fcode=${filebase}.f
>  	local ret
>  	local _fc=$(tc-getFC)
>  
>  	cat <<- EOF > "${fcode}"

|| die

>  	       call omp_get_num_threads
>  	       end
>  	EOF
>  
>  	for flag in -fopenmp -xopenmp -openmp -mp -omp -qsmp=omp; do
>  		${_fc} ${flag} "${fcode}" -o "${fcode}.x" \
>  			&>> "${T}"/_fortran_compile_test.log
>  		ret=$?
>  		(( ${ret} )) || break

This (( ... )) is unreadable at best; please replace it with clear
condition.

>  	done
>  
>  	rm -f "${fcode}.x"
>  	return ${ret}
>  }
>  
>  # @FUNCTION: _fortran_die_msg
>  # @INTERNAL
>  # @DESCRIPTION:
>  # Detailed description how to handle fortran support
>  _fortran_die_msg() {
>  	debug-print-function ${FUNCNAME} "${@}"
>  
>  	echo

Don't mix echo with eerror.

>  	eerror "Please install currently selected gcc version with USE=fortran."
>  	eerror "If you intend to use a different compiler then gfortran, please"
>  	eerror "set FC variable accordingly and take care that the necessary"
>  	eerror "fortran dialects are supported."
>  	echo
>  	die "Currently no working fortran compiler is available (see ${T}/_fortran_compile_test.log for information)"
>  }
>  
>  # @FUNCTION: _fortran_test_function
>  # @INTERNAL
>  # @DESCRIPTION:
>  # Internal test function for working fortran compiler.
>  # It is called in fortran-2_pkg_setup.
>  _fortran_test_function() {
>  	debug-print-function ${FUNCNAME} "${@}"
>  
>  	local dialect
>  
>  	: ${F77:=$(tc-getFC)}
>  
>  	: ${FORTRAN_STANDARD:=77}
>  	for dialect in ${FORTRAN_STANDARD}; do
>  		case ${dialect} in
>  			77) _fortran_compile_test $(tc-getF77) || \
>  				_fortran_die_msg ;;
>  			90|95) _fortran_compile_test $(tc-getFC) 90 || \
>  				_fortran_die_msg ;;
>  			2003) _fortran_compile_test $(tc-getFC) 03 || \
>  				_fortran_die_msg ;;
>  			2008) die "Future" ;;
>  			*) die "${dialect} is not a Fortran dialect." ;;
>  		esac
>  	done
>  
>  	tc-export F77 FC
>  	einfo "Using following Fortran compiler:"
>  	einfo "  F77: ${F77}"
>  	einfo "  FC:  ${FC}"
>  
>  	if [[ ${FORTRAN_NEED_OPENMP} == 1 ]]; then
>  		if _fortran-has-openmp; then
>  			einfo "${FC} has OPENMP support"
>  		else
>  			die "Please install current gcc with USE=openmp or set the FC variable to a compiler that supports OpenMP"
>  		fi
>  	fi
>  }
>  
>  # @FUNCTION: _fortran-2_pkg_setup
>  # @INTERNAL
>  # @DESCRIPTION:
>  # _The_ fortran-2_pkg_setup() code
>  _fortran-2_pkg_setup() {
>  	for _f_use in ${FORTRAN_NEEDED}; do
>  	case ${_f_use} in
>  		always)
>  			_fortran_test_function && break
>  			;;
>  		no)
>  			einfo "Forcing fortran support off"
>  			break
>  			;;
>  		*)
>  			if use ${_f_use}; then
>  				_fortran_test_function && break
>  			else
>  				unset FC
>  				unset F77
>  			fi

This contradicts the dependency atoms.

If FORTRAN_NEEDED="foo bar", you'll get:

  DEP="foo? ( virtual/fortran ) bar? ( virtual/fortran )"

However, with USE="foo -bar" this will first set the compiler
for USE=foo, then reset it for USE=bar.

>  			;;
>  		esac
>  	done
>  }
>  
>  
>  # @FUNCTION: fortran-2_pkg_setup
>  # @DESCRIPTION:
>  # Setup functionality,
>  # checks for a valid fortran compiler and optionally for its openmp support.
>  fortran-2_pkg_setup() {
>  	debug-print-function ${FUNCNAME} "${@}"
>  
>  	if [[ ${MERGE_TYPE} != binary ]]; then
>  		_fortran-2_pkg_setup
>  	fi
>  }
>  
>  _FORTRAN_2_ECLASS=1
>  fi

-- 
Best regards,
Michał Górny

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

Re: [gentoo-dev] [PATCH v2] fortran-2.eclass: support EAPI 7

[Andrew Savchenko]

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

Hi all!

Here follows the updated version with improvements proposed by
mgorny and dilfridge.

diff --git a/eclass/fortran-2.eclass b/eclass/fortran-2.eclass
index 820cbbcb49bd..a0243714b674 100644
--- a/eclass/fortran-2.eclass
+++ b/eclass/fortran-2.eclass
@@ -1,285 +1,289 @@
-# Copyright 1999-2017 Gentoo Foundation
+# Copyright 1999-2018 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 # @ECLASS: fortran-2.eclass
 # @MAINTAINER:
 # jlec@gentoo.org
 # sci@gentoo.org
 # @AUTHOR:
 # Author Justin Lecher <jlec@gentoo.org>
 # Test functions provided by Sebastien Fabbro and Kacper Kowalik
-# @SUPPORTED_EAPIS: 4 5 6
+# @SUPPORTED_EAPIS: 4 5 6 7
 # @BLURB: Simplify fortran compiler management
 # @DESCRIPTION:
 # If you need a fortran compiler, then you should be inheriting this eclass.
 # In case you only need optional support, please export FORTRAN_NEEDED before
 # inheriting the eclass.
 #
 # The eclass tests for working fortran compilers
 # and exports the variables FC and F77.
 # Optionally, it checks for extended capabilities based on
 # the variable options selected in the ebuild
 # The only phase function exported is fortran-2_pkg_setup.
 # @EXAMPLE:
 # FORTRAN_NEEDED="lapack fortran"
 #
 # inherit fortran-2
 #
 # FORTRAN_NEED_OPENMP=1
 
-inherit eutils toolchain-funcs
+inherit toolchain-funcs
+case ${EAPI:-0} in
+	# not used in the eclass, but left for backward compatibility with legacy users
+	4|5|6) inherit eutils ;;
+	7) ;;
+	*) die "EAPI=${EAPI} is not supported" ;;
+esac
 
 case ${EAPI:-0} in
-	4|5|6) EXPORT_FUNCTIONS pkg_setup ;;
+	4|5|6|7) EXPORT_FUNCTIONS pkg_setup ;;
 	*) die "EAPI=${EAPI} is not supported" ;;
 esac
 
 if [[ ! ${_FORTRAN_2_CLASS} ]]; then
 
 # @ECLASS-VARIABLE: FORTRAN_NEED_OPENMP
 # @DESCRIPTION:
 # Set to "1" in order to automatically have the eclass abort if the fortran
 # compiler lacks openmp support.
 : ${FORTRAN_NEED_OPENMP:=0}
 
 # @ECLASS-VARIABLE: FORTRAN_STANDARD
 # @DESCRIPTION:
 # Set this, if a special dialect needs to be supported.
 # Generally not needed as default is sufficient.
 #
 # Valid settings are any combination of: 77 90 95 2003
 : ${FORTRAN_STANDARD:=77}
 
 # @ECLASS-VARIABLE: FORTRAN_NEEDED
 # @DESCRIPTION:
 # If your package has an optional fortran support, set this variable
 # to the space separated list of USE triggering the fortran
 # dependency.
 #
 # e.g. FORTRAN_NEEDED=lapack would result in
 #
 # DEPEND="lapack? ( virtual/fortran )"
 #
 # If unset, we always depend on virtual/fortran.
 : ${FORTRAN_NEEDED:=always}
 
 for _f_use in ${FORTRAN_NEEDED}; do
 	case ${_f_use} in
 		always)
 			DEPEND+=" virtual/fortran"
 			RDEPEND+=" virtual/fortran"
 			break
 			;;
 		no)
 			break
 			;;
 		test)
 			DEPEND+=" ${_f_use}? ( virtual/fortran )"
 			;;
 		*)
 			DEPEND+=" ${_f_use}? ( virtual/fortran )"
 			RDEPEND+=" ${_f_use}? ( virtual/fortran )"
 			;;
 	esac
 done
 unset _f_use
 
 # @FUNCTION: fortran_int64_abi_fflags
 # @DESCRIPTION:
 # Return the Fortran compiler flag to enable 64 bit integers for
 # array indices
 # @CODE
 fortran_int64_abi_fflags() {
 	debug-print-function ${FUNCNAME} "${@}"
 
-	_FC=$(tc-getFC)
+	local _FC=$(tc-getFC)
 	if [[ ${_FC} == *gfortran* ]]; then
 		echo "-fdefault-integer-8"
 	elif [[ ${_FC} == ifort ]]; then
 		echo "-integer-size 64"
 	else
 		die "Compiler flag for 64bit interger for ${_FC} unknown"
 	fi
 }
 
 # @FUNCTION: _fortran_write_testsuite
 # @INTERNAL
 # @DESCRIPTION:
 # writes fortran test code
 _fortran_write_testsuite() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	local filebase=${T}/test-fortran
 
 	# f77 code
-	cat <<- EOF > "${filebase}.f"
+	cat <<- EOF > "${filebase}.f" || die
 	       end
 	EOF
 
 	# f90/95 code
-	cat <<- EOF > "${filebase}.f90"
+	cat <<- EOF > "${filebase}.f90" || die
 	end
 	EOF
 
 	# f2003 code
-	cat <<- EOF > "${filebase}.f03"
+	cat <<- EOF > "${filebase}.f03" || die
 	       procedure(), pointer :: p
 	       end
 	EOF
 }
 
 # @FUNCTION: _fortran_compile_test
 # @USAGE: <compiler> [dialect]
 # @INTERNAL
 # @DESCRIPTION:
 # Takes fortran compiler as first argument and dialect as second.
 # Checks whether the passed fortran compiler speaks the fortran dialect
 _fortran_compile_test() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	local filebase=${T}/test-fortran
 	local fcomp=${1}
 	local fdia=${2}
 	local fcode=${filebase}.f${fdia}
 	local ret
 
 	[[ $# -lt 1 ]] && \
 		die "_fortran_compile_test() needs at least one argument"
 
 	[[ -f ${fcode} ]] || _fortran_write_testsuite
 
 	${fcomp} "${fcode}" -o "${fcode}.x" \
 		>> "${T}"/_fortran_compile_test.log 2>&1
 	ret=$?
 
 	rm -f "${fcode}.x"
 	return ${ret}
 }
 
 # @FUNCTION: _fortran-has-openmp
 # @RETURN: return code of the compiler
 # @INTERNAL
 # @DESCRIPTION:
 # See if the fortran supports OpenMP.
 _fortran-has-openmp() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	local flag
 	local filebase=${T}/test-fc-openmp
 	local fcode=${filebase}.f
-	local ret
 	local _fc=$(tc-getFC)
 
-	cat <<- EOF > "${fcode}"
+	cat <<- EOF > "${fcode}" || die
 	       call omp_get_num_threads
 	       end
 	EOF
 
 	for flag in -fopenmp -xopenmp -openmp -mp -omp -qsmp=omp; do
 		${_fc} ${flag} "${fcode}" -o "${fcode}.x" \
 			&>> "${T}"/_fortran_compile_test.log
-		ret=$?
-		(( ${ret} )) || break
+		[[ $? == 0 ]] && break
 	done
 
 	rm -f "${fcode}.x"
 	return ${ret}
 }
 
 # @FUNCTION: _fortran_die_msg
 # @INTERNAL
 # @DESCRIPTION:
 # Detailed description how to handle fortran support
 _fortran_die_msg() {
 	debug-print-function ${FUNCNAME} "${@}"
 
-	echo
+	eerror
 	eerror "Please install currently selected gcc version with USE=fortran."
 	eerror "If you intend to use a different compiler then gfortran, please"
 	eerror "set FC variable accordingly and take care that the necessary"
 	eerror "fortran dialects are supported."
-	echo
+	eerror
 	die "Currently no working fortran compiler is available (see ${T}/_fortran_compile_test.log for information)"
 }
 
 # @FUNCTION: _fortran_test_function
 # @INTERNAL
 # @DESCRIPTION:
 # Internal test function for working fortran compiler.
 # It is called in fortran-2_pkg_setup.
 _fortran_test_function() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	local dialect
 
 	: ${F77:=$(tc-getFC)}
 
 	: ${FORTRAN_STANDARD:=77}
 	for dialect in ${FORTRAN_STANDARD}; do
 		case ${dialect} in
 			77) _fortran_compile_test $(tc-getF77) || \
 				_fortran_die_msg ;;
 			90|95) _fortran_compile_test $(tc-getFC) 90 || \
 				_fortran_die_msg ;;
 			2003) _fortran_compile_test $(tc-getFC) 03 || \
 				_fortran_die_msg ;;
 			2008) die "Future" ;;
 			*) die "${dialect} is not a Fortran dialect." ;;
 		esac
 	done
 
 	tc-export F77 FC
 	einfo "Using following Fortran compiler:"
 	einfo "  F77: ${F77}"
 	einfo "  FC:  ${FC}"
 
 	if [[ ${FORTRAN_NEED_OPENMP} == 1 ]]; then
 		if _fortran-has-openmp; then
 			einfo "${FC} has OPENMP support"
 		else
 			die "Please install current gcc with USE=openmp or set the FC variable to a compiler that supports OpenMP"
 		fi
 	fi
 }
 
 # @FUNCTION: _fortran-2_pkg_setup
 # @INTERNAL
 # @DESCRIPTION:
 # _The_ fortran-2_pkg_setup() code
 _fortran-2_pkg_setup() {
 	for _f_use in ${FORTRAN_NEEDED}; do
 	case ${_f_use} in
 		always)
-			_fortran_test_function && break
+			_fortran_test_function && break 2
 			;;
 		no)
 			einfo "Forcing fortran support off"
 			break
 			;;
 		*)
 			if use ${_f_use}; then
-				_fortran_test_function && break
+				_fortran_test_function && break 2
 			else
 				unset FC
 				unset F77
 			fi
 			;;
 		esac
 	done
 }
 
 
 # @FUNCTION: fortran-2_pkg_setup
 # @DESCRIPTION:
 # Setup functionality,
 # checks for a valid fortran compiler and optionally for its openmp support.
 fortran-2_pkg_setup() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	if [[ ${MERGE_TYPE} != binary ]]; then
 		_fortran-2_pkg_setup
 	fi
 }
 
 _FORTRAN_2_ECLASS=1
 fi


Best regards,
Andrew Savchenko

[-- Attachment #2: Type: application/pgp-signature, Size: 833 bytes --]

Re: [gentoo-dev] [PATCH] fortran-2.eclass: support EAPI 7

[Andrew Savchenko]

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

Hi!

On Mon, 29 Oct 2018 23:52:31 +0100 Andreas K. Huettel wrote:
> > 
> > -inherit eutils toolchain-funcs
> > +inherit toolchain-funcs
> > 
> >  case ${EAPI:-0} in
> > -	4|5|6) EXPORT_FUNCTIONS pkg_setup ;;
> > +	4|5|6|7) EXPORT_FUNCTIONS pkg_setup ;;
> >  	*) die "EAPI=${EAPI} is not supported" ;;
> >  esac
> > 
> 
> ^ The disadvantage of this is that eutils inheritance then suddenly disappears 
> for all ebuilds. And you don't know who assumed implicitly somewhere that 
> inheriting fortran-2 makes epatch available...
> 
> So how about still inheriting eutils for EAPI 4,5,6 and only dropping it for 
> EAPI 7 ?!

I agree, backward compatibility matters. Fixed in v2 patch.


Best regards,
Andrew Savchenko

[-- Attachment #2: Type: application/pgp-signature, Size: 833 bytes --]

Re: [gentoo-dev] [PATCH] fortran-2.eclass: support EAPI 7

[Andrew Savchenko]

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

Hi!

On Tue, 30 Oct 2018 08:18:58 +0100 Michał Górny wrote:
> On Mon, 2018-10-29 at 03:57 +0300, Andrew Savchenko wrote:
> > On Sun, 28 Oct 2018 19:29:28 +0100 Michał Górny wrote:
> > > On Sun, 2018-10-28 at 01:38 +0300, Andrew Savchenko wrote:
> > > > Hi all!
> > > > 
> > > > The only blocker for EAPI 7 update is eutils inheritance, but it
> > > > seems to be not used within the current eclass code, probably a
> > > > remnant from older days. So it is removed.
> > > > 
> > > > Looks like no other EAPI 7 specific changes needed.
> > > > 
> > > 
> > > Please use -U99999 to include more context to the patches.
> 
> I'm going to include a few 'easy cleanup' comments since EAPI 7
> is a good opportunity to improve the eclass.  I'm going to skip horribly
> bad design decisions since I suppose nobody cares.

Should we really mix EAPI bump with full code review?

This eclass is small, so no harm here. But for larger eclasses
(hello java-*.eclass) this will hinder updates considerably. I
prefer to fix something rather than to fix nothing while
frustrating in attempt to fix everything at once.

Also this make git history review harder as fixes for independent
issues will be mixed together.

So I kindly ask you for future updates (from everyone, not just
me) focus on review of the proposed changes instead of reviewing
full code. Thank you for understanding.

> >  # @FUNCTION: fortran_int64_abi_fflags
> >  # @DESCRIPTION:
> >  # Return the Fortran compiler flag to enable 64 bit integers for
> >  # array indices
> >  # @CODE
> >  fortran_int64_abi_fflags() {
> >  	debug-print-function ${FUNCNAME} "${@}"
> >  
> >  	_FC=$(tc-getFC)
> 
> Any reason not to make it local?

Fixed.
 
> >  # @FUNCTION: _fortran_write_testsuite
> >  # @INTERNAL
> >  # @DESCRIPTION:
> >  # writes fortran test code
> >  _fortran_write_testsuite() {
> >  	debug-print-function ${FUNCNAME} "${@}"
> >  
> >  	local filebase=${T}/test-fortran
> >  
> >  	# f77 code
> >  	cat <<- EOF > "${filebase}.f"
> 
> || die

Done.

> >  	       end
> >  	EOF
> >  
> >  	# f90/95 code
> >  	cat <<- EOF > "${filebase}.f90"
> 
> || die

Done.

> >  	end
> 
> Also, why different indentation?

I prefer not to touch it. Fortran compilers are quite picky with
leading spaces or tabs.

> >  	EOF
> >  
> >  	# f2003 code
> >  	cat <<- EOF > "${filebase}.f03"
> 
> || die

Done.

> >  # @FUNCTION: _fortran-has-openmp
> >  # @RETURN: return code of the compiler
> >  # @INTERNAL
> >  # @DESCRIPTION:
> >  # See if the fortran supports OpenMP.
> >  _fortran-has-openmp() {
> >  	debug-print-function ${FUNCNAME} "${@}"
> >  
> >  	local flag
> >  	local filebase=${T}/test-fc-openmp
> >  	local fcode=${filebase}.f
> >  	local ret
> >  	local _fc=$(tc-getFC)
> >  
> >  	cat <<- EOF > "${fcode}"
> 
> || die

Done.
 
> >  	for flag in -fopenmp -xopenmp -openmp -mp -omp -qsmp=omp; do
> >  		${_fc} ${flag} "${fcode}" -o "${fcode}.x" \
> >  			&>> "${T}"/_fortran_compile_test.log
> >  		ret=$?
> >  		(( ${ret} )) || break
> 
> This (( ... )) is unreadable at best; please replace it with clear
> condition.

Fixed. ret variable is not needed at all.

> >  # @FUNCTION: _fortran_die_msg
> >  # @INTERNAL
> >  # @DESCRIPTION:
> >  # Detailed description how to handle fortran support
> >  _fortran_die_msg() {
> >  	debug-print-function ${FUNCNAME} "${@}"
> >  
> >  	echo
> 
> Don't mix echo with eerror.

Done.

> >  # @FUNCTION: _fortran-2_pkg_setup
> >  # @INTERNAL
> >  # @DESCRIPTION:
> >  # _The_ fortran-2_pkg_setup() code
> >  _fortran-2_pkg_setup() {
> >  	for _f_use in ${FORTRAN_NEEDED}; do
> >  	case ${_f_use} in
> >  		always)
> >  			_fortran_test_function && break
> >  			;;
> >  		no)
> >  			einfo "Forcing fortran support off"
> >  			break
> >  			;;
> >  		*)
> >  			if use ${_f_use}; then
> >  				_fortran_test_function && break
> >  			else
> >  				unset FC
> >  				unset F77
> >  			fi
> 
> This contradicts the dependency atoms.
> 
> If FORTRAN_NEEDED="foo bar", you'll get:
> 
>   DEP="foo? ( virtual/fortran ) bar? ( virtual/fortran )"
> 
> However, with USE="foo -bar" this will first set the compiler
> for USE=foo, then reset it for USE=bar.

Ok, now both case and for will break immediately if fortran
compiler is found and passed tests.
 
The updated full v2 patch will be sent as a separate e-mail.

Best regards,
Andrew Savchenko

[-- Attachment #2: Type: application/pgp-signature, Size: 833 bytes --]

Re: [gentoo-dev] [PATCH] fortran-2.eclass: support EAPI 7

[Michael Orlitzky]

On 11/01/2018 06:27 PM, Andrew Savchenko wrote:
> 
> This eclass is small, so no harm here. But for larger eclasses
> (hello java-*.eclass) this will hinder updates considerably. I
> prefer to fix something rather than to fix nothing while
> frustrating in attempt to fix everything at once.
> 
> Also this make git history review harder as fixes for independent
> issues will be mixed together.
> 
> So I kindly ask you for future updates (from everyone, not just
> me) focus on review of the proposed changes instead of reviewing
> full code. Thank you for understanding.
> 

You don't have to fix everything at once. A thorough code review is
incredibly valuable we shouldn't discourage anyone from doing them.

On the other hand, if you decide to fix only some of the issues, that's
your prerogative. I would however suggest that afterwards, you open a
bug for the remaining improvements so that the valuable time of the
reviewer is not wasted.

Re: [gentoo-dev] [PATCH v2] fortran-2.eclass: support EAPI 7

[Ulrich Mueller]

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

>>>>> On Thu, 01 Nov 2018, Andrew Savchenko wrote:
 
> -inherit eutils toolchain-funcs
> +inherit toolchain-funcs
> +case ${EAPI:-0} in
> +	# not used in the eclass, but left for backward compatibility with legacy users
> +	4|5|6) inherit eutils ;;
> +	7) ;;
> +	*) die "EAPI=${EAPI} is not supported" ;;
> +esac
>  
>  case ${EAPI:-0} in
> -	4|5|6) EXPORT_FUNCTIONS pkg_setup ;;
> +	4|5|6|7) EXPORT_FUNCTIONS pkg_setup ;;
>  	*) die "EAPI=${EAPI} is not supported" ;;
>  esac

Why is the second case statement needed? Program flow can only reach it
if the EAPI is 4, 5, 6, or 7.

Ulrich

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

Re: [gentoo-dev] [PATCH] fortran-2.eclass: support EAPI 7

[Michał Górny]

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

On Fri, 2018-11-02 at 01:27 +0300, Andrew Savchenko wrote:
> Hi!
> 
> On Tue, 30 Oct 2018 08:18:58 +0100 Michał Górny wrote:
> > On Mon, 2018-10-29 at 03:57 +0300, Andrew Savchenko wrote:
> > > On Sun, 28 Oct 2018 19:29:28 +0100 Michał Górny wrote:
> > > > On Sun, 2018-10-28 at 01:38 +0300, Andrew Savchenko wrote:
> > > > > Hi all!
> > > > > 
> > > > > The only blocker for EAPI 7 update is eutils inheritance, but it
> > > > > seems to be not used within the current eclass code, probably a
> > > > > remnant from older days. So it is removed.
> > > > > 
> > > > > Looks like no other EAPI 7 specific changes needed.
> > > > > 
> > > > 
> > > > Please use -U99999 to include more context to the patches.
> > 
> > I'm going to include a few 'easy cleanup' comments since EAPI 7
> > is a good opportunity to improve the eclass.  I'm going to skip horribly
> > bad design decisions since I suppose nobody cares.
> 
> Should we really mix EAPI bump with full code review?

Yes, for two reasons.

Firstly, because an EAPI bump effectively requires reviewing all
the eclass logic for constraints imposed by the new EAPI.  While
reviewing code, it is natural that people may notice other issues. 
Ignoring them once noticed would be a waste of effort.

Secondly, changes to frequently used eclass have a large overhead of
metadata cache updates.  Given most of the listed issues are rather
trivial to fix, it would be wasteful to defer them for a second metadata
cache update.

> This eclass is small, so no harm here. But for larger eclasses
> (hello java-*.eclass) this will hinder updates considerably. I
> prefer to fix something rather than to fix nothing while
> frustrating in attempt to fix everything at once.
> 
> Also this make git history review harder as fixes for independent
> issues will be mixed together.

Why would you mix them together?  The whole point of using git (and not
CVS) is that you can trivially make separate commits addressing
different kinds of issues.  It also makes it trivial to send them for
review afterwards.

> So I kindly ask you for future updates (from everyone, not just
> me) focus on review of the proposed changes instead of reviewing
> full code. Thank you for understanding.

As explained above, the proposed change is meaningless without context
(as it affects how everything else in eclass works).  If we were to
ignore context, we'd even ACK eclass changes that resulted in the eclass
immediately dying due to programmer's mistake.

Finally, I'd like to point out that peer review is one of foundations
of open source.  Sadly, Gentoo has failed to embrace this, and right now reviews of existing code are rather an exception than a rule.  What
makes it even worse is that some developers are actively hostile to
the criticism of their code.  It is as if Gentoo's bazaar was dominated by makeshift cathedrals.

-- 
Best regards,
Michał Górny

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

Re: [gentoo-dev] [PATCH v2] fortran-2.eclass: support EAPI 7

[Andrew Savchenko]

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

On Fri, 02 Nov 2018 11:27:29 +0100 Ulrich Mueller wrote:
> >>>>> On Thu, 01 Nov 2018, Andrew Savchenko wrote:
>  
> > -inherit eutils toolchain-funcs
> > +inherit toolchain-funcs
> > +case ${EAPI:-0} in
> > +	# not used in the eclass, but left for backward compatibility with legacy users
> > +	4|5|6) inherit eutils ;;
> > +	7) ;;
> > +	*) die "EAPI=${EAPI} is not supported" ;;
> > +esac
> >  
> >  case ${EAPI:-0} in
> > -	4|5|6) EXPORT_FUNCTIONS pkg_setup ;;
> > +	4|5|6|7) EXPORT_FUNCTIONS pkg_setup ;;
> >  	*) die "EAPI=${EAPI} is not supported" ;;
> >  esac
> 
> Why is the second case statement needed? Program flow can only reach it
> if the EAPI is 4, 5, 6, or 7.

Just a remnant from older version. Fixed.

Best regards,
Andrew Savchenko

[-- Attachment #2: Type: application/pgp-signature, Size: 833 bytes --]

Re: [gentoo-dev] [PATCH] fortran-2.eclass: support EAPI 7

[Andrew Savchenko]

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

On Fri, 02 Nov 2018 15:20:16 +0100 Michał Górny wrote:
> On Fri, 2018-11-02 at 01:27 +0300, Andrew Savchenko wrote:
> > Hi!
> > 
> > On Tue, 30 Oct 2018 08:18:58 +0100 Michał Górny wrote:
> > > On Mon, 2018-10-29 at 03:57 +0300, Andrew Savchenko wrote:
> > > > On Sun, 28 Oct 2018 19:29:28 +0100 Michał Górny wrote:
> > > > > On Sun, 2018-10-28 at 01:38 +0300, Andrew Savchenko wrote:
> > > > > > Hi all!
> > > > > > 
> > > > > > The only blocker for EAPI 7 update is eutils inheritance, but it
> > > > > > seems to be not used within the current eclass code, probably a
> > > > > > remnant from older days. So it is removed.
> > > > > > 
> > > > > > Looks like no other EAPI 7 specific changes needed.
> > > > > > 
> > > > > 
> > > > > Please use -U99999 to include more context to the patches.
> > > 
> > > I'm going to include a few 'easy cleanup' comments since EAPI 7
> > > is a good opportunity to improve the eclass.  I'm going to skip horribly
> > > bad design decisions since I suppose nobody cares.
> > 
> > Should we really mix EAPI bump with full code review?
> 
> Yes, for two reasons.
> 
> Firstly, because an EAPI bump effectively requires reviewing all
> the eclass logic for constraints imposed by the new EAPI.  While
> reviewing code, it is natural that people may notice other issues. 
> Ignoring them once noticed would be a waste of effort.

EAPI update usually don't affect full ebuild scope. For EAPI 6->7
update grep is sufficient to find affected parts of the eclass.

> Secondly, changes to frequently used eclass have a large overhead of
> metadata cache updates.  Given most of the listed issues are rather
> trivial to fix, it would be wasteful to defer them for a second metadata
> cache update.

Not all eclasses are frequently used, including fortran-2 eclass.

> > So I kindly ask you for future updates (from everyone, not just
> > me) focus on review of the proposed changes instead of reviewing
> > full code. Thank you for understanding.
> 
> As explained above, the proposed change is meaningless without context
> (as it affects how everything else in eclass works).  If we were to
> ignore context, we'd even ACK eclass changes that resulted in the eclass
> immediately dying due to programmer's mistake.
> 
> Finally, I'd like to point out that peer review is one of foundations
> of open source.

While peer review is important in many cases, the statement above
is wrong. The free software (and not just open source and Gentoo
is free software) founds on the four freedoms as defined by FSF.

> Sadly, Gentoo has failed to embrace this, and right now reviews
> of existing code are rather an exception than a rule.  What
> makes it even worse is that some developers are actively hostile to
> the criticism of their code.

Everything is good only within sane limits. While peer review is
often useful, it may be harmful as well:

1) It at least doubles human time required to do the job. And human
resources are the only resources we are really short of.

2) Sometimes it turns into bikeshedding, e.g. of how variables
should be named or what indentation style should be used.

3) Unreasonably long and complicated process for routine changes
may discourage people from further contributions. Right now we have
dozens of eclasses still on EAPI 6. I would prefer to see them
EAPI 7 so they would not block EAPI 7 updates of dependent packages,
rather than require them to underdo complete overhaul during EAPI 7
update effectively postponing it for a long time.

Sometimes the best is the enemy of the good.

Best regards,
Andrew Savchenko

[-- Attachment #2: Type: application/pgp-signature, Size: 833 bytes --]

Re: [gentoo-dev] [PATCH v3 2/2] fortran-2.eclass: support EAPI 7

[Andrew Savchenko]

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

Hi all!

Here follow updated patches for fortran-2.eclass EAPI 7 update.

Patch 2 contains only code cleanup and fixes unrelated to EAPI 7
update:

diff --git a/eclass/fortran-2.eclass b/eclass/fortran-2.eclass
index 820cbbcb49bd..b871d16e3e05 100644
--- a/eclass/fortran-2.eclass
+++ b/eclass/fortran-2.eclass
@@ -1,4 +1,4 @@
-# Copyright 1999-2017 Gentoo Foundation
+# Copyright 1999-2018 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 # @ECLASS: fortran-2.eclass
@@ -92,7 +95,7 @@ unset _f_use
 fortran_int64_abi_fflags() {
 	debug-print-function ${FUNCNAME} "${@}"
 
-	_FC=$(tc-getFC)
+	local _FC=$(tc-getFC)
 	if [[ ${_FC} == *gfortran* ]]; then
 		echo "-fdefault-integer-8"
 	elif [[ ${_FC} == ifort ]]; then
@@ -112,17 +115,17 @@ _fortran_write_testsuite() {
 	local filebase=${T}/test-fortran
 
 	# f77 code
-	cat <<- EOF > "${filebase}.f"
+	cat <<- EOF > "${filebase}.f" || die
 	       end
 	EOF
 
 	# f90/95 code
-	cat <<- EOF > "${filebase}.f90"
+	cat <<- EOF > "${filebase}.f90" || die
 	end
 	EOF
 
 	# f2003 code
-	cat <<- EOF > "${filebase}.f03"
+	cat <<- EOF > "${filebase}.f03" || die
 	       procedure(), pointer :: p
 	       end
 	EOF
@@ -170,7 +173,7 @@ _fortran-has-openmp() {
 	local ret
 	local _fc=$(tc-getFC)
 
-	cat <<- EOF > "${fcode}"
+	cat <<- EOF > "${fcode}" || die
 	       call omp_get_num_threads
 	       end
 	EOF
@@ -179,7 +182,7 @@ _fortran-has-openmp() {
 		${_fc} ${flag} "${fcode}" -o "${fcode}.x" \
 			&>> "${T}"/_fortran_compile_test.log
 		ret=$?
-		(( ${ret} )) || break
+		[[ ${ret} == 0 ]] && break
 	done
 
 	rm -f "${fcode}.x"
@@ -193,12 +196,12 @@ _fortran-has-openmp() {
 _fortran_die_msg() {
 	debug-print-function ${FUNCNAME} "${@}"
 
-	echo
+	eerror
 	eerror "Please install currently selected gcc version with USE=fortran."
 	eerror "If you intend to use a different compiler then gfortran, please"
 	eerror "set FC variable accordingly and take care that the necessary"
 	eerror "fortran dialects are supported."
-	echo
+	eerror
 	die "Currently no working fortran compiler is available (see ${T}/_fortran_compile_test.log for information)"
 }
 
@@ -250,7 +253,7 @@ _fortran-2_pkg_setup() {
 	for _f_use in ${FORTRAN_NEEDED}; do
 	case ${_f_use} in
 		always)
-			_fortran_test_function && break
+			_fortran_test_function && break 2
 			;;
 		no)
 			einfo "Forcing fortran support off"
@@ -258,7 +261,7 @@ _fortran-2_pkg_setup() {
 			;;
 		*)
 			if use ${_f_use}; then
-				_fortran_test_function && break
+				_fortran_test_function && break 2
 			else
 				unset FC
 				unset F77


Best regards,
Andrew Savchenko

[-- Attachment #2: Type: application/pgp-signature, Size: 833 bytes --]

Re: [gentoo-dev] [PATCH v3 1/2] fortran-2.eclass: support EAPI 7

[Andrew Savchenko]

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

Hi all!

Here follow updated patches for fortran-2.eclass EAPI 7 update.

Patch 1 contains only EAPI 7 related changes:

diff --git a/eclass/fortran-2.eclass b/eclass/fortran-2.eclass
index 820cbbcb49bd..b871d16e3e05 100644
--- a/eclass/fortran-2.eclass
+++ b/eclass/fortran-2.eclass
@@ -8,7 +8,7 @@
 # @AUTHOR:
 # Author Justin Lecher <jlec@gentoo.org>
 # Test functions provided by Sebastien Fabbro and Kacper Kowalik
-# @SUPPORTED_EAPIS: 4 5 6
+# @SUPPORTED_EAPIS: 4 5 6 7
 # @BLURB: Simplify fortran compiler management
 # @DESCRIPTION:
 # If you need a fortran compiler, then you should be inheriting
this eclass. @@ -27,13 +27,16 @@
 #
 # FORTRAN_NEED_OPENMP=1
 
-inherit eutils toolchain-funcs
-
+inherit toolchain-funcs
 case ${EAPI:-0} in
-	4|5|6) EXPORT_FUNCTIONS pkg_setup ;;
+	# not used in the eclass, but left for backward
compatibility with legacy users
+	4|5|6) inherit eutils ;;
+	7) ;;
 	*) die "EAPI=${EAPI} is not supported" ;;
 esac
 
+EXPORT_FUNCTIONS pkg_setup
+
 if [[ ! ${_FORTRAN_2_CLASS} ]]; then
 
 # @ECLASS-VARIABLE: FORTRAN_NEED_OPENMP


Best regards,
Andrew Savchenko

[-- Attachment #2: Type: application/pgp-signature, Size: 833 bytes --]

Re: [gentoo-dev] [PATCH v3 2/2] fortran-2.eclass: support EAPI 7

[Andrew Savchenko]

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

On Mon, 5 Nov 2018 18:37:55 +0300 Andrew Savchenko wrote:
> Hi all!
> 
> Here follow updated patches for fortran-2.eclass EAPI 7 update.
> 
> Patch 2 contains only code cleanup and fixes unrelated to EAPI 7
> update:

With no comments for ~12 days both patches are applied now.

Best regards,
Andrew Savchenko

[-- Attachment #2: Type: application/pgp-signature, Size: 833 bytes --]