[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gentoo-dev
Subject:    Re: [gentoo-dev] [PATCH] fortran-2.eclass: support EAPI 7
From:       Michał_Górny <mgorny () gentoo ! org>
Date:       2018-10-30 7:18:58
Message-ID: 1540883938.1250.6.camel () gentoo ! org
[Download RAW message or body]


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


["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]