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

[Michał Górny]

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@g.o
>  # sci@g.o
>  # @AUTHOR:
>  # Author Justin Lecher <jlec@g.o>
>  # 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