public inbox for gentoo-dev@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-dev] RFC: intel-sdp-r1.eclass
@ 2016-02-15  8:16 Justin Lecher (jlec)
  2016-02-15 12:59 ` Michał Górny
  0 siblings, 1 reply; 12+ messages in thread
From: Justin Lecher (jlec) @ 2016-02-15  8:16 UTC (permalink / raw
  To: Gentoo Dev


[-- Attachment #1.1: Type: text/plain, Size: 368 bytes --]

Hi everyone,

We (actually mostly soap) rewrote parts of the intel-sdp.eclass and
decided to revbump it. Please review our changes.

Changes are:

* Move from EAPI=5 to 6
* Adopt to changed packaging layout
* Use ABI_ instead of multilib
* Drop eclipse support
* Require all RPM lists to be arrays
* Don't record in installation db


Thanks,

Justin

[-- Attachment #1.2: intel-sdp-r1.eclass --]
[-- Type: text/plain, Size: 15167 bytes --]

# Copyright 1999-2016 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Id$

# @ECLASS: intel-sdp-r1.eclass
# @MAINTAINER:
# Justin Lecher <jlec@gentoo.org>
# David Seifert <soap@gentoo.org>
# Sci Team <sci@gentoo.org>
# @BLURB: Handling of Intel's Software Development Products package management

if [[ ! ${_INTEL_SDP_R1_ECLASS_} ]]; then

case "${EAPI:-0}" in
	6) ;;
	*) die "EAPI=${EAPI} is not supported" ;;
esac

# @ECLASS-VARIABLE: INTEL_DID
# @DEFAULT_UNSET
# @DESCRIPTION:
# The package download ID from Intel.
# To find out its value, see the links to download in
# https://registrationcenter.intel.com/RegCenter/MyProducts.aspx
#
# e.g. 8365
#
# Must be defined before inheriting the eclass

# @ECLASS-VARIABLE: INTEL_DPN
# @DEFAULT_UNSET
# @DESCRIPTION:
# The package name to download from Intel.
# To find out its value, see the links to download in
# https://registrationcenter.intel.com/RegCenter/MyProducts.aspx
#
# e.g. parallel_studio_xe
#
# Must be defined before inheriting the eclass

# @ECLASS-VARIABLE: INTEL_DPV
# @DEFAULT_UNSET
# @DESCRIPTION:
# The package download version from Intel.
# To find out its value, see the links to download in
# https://registrationcenter.intel.com/RegCenter/MyProducts.aspx
#
# e.g. 2016_update1
#
# Must be defined before inheriting the eclass

# @ECLASS-VARIABLE: INTEL_TARX
# @DESCRIPTION:
# The package extention.
# To find out its value, see the links to download in
# https://registrationcenter.intel.com/RegCenter/MyProducts.aspx
#
# e.g. tar.gz
#
# Must be defined before inheriting the eclass
: ${INTEL_TARX:=tgz}

# @ECLASS-VARIABLE: INTEL_SUBDIR
# @DEFAULT_UNSET
# @DESCRIPTION:
# The package sub-directory where it will end-up in /opt/intel
# To find out its value, you have to do a raw install from the Intel tar ball

# @ECLASS-VARIABLE: INTEL_SKIP_LICENSE
# @DEFAULT_UNSET
# @DESCRIPTION:
# Possibility to skip the mandatory check for licenses. Only set this if there
# is really no fix.

# @ECLASS-VARIABLE: INTEL_RPMS_DIR
# @DESCRIPTION:
# Main subdirectory which contains the rpms to extract.
: ${INTEL_RPMS_DIR:=rpm}

# @ECLASS-VARIABLE: INTEL_X86
# @DESCRIPTION:
# 32bit arch in rpm names
#
# e.g. i486
: ${INTEL_X86:=i486}

# @ECLASS-VARIABLE: INTEL_BIN_RPMS
# @DESCRIPTION:
# Functional name of rpm without any version/arch tag
# Has to be a bash array
#
# e.g. ("icc-l-all-devel")
#
# if the rpm is located in a directory other than INTEL_RPMS_DIR you can
# specify the full path
#
# e.g. CLI_install/rpm/intel-vtune-amplifier-xe-cli
: ${INTEL_BIN_RPMS:=()}

# @ECLASS-VARIABLE: INTEL_AMD64_RPMS
# @DESCRIPTION:
# AMD64 single arch rpms. Same syntax as INTEL_BIN_RPMS
# Has to be a bash array
: ${INTEL_AMD64_RPMS:=()}

# @ECLASS-VARIABLE: INTEL_X86_RPMS
# @DESCRIPTION:
# X86 single arch rpms. Same syntax as INTEL_BIN_RPMS
# Has to be a bash array
: ${INTEL_X86_RPMS:=()}

# @ECLASS-VARIABLE: INTEL_DAT_RPMS
# @DESCRIPTION:
# Functional name of rpm of common data which are arch free
# without any version tag
# Has to be a bash array
#
# e.g. ("openmp-l-all-devel")
#
# if the rpm is located in a directory different to INTEL_RPMS_DIR you can
# specify the full path
#
# e.g. CLI_install/rpm/intel-vtune-amplifier-xe-cli-common
: ${INTEL_DAT_RPMS:=()}

# @ECLASS-VARIABLE: INTEL_SINGLE_ARCH
# @DESCRIPTION:
# Unset, if only the multilib package will be provided by intel
: ${INTEL_SINGLE_ARCH:=true}

MULTILIB_COMPAT=( abi_x86_{32,64} )

inherit check-reqs eutils multilib-build versionator

_INTEL_PV1=$(get_version_component_range 1)
_INTEL_PV2=$(get_version_component_range 2)
_INTEL_PV3=$(get_version_component_range 3)
_INTEL_PV4=$(get_version_component_range 4)
_INTEL_PV=""
[[ -n ${_INTEL_PV4} ]] && _INTEL_PV+="${_INTEL_PV4}-"
[[ -n ${_INTEL_PV1} ]] && _INTEL_PV+="${_INTEL_PV1}"
[[ -n ${_INTEL_PV2} ]] && _INTEL_PV+=".${_INTEL_PV2}"
[[ -n ${_INTEL_PV3} ]] && _INTEL_PV+=".${_INTEL_PV3}"
[[ -n ${_INTEL_PV4} ]] && _INTEL_PV+="-${_INTEL_PV4}"

_INTEL_URI="http://registrationcenter-download.intel.com/akdlm/irc_nas/${INTEL_DID}/${INTEL_DPN}"

if [ ${INTEL_SINGLE_ARCH} == true ]; then
	SRC_URI="
		abi_x86_32? ( ${_INTEL_URI}_${INTEL_DPV}_ia32.${INTEL_TARX} )
		abi_x86_64? ( ${_INTEL_URI}_${INTEL_DPV}_intel64.${INTEL_TARX} )"
else
	SRC_URI="${_INTEL_URI}_${INTEL_DPV}.${INTEL_TARX}"
fi

LICENSE="Intel-SDP"
# Future work, #394411
#SLOT="${_INTEL_PV1}.${_INTEL_PV2}"
SLOT="0"

RESTRICT="mirror"

RDEPEND=""
DEPEND="app-arch/rpm2targz"

_INTEL_SDP_YEAR=${INTEL_DPV}
_INTEL_SDP_YEAR=${_INTEL_SDP_YEAR%_sp*}
_INTEL_SDP_YEAR=${_INTEL_SDP_YEAR%_update*}

# @ECLASS-VARIABLE: INTEL_SDP_DIR
# @DESCRIPTION:
# Full rootless path to installation dir
INTEL_SDP_DIR="opt/intel/${INTEL_SUBDIR}_${_INTEL_SDP_YEAR:-${_INTEL_PV1}}"
[[ -n ${_INTEL_PV3} ]] && INTEL_SDP_DIR+=".${_INTEL_PV3}"
[[ -n ${_INTEL_PV4} ]] && INTEL_SDP_DIR+=".${_INTEL_PV4}"

# @ECLASS-VARIABLE: INTEL_SDP_EDIR
# @DESCRIPTION:
# Full rooted path to installation dir
INTEL_SDP_EDIR="${EROOT%/}/${INTEL_SDP_DIR}"

S="${WORKDIR}"

QA_PREBUILT="${INTEL_SDP_DIR}/*"

# @ECLASS-VARIABLE: INTEL_ARCH
# @DEFAULT_UNSET
# @DESCRIPTION:
# Intels internal names of the arches; will be set at runtime accordingly
#
# e.g. amd64-multilib -> INTEL_ARCH="intel64 ia32"

# @FUNCTION: _isdp_big-warning
# @USAGE: [pre-check | test-failed]
# @INTERNAL
# @DESCRIPTION:
# warn user that we really require a license
_isdp_big-warning() {
	debug-print-function ${FUNCNAME} "${@}"

	case ${1} in
		pre-check )
			echo ""
			ewarn "License file not found!"
			;;

		test-failed )
			echo ""
			ewarn "Function test failed. Most probably due to an invalid license."
			ewarn "This means you already tried to bypass the license check once."
			;;
	esac

	echo ""
	ewarn "Make sure you have received an Intel license."
	ewarn "To receive a non-commercial license, you need to register at:"
	ewarn "https://software.intel.com/en-us/qualify-for-free-software"
	ewarn "Install the license file into ${EPREFIX}/opt/intel/licenses"
	ewarn ""
	ewarn "Beginning with the 2016 suite of tools, license files are keyed"
	ewarn "to the MAC address of the eth0 interface. In order to retrieve"
	ewarn "a personalized license file, follow the instructions at"
	ewarn "https://software.intel.com/en-us/articles/how-do-i-get-my-license-file-for-intel-parallel-studio-xe-2016"

	case ${1} in
		pre-check )
			ewarn "before proceeding with installation of ${P}"
			echo ""
			;;
		* )
			echo ""
			;;
			esac
}

# @FUNCTION: _isdp_version_test
# @INTERNAL
# @DESCRIPTION:
# Testing for valid license by asking for version information of the compiler
_isdp_version_test() {
	debug-print-function ${FUNCNAME} "${@}"

	local comp comp_full arch warn
	case ${PN} in
		ifc )
			debug-print "Testing ifort"
			comp=ifort
			;;
		icc )
			debug-print "Testing icc"
			comp=icc
			;;
		*)
			die "${PN} is not supported for testing"
			;;
	esac

	for arch in ${INTEL_ARCH}; do
		case ${EBUILD_PHASE} in
			install )
				comp_full="${ED}/${INTEL_SDP_DIR}/linux/bin/${arch}/${comp}"
				;;
			postinst )
				comp_full="${INTEL_SDP_EDIR}/linux/bin/${arch}/${comp}"
				;;
			* )
				ewarn "Compile test not supported in ${EBUILD_PHASE}"
				continue
				;;
		esac

		debug-print "LD_LIBRARY_PATH=\"${INTEL_SDP_EDIR}/linux/bin/${arch}/\" \"${comp_full}\" -V"

		LD_LIBRARY_PATH="${INTEL_SDP_EDIR}/linux/bin/${arch}/" "${comp_full}" -V &>/dev/null
		[[ $? -ne 0 ]] && warn=yes
	done
	[[ "${warn}" == "yes" ]] && _isdp_big-warning test-failed
}

# @FUNCTION: _isdp_run-test
# @INTERNAL
# @DESCRIPTION:
# Test if installed compiler is working
_isdp_run-test() {
	debug-print-function ${FUNCNAME} "${@}"

	if [[ -z ${INTEL_SKIP_LICENSE} ]]; then
		case ${PN} in
			ifc | icc )
				_isdp_version_test ;;
			* )
				debug-print "No test available for ${PN}"
				;;
		esac
	fi
}

# @FUNCTION: convert2intel_arch
# @USAGE: <arch>
# @DESCRIPTION:
# Convert between portage arch (e.g. amd64, x86) and intel arch
# nomenclature (e.g. intel64, ia32)
convert2intel_arch() {
	debug-print-function ${FUNCNAME} "${@}"

	case $1 in
		amd64|abi_x86_64|*amd64*)
			echo "intel64"
			;;
		x86|abi_x86_32|*x86*)
			echo "ia32"
			;;
		*)
			die "Abi \'$1\' is unsupported"
			;;
	esac
}

# @FUNCTION: intel-sdp-r1_pkg_pretend
# @DESCRIPTION:
# @CODE
# * Check that the user has a (valid) license file before going on.
# * Check for space requirements being fullfilled
# @CODE
intel-sdp-r1_pkg_pretend() {
	debug-print-function ${FUNCNAME} "${@}"

	local warn=1 dir dirs ret arch a p

	: ${CHECKREQS_DISK_BUILD:=256M}
	check-reqs_pkg_pretend

	if [[ -z ${INTEL_SKIP_LICENSE} ]]; then
		if echo ${INTEL_LICENSE_FILE} | grep -q @; then
			einfo "Looks like you are using following license server:"
			einfo "   ${INTEL_LICENSE_FILE}"
			return 0
		fi

		dirs=(
			"${EPREFIX}/opt/intel/licenses"
			"${INTEL_SDP_EDIR}/licenses"
			"${INTEL_SDP_EDIR}/Licenses"
			)
		for dir in "${dirs[@]}" ; do
			ebegin "Checking for a license in: ${dir}"
			#maybe use nullglob or [[ $(echo ${dir/*lic) != "${dir}/*lic" ]]
			[[ $( ls "${dir}"/*lic 2>/dev/null ) ]]; ret=$?
			eend ${ret}
			if [[ ${ret} == "0" ]]; then
				warn=${ret}
				break
			fi
		done
		if [[ ${warn} == "1" ]]; then
			_isdp_big-warning pre-check
			die "Could not find license file"
		fi
	else
		eqawarn "The ebuild doesn't check for presence of a proper intel license!"
		eqawarn "This shouldn't be done unless there is a very good reason."
	fi
}

# @FUNCTION: intel-sdp-r1_pkg_setup
# @DESCRIPTION:
# Setting up and sorting some internal variables
intel-sdp-r1_pkg_setup() {
	debug-print-function ${FUNCNAME} "${@}"
	local arch a p

	INTEL_ARCH=""

	if use abi_x86_64; then
		arch+=" x86_64"
		INTEL_ARCH+=" intel64"
	fi
	if use abi_x86_32; then
		arch+=" ${INTEL_X86}"
		INTEL_ARCH+=" ia32"
	fi
	INTEL_RPMS=()
	INTEL_RPMS_FULL=()

	for p in "${INTEL_BIN_RPMS[@]}"; do
		for a in ${arch}; do
			if [ ${p} == $(basename ${p}) ]; then
				# check for variables ending in ".rpm"
				# these are excluded from version expansion, due to Intel's
				# idiosyncratic versioning scheme beginning with their 2016
				# suite of tools.
				if [[ "${p}" == *.rpm ]]; then
					INTEL_RPMS+=( intel-${p} )
				else
					INTEL_RPMS+=( intel-${p}-${_INTEL_PV}.${a}.rpm )
				fi
			else
				if [[ "${p}" == *.rpm ]]; then
					INTEL_RPMS_FULL+=( ${p} )
				else
					INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV}.${a}.rpm )
				fi
			fi
		done
	done

	if use amd64; then
		for p in "${INTEL_AMD64_RPMS[@]}"; do
			if [ ${p} == $(basename ${p}) ]; then
				if [[ "${p}" == *.rpm ]]; then
					INTEL_RPMS+=( intel-${p} )
				else
					INTEL_RPMS+=( intel-${p}-${_INTEL_PV}.x86_64.rpm )
				fi
			else
				if [[ "${p}" == *.rpm ]]; then
					INTEL_RPMS_FULL+=( ${p} )
				else
					INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV}.x86_64.rpm )
				fi
			fi
		done
	fi

	for p in "${INTEL_X86_RPMS[@]}"; do
		if [ ${p} == $(basename ${p}) ]; then
			if [[ "${p}" == *.rpm ]]; then
				INTEL_RPMS+=( intel-${p} )
			else
				INTEL_RPMS+=( intel-${p}-${_INTEL_PV}.${INTEL_X86}.rpm )
			fi
		else
			if [[ "${p}" == *.rpm ]]; then
				INTEL_RPMS_FULL+=( ${p} )
			else
				INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV}.${INTEL_X86}.rpm )
			fi
		fi
	done

	for p in "${INTEL_DAT_RPMS[@]}"; do
		if [ ${p} == $(basename ${p}) ]; then
			if [[ "${p}" == *.rpm ]]; then
				INTEL_RPMS+=( intel-${p} )
			else
				INTEL_RPMS+=( intel-${p}-${_INTEL_PV}.noarch.rpm )
			fi
		else
			if [[ "${p}" == *.rpm ]]; then
				INTEL_RPMS_FULL+=( ${p} )
			else
				INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV}.noarch.rpm )
			fi
		fi
	done
}

# @FUNCTION: intel-sdp-r1_src_unpack
# @DESCRIPTION:
# Unpacking necessary rpms from tarball, extract them and rearrange the output.
intel-sdp-r1_src_unpack() {
	local l r subdir rb t list=() debug_list

	for t in ${A}; do
		for r in "${INTEL_RPMS[@]}"; do
			rpmdir=${t%%.*}/${INTEL_RPMS_DIR}
			list+=( ${rpmdir}/${r} )
		done

		for r in "${INTEL_RPMS_FULL[@]}"; do
			list+=( ${t%%.*}/${r} )
		done

		debug_list="$(IFS=$'\n'; echo ${list[@]} )"

		debug-print "Adding to decompression list:"
		debug-print ${debug_list}

		tar xvf "${DISTDIR}"/${t} ${list[@]} &> "${T}"/rpm-extraction.log

		for r in ${list[@]}; do
			rb=$(basename ${r})
			einfo "Unpacking ${rb}"
			rpm2tar -O ${r} | tar xvf - | sed -e \
				"s:^\.:${EROOT#/}:g" > /dev/null; assert "unpacking ${r} failed"
		done
	done
}

# @FUNCTION: intel-sdp-r1_src_install
# @DESCRIPTION:
# Install everything
intel-sdp-r1_src_install() {
	debug-print-function ${FUNCNAME} "${@}"

	# remove uninstall information
	if path_exists opt/intel/"${INTEL_DPN}"*/uninstall; then
		ebegin "Cleaning out uninstall"
		rm -r opt/intel/"${INTEL_DPN}"*/uninstall || die
		eend
	fi

	# handle documentation
	if path_exists "opt/intel/documentation_${_INTEL_SDP_YEAR}"; then
		if path_exists "opt/intel/documentation_${_INTEL_SDP_YEAR}/en/man/common/man1"; then
			doman opt/intel/documentation_"${_INTEL_SDP_YEAR}"/en/man/common/man1/*
			rm -r opt/intel/documentation_"${_INTEL_SDP_YEAR}"/en/man || die
		fi

		if use doc; then
			if ! use linguas_ja; then
				rm -r opt/intel/documentation_"${_INTEL_SDP_YEAR}"/ja || die
			fi
			dodoc -r opt/intel/documentation_"${_INTEL_SDP_YEAR}"/*
		fi

		ebegin "Cleaning out documentation"
		rm -r "opt/intel/documentation_${_INTEL_SDP_YEAR}" || die
		rm "${INTEL_SDP_DIR}"/linux/{documentation,man} || die
		eend
	fi

	# handle examples
	if path_exists "opt/intel/samples_${_INTEL_SDP_YEAR}"; then
		if use examples; then
			if ! use linguas_ja; then
				rm -r opt/intel/samples_"${_INTEL_SDP_YEAR}"/ja || die
			fi
			dodoc -r opt/intel/samples_"${_INTEL_SDP_YEAR}"/*
		fi

		ebegin "Cleaning out examples"
		rm -r "opt/intel/samples_${_INTEL_SDP_YEAR}" || die
		eend
	fi

	# remove eclipse
	rm -rf opt/intel/ide_support_* || die

	ebegin "Tagging ${PN}"
	find opt -name \*sh -type f -exec sed -i \
		-e "s:<.*DIR>:${INTEL_SDP_EDIR}/linux:g" \
		'{}' + || die
	eend

	[[ -d "${ED}" ]] || dodir /
	mv opt "${ED}"/ || die "moving files failed"

	dodir "${INTEL_SDP_DIR}"/licenses /opt/intel/ism/rm
	keepdir "${INTEL_SDP_DIR}"/licenses /opt/intel/ism/rm
}

# @FUNCTION: intel-sdp-r1_pkg_postinst
# @DESCRIPTION:
# Test for all things working
intel-sdp-r1_pkg_postinst() {
	debug-print-function ${FUNCNAME} "${@}"

	_isdp_run-test

	if [[ ${PN} = icc ]] && has_version ">=dev-util/ccache-3.1.9-r2" ; then
		#add ccache links as icc might get installed after ccache
		"${EROOT}"/usr/bin/ccache-config --install-links
	fi

	elog "Beginning with the 2016 suite of Intel tools, Gentoo has removed"
	elog "support for the eclipse plugin. If you require the IDE support,"
	elog "you will have to install the suite on your own, outside portage."
}

# @FUNCTION: intel-sdp-r1_pkg_postrm
# @DESCRIPTION:
# Sanitize cache links
intel-sdp-r1_pkg_postrm() {
	debug-print-function ${FUNCNAME} "${@}"

	if [[ ${PN} = icc ]] && has_version ">=dev-util/ccache-3.1.9-r2" && [[ -z ${REPLACED_BY_VERSION} ]]; then
		# --remove-links would remove all links, --install-links updates them
		"${EROOT}"/usr/bin/ccache-config --install-links
	fi
}

EXPORT_FUNCTIONS pkg_setup src_unpack src_install pkg_postinst pkg_postrm pkg_pretend

_INTEL_SDP_R1_ECLASS_=1
fi

[-- Attachment #1.3: intel-sdp.eclass.diff --]
[-- Type: text/plain, Size: 18158 bytes --]

--- eclass/intel-sdp.eclass	2016-02-15 09:15:03.000000000 +0100
+++ /Volumes/iExtra/justin/src/gentoo/science/eclass/intel-sdp-r1.eclass	2016-02-15 09:13:59.000000000 +0100
@@ -2,16 +2,17 @@
 # Distributed under the terms of the GNU General Public License v2
 # $Id$
 
-# @ECLASS: intel-sdp.eclass
+# @ECLASS: intel-sdp-r1.eclass
 # @MAINTAINER:
 # Justin Lecher <jlec@gentoo.org>
+# David Seifert <soap@gentoo.org>
 # Sci Team <sci@gentoo.org>
 # @BLURB: Handling of Intel's Software Development Products package management
 
-if [[ ! ${_INTEL_SDP_ECLASS_} ]]; then
+if [[ ! ${_INTEL_SDP_R1_ECLASS_} ]]; then
 
 case "${EAPI:-0}" in
-	5) ;;
+	6) ;;
 	*) die "EAPI=${EAPI} is not supported" ;;
 esac
 
@@ -22,7 +23,7 @@
 # To find out its value, see the links to download in
 # https://registrationcenter.intel.com/RegCenter/MyProducts.aspx
 #
-# e.g. 2504
+# e.g. 8365
 #
 # Must be defined before inheriting the eclass
 
@@ -44,7 +45,7 @@
 # To find out its value, see the links to download in
 # https://registrationcenter.intel.com/RegCenter/MyProducts.aspx
 #
-# e.g. 2011_sp1_update2
+# e.g. 2016_update1
 #
 # Must be defined before inheriting the eclass
 
@@ -80,56 +81,74 @@
 # @DESCRIPTION:
 # 32bit arch in rpm names
 #
-# e.g. i484
+# e.g. i486
 : ${INTEL_X86:=i486}
 
 # @ECLASS-VARIABLE: INTEL_BIN_RPMS
 # @DESCRIPTION:
 # Functional name of rpm without any version/arch tag
+# Has to be a bash array
 #
-# e.g. compilerprof
+# e.g. ("icc-l-all-devel")
 #
-# if the rpm is located in a directory different to INTEL_RPMS_DIR you can
+# if the rpm is located in a directory other than INTEL_RPMS_DIR you can
 # specify the full path
 #
 # e.g. CLI_install/rpm/intel-vtune-amplifier-xe-cli
+: ${INTEL_BIN_RPMS:=()}
+
+# @ECLASS-VARIABLE: INTEL_AMD64_RPMS
+# @DESCRIPTION:
+# AMD64 single arch rpms. Same syntax as INTEL_BIN_RPMS
+# Has to be a bash array
+: ${INTEL_AMD64_RPMS:=()}
+
+# @ECLASS-VARIABLE: INTEL_X86_RPMS
+# @DESCRIPTION:
+# X86 single arch rpms. Same syntax as INTEL_BIN_RPMS
+# Has to be a bash array
+: ${INTEL_X86_RPMS:=()}
 
 # @ECLASS-VARIABLE: INTEL_DAT_RPMS
 # @DESCRIPTION:
 # Functional name of rpm of common data which are arch free
 # without any version tag
+# Has to be a bash array
 #
-# e.g. openmp
+# e.g. ("openmp-l-all-devel")
 #
 # if the rpm is located in a directory different to INTEL_RPMS_DIR you can
 # specify the full path
 #
 # e.g. CLI_install/rpm/intel-vtune-amplifier-xe-cli-common
-: ${INTEL_DAT_RPMS:=""}
+: ${INTEL_DAT_RPMS:=()}
 
 # @ECLASS-VARIABLE: INTEL_SINGLE_ARCH
 # @DESCRIPTION:
 # Unset, if only the multilib package will be provided by intel
 : ${INTEL_SINGLE_ARCH:=true}
 
-# @ECLASS-VARIABLE: INTEL_SDP_DB
-# @DESCRIPTION:
-# Full path to intel registry db
-INTEL_SDP_DB="${EROOT%/}"/opt/intel/intel-sdp-products.db
+MULTILIB_COMPAT=( abi_x86_{32,64} )
 
-inherit check-reqs eutils multilib versionator
+inherit check-reqs eutils multilib-build versionator
 
 _INTEL_PV1=$(get_version_component_range 1)
 _INTEL_PV2=$(get_version_component_range 2)
 _INTEL_PV3=$(get_version_component_range 3)
 _INTEL_PV4=$(get_version_component_range 4)
-_INTEL_URI="http://registrationcenter-download.intel.com/irc_nas/${INTEL_DID}/${INTEL_DPN}"
+_INTEL_PV=""
+[[ -n ${_INTEL_PV4} ]] && _INTEL_PV+="${_INTEL_PV4}-"
+[[ -n ${_INTEL_PV1} ]] && _INTEL_PV+="${_INTEL_PV1}"
+[[ -n ${_INTEL_PV2} ]] && _INTEL_PV+=".${_INTEL_PV2}"
+[[ -n ${_INTEL_PV3} ]] && _INTEL_PV+=".${_INTEL_PV3}"
+[[ -n ${_INTEL_PV4} ]] && _INTEL_PV+="-${_INTEL_PV4}"
+
+_INTEL_URI="http://registrationcenter-download.intel.com/akdlm/irc_nas/${INTEL_DID}/${INTEL_DPN}"
 
 if [ ${INTEL_SINGLE_ARCH} == true ]; then
 	SRC_URI="
-		amd64? ( multilib? ( ${_INTEL_URI}_${INTEL_DPV}.${INTEL_TARX} ) )
-		amd64? ( !multilib? ( ${_INTEL_URI}_${INTEL_DPV}_intel64.${INTEL_TARX} ) )
-		x86?	( ${_INTEL_URI}_${INTEL_DPV}_ia32.${INTEL_TARX} )"
+		abi_x86_32? ( ${_INTEL_URI}_${INTEL_DPV}_ia32.${INTEL_TARX} )
+		abi_x86_64? ( ${_INTEL_URI}_${INTEL_DPV}_intel64.${INTEL_TARX} )"
 else
 	SRC_URI="${_INTEL_URI}_${INTEL_DPV}.${INTEL_TARX}"
 fi
@@ -138,20 +157,22 @@
 # Future work, #394411
 #SLOT="${_INTEL_PV1}.${_INTEL_PV2}"
 SLOT="0"
-IUSE="examples multilib"
 
 RESTRICT="mirror"
 
 RDEPEND=""
 DEPEND="app-arch/rpm2targz"
 
-_INTEL_SDP_YEAR=${INTEL_DPV%_update*}
-_INTEL_SDP_YEAR=${INTEL_DPV%_sp*}
+_INTEL_SDP_YEAR=${INTEL_DPV}
+_INTEL_SDP_YEAR=${_INTEL_SDP_YEAR%_sp*}
+_INTEL_SDP_YEAR=${_INTEL_SDP_YEAR%_update*}
 
 # @ECLASS-VARIABLE: INTEL_SDP_DIR
 # @DESCRIPTION:
 # Full rootless path to installation dir
-INTEL_SDP_DIR="opt/intel/${INTEL_SUBDIR}-${_INTEL_SDP_YEAR:-${_INTEL_PV1}}.${_INTEL_PV3}.${_INTEL_PV4}"
+INTEL_SDP_DIR="opt/intel/${INTEL_SUBDIR}_${_INTEL_SDP_YEAR:-${_INTEL_PV1}}"
+[[ -n ${_INTEL_PV3} ]] && INTEL_SDP_DIR+=".${_INTEL_PV3}"
+[[ -n ${_INTEL_PV4} ]] && INTEL_SDP_DIR+=".${_INTEL_PV4}"
 
 # @ECLASS-VARIABLE: INTEL_SDP_EDIR
 # @DESCRIPTION:
@@ -169,34 +190,6 @@
 #
 # e.g. amd64-multilib -> INTEL_ARCH="intel64 ia32"
 
-# @FUNCTION: _isdp_link_eclipse_plugins
-# @INTERNAL
-# @DESCRIPTION:
-# Creating necessary links to use intel compiler with eclipse
-_isdp_link_eclipse_plugins() {
-	debug-print-function ${FUNCNAME} "${@}"
-
-	local c f
-	pushd ${INTEL_SDP_DIR}/eclipse_support > /dev/null || die
-		for c in cdt*; do
-			local cv=${c#cdt} ev=3.$(( ${cv:0:1} - 1))
-			if has_version "dev-util/eclipse-sdk:${ev}"; then
-				einfo "Linking eclipse (v${ev}) plugin cdt (v${cv})"
-				for f in cdt${cv}/eclipse/features/*; do
-					dodir /usr/$(get_libdir)/eclipse-${ev}/features
-					dosym "${INTEL_SDP_EDIR}"/eclipse_support/${f} \
-						/usr/$(get_libdir)/eclipse-${ev}/features/ || die
-				done
-				for f in cdt${cv}/eclipse/plugins/*; do
-					dodir /usr/$(get_libdir)/eclipse-${ev}/plugins
-					dosym "${INTEL_SDP_EDIR}"/eclipse_support/${f} \
-						/usr/$(get_libdir)/eclipse-${ev}/plugins/ || die
-				done
-			fi
-		done
-	popd > /dev/null || die
-}
-
 # @FUNCTION: _isdp_big-warning
 # @USAGE: [pre-check | test-failed]
 # @INTERNAL
@@ -212,7 +205,7 @@
 			;;
 
 		test-failed )
-			echo
+			echo ""
 			ewarn "Function test failed. Most probably due to an invalid license."
 			ewarn "This means you already tried to bypass the license check once."
 			;;
@@ -222,7 +215,12 @@
 	ewarn "Make sure you have received an Intel license."
 	ewarn "To receive a non-commercial license, you need to register at:"
 	ewarn "https://software.intel.com/en-us/qualify-for-free-software"
-	ewarn "Install the license file into ${INTEL_SDP_EDIR}/licenses/"
+	ewarn "Install the license file into ${EPREFIX}/opt/intel/licenses"
+	ewarn ""
+	ewarn "Beginning with the 2016 suite of tools, license files are keyed"
+	ewarn "to the MAC address of the eth0 interface. In order to retrieve"
+	ewarn "a personalized license file, follow the instructions at"
+	ewarn "https://software.intel.com/en-us/articles/how-do-i-get-my-license-file-for-intel-parallel-studio-xe-2016"
 
 	case ${1} in
 		pre-check )
@@ -260,10 +258,10 @@
 	for arch in ${INTEL_ARCH}; do
 		case ${EBUILD_PHASE} in
 			install )
-				comp_full="${ED}/${INTEL_SDP_DIR}/bin/${arch}/${comp}"
+				comp_full="${ED}/${INTEL_SDP_DIR}/linux/bin/${arch}/${comp}"
 				;;
 			postinst )
-				comp_full="${INTEL_SDP_EDIR}/bin/${arch}/${comp}"
+				comp_full="${INTEL_SDP_EDIR}/linux/bin/${arch}/${comp}"
 				;;
 			* )
 				ewarn "Compile test not supported in ${EBUILD_PHASE}"
@@ -271,9 +269,9 @@
 				;;
 		esac
 
-		debug-print "LD_LIBRARY_PATH=\"${INTEL_SDP_EDIR}/bin/${arch}/\" \"${comp_full}\" -V"
+		debug-print "LD_LIBRARY_PATH=\"${INTEL_SDP_EDIR}/linux/bin/${arch}/\" \"${comp_full}\" -V"
 
-		LD_LIBRARY_PATH="${INTEL_SDP_EDIR}/bin/${arch}/" "${comp_full}" -V &>/dev/null
+		LD_LIBRARY_PATH="${INTEL_SDP_EDIR}/linux/bin/${arch}/" "${comp_full}" -V &>/dev/null
 		[[ $? -ne 0 ]] && warn=yes
 	done
 	[[ "${warn}" == "yes" ]] && _isdp_big-warning test-failed
@@ -297,13 +295,34 @@
 	fi
 }
 
-# @FUNCTION: intel-sdp_pkg_pretend
+# @FUNCTION: convert2intel_arch
+# @USAGE: <arch>
+# @DESCRIPTION:
+# Convert between portage arch (e.g. amd64, x86) and intel arch
+# nomenclature (e.g. intel64, ia32)
+convert2intel_arch() {
+	debug-print-function ${FUNCNAME} "${@}"
+
+	case $1 in
+		amd64|abi_x86_64|*amd64*)
+			echo "intel64"
+			;;
+		x86|abi_x86_32|*x86*)
+			echo "ia32"
+			;;
+		*)
+			die "Abi \'$1\' is unsupported"
+			;;
+	esac
+}
+
+# @FUNCTION: intel-sdp-r1_pkg_pretend
 # @DESCRIPTION:
 # @CODE
 # * Check that the user has a (valid) license file before going on.
 # * Check for space requirements being fullfilled
 # @CODE
-intel-sdp_pkg_pretend() {
+intel-sdp-r1_pkg_pretend() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	local warn=1 dir dirs ret arch a p
@@ -319,9 +338,9 @@
 		fi
 
 		dirs=(
+			"${EPREFIX}/opt/intel/licenses"
 			"${INTEL_SDP_EDIR}/licenses"
 			"${INTEL_SDP_EDIR}/Licenses"
-			"${EPREFIX}/opt/intel/licenses"
 			)
 		for dir in "${dirs[@]}" ; do
 			ebegin "Checking for a license in: ${dir}"
@@ -338,63 +357,108 @@
 			die "Could not find license file"
 		fi
 	else
-		eqawarn "The ebuild doesn't check for presents of a proper intel license!"
-		eqawarn "This shouldn't be done unless there is a serious reason."
+		eqawarn "The ebuild doesn't check for presence of a proper intel license!"
+		eqawarn "This shouldn't be done unless there is a very good reason."
 	fi
 }
 
-# @FUNCTION: intel-sdp_pkg_setup
+# @FUNCTION: intel-sdp-r1_pkg_setup
 # @DESCRIPTION:
 # Setting up and sorting some internal variables
-intel-sdp_pkg_setup() {
+intel-sdp-r1_pkg_setup() {
 	debug-print-function ${FUNCNAME} "${@}"
 	local arch a p
 
-	if use x86; then
-		arch=${INTEL_X86}
-		INTEL_ARCH="ia32"
-	elif use amd64; then
-		arch=x86_64
-		INTEL_ARCH="intel64"
-		if has_multilib_profile; then
-			arch="x86_64 ${INTEL_X86}"
-			INTEL_ARCH="intel64 ia32"
-		fi
+	INTEL_ARCH=""
+
+	if use abi_x86_64; then
+		arch+=" x86_64"
+		INTEL_ARCH+=" intel64"
+	fi
+	if use abi_x86_32; then
+		arch+=" ${INTEL_X86}"
+		INTEL_ARCH+=" ia32"
 	fi
 	INTEL_RPMS=()
 	INTEL_RPMS_FULL=()
-	if [[ $(declare -p INTEL_BIN_RPMS) = "declare -a "* ]] ; then
-		_INTEL_BIN_RPMS=( ${INTEL_BIN_RPMS[@]} )
-	else
-		read -r -d '' -a _INTEL_BIN_RPMS <<<"${INTEL_BIN_RPMS}"
-	fi
-	for p in "${_INTEL_BIN_RPMS[@]}"; do
+
+	for p in "${INTEL_BIN_RPMS[@]}"; do
 		for a in ${arch}; do
 			if [ ${p} == $(basename ${p}) ]; then
-				INTEL_RPMS+=( intel-${p}-${_INTEL_PV4}-${_INTEL_PV1}.${_INTEL_PV2}-${_INTEL_PV3}.${a}.rpm )
+				# check for variables ending in ".rpm"
+				# these are excluded from version expansion, due to Intel's
+				# idiosyncratic versioning scheme beginning with their 2016
+				# suite of tools.
+				if [[ "${p}" == *.rpm ]]; then
+					INTEL_RPMS+=( intel-${p} )
+				else
+					INTEL_RPMS+=( intel-${p}-${_INTEL_PV}.${a}.rpm )
+				fi
 			else
-				INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV4}-${_INTEL_PV1}.${_INTEL_PV2}-${_INTEL_PV3}.${a}.rpm )
+				if [[ "${p}" == *.rpm ]]; then
+					INTEL_RPMS_FULL+=( ${p} )
+				else
+					INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV}.${a}.rpm )
+				fi
 			fi
 		done
 	done
-	if [[ $(declare -p INTEL_DAT_RPMS) = "declare -a "* ]] ; then
-		_INTEL_DAT_RPMS=( ${INTEL_DAT_RPMS[@]} )
-	else
-		read -r -d '' -a _INTEL_DAT_RPMS <<<"${INTEL_DAT_RPMS}"
+
+	if use amd64; then
+		for p in "${INTEL_AMD64_RPMS[@]}"; do
+			if [ ${p} == $(basename ${p}) ]; then
+				if [[ "${p}" == *.rpm ]]; then
+					INTEL_RPMS+=( intel-${p} )
+				else
+					INTEL_RPMS+=( intel-${p}-${_INTEL_PV}.x86_64.rpm )
+				fi
+			else
+				if [[ "${p}" == *.rpm ]]; then
+					INTEL_RPMS_FULL+=( ${p} )
+				else
+					INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV}.x86_64.rpm )
+				fi
+			fi
+		done
 	fi
-	for p in "${_INTEL_DAT_RPMS[@]}"; do
+
+	for p in "${INTEL_X86_RPMS[@]}"; do
+		if [ ${p} == $(basename ${p}) ]; then
+			if [[ "${p}" == *.rpm ]]; then
+				INTEL_RPMS+=( intel-${p} )
+			else
+				INTEL_RPMS+=( intel-${p}-${_INTEL_PV}.${INTEL_X86}.rpm )
+			fi
+		else
+			if [[ "${p}" == *.rpm ]]; then
+				INTEL_RPMS_FULL+=( ${p} )
+			else
+				INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV}.${INTEL_X86}.rpm )
+			fi
+		fi
+	done
+
+	for p in "${INTEL_DAT_RPMS[@]}"; do
 		if [ ${p} == $(basename ${p}) ]; then
-			INTEL_RPMS+=( intel-${p}-${_INTEL_PV4}-${_INTEL_PV1}.${_INTEL_PV2}-${_INTEL_PV3}.noarch.rpm )
+			if [[ "${p}" == *.rpm ]]; then
+				INTEL_RPMS+=( intel-${p} )
+			else
+				INTEL_RPMS+=( intel-${p}-${_INTEL_PV}.noarch.rpm )
+			fi
 		else
-			INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV4}-${_INTEL_PV1}.${_INTEL_PV2}-${_INTEL_PV3}.noarch.rpm )
+			if [[ "${p}" == *.rpm ]]; then
+				INTEL_RPMS_FULL+=( ${p} )
+			else
+				INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV}.noarch.rpm )
+			fi
 		fi
 	done
 }
 
-# @FUNCTION: intel-sdp_src_unpack
+# @FUNCTION: intel-sdp-r1_src_unpack
 # @DESCRIPTION:
 # Unpacking necessary rpms from tarball, extract them and rearrange the output.
-intel-sdp_src_unpack() {
+intel-sdp-r1_src_unpack() {
 	local l r subdir rb t list=() debug_list
 
 	for t in ${A}; do
@@ -416,71 +480,66 @@
 
 		for r in ${list[@]}; do
 			rb=$(basename ${r})
-			l=.${rb}_$(date +'%d%m%y_%H%M%S').log
 			einfo "Unpacking ${rb}"
 			rpm2tar -O ${r} | tar xvf - | sed -e \
-				"s:^\.:${EROOT#/}:g" > ${l}; assert "unpacking ${r} failed"
-			mv ${l} opt/intel/ || die "failed moving extract log file"
+				"s:^\.:${EROOT#/}:g" > /dev/null; assert "unpacking ${r} failed"
 		done
 	done
-
-	mv opt/intel/* ${INTEL_SDP_DIR} || die "mv to INTEL_SDP_DIR failed"
 }
 
-# @FUNCTION: intel-sdp_src_install
+# @FUNCTION: intel-sdp-r1_src_install
 # @DESCRIPTION:
 # Install everything
-intel-sdp_src_install() {
+intel-sdp-r1_src_install() {
 	debug-print-function ${FUNCNAME} "${@}"
 
-	if path_exists "${INTEL_SDP_DIR}"/uninstall*; then
-		ebegin "Cleaning out uninstall information"
-		find "${INTEL_SDP_DIR}"/uninstall* -delete || die
+	# remove uninstall information
+	if path_exists opt/intel/"${INTEL_DPN}"*/uninstall; then
+		ebegin "Cleaning out uninstall"
+		rm -r opt/intel/"${INTEL_DPN}"*/uninstall || die
 		eend
 	fi
 
-	if path_exists "${INTEL_SDP_DIR}"/Documentation; then
-		dodoc -r "${INTEL_SDP_DIR}"/Documentation/*
+	# handle documentation
+	if path_exists "opt/intel/documentation_${_INTEL_SDP_YEAR}"; then
+		if path_exists "opt/intel/documentation_${_INTEL_SDP_YEAR}/en/man/common/man1"; then
+			doman opt/intel/documentation_"${_INTEL_SDP_YEAR}"/en/man/common/man1/*
+			rm -r opt/intel/documentation_"${_INTEL_SDP_YEAR}"/en/man || die
+		fi
+
+		if use doc; then
+			if ! use linguas_ja; then
+				rm -r opt/intel/documentation_"${_INTEL_SDP_YEAR}"/ja || die
+			fi
+			dodoc -r opt/intel/documentation_"${_INTEL_SDP_YEAR}"/*
+		fi
 
 		ebegin "Cleaning out documentation"
-		find "${INTEL_SDP_DIR}"/Documentation -delete || die
+		rm -r "opt/intel/documentation_${_INTEL_SDP_YEAR}" || die
+		rm "${INTEL_SDP_DIR}"/linux/{documentation,man} || die
 		eend
 	fi
 
-	if path_exists "${INTEL_SDP_DIR}"/Samples; then
-		if use examples ; then
-			insinto /usr/share/${P}/examples/
-			doins -r "${INTEL_SDP_DIR}"/Samples/*
+	# handle examples
+	if path_exists "opt/intel/samples_${_INTEL_SDP_YEAR}"; then
+		if use examples; then
+			if ! use linguas_ja; then
+				rm -r opt/intel/samples_"${_INTEL_SDP_YEAR}"/ja || die
+			fi
+			dodoc -r opt/intel/samples_"${_INTEL_SDP_YEAR}"/*
 		fi
+
 		ebegin "Cleaning out examples"
-		find "${INTEL_SDP_DIR}"/Samples -delete || die
+		rm -r "opt/intel/samples_${_INTEL_SDP_YEAR}" || die
 		eend
 	fi
 
-	if path_exists "${INTEL_SDP_DIR}"/eclipse_support; then
-		if has eclipse ${IUSE} && use eclipse; then
-			_isdp_link_eclipse_plugins
-		else
-			ebegin "Cleaning out eclipse plugin"
-			find "${INTEL_SDP_DIR}"/eclipse_support -delete || die
-			eend
-		fi
-	fi
-
-	if path_exists "${INTEL_SDP_DIR}"/man; then
-		path_exists "${INTEL_SDP_DIR}"/man/en_US/man1/* && \
-			doman "${INTEL_SDP_DIR}"/man/en_US/man1/*
-		path_exists "${INTEL_SDP_DIR}"/man/man1/* && \
-			doman "${INTEL_SDP_DIR}"/man/man1/*
-		has linguas_ja ${IUSE} && use linguas_ja && \
-			doman -i18n=ja_JP "${INTEL_SDP_DIR}"/man/ja_JP/man1/*
-
-		find "${INTEL_SDP_DIR}"/man -delete || die
-	fi
+	# remove eclipse
+	rm -rf opt/intel/ide_support_* || die
 
 	ebegin "Tagging ${PN}"
 	find opt -name \*sh -type f -exec sed -i \
-		-e "s:<.*DIR>:${INTEL_SDP_EDIR}:g" \
+		-e "s:<.*DIR>:${INTEL_SDP_EDIR}/linux:g" \
 		'{}' + || die
 	eend
 
@@ -491,43 +550,30 @@
 	keepdir "${INTEL_SDP_DIR}"/licenses /opt/intel/ism/rm
 }
 
-# @FUNCTION: intel-sdp_pkg_postinst
+# @FUNCTION: intel-sdp-r1_pkg_postinst
 # @DESCRIPTION:
-# Add things to intel database
-intel-sdp_pkg_postinst() {
+# Test for all things working
+intel-sdp-r1_pkg_postinst() {
 	debug-print-function ${FUNCNAME} "${@}"
 
-	# add product registry to intel "database"
-	local l r
-	for r in ${INTEL_RPMS}; do
-		l="$(ls -1 ${EROOT%/}/opt/intel/.${r}_*.log | head -n 1)"
-		echo >> ${INTEL_SDP_DB} \
-			"<:${r%-${_INTEL_PV4}*}-${_INTEL_PV4}:${r}:${INTEL_SDP_EDIR}:${l}:>"
-	done
 	_isdp_run-test
 
 	if [[ ${PN} = icc ]] && has_version ">=dev-util/ccache-3.1.9-r2" ; then
 		#add ccache links as icc might get installed after ccache
 		"${EROOT}"/usr/bin/ccache-config --install-links
 	fi
+
+	elog "Beginning with the 2016 suite of Intel tools, Gentoo has removed"
+	elog "support for the eclipse plugin. If you require the IDE support,"
+	elog "you will have to install the suite on your own, outside portage."
 }
 
-# @FUNCTION: intel-sdp_pkg_postrm
+# @FUNCTION: intel-sdp-r1_pkg_postrm
 # @DESCRIPTION:
-# Sanitize intel database
-intel-sdp_pkg_postrm() {
+# Sanitize cache links
+intel-sdp-r1_pkg_postrm() {
 	debug-print-function ${FUNCNAME} "${@}"
 
-	# remove from intel "database"
-	if [[ -e ${INTEL_SDP_DB} ]]; then
-		local r
-		for r in ${INTEL_RPMS}; do
-			sed -i \
-				-e "/${r}/d" \
-				${INTEL_SDP_DB}
-		done
-	fi
-
 	if [[ ${PN} = icc ]] && has_version ">=dev-util/ccache-3.1.9-r2" && [[ -z ${REPLACED_BY_VERSION} ]]; then
 		# --remove-links would remove all links, --install-links updates them
 		"${EROOT}"/usr/bin/ccache-config --install-links
@@ -536,5 +582,5 @@
 
 EXPORT_FUNCTIONS pkg_setup src_unpack src_install pkg_postinst pkg_postrm pkg_pretend
 
-_INTEL_SDP_ECLASS_=1
+_INTEL_SDP_R1_ECLASS_=1
 fi

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [gentoo-dev] RFC: intel-sdp-r1.eclass
  2016-02-15  8:16 [gentoo-dev] RFC: intel-sdp-r1.eclass Justin Lecher (jlec)
@ 2016-02-15 12:59 ` Michał Górny
  2016-02-15 13:37   ` Justin Lecher (jlec)
  0 siblings, 1 reply; 12+ messages in thread
From: Michał Górny @ 2016-02-15 12:59 UTC (permalink / raw
  To: Justin Lecher (jlec); +Cc: Gentoo Dev

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

On Mon, 15 Feb 2016 09:16:53 +0100
"Justin Lecher (jlec)" <jlec@gentoo.org> wrote:

> # Copyright 1999-2016 Gentoo Foundation
> # Distributed under the terms of the GNU General Public License v2
> # $Id$
> 
> # @ECLASS: intel-sdp-r1.eclass
> # @MAINTAINER:
> # Justin Lecher <jlec@gentoo.org>
> # David Seifert <soap@gentoo.org>
> # Sci Team <sci@gentoo.org>
> # @BLURB: Handling of Intel's Software Development Products package management
> 
> if [[ ! ${_INTEL_SDP_R1_ECLASS_} ]]; then
> 
> case "${EAPI:-0}" in

:-0 is meaningless here.

> 	6) ;;
> 	*) die "EAPI=${EAPI} is not supported" ;;

If at all, it could be helpful here.

> esac
> 
> # @ECLASS-VARIABLE: INTEL_DID
> # @DEFAULT_UNSET
> # @DESCRIPTION:
> # The package download ID from Intel.
> # To find out its value, see the links to download in
> # https://registrationcenter.intel.com/RegCenter/MyProducts.aspx
> #
> # e.g. 8365
> #
> # Must be defined before inheriting the eclass
> 
> # @ECLASS-VARIABLE: INTEL_DPN
> # @DEFAULT_UNSET
> # @DESCRIPTION:
> # The package name to download from Intel.
> # To find out its value, see the links to download in
> # https://registrationcenter.intel.com/RegCenter/MyProducts.aspx
> #
> # e.g. parallel_studio_xe
> #
> # Must be defined before inheriting the eclass
> 
> # @ECLASS-VARIABLE: INTEL_DPV
> # @DEFAULT_UNSET
> # @DESCRIPTION:
> # The package download version from Intel.
> # To find out its value, see the links to download in
> # https://registrationcenter.intel.com/RegCenter/MyProducts.aspx
> #
> # e.g. 2016_update1
> #
> # Must be defined before inheriting the eclass
> 
> # @ECLASS-VARIABLE: INTEL_TARX
> # @DESCRIPTION:
> # The package extention.

Extension. Or if you're not on Windows, then 'suffix'.

> # To find out its value, see the links to download in
> # https://registrationcenter.intel.com/RegCenter/MyProducts.aspx
> #
> # e.g. tar.gz
> #
> # Must be defined before inheriting the eclass
> : ${INTEL_TARX:=tgz}
> 
> # @ECLASS-VARIABLE: INTEL_SUBDIR
> # @DEFAULT_UNSET
> # @DESCRIPTION:
> # The package sub-directory where it will end-up in /opt/intel
> # To find out its value, you have to do a raw install from the Intel tar ball

To be honest, I find this kinda terrible. There's a huge block of docs
which makes me feel small and confused. Maybe it'd useful to give some
semi-complete example on top (in global doc)?

> # @ECLASS-VARIABLE: INTEL_SKIP_LICENSE
> # @DEFAULT_UNSET
> # @DESCRIPTION:
> # Possibility to skip the mandatory check for licenses. Only set this if there
> # is really no fix.
> 
> # @ECLASS-VARIABLE: INTEL_RPMS_DIR
> # @DESCRIPTION:
> # Main subdirectory which contains the rpms to extract.
> : ${INTEL_RPMS_DIR:=rpm}
> 
> # @ECLASS-VARIABLE: INTEL_X86
> # @DESCRIPTION:
> # 32bit arch in rpm names
> #
> # e.g. i486
> : ${INTEL_X86:=i486}
> 
> # @ECLASS-VARIABLE: INTEL_BIN_RPMS
> # @DESCRIPTION:
> # Functional name of rpm without any version/arch tag
> # Has to be a bash array
> #
> # e.g. ("icc-l-all-devel")
> #
> # if the rpm is located in a directory other than INTEL_RPMS_DIR you can
> # specify the full path
> #
> # e.g. CLI_install/rpm/intel-vtune-amplifier-xe-cli
> : ${INTEL_BIN_RPMS:=()}

$ : ${foo:=()}
$ declare -p foo
declare -- foo="()"

In other words, it doesn't work the way you expect it to.

> # @ECLASS-VARIABLE: INTEL_AMD64_RPMS
> # @DESCRIPTION:
> # AMD64 single arch rpms. Same syntax as INTEL_BIN_RPMS
> # Has to be a bash array
> : ${INTEL_AMD64_RPMS:=()}
> 
> # @ECLASS-VARIABLE: INTEL_X86_RPMS
> # @DESCRIPTION:
> # X86 single arch rpms. Same syntax as INTEL_BIN_RPMS
> # Has to be a bash array
> : ${INTEL_X86_RPMS:=()}
> 
> # @ECLASS-VARIABLE: INTEL_DAT_RPMS
> # @DESCRIPTION:
> # Functional name of rpm of common data which are arch free
> # without any version tag
> # Has to be a bash array
> #
> # e.g. ("openmp-l-all-devel")
> #
> # if the rpm is located in a directory different to INTEL_RPMS_DIR you can
> # specify the full path
> #
> # e.g. CLI_install/rpm/intel-vtune-amplifier-xe-cli-common
> : ${INTEL_DAT_RPMS:=()}
> 
> # @ECLASS-VARIABLE: INTEL_SINGLE_ARCH
> # @DESCRIPTION:
> # Unset, if only the multilib package will be provided by intel
> : ${INTEL_SINGLE_ARCH:=true}

This is really weird. It sounds like I'm supposed to do:

  inherit intel-sdp-r1
  unset INTEL_SINGLE_ARCH

I suggest you used positive logic instead.

> MULTILIB_COMPAT=( abi_x86_{32,64} )
> 
> inherit check-reqs eutils multilib-build versionator
> 
> _INTEL_PV1=$(get_version_component_range 1)
> _INTEL_PV2=$(get_version_component_range 2)
> _INTEL_PV3=$(get_version_component_range 3)
> _INTEL_PV4=$(get_version_component_range 4)

I'm pretty sure there's a better way than calling the same function
four times. For example:

  _INTEL_PV=( $(get_version_components) )

Plus, please reduce the environment pollution. Either unset all those
variables when no longer needed, or preferably create a function for
setting globals and make them local.

> _INTEL_PV=""
> [[ -n ${_INTEL_PV4} ]] && _INTEL_PV+="${_INTEL_PV4}-"
> [[ -n ${_INTEL_PV1} ]] && _INTEL_PV+="${_INTEL_PV1}"
> [[ -n ${_INTEL_PV2} ]] && _INTEL_PV+=".${_INTEL_PV2}"
> [[ -n ${_INTEL_PV3} ]] && _INTEL_PV+=".${_INTEL_PV3}"
> [[ -n ${_INTEL_PV4} ]] && _INTEL_PV+="-${_INTEL_PV4}"

Now, this is crazy ;-). I don't see immediately how to improve that,
but I suggest you thought about that.

Plus, it's kinda confusing having _INTEL_PV and INTEL_DPV used in all
different places. Could you try to use more explanatory names?

> _INTEL_URI="http://registrationcenter-download.intel.com/akdlm/irc_nas/${INTEL_DID}/${INTEL_DPN}"
> 
> if [ ${INTEL_SINGLE_ARCH} == true ]; then

Please use [[ ]] for consistency, and quoting safety.

> 	SRC_URI="
> 		abi_x86_32? ( ${_INTEL_URI}_${INTEL_DPV}_ia32.${INTEL_TARX} )
> 		abi_x86_64? ( ${_INTEL_URI}_${INTEL_DPV}_intel64.${INTEL_TARX} )"
> else
> 	SRC_URI="${_INTEL_URI}_${INTEL_DPV}.${INTEL_TARX}"
> fi
> 
> LICENSE="Intel-SDP"
> # Future work, #394411
> #SLOT="${_INTEL_PV1}.${_INTEL_PV2}"
> SLOT="0"
> 
> RESTRICT="mirror"
> 
> RDEPEND=""
> DEPEND="app-arch/rpm2targz"
> 
> _INTEL_SDP_YEAR=${INTEL_DPV}
> _INTEL_SDP_YEAR=${_INTEL_SDP_YEAR%_sp*}
> _INTEL_SDP_YEAR=${_INTEL_SDP_YEAR%_update*}
> 
> # @ECLASS-VARIABLE: INTEL_SDP_DIR
> # @DESCRIPTION:
> # Full rootless path to installation dir
> INTEL_SDP_DIR="opt/intel/${INTEL_SUBDIR}_${_INTEL_SDP_YEAR:-${_INTEL_PV1}}"
> [[ -n ${_INTEL_PV3} ]] && INTEL_SDP_DIR+=".${_INTEL_PV3}"
> [[ -n ${_INTEL_PV4} ]] && INTEL_SDP_DIR+=".${_INTEL_PV4}"
> 
> # @ECLASS-VARIABLE: INTEL_SDP_EDIR
> # @DESCRIPTION:
> # Full rooted path to installation dir
> INTEL_SDP_EDIR="${EROOT%/}/${INTEL_SDP_DIR}"

EROOT can't be used in global scope. Doing things like this, you could
end up with wrong value being carried on to phases.

> S="${WORKDIR}"
> 
> QA_PREBUILT="${INTEL_SDP_DIR}/*"
> 
> # @ECLASS-VARIABLE: INTEL_ARCH
> # @DEFAULT_UNSET
> # @DESCRIPTION:
> # Intels internal names of the arches; will be set at runtime accordingly
> #
> # e.g. amd64-multilib -> INTEL_ARCH="intel64 ia32"

So we're not supposed to set this?

> # @FUNCTION: _isdp_big-warning
> # @USAGE: [pre-check | test-failed]
> # @INTERNAL
> # @DESCRIPTION:
> # warn user that we really require a license

Description really need to be improved. They should tell how to call
the function and what it does accordingly. This applies to all
functions.

> _isdp_big-warning() {
> 	debug-print-function ${FUNCNAME} "${@}"
> 
> 	case ${1} in
> 		pre-check )
> 			echo ""

Don't mix echo with ewarn.

> 			ewarn "License file not found!"
> 			;;
> 
> 		test-failed )
> 			echo ""
> 			ewarn "Function test failed. Most probably due to an invalid license."
> 			ewarn "This means you already tried to bypass the license check once."
> 			;;
> 	esac
> 
> 	echo ""
> 	ewarn "Make sure you have received an Intel license."
> 	ewarn "To receive a non-commercial license, you need to register at:"
> 	ewarn "https://software.intel.com/en-us/qualify-for-free-software"
> 	ewarn "Install the license file into ${EPREFIX}/opt/intel/licenses"
> 	ewarn ""
> 	ewarn "Beginning with the 2016 suite of tools, license files are keyed"
> 	ewarn "to the MAC address of the eth0 interface. In order to retrieve"
> 	ewarn "a personalized license file, follow the instructions at"
> 	ewarn "https://software.intel.com/en-us/articles/how-do-i-get-my-license-file-for-intel-parallel-studio-xe-2016"
> 
> 	case ${1} in
> 		pre-check )
> 			ewarn "before proceeding with installation of ${P}"
> 			echo ""
> 			;;
> 		* )
> 			echo ""
> 			;;
> 			esac
> }
> 
> # @FUNCTION: _isdp_version_test
> # @INTERNAL
> # @DESCRIPTION:
> # Testing for valid license by asking for version information of the compiler
> _isdp_version_test() {
> 	debug-print-function ${FUNCNAME} "${@}"
> 
> 	local comp comp_full arch warn
> 	case ${PN} in
> 		ifc )
> 			debug-print "Testing ifort"
> 			comp=ifort
> 			;;
> 		icc )
> 			debug-print "Testing icc"
> 			comp=icc
> 			;;
> 		*)
> 			die "${PN} is not supported for testing"
> 			;;
> 	esac
> 
> 	for arch in ${INTEL_ARCH}; do
> 		case ${EBUILD_PHASE} in
> 			install )
> 				comp_full="${ED}/${INTEL_SDP_DIR}/linux/bin/${arch}/${comp}"

Double slash imminent (ED has one).

> 				;;
> 			postinst )
> 				comp_full="${INTEL_SDP_EDIR}/linux/bin/${arch}/${comp}"
> 				;;
> 			* )
> 				ewarn "Compile test not supported in ${EBUILD_PHASE}"

Why not die? It's a new eclass after all.

> 				continue
> 				;;
> 		esac
> 
> 		debug-print "LD_LIBRARY_PATH=\"${INTEL_SDP_EDIR}/linux/bin/${arch}/\" \"${comp_full}\" -V"
> 
> 		LD_LIBRARY_PATH="${INTEL_SDP_EDIR}/linux/bin/${arch}/" "${comp_full}" -V &>/dev/null
> 		[[ $? -ne 0 ]] && warn=yes

Why not just:

  LD_L...  "${comp_full}" -V &>/dev/null || warn=yes

> 	done
> 	[[ "${warn}" == "yes" ]] && _isdp_big-warning test-failed

Unnecessary quoting.

> }
> 
> # @FUNCTION: _isdp_run-test
> # @INTERNAL
> # @DESCRIPTION:
> # Test if installed compiler is working
> _isdp_run-test() {
> 	debug-print-function ${FUNCNAME} "${@}"
> 
> 	if [[ -z ${INTEL_SKIP_LICENSE} ]]; then
> 		case ${PN} in
> 			ifc | icc )
> 				_isdp_version_test ;;
> 			* )
> 				debug-print "No test available for ${PN}"
> 				;;
> 		esac
> 	fi
> }
> 
> # @FUNCTION: convert2intel_arch
> # @USAGE: <arch>
> # @DESCRIPTION:
> # Convert between portage arch (e.g. amd64, x86) and intel arch
> # nomenclature (e.g. intel64, ia32)
> convert2intel_arch() {

Namespace, please.

> 	debug-print-function ${FUNCNAME} "${@}"
> 
> 	case $1 in
> 		amd64|abi_x86_64|*amd64*)

Err, *amd64* catches amd64, you know.

> 			echo "intel64"
> 			;;
> 		x86|abi_x86_32|*x86*)
> 			echo "ia32"
> 			;;
> 		*)
> 			die "Abi \'$1\' is unsupported"
> 			;;
> 	esac
> }
> 
> # @FUNCTION: intel-sdp-r1_pkg_pretend
> # @DESCRIPTION:
> # @CODE
> # * Check that the user has a (valid) license file before going on.
> # * Check for space requirements being fullfilled

fulfilled.

> # @CODE

Err, this is not code, you know.

> intel-sdp-r1_pkg_pretend() {
> 	debug-print-function ${FUNCNAME} "${@}"
> 
> 	local warn=1 dir dirs ret arch a p
> 
> 	: ${CHECKREQS_DISK_BUILD:=256M}
> 	check-reqs_pkg_pretend
> 
> 	if [[ -z ${INTEL_SKIP_LICENSE} ]]; then
> 		if echo ${INTEL_LICENSE_FILE} | grep -q @; then

Err... is this some fancy way of saying:

  [[ ${INTEL_LICENSE_FILE} == *@* ]]

? Or is @ special here?

> 			einfo "Looks like you are using following license server:"
> 			einfo "   ${INTEL_LICENSE_FILE}"
> 			return 0
> 		fi
> 
> 		dirs=(
> 			"${EPREFIX}/opt/intel/licenses"
> 			"${INTEL_SDP_EDIR}/licenses"
> 			"${INTEL_SDP_EDIR}/Licenses"
> 			)
> 		for dir in "${dirs[@]}" ; do
> 			ebegin "Checking for a license in: ${dir}"
> 			#maybe use nullglob or [[ $(echo ${dir/*lic) != "${dir}/*lic" ]]
> 			[[ $( ls "${dir}"/*lic 2>/dev/null ) ]]; ret=$?

Maybe you should use something sane indeed.

> 			eend ${ret}
> 			if [[ ${ret} == "0" ]]; then
> 				warn=${ret}
> 				break
> 			fi

I would have to check PMS here but I think eend preserves (returns)
the status passed to it.

> 		done
> 		if [[ ${warn} == "1" ]]; then
> 			_isdp_big-warning pre-check
> 			die "Could not find license file"
> 		fi
> 	else
> 		eqawarn "The ebuild doesn't check for presence of a proper intel license!"
> 		eqawarn "This shouldn't be done unless there is a very good reason."

This looks like a bad idea. You either require it, and fail it doesn't
exist, or let people disable it. You don't warn endlessly if this is
allowed.

> 	fi
> }
> 
> # @FUNCTION: intel-sdp-r1_pkg_setup
> # @DESCRIPTION:
> # Setting up and sorting some internal variables
> intel-sdp-r1_pkg_setup() {
> 	debug-print-function ${FUNCNAME} "${@}"
> 	local arch a p
> 
> 	INTEL_ARCH=""
> 
> 	if use abi_x86_64; then
> 		arch+=" x86_64"
> 		INTEL_ARCH+=" intel64"

Wouldn't it be better to use an array here?

> 	fi
> 	if use abi_x86_32; then
> 		arch+=" ${INTEL_X86}"
> 		INTEL_ARCH+=" ia32"
> 	fi
> 	INTEL_RPMS=()
> 	INTEL_RPMS_FULL=()
> 
> 	for p in "${INTEL_BIN_RPMS[@]}"; do
> 		for a in ${arch}; do
> 			if [ ${p} == $(basename ${p}) ]; then

${p##*/}.

> 				# check for variables ending in ".rpm"
> 				# these are excluded from version expansion, due to Intel's
> 				# idiosyncratic versioning scheme beginning with their 2016
> 				# suite of tools.
> 				if [[ "${p}" == *.rpm ]]; then
> 					INTEL_RPMS+=( intel-${p} )
> 				else
> 					INTEL_RPMS+=( intel-${p}-${_INTEL_PV}.${a}.rpm )
> 				fi
> 			else
> 				if [[ "${p}" == *.rpm ]]; then
> 					INTEL_RPMS_FULL+=( ${p} )
> 				else
> 					INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV}.${a}.rpm )
> 				fi
> 			fi
> 		done
> 	done
> 
> 	if use amd64; then
> 		for p in "${INTEL_AMD64_RPMS[@]}"; do
> 			if [ ${p} == $(basename ${p}) ]; then
> 				if [[ "${p}" == *.rpm ]]; then
> 					INTEL_RPMS+=( intel-${p} )
> 				else
> 					INTEL_RPMS+=( intel-${p}-${_INTEL_PV}.x86_64.rpm )
> 				fi
> 			else
> 				if [[ "${p}" == *.rpm ]]; then
> 					INTEL_RPMS_FULL+=( ${p} )
> 				else
> 					INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV}.x86_64.rpm )
> 				fi
> 			fi
> 		done
> 	fi
> 
> 	for p in "${INTEL_X86_RPMS[@]}"; do
> 		if [ ${p} == $(basename ${p}) ]; then
> 			if [[ "${p}" == *.rpm ]]; then
> 				INTEL_RPMS+=( intel-${p} )
> 			else
> 				INTEL_RPMS+=( intel-${p}-${_INTEL_PV}.${INTEL_X86}.rpm )
> 			fi
> 		else
> 			if [[ "${p}" == *.rpm ]]; then
> 				INTEL_RPMS_FULL+=( ${p} )
> 			else
> 				INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV}.${INTEL_X86}.rpm )
> 			fi
> 		fi
> 	done
> 
> 	for p in "${INTEL_DAT_RPMS[@]}"; do
> 		if [ ${p} == $(basename ${p}) ]; then
> 			if [[ "${p}" == *.rpm ]]; then
> 				INTEL_RPMS+=( intel-${p} )
> 			else
> 				INTEL_RPMS+=( intel-${p}-${_INTEL_PV}.noarch.rpm )
> 			fi
> 		else
> 			if [[ "${p}" == *.rpm ]]; then
> 				INTEL_RPMS_FULL+=( ${p} )
> 			else
> 				INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV}.noarch.rpm )
> 			fi
> 		fi
> 	done

Wouldn't you be able to collapse that into one loop?

> }
> 
> # @FUNCTION: intel-sdp-r1_src_unpack
> # @DESCRIPTION:
> # Unpacking necessary rpms from tarball, extract them and rearrange the output.
> intel-sdp-r1_src_unpack() {
> 	local l r subdir rb t list=() debug_list
> 
> 	for t in ${A}; do
> 		for r in "${INTEL_RPMS[@]}"; do
> 			rpmdir=${t%%.*}/${INTEL_RPMS_DIR}
> 			list+=( ${rpmdir}/${r} )
> 		done
> 
> 		for r in "${INTEL_RPMS_FULL[@]}"; do
> 			list+=( ${t%%.*}/${r} )
> 		done
> 
> 		debug_list="$(IFS=$'\n'; echo ${list[@]} )"
> 
> 		debug-print "Adding to decompression list:"
> 		debug-print ${debug_list}
> 
> 		tar xvf "${DISTDIR}"/${t} ${list[@]} &> "${T}"/rpm-extraction.log

I suggest -xvf. Looks more friendly than ye-ol-syntax.

> 
> 		for r in ${list[@]}; do
> 			rb=$(basename ${r})

basename again.

> 			einfo "Unpacking ${rb}"
> 			rpm2tar -O ${r} | tar xvf - | sed -e \
> 				"s:^\.:${EROOT#/}:g" > /dev/null; assert "unpacking ${r} failed"

What's the deal with this sed?

> 		done
> 	done
> }
> 
> # @FUNCTION: intel-sdp-r1_src_install
> # @DESCRIPTION:
> # Install everything
> intel-sdp-r1_src_install() {
> 	debug-print-function ${FUNCNAME} "${@}"
> 
> 	# remove uninstall information
> 	if path_exists opt/intel/"${INTEL_DPN}"*/uninstall; then
> 		ebegin "Cleaning out uninstall"
> 		rm -r opt/intel/"${INTEL_DPN}"*/uninstall || die
> 		eend
> 	fi
> 
> 	# handle documentation
> 	if path_exists "opt/intel/documentation_${_INTEL_SDP_YEAR}"; then
> 		if path_exists "opt/intel/documentation_${_INTEL_SDP_YEAR}/en/man/common/man1"; then
> 			doman opt/intel/documentation_"${_INTEL_SDP_YEAR}"/en/man/common/man1/*
> 			rm -r opt/intel/documentation_"${_INTEL_SDP_YEAR}"/en/man || die
> 		fi
> 
> 		if use doc; then
> 			if ! use linguas_ja; then
> 				rm -r opt/intel/documentation_"${_INTEL_SDP_YEAR}"/ja || die
> 			fi
> 			dodoc -r opt/intel/documentation_"${_INTEL_SDP_YEAR}"/*
> 		fi
> 
> 		ebegin "Cleaning out documentation"
> 		rm -r "opt/intel/documentation_${_INTEL_SDP_YEAR}" || die
> 		rm "${INTEL_SDP_DIR}"/linux/{documentation,man} || die
> 		eend
> 	fi
> 
> 	# handle examples
> 	if path_exists "opt/intel/samples_${_INTEL_SDP_YEAR}"; then
> 		if use examples; then
> 			if ! use linguas_ja; then
> 				rm -r opt/intel/samples_"${_INTEL_SDP_YEAR}"/ja || die
> 			fi
> 			dodoc -r opt/intel/samples_"${_INTEL_SDP_YEAR}"/*
> 		fi
> 
> 		ebegin "Cleaning out examples"
> 		rm -r "opt/intel/samples_${_INTEL_SDP_YEAR}" || die
> 		eend
> 	fi
> 
> 	# remove eclipse
> 	rm -rf opt/intel/ide_support_* || die
> 
> 	ebegin "Tagging ${PN}"
> 	find opt -name \*sh -type f -exec sed -i \
> 		-e "s:<.*DIR>:${INTEL_SDP_EDIR}/linux:g" \
> 		'{}' + || die
> 	eend
> 
> 	[[ -d "${ED}" ]] || dodir /

Do we actually need that, ever?

> 	mv opt "${ED}"/ || die "moving files failed"
> 
> 	dodir "${INTEL_SDP_DIR}"/licenses /opt/intel/ism/rm
> 	keepdir "${INTEL_SDP_DIR}"/licenses /opt/intel/ism/rm

dodir is redundant.

> }
> 
> # @FUNCTION: intel-sdp-r1_pkg_postinst
> # @DESCRIPTION:
> # Test for all things working
> intel-sdp-r1_pkg_postinst() {
> 	debug-print-function ${FUNCNAME} "${@}"
> 
> 	_isdp_run-test
> 
> 	if [[ ${PN} = icc ]] && has_version ">=dev-util/ccache-3.1.9-r2" ; then
> 		#add ccache links as icc might get installed after ccache
> 		"${EROOT}"/usr/bin/ccache-config --install-links
> 	fi
> 
> 	elog "Beginning with the 2016 suite of Intel tools, Gentoo has removed"
> 	elog "support for the eclipse plugin. If you require the IDE support,"
> 	elog "you will have to install the suite on your own, outside portage."
> }
> 
> # @FUNCTION: intel-sdp-r1_pkg_postrm
> # @DESCRIPTION:
> # Sanitize cache links
> intel-sdp-r1_pkg_postrm() {
> 	debug-print-function ${FUNCNAME} "${@}"
> 
> 	if [[ ${PN} = icc ]] && has_version ">=dev-util/ccache-3.1.9-r2" && [[ -z ${REPLACED_BY_VERSION} ]]; then
> 		# --remove-links would remove all links, --install-links updates them
> 		"${EROOT}"/usr/bin/ccache-config --install-links
> 	fi
> }
> 
> EXPORT_FUNCTIONS pkg_setup src_unpack src_install pkg_postinst pkg_postrm pkg_pretend

We usually do this on top, and it's better to do it outside guards so
that order from inherit is always respected.

> 
> _INTEL_SDP_R1_ECLASS_=1
> fi



-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [gentoo-dev] RFC: intel-sdp-r1.eclass
  2016-02-15 12:59 ` Michał Górny
@ 2016-02-15 13:37   ` Justin Lecher (jlec)
  2016-02-15 14:35     ` Michał Górny
  2016-02-15 18:48     ` Martin Vaeth
  0 siblings, 2 replies; 12+ messages in thread
From: Justin Lecher (jlec) @ 2016-02-15 13:37 UTC (permalink / raw
  To: gentoo-dev

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

On 15/02/16 13:59, Michał Górny wrote:
> On Mon, 15 Feb 2016 09:16:53 +0100
> "Justin Lecher (jlec)" <jlec@gentoo.org> wrote:
> 
>> # Copyright 1999-2016 Gentoo Foundation
>> # Distributed under the terms of the GNU General Public License v2
>> # $Id$
>>
>> # @ECLASS: intel-sdp-r1.eclass
>> # @MAINTAINER:
>> # Justin Lecher <jlec@gentoo.org>
>> # David Seifert <soap@gentoo.org>
>> # Sci Team <sci@gentoo.org>
>> # @BLURB: Handling of Intel's Software Development Products package management
>>
>> if [[ ! ${_INTEL_SDP_R1_ECLASS_} ]]; then
>>
>> case "${EAPI:-0}" in
> 
> :-0 is meaningless here.
> 
>> 	6) ;;
>> 	*) die "EAPI=${EAPI} is not supported" ;;
> 
> If at all, it could be helpful here.

We had that before, will fix it.

> 
>> esac
>>
>> # @ECLASS-VARIABLE: INTEL_DID
>> # @DEFAULT_UNSET
>> # @DESCRIPTION:
>> # The package download ID from Intel.
>> # To find out its value, see the links to download in
>> # https://registrationcenter.intel.com/RegCenter/MyProducts.aspx
>> #
>> # e.g. 8365
>> #
>> # Must be defined before inheriting the eclass
>>
>> # @ECLASS-VARIABLE: INTEL_DPN
>> # @DEFAULT_UNSET
>> # @DESCRIPTION:
>> # The package name to download from Intel.
>> # To find out its value, see the links to download in
>> # https://registrationcenter.intel.com/RegCenter/MyProducts.aspx
>> #
>> # e.g. parallel_studio_xe
>> #
>> # Must be defined before inheriting the eclass
>>
>> # @ECLASS-VARIABLE: INTEL_DPV
>> # @DEFAULT_UNSET
>> # @DESCRIPTION:
>> # The package download version from Intel.
>> # To find out its value, see the links to download in
>> # https://registrationcenter.intel.com/RegCenter/MyProducts.aspx
>> #
>> # e.g. 2016_update1
>> #
>> # Must be defined before inheriting the eclass
>>
>> # @ECLASS-VARIABLE: INTEL_TARX
>> # @DESCRIPTION:
>> # The package extention.
> 
> Extension. Or if you're not on Windows, then 'suffix'.

Fair enough.

> 
>> # To find out its value, see the links to download in
>> # https://registrationcenter.intel.com/RegCenter/MyProducts.aspx
>> #
>> # e.g. tar.gz
>> #
>> # Must be defined before inheriting the eclass
>> : ${INTEL_TARX:=tgz}
>>
>> # @ECLASS-VARIABLE: INTEL_SUBDIR
>> # @DEFAULT_UNSET
>> # @DESCRIPTION:
>> # The package sub-directory where it will end-up in /opt/intel
>> # To find out its value, you have to do a raw install from the Intel tar ball
> 
> To be honest, I find this kinda terrible. There's a huge block of docs
> which makes me feel small and confused. Maybe it'd useful to give some
> semi-complete example on top (in global doc)?

That makes definitely make sense. We will add one.

Although nobody other then the maintainer of this eclass will ever use it.

> 
>> # @ECLASS-VARIABLE: INTEL_SKIP_LICENSE
>> # @DEFAULT_UNSET
>> # @DESCRIPTION:
>> # Possibility to skip the mandatory check for licenses. Only set this if there
>> # is really no fix.
>>
>> # @ECLASS-VARIABLE: INTEL_RPMS_DIR
>> # @DESCRIPTION:
>> # Main subdirectory which contains the rpms to extract.
>> : ${INTEL_RPMS_DIR:=rpm}
>>
>> # @ECLASS-VARIABLE: INTEL_X86
>> # @DESCRIPTION:
>> # 32bit arch in rpm names
>> #
>> # e.g. i486
>> : ${INTEL_X86:=i486}
>>
>> # @ECLASS-VARIABLE: INTEL_BIN_RPMS
>> # @DESCRIPTION:
>> # Functional name of rpm without any version/arch tag
>> # Has to be a bash array
>> #
>> # e.g. ("icc-l-all-devel")
>> #
>> # if the rpm is located in a directory other than INTEL_RPMS_DIR you can
>> # specify the full path
>> #
>> # e.g. CLI_install/rpm/intel-vtune-amplifier-xe-cli
>> : ${INTEL_BIN_RPMS:=()}
> 
> $ : ${foo:=()}
> $ declare -p foo
> declare -- foo="()"
> 
> In other words, it doesn't work the way you expect it to.

I already wondered about this. Is there any way to force a variable to
be an array in bash? Or define it as an empty array?

> 
>> # @ECLASS-VARIABLE: INTEL_AMD64_RPMS
>> # @DESCRIPTION:
>> # AMD64 single arch rpms. Same syntax as INTEL_BIN_RPMS
>> # Has to be a bash array
>> : ${INTEL_AMD64_RPMS:=()}
>>
>> # @ECLASS-VARIABLE: INTEL_X86_RPMS
>> # @DESCRIPTION:
>> # X86 single arch rpms. Same syntax as INTEL_BIN_RPMS
>> # Has to be a bash array
>> : ${INTEL_X86_RPMS:=()}
>>
>> # @ECLASS-VARIABLE: INTEL_DAT_RPMS
>> # @DESCRIPTION:
>> # Functional name of rpm of common data which are arch free
>> # without any version tag
>> # Has to be a bash array
>> #
>> # e.g. ("openmp-l-all-devel")
>> #
>> # if the rpm is located in a directory different to INTEL_RPMS_DIR you can
>> # specify the full path
>> #
>> # e.g. CLI_install/rpm/intel-vtune-amplifier-xe-cli-common
>> : ${INTEL_DAT_RPMS:=()}
>>
>> # @ECLASS-VARIABLE: INTEL_SINGLE_ARCH
>> # @DESCRIPTION:
>> # Unset, if only the multilib package will be provided by intel
>> : ${INTEL_SINGLE_ARCH:=true}
> 
> This is really weird. It sounds like I'm supposed to do:
> 
>   inherit intel-sdp-r1
>   unset INTEL_SINGLE_ARCH
> 
> I suggest you used positive logic instead.

The wording is wrong. Setting it to anything but "true" like
"INTEL_SINGLE_ARCH=false" works. We will fix the wording.

> 
>> MULTILIB_COMPAT=( abi_x86_{32,64} )
>>
>> inherit check-reqs eutils multilib-build versionator
>>
>> _INTEL_PV1=$(get_version_component_range 1)
>> _INTEL_PV2=$(get_version_component_range 2)
>> _INTEL_PV3=$(get_version_component_range 3)
>> _INTEL_PV4=$(get_version_component_range 4)
> 
> I'm pretty sure there's a better way than calling the same function
> four times. For example:
> 
>   _INTEL_PV=( $(get_version_components) )
> 
> Plus, please reduce the environment pollution. Either unset all those
> variables when no longer needed, or preferably create a function for
> setting globals and make them local.

yes, thanks for the comment.

> 
>> _INTEL_PV=""
>> [[ -n ${_INTEL_PV4} ]] && _INTEL_PV+="${_INTEL_PV4}-"
>> [[ -n ${_INTEL_PV1} ]] && _INTEL_PV+="${_INTEL_PV1}"
>> [[ -n ${_INTEL_PV2} ]] && _INTEL_PV+=".${_INTEL_PV2}"
>> [[ -n ${_INTEL_PV3} ]] && _INTEL_PV+=".${_INTEL_PV3}"
>> [[ -n ${_INTEL_PV4} ]] && _INTEL_PV+="-${_INTEL_PV4}"
> 
> Now, this is crazy ;-). I don't see immediately how to improve that,
> but I suggest you thought about that.

Intel defines the version in roughly the following schema
MICRO.MAJOR.MINOR or YEAR-MINOR. So there is no sanity other then
reshuffling.

> 
> Plus, it's kinda confusing having _INTEL_PV and INTEL_DPV used in all
> different places. Could you try to use more explanatory names?

We will see how much can be made local so we don't need to care about
better names.

> 
>> _INTEL_URI="http://registrationcenter-download.intel.com/akdlm/irc_nas/${INTEL_DID}/${INTEL_DPN}"
>>
>> if [ ${INTEL_SINGLE_ARCH} == true ]; then
> 
> Please use [[ ]] for consistency, and quoting safety.

yes.

> 
>> 	SRC_URI="
>> 		abi_x86_32? ( ${_INTEL_URI}_${INTEL_DPV}_ia32.${INTEL_TARX} )
>> 		abi_x86_64? ( ${_INTEL_URI}_${INTEL_DPV}_intel64.${INTEL_TARX} )"
>> else
>> 	SRC_URI="${_INTEL_URI}_${INTEL_DPV}.${INTEL_TARX}"
>> fi
>>
>> LICENSE="Intel-SDP"
>> # Future work, #394411
>> #SLOT="${_INTEL_PV1}.${_INTEL_PV2}"
>> SLOT="0"
>>
>> RESTRICT="mirror"
>>
>> RDEPEND=""
>> DEPEND="app-arch/rpm2targz"
>>
>> _INTEL_SDP_YEAR=${INTEL_DPV}
>> _INTEL_SDP_YEAR=${_INTEL_SDP_YEAR%_sp*}
>> _INTEL_SDP_YEAR=${_INTEL_SDP_YEAR%_update*}
>>
>> # @ECLASS-VARIABLE: INTEL_SDP_DIR
>> # @DESCRIPTION:
>> # Full rootless path to installation dir
>> INTEL_SDP_DIR="opt/intel/${INTEL_SUBDIR}_${_INTEL_SDP_YEAR:-${_INTEL_PV1}}"
>> [[ -n ${_INTEL_PV3} ]] && INTEL_SDP_DIR+=".${_INTEL_PV3}"
>> [[ -n ${_INTEL_PV4} ]] && INTEL_SDP_DIR+=".${_INTEL_PV4}"
>>
>> # @ECLASS-VARIABLE: INTEL_SDP_EDIR
>> # @DESCRIPTION:
>> # Full rooted path to installation dir
>> INTEL_SDP_EDIR="${EROOT%/}/${INTEL_SDP_DIR}"
> 
> EROOT can't be used in global scope. Doing things like this, you could
> end up with wrong value being carried on to phases.

Good spot. We will work on this.

> 
>> S="${WORKDIR}"
>>
>> QA_PREBUILT="${INTEL_SDP_DIR}/*"
>>
>> # @ECLASS-VARIABLE: INTEL_ARCH
>> # @DEFAULT_UNSET
>> # @DESCRIPTION:
>> # Intels internal names of the arches; will be set at runtime accordingly
>> #
>> # e.g. amd64-multilib -> INTEL_ARCH="intel64 ia32"
> 
> So we're not supposed to set this?

No, this is correctly set in pkg_setup(). We will add some @INTERNAL.

> 
>> # @FUNCTION: _isdp_big-warning
>> # @USAGE: [pre-check | test-failed]
>> # @INTERNAL
>> # @DESCRIPTION:
>> # warn user that we really require a license
> 
> Description really need to be improved. They should tell how to call
> the function and what it does accordingly. This applies to all
> functions.

:/ but yes, we will look into that.

> 
>> _isdp_big-warning() {
>> 	debug-print-function ${FUNCNAME} "${@}"
>>
>> 	case ${1} in
>> 		pre-check )
>> 			echo ""
> 
> Don't mix echo with ewarn.

Why?

> 
>> 			ewarn "License file not found!"
>> 			;;
>>
>> 		test-failed )
>> 			echo ""
>> 			ewarn "Function test failed. Most probably due to an invalid license."
>> 			ewarn "This means you already tried to bypass the license check once."
>> 			;;
>> 	esac
>>
>> 	echo ""
>> 	ewarn "Make sure you have received an Intel license."
>> 	ewarn "To receive a non-commercial license, you need to register at:"
>> 	ewarn "https://software.intel.com/en-us/qualify-for-free-software"
>> 	ewarn "Install the license file into ${EPREFIX}/opt/intel/licenses"
>> 	ewarn ""
>> 	ewarn "Beginning with the 2016 suite of tools, license files are keyed"
>> 	ewarn "to the MAC address of the eth0 interface. In order to retrieve"
>> 	ewarn "a personalized license file, follow the instructions at"
>> 	ewarn "https://software.intel.com/en-us/articles/how-do-i-get-my-license-file-for-intel-parallel-studio-xe-2016"
>>
>> 	case ${1} in
>> 		pre-check )
>> 			ewarn "before proceeding with installation of ${P}"
>> 			echo ""
>> 			;;
>> 		* )
>> 			echo ""
>> 			;;
>> 			esac
>> }
>>
>> # @FUNCTION: _isdp_version_test
>> # @INTERNAL
>> # @DESCRIPTION:
>> # Testing for valid license by asking for version information of the compiler
>> _isdp_version_test() {
>> 	debug-print-function ${FUNCNAME} "${@}"
>>
>> 	local comp comp_full arch warn
>> 	case ${PN} in
>> 		ifc )
>> 			debug-print "Testing ifort"
>> 			comp=ifort
>> 			;;
>> 		icc )
>> 			debug-print "Testing icc"
>> 			comp=icc
>> 			;;
>> 		*)
>> 			die "${PN} is not supported for testing"
>> 			;;
>> 	esac
>>
>> 	for arch in ${INTEL_ARCH}; do
>> 		case ${EBUILD_PHASE} in
>> 			install )
>> 				comp_full="${ED}/${INTEL_SDP_DIR}/linux/bin/${arch}/${comp}"
> 
> Double slash imminent (ED has one).

Always? Per definition?

> 
>> 				;;
>> 			postinst )
>> 				comp_full="${INTEL_SDP_EDIR}/linux/bin/${arch}/${comp}"
>> 				;;
>> 			* )
>> 				ewarn "Compile test not supported in ${EBUILD_PHASE}"
> 
> Why not die? It's a new eclass after all.

Fair enough.

> 
>> 				continue
>> 				;;
>> 		esac
>>
>> 		debug-print "LD_LIBRARY_PATH=\"${INTEL_SDP_EDIR}/linux/bin/${arch}/\" \"${comp_full}\" -V"
>>
>> 		LD_LIBRARY_PATH="${INTEL_SDP_EDIR}/linux/bin/${arch}/" "${comp_full}" -V &>/dev/null
>> 		[[ $? -ne 0 ]] && warn=yes
> 
> Why not just:
> 
>   LD_L...  "${comp_full}" -V &>/dev/null || warn=yes

good question.

> 
>> 	done
>> 	[[ "${warn}" == "yes" ]] && _isdp_big-warning test-failed
> 
> Unnecessary quoting.

right.

> 
>> }
>>
>> # @FUNCTION: _isdp_run-test
>> # @INTERNAL
>> # @DESCRIPTION:
>> # Test if installed compiler is working
>> _isdp_run-test() {
>> 	debug-print-function ${FUNCNAME} "${@}"
>>
>> 	if [[ -z ${INTEL_SKIP_LICENSE} ]]; then
>> 		case ${PN} in
>> 			ifc | icc )
>> 				_isdp_version_test ;;
>> 			* )
>> 				debug-print "No test available for ${PN}"
>> 				;;
>> 		esac
>> 	fi
>> }
>>
>> # @FUNCTION: convert2intel_arch
>> # @USAGE: <arch>
>> # @DESCRIPTION:
>> # Convert between portage arch (e.g. amd64, x86) and intel arch
>> # nomenclature (e.g. intel64, ia32)
>> convert2intel_arch() {
> 
> Namespace, please.

true.

> 
>> 	debug-print-function ${FUNCNAME} "${@}"
>>
>> 	case $1 in
>> 		amd64|abi_x86_64|*amd64*)
> 
> Err, *amd64* catches amd64, you know.

correct. same for x86 below.

> 
>> 			echo "intel64"
>> 			;;
>> 		x86|abi_x86_32|*x86*)
>> 			echo "ia32"
>> 			;;
>> 		*)
>> 			die "Abi \'$1\' is unsupported"
>> 			;;
>> 	esac
>> }
>>
>> # @FUNCTION: intel-sdp-r1_pkg_pretend
>> # @DESCRIPTION:
>> # @CODE
>> # * Check that the user has a (valid) license file before going on.
>> # * Check for space requirements being fullfilled
> 
> fulfilled.

thanks.

> 
>> # @CODE
> 
> Err, this is not code, you know.

This is needed for nice formatting. Otherwise there is no line break

> 
>> intel-sdp-r1_pkg_pretend() {
>> 	debug-print-function ${FUNCNAME} "${@}"
>>
>> 	local warn=1 dir dirs ret arch a p
>>
>> 	: ${CHECKREQS_DISK_BUILD:=256M}
>> 	check-reqs_pkg_pretend
>>
>> 	if [[ -z ${INTEL_SKIP_LICENSE} ]]; then
>> 		if echo ${INTEL_LICENSE_FILE} | grep -q @; then
> 
> Err... is this some fancy way of saying:
> 
>   [[ ${INTEL_LICENSE_FILE} == *@* ]]
> 
> ? Or is @ special here?

no, you are right, that can be done more elegant.

> 
>> 			einfo "Looks like you are using following license server:"
>> 			einfo "   ${INTEL_LICENSE_FILE}"
>> 			return 0
>> 		fi
>>
>> 		dirs=(
>> 			"${EPREFIX}/opt/intel/licenses"
>> 			"${INTEL_SDP_EDIR}/licenses"
>> 			"${INTEL_SDP_EDIR}/Licenses"
>> 			)
>> 		for dir in "${dirs[@]}" ; do
>> 			ebegin "Checking for a license in: ${dir}"
>> 			#maybe use nullglob or [[ $(echo ${dir/*lic) != "${dir}/*lic" ]]
>> 			[[ $( ls "${dir}"/*lic 2>/dev/null ) ]]; ret=$?
> 
> Maybe you should use something sane indeed.
> 
>> 			eend ${ret}
>> 			if [[ ${ret} == "0" ]]; then
>> 				warn=${ret}
>> 				break
>> 			fi
> 
> I would have to check PMS here but I think eend preserves (returns)
> the status passed to it.

... "Returns its first argument as exit status."

You are right.

> 
>> 		done
>> 		if [[ ${warn} == "1" ]]; then
>> 			_isdp_big-warning pre-check
>> 			die "Could not find license file"
>> 		fi
>> 	else
>> 		eqawarn "The ebuild doesn't check for presence of a proper intel license!"
>> 		eqawarn "This shouldn't be done unless there is a very good reason."
> 
> This looks like a bad idea. You either require it, and fail it doesn't
> exist, or let people disable it. You don't warn endlessly if this is
> allowed.

perhaps you are right.

> 
>> 	fi
>> }
>>
>> # @FUNCTION: intel-sdp-r1_pkg_setup
>> # @DESCRIPTION:
>> # Setting up and sorting some internal variables
>> intel-sdp-r1_pkg_setup() {
>> 	debug-print-function ${FUNCNAME} "${@}"
>> 	local arch a p
>>
>> 	INTEL_ARCH=""
>>
>> 	if use abi_x86_64; then
>> 		arch+=" x86_64"
>> 		INTEL_ARCH+=" intel64"
> 
> Wouldn't it be better to use an array here?

yes.

> 
>> 	fi
>> 	if use abi_x86_32; then
>> 		arch+=" ${INTEL_X86}"
>> 		INTEL_ARCH+=" ia32"
>> 	fi
>> 	INTEL_RPMS=()
>> 	INTEL_RPMS_FULL=()
>>
>> 	for p in "${INTEL_BIN_RPMS[@]}"; do
>> 		for a in ${arch}; do
>> 			if [ ${p} == $(basename ${p}) ]; then
> 
> ${p##*/}.

make sense.

> 
>> 				# check for variables ending in ".rpm"
>> 				# these are excluded from version expansion, due to Intel's
>> 				# idiosyncratic versioning scheme beginning with their 2016
>> 				# suite of tools.
>> 				if [[ "${p}" == *.rpm ]]; then
>> 					INTEL_RPMS+=( intel-${p} )
>> 				else
>> 					INTEL_RPMS+=( intel-${p}-${_INTEL_PV}.${a}.rpm )
>> 				fi
>> 			else
>> 				if [[ "${p}" == *.rpm ]]; then
>> 					INTEL_RPMS_FULL+=( ${p} )
>> 				else
>> 					INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV}.${a}.rpm )
>> 				fi
>> 			fi
>> 		done
>> 	done
>>
>> 	if use amd64; then
>> 		for p in "${INTEL_AMD64_RPMS[@]}"; do
>> 			if [ ${p} == $(basename ${p}) ]; then
>> 				if [[ "${p}" == *.rpm ]]; then
>> 					INTEL_RPMS+=( intel-${p} )
>> 				else
>> 					INTEL_RPMS+=( intel-${p}-${_INTEL_PV}.x86_64.rpm )
>> 				fi
>> 			else
>> 				if [[ "${p}" == *.rpm ]]; then
>> 					INTEL_RPMS_FULL+=( ${p} )
>> 				else
>> 					INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV}.x86_64.rpm )
>> 				fi
>> 			fi
>> 		done
>> 	fi
>>
>> 	for p in "${INTEL_X86_RPMS[@]}"; do
>> 		if [ ${p} == $(basename ${p}) ]; then
>> 			if [[ "${p}" == *.rpm ]]; then
>> 				INTEL_RPMS+=( intel-${p} )
>> 			else
>> 				INTEL_RPMS+=( intel-${p}-${_INTEL_PV}.${INTEL_X86}.rpm )
>> 			fi
>> 		else
>> 			if [[ "${p}" == *.rpm ]]; then
>> 				INTEL_RPMS_FULL+=( ${p} )
>> 			else
>> 				INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV}.${INTEL_X86}.rpm )
>> 			fi
>> 		fi
>> 	done
>>
>> 	for p in "${INTEL_DAT_RPMS[@]}"; do
>> 		if [ ${p} == $(basename ${p}) ]; then
>> 			if [[ "${p}" == *.rpm ]]; then
>> 				INTEL_RPMS+=( intel-${p} )
>> 			else
>> 				INTEL_RPMS+=( intel-${p}-${_INTEL_PV}.noarch.rpm )
>> 			fi
>> 		else
>> 			if [[ "${p}" == *.rpm ]]; then
>> 				INTEL_RPMS_FULL+=( ${p} )
>> 			else
>> 				INTEL_RPMS_FULL+=( ${p}-${_INTEL_PV}.noarch.rpm )
>> 			fi
>> 		fi
>> 	done
> 
> Wouldn't you be able to collapse that into one loop?

no, because the first has ${INTEL_X86}.rpm as suffeix and the later has
${INTEL_X86}.rpm.

> 
>> }
>>
>> # @FUNCTION: intel-sdp-r1_src_unpack
>> # @DESCRIPTION:
>> # Unpacking necessary rpms from tarball, extract them and rearrange the output.
>> intel-sdp-r1_src_unpack() {
>> 	local l r subdir rb t list=() debug_list
>>
>> 	for t in ${A}; do
>> 		for r in "${INTEL_RPMS[@]}"; do
>> 			rpmdir=${t%%.*}/${INTEL_RPMS_DIR}
>> 			list+=( ${rpmdir}/${r} )
>> 		done
>>
>> 		for r in "${INTEL_RPMS_FULL[@]}"; do
>> 			list+=( ${t%%.*}/${r} )
>> 		done
>>
>> 		debug_list="$(IFS=$'\n'; echo ${list[@]} )"
>>
>> 		debug-print "Adding to decompression list:"
>> 		debug-print ${debug_list}
>>
>> 		tar xvf "${DISTDIR}"/${t} ${list[@]} &> "${T}"/rpm-extraction.log
> 
> I suggest -xvf. Looks more friendly than ye-ol-syntax.

We don't mind.

> 
>>
>> 		for r in ${list[@]}; do
>> 			rb=$(basename ${r})
> 
> basename again.

yes, will be fixed.

> 
>> 			einfo "Unpacking ${rb}"
>> 			rpm2tar -O ${r} | tar xvf - | sed -e \
>> 				"s:^\.:${EROOT#/}:g" > /dev/null; assert "unpacking ${r} failed"
> 
> What's the deal with this sed?

Good question, but it was there since always and probably the original
author had good reasons for it. We will look into it and comment the code.

> 
>> 		done
>> 	done
>> }
>>
>> # @FUNCTION: intel-sdp-r1_src_install
>> # @DESCRIPTION:
>> # Install everything
>> intel-sdp-r1_src_install() {
>> 	debug-print-function ${FUNCNAME} "${@}"
>>
>> 	# remove uninstall information
>> 	if path_exists opt/intel/"${INTEL_DPN}"*/uninstall; then
>> 		ebegin "Cleaning out uninstall"
>> 		rm -r opt/intel/"${INTEL_DPN}"*/uninstall || die
>> 		eend
>> 	fi
>>
>> 	# handle documentation
>> 	if path_exists "opt/intel/documentation_${_INTEL_SDP_YEAR}"; then
>> 		if path_exists "opt/intel/documentation_${_INTEL_SDP_YEAR}/en/man/common/man1"; then
>> 			doman opt/intel/documentation_"${_INTEL_SDP_YEAR}"/en/man/common/man1/*
>> 			rm -r opt/intel/documentation_"${_INTEL_SDP_YEAR}"/en/man || die
>> 		fi
>>
>> 		if use doc; then
>> 			if ! use linguas_ja; then
>> 				rm -r opt/intel/documentation_"${_INTEL_SDP_YEAR}"/ja || die
>> 			fi
>> 			dodoc -r opt/intel/documentation_"${_INTEL_SDP_YEAR}"/*
>> 		fi
>>
>> 		ebegin "Cleaning out documentation"
>> 		rm -r "opt/intel/documentation_${_INTEL_SDP_YEAR}" || die
>> 		rm "${INTEL_SDP_DIR}"/linux/{documentation,man} || die
>> 		eend
>> 	fi
>>
>> 	# handle examples
>> 	if path_exists "opt/intel/samples_${_INTEL_SDP_YEAR}"; then
>> 		if use examples; then
>> 			if ! use linguas_ja; then
>> 				rm -r opt/intel/samples_"${_INTEL_SDP_YEAR}"/ja || die
>> 			fi
>> 			dodoc -r opt/intel/samples_"${_INTEL_SDP_YEAR}"/*
>> 		fi
>>
>> 		ebegin "Cleaning out examples"
>> 		rm -r "opt/intel/samples_${_INTEL_SDP_YEAR}" || die
>> 		eend
>> 	fi
>>
>> 	# remove eclipse
>> 	rm -rf opt/intel/ide_support_* || die
>>
>> 	ebegin "Tagging ${PN}"
>> 	find opt -name \*sh -type f -exec sed -i \
>> 		-e "s:<.*DIR>:${INTEL_SDP_EDIR}/linux:g" \
>> 		'{}' + || die
>> 	eend
>>
>> 	[[ -d "${ED}" ]] || dodir /
> 
> Do we actually need that, ever?

Probably not.

> 
>> 	mv opt "${ED}"/ || die "moving files failed"
>>
>> 	dodir "${INTEL_SDP_DIR}"/licenses /opt/intel/ism/rm
>> 	keepdir "${INTEL_SDP_DIR}"/licenses /opt/intel/ism/rm
> 
> dodir is redundant.

true.

> 
>> }
>>
>> # @FUNCTION: intel-sdp-r1_pkg_postinst
>> # @DESCRIPTION:
>> # Test for all things working
>> intel-sdp-r1_pkg_postinst() {
>> 	debug-print-function ${FUNCNAME} "${@}"
>>
>> 	_isdp_run-test
>>
>> 	if [[ ${PN} = icc ]] && has_version ">=dev-util/ccache-3.1.9-r2" ; then
>> 		#add ccache links as icc might get installed after ccache
>> 		"${EROOT}"/usr/bin/ccache-config --install-links
>> 	fi
>>
>> 	elog "Beginning with the 2016 suite of Intel tools, Gentoo has removed"
>> 	elog "support for the eclipse plugin. If you require the IDE support,"
>> 	elog "you will have to install the suite on your own, outside portage."
>> }
>>
>> # @FUNCTION: intel-sdp-r1_pkg_postrm
>> # @DESCRIPTION:
>> # Sanitize cache links
>> intel-sdp-r1_pkg_postrm() {
>> 	debug-print-function ${FUNCNAME} "${@}"
>>
>> 	if [[ ${PN} = icc ]] && has_version ">=dev-util/ccache-3.1.9-r2" && [[ -z ${REPLACED_BY_VERSION} ]]; then
>> 		# --remove-links would remove all links, --install-links updates them
>> 		"${EROOT}"/usr/bin/ccache-config --install-links
>> 	fi
>> }
>>
>> EXPORT_FUNCTIONS pkg_setup src_unpack src_install pkg_postinst pkg_postrm pkg_pretend
> 
> We usually do this on top, and it's better to do it outside guards so
> that order from inherit is always respected.

we will move it up. I don't get your second comment. Do you mean the
case someone does

inherit intel-sdp-r1 some-other-eclass intel-sdp

?

> 
>>
>> _INTEL_SDP_R1_ECLASS_=1
>> fi
> 
> 
> 


thanks for your comments,

Justin


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [gentoo-dev] RFC: intel-sdp-r1.eclass
  2016-02-15 13:37   ` Justin Lecher (jlec)
@ 2016-02-15 14:35     ` Michał Górny
  2016-02-15 14:41       ` Justin Lecher (jlec)
  2016-02-17  4:48       ` [gentoo-dev] " Ryan Hill
  2016-02-15 18:48     ` Martin Vaeth
  1 sibling, 2 replies; 12+ messages in thread
From: Michał Górny @ 2016-02-15 14:35 UTC (permalink / raw
  To: Justin Lecher (jlec); +Cc: gentoo-dev

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

On Mon, 15 Feb 2016 14:37:41 +0100
"Justin Lecher (jlec)" <jlec@gentoo.org> wrote:

> On 15/02/16 13:59, Michał Górny wrote:
> > On Mon, 15 Feb 2016 09:16:53 +0100
> > "Justin Lecher (jlec)" <jlec@gentoo.org> wrote:
> >   
> >> # @ECLASS-VARIABLE: INTEL_SUBDIR
> >> # @DEFAULT_UNSET
> >> # @DESCRIPTION:
> >> # The package sub-directory where it will end-up in /opt/intel
> >> # To find out its value, you have to do a raw install from the Intel tar ball  
> > 
> > To be honest, I find this kinda terrible. There's a huge block of docs
> > which makes me feel small and confused. Maybe it'd useful to give some
> > semi-complete example on top (in global doc)?  
> 
> That makes definitely make sense. We will add one.
> 
> Although nobody other then the maintainer of this eclass will ever use it.

Remember that maintainers can change. It's better to have good then
have new maintainers figure out all stuff over again.

> >> # e.g. CLI_install/rpm/intel-vtune-amplifier-xe-cli
> >> : ${INTEL_BIN_RPMS:=()}  
> > 
> > $ : ${foo:=()}
> > $ declare -p foo
> > declare -- foo="()"
> > 
> > In other words, it doesn't work the way you expect it to.  
> 
> I already wondered about this. Is there any way to force a variable to
> be an array in bash? Or define it as an empty array?

Look at e.g. python-utils-r1.

To check for array:

  if [[ $(declare -p foo) != "declare -a"* ]]; then
    ...
  fi

To default to empty, simple (yet a bit imperfect) way:

  [[ ${foo[@]} }] || foo=()

> >> # @ECLASS-VARIABLE: INTEL_SINGLE_ARCH
> >> # @DESCRIPTION:
> >> # Unset, if only the multilib package will be provided by intel
> >> : ${INTEL_SINGLE_ARCH:=true}  
> > 
> > This is really weird. It sounds like I'm supposed to do:
> > 
> >   inherit intel-sdp-r1
> >   unset INTEL_SINGLE_ARCH
> > 
> > I suggest you used positive logic instead.  
> 
> The wording is wrong. Setting it to anything but "true" like
> "INTEL_SINGLE_ARCH=false" works. We will fix the wording.

I still think positive logic is better. That is, a variable which
defaults to, say, unset, and changes behavior if it becomes set to
non-empty value.

> >> _isdp_big-warning() {
> >> 	debug-print-function ${FUNCNAME} "${@}"
> >>
> >> 	case ${1} in
> >> 		pre-check )
> >> 			echo ""  
> > 
> > Don't mix echo with ewarn.  
> 
> Why?

Because they won't go through the same output channels.

> >> 				comp_full="${ED}/${INTEL_SDP_DIR}/linux/bin/${arch}/${comp}"  
> > 
> > Double slash imminent (ED has one).  
> 
> Always? Per definition?

Yes, sadly. i wanted to change this but it's unlikely to go since it
makes EAPI migration hard. If you really want to cover both cases, you
can always do ${foo%/}/bar.

> >> # @CODE  
> > 
> > Err, this is not code, you know.  
> 
> This is needed for nice formatting. Otherwise there is no line break

Add an empty line between the two. That should do it correctly, without
code blocks in devmanual.

> >> 			#maybe use nullglob or [[ $(echo ${dir/*lic) != "${dir}/*lic" ]]
> >> 			[[ $( ls "${dir}"/*lic 2>/dev/null ) ]]; ret=$?  
> > 
> > Maybe you should use something sane indeed.

Maybe file_exists from eutils could help here btw.

> > Wouldn't you be able to collapse that into one loop?  
> 
> no, because the first has ${INTEL_X86}.rpm as suffeix and the later has
> ${INTEL_X86}.rpm.

Errrrr... am I reading wrong, or did you just type the same thing twice?

> >> 			einfo "Unpacking ${rb}"
> >> 			rpm2tar -O ${r} | tar xvf - | sed -e \
> >> 				"s:^\.:${EROOT#/}:g" > /dev/null; assert "unpacking ${r} failed"  
> > 
> > What's the deal with this sed?  
> 
> Good question, but it was there since always and probably the original
> author had good reasons for it. We will look into it and comment the code.

Didn't it change in the past? As I see it, tar here outputs file names,
sed edits them and then you discard them to /dev/null. In this case,
sed doesn't return any specific exit code so it seems to be a complete
no-op.

Maybe originally it verbosely output the extracted files.

> >> EXPORT_FUNCTIONS pkg_setup src_unpack src_install pkg_postinst pkg_postrm pkg_pretend  
> > 
> > We usually do this on top, and it's better to do it outside guards so
> > that order from inherit is always respected.  
> 
> we will move it up. I don't get your second comment. Do you mean the
> case someone does
> 
> inherit intel-sdp-r1 some-other-eclass intel-sdp
> 
> ?

Rather something unlikely like:

  inherit foo bar intel-sdp-r1

where foo inherits intel-sdp-r1, and therefore the exports occur before
bar, and bar takes over even though intel-sdp-r1 is explicitly
listed last.

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [gentoo-dev] RFC: intel-sdp-r1.eclass
  2016-02-15 14:35     ` Michał Górny
@ 2016-02-15 14:41       ` Justin Lecher (jlec)
  2016-02-15 14:55         ` Michał Górny
  2016-02-17  4:48       ` [gentoo-dev] " Ryan Hill
  1 sibling, 1 reply; 12+ messages in thread
From: Justin Lecher (jlec) @ 2016-02-15 14:41 UTC (permalink / raw
  To: gentoo-dev

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

On 15/02/16 15:35, Michał Górny wrote:
> On Mon, 15 Feb 2016 14:37:41 +0100
> "Justin Lecher (jlec)" <jlec@gentoo.org> wrote:
> 
>> On 15/02/16 13:59, Michał Górny wrote:
>>> On Mon, 15 Feb 2016 09:16:53 +0100
>>> "Justin Lecher (jlec)" <jlec@gentoo.org> wrote:
>>>   
>>>> # @ECLASS-VARIABLE: INTEL_SUBDIR
>>>> # @DEFAULT_UNSET
>>>> # @DESCRIPTION:
>>>> # The package sub-directory where it will end-up in /opt/intel
>>>> # To find out its value, you have to do a raw install from the Intel tar ball  
>>>
>>> To be honest, I find this kinda terrible. There's a huge block of docs
>>> which makes me feel small and confused. Maybe it'd useful to give some
>>> semi-complete example on top (in global doc)?  
>>
>> That makes definitely make sense. We will add one.
>>
>> Although nobody other then the maintainer of this eclass will ever use it.
> 
> Remember that maintainers can change. It's better to have good then
> have new maintainers figure out all stuff over again.
> 
>>>> # e.g. CLI_install/rpm/intel-vtune-amplifier-xe-cli
>>>> : ${INTEL_BIN_RPMS:=()}  
>>>
>>> $ : ${foo:=()}
>>> $ declare -p foo
>>> declare -- foo="()"
>>>
>>> In other words, it doesn't work the way you expect it to.  
>>
>> I already wondered about this. Is there any way to force a variable to
>> be an array in bash? Or define it as an empty array?
> 
> Look at e.g. python-utils-r1.
> 
> To check for array:
> 
>   if [[ $(declare -p foo) != "declare -a"* ]]; then
>     ...
>   fi
> 
> To default to empty, simple (yet a bit imperfect) way:
> 
>   [[ ${foo[@]} }] || foo=()

And what about the default assignment for the man page?

> 
>>>> # @ECLASS-VARIABLE: INTEL_SINGLE_ARCH
>>>> # @DESCRIPTION:
>>>> # Unset, if only the multilib package will be provided by intel
>>>> : ${INTEL_SINGLE_ARCH:=true}  
>>>
>>> This is really weird. It sounds like I'm supposed to do:
>>>
>>>   inherit intel-sdp-r1
>>>   unset INTEL_SINGLE_ARCH
>>>
>>> I suggest you used positive logic instead.  
>>
>> The wording is wrong. Setting it to anything but "true" like
>> "INTEL_SINGLE_ARCH=false" works. We will fix the wording.
> 
> I still think positive logic is better. That is, a variable which
> defaults to, say, unset, and changes behavior if it becomes set to
> non-empty value.

we will look into that.

> 
>>>> _isdp_big-warning() {
>>>> 	debug-print-function ${FUNCNAME} "${@}"
>>>>
>>>> 	case ${1} in
>>>> 		pre-check )
>>>> 			echo ""  
>>>
>>> Don't mix echo with ewarn.  
>>
>> Why?
> 
> Because they won't go through the same output channels.
> 
>>>> 				comp_full="${ED}/${INTEL_SDP_DIR}/linux/bin/${arch}/${comp}"  
>>>
>>> Double slash imminent (ED has one).  
>>
>> Always? Per definition?
> 
> Yes, sadly. i wanted to change this but it's unlikely to go since it
> makes EAPI migration hard. If you really want to cover both cases, you
> can always do ${foo%/}/bar.
> 
>>>> # @CODE  
>>>
>>> Err, this is not code, you know.  
>>
>> This is needed for nice formatting. Otherwise there is no line break
> 
> Add an empty line between the two. That should do it correctly, without
> code blocks in devmanual.

That will introduce an empty line between the two points.

> 
>>>> 			#maybe use nullglob or [[ $(echo ${dir/*lic) != "${dir}/*lic" ]]
>>>> 			[[ $( ls "${dir}"/*lic 2>/dev/null ) ]]; ret=$?  
>>>
>>> Maybe you should use something sane indeed.
> 
> Maybe file_exists from eutils could help here btw.
> 
>>> Wouldn't you be able to collapse that into one loop?  
>>
>> no, because the first has ${INTEL_X86}.rpm as suffeix and the later has
>> ${INTEL_X86}.rpm.
> 
> Errrrr... am I reading wrong, or did you just type the same thing twice?

right, it should be ${INTEL_X86}.rpm vs noarch.rpm

> 
>>>> 			einfo "Unpacking ${rb}"
>>>> 			rpm2tar -O ${r} | tar xvf - | sed -e \
>>>> 				"s:^\.:${EROOT#/}:g" > /dev/null; assert "unpacking ${r} failed"  
>>>
>>> What's the deal with this sed?  
>>
>> Good question, but it was there since always and probably the original
>> author had good reasons for it. We will look into it and comment the code.
> 
> Didn't it change in the past? As I see it, tar here outputs file names,
> sed edits them and then you discard them to /dev/null. In this case,
> sed doesn't return any specific exit code so it seems to be a complete
> no-op.
> 
> Maybe originally it verbosely output the extracted files.
> 
>>>> EXPORT_FUNCTIONS pkg_setup src_unpack src_install pkg_postinst pkg_postrm pkg_pretend  
>>>
>>> We usually do this on top, and it's better to do it outside guards so
>>> that order from inherit is always respected.  
>>
>> we will move it up. I don't get your second comment. Do you mean the
>> case someone does
>>
>> inherit intel-sdp-r1 some-other-eclass intel-sdp
>>
>> ?
> 
> Rather something unlikely like:
> 
>   inherit foo bar intel-sdp-r1
> 
> where foo inherits intel-sdp-r1, and therefore the exports occur before
> bar, and bar takes over even though intel-sdp-r1 is explicitly
> listed last.
> 



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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [gentoo-dev] RFC: intel-sdp-r1.eclass
  2016-02-15 14:41       ` Justin Lecher (jlec)
@ 2016-02-15 14:55         ` Michał Górny
  0 siblings, 0 replies; 12+ messages in thread
From: Michał Górny @ 2016-02-15 14:55 UTC (permalink / raw
  To: Justin Lecher (jlec); +Cc: gentoo-dev

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

On Mon, 15 Feb 2016 15:41:58 +0100
"Justin Lecher (jlec)" <jlec@gentoo.org> wrote:

> On 15/02/16 15:35, Michał Górny wrote:
> > On Mon, 15 Feb 2016 14:37:41 +0100
> > "Justin Lecher (jlec)" <jlec@gentoo.org> wrote:
> >   
> >> On 15/02/16 13:59, Michał Górny wrote:  
> >>> On Mon, 15 Feb 2016 09:16:53 +0100
> >>> "Justin Lecher (jlec)" <jlec@gentoo.org> wrote:
> >>>     
> >>>> # @ECLASS-VARIABLE: INTEL_SUBDIR
> >>>> # @DEFAULT_UNSET
> >>>> # @DESCRIPTION:
> >>>> # The package sub-directory where it will end-up in /opt/intel
> >>>> # To find out its value, you have to do a raw install from the Intel tar ball    
> >>>
> >>> To be honest, I find this kinda terrible. There's a huge block of docs
> >>> which makes me feel small and confused. Maybe it'd useful to give some
> >>> semi-complete example on top (in global doc)?    
> >>
> >> That makes definitely make sense. We will add one.
> >>
> >> Although nobody other then the maintainer of this eclass will ever use it.  
> > 
> > Remember that maintainers can change. It's better to have good then
> > have new maintainers figure out all stuff over again.
> >   
> >>>> # e.g. CLI_install/rpm/intel-vtune-amplifier-xe-cli
> >>>> : ${INTEL_BIN_RPMS:=()}    
> >>>
> >>> $ : ${foo:=()}
> >>> $ declare -p foo
> >>> declare -- foo="()"
> >>>
> >>> In other words, it doesn't work the way you expect it to.    
> >>
> >> I already wondered about this. Is there any way to force a variable to
> >> be an array in bash? Or define it as an empty array?  
> > 
> > Look at e.g. python-utils-r1.
> > 
> > To check for array:
> > 
> >   if [[ $(declare -p foo) != "declare -a"* ]]; then
> >     ...
> >   fi
> > 
> > To default to empty, simple (yet a bit imperfect) way:
> > 
> >   [[ ${foo[@]} }] || foo=()  
> 
> And what about the default assignment for the man page?

Have no clue. I think someone mentioned some hack somewhere. Or maybe
we could finally fix eclass-manpages script to handle this.

> >>> Err, this is not code, you know.    
> >>
> >> This is needed for nice formatting. Otherwise there is no line break  
> > 
> > Add an empty line between the two. That should do it correctly, without
> > code blocks in devmanual.  
> 
> That will introduce an empty line between the two points.

Which is quite correct. And in any case, it's definitely not worse than
what you're causing now:

https://devmanual.gentoo.org/eclass-reference/intel-sdp.eclass/index.html

> >>> Wouldn't you be able to collapse that into one loop?    
> >>
> >> no, because the first has ${INTEL_X86}.rpm as suffeix and the later has
> >> ${INTEL_X86}.rpm.  
> > 
> > Errrrr... am I reading wrong, or did you just type the same thing twice?  
> 
> right, it should be ${INTEL_X86}.rpm vs noarch.rpm

Well, I think you still could handle this with some extra code
and conditionals, at least reduced code duplication.

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [gentoo-dev] Re: RFC: intel-sdp-r1.eclass
  2016-02-15 13:37   ` Justin Lecher (jlec)
  2016-02-15 14:35     ` Michał Górny
@ 2016-02-15 18:48     ` Martin Vaeth
  1 sibling, 0 replies; 12+ messages in thread
From: Martin Vaeth @ 2016-02-15 18:48 UTC (permalink / raw
  To: gentoo-dev

Justin Lecher (jlec) <jlec@gentoo.org> wrote:
>>> [[ -n ${_INTEL_PV4} ]] && _INTEL_PV+="${_INTEL_PV4}-"
>>> [[ -n ${_INTEL_PV1} ]] && _INTEL_PV+="${_INTEL_PV1}"
>>> [[ -n ${_INTEL_PV2} ]] && _INTEL_PV+=".${_INTEL_PV2}"
>>> [[ -n ${_INTEL_PV3} ]] && _INTEL_PV+=".${_INTEL_PV3}"
>>> [[ -n ${_INTEL_PV4} ]] && _INTEL_PV+="-${_INTEL_PV4}"
>>
>> Now, this is crazy ;-). I don't see immediately how to improve that,
>> but I suggest you thought about that.
>
> [...] So there is no sanity other then reshuffling.

You could put the logic into the string:

_INTEL_PV+="${_INTEL_PV4}${_INTEL_PV4:+-}${_INTEL_PV1}"
_INTEL_PV+="${_INTEL_PV2:+.}${_INTEL_PV2}"
_INTEL_PV+="${_INTEL_PV3:+.}${_INTEL_PV3}"
_INTEL_PV+="${_INTEL_PV4:+-}${_INTEL_PV4}"



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [gentoo-dev] Re: RFC: intel-sdp-r1.eclass
  2016-02-15 14:35     ` Michał Górny
  2016-02-15 14:41       ` Justin Lecher (jlec)
@ 2016-02-17  4:48       ` Ryan Hill
  2016-02-17  6:47         ` Michał Górny
  1 sibling, 1 reply; 12+ messages in thread
From: Ryan Hill @ 2016-02-17  4:48 UTC (permalink / raw
  To: gentoo-dev

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

On Mon, 15 Feb 2016 15:35:12 +0100
Michał Górny <mgorny@gentoo.org> wrote:
> On Mon, 15 Feb 2016 14:37:41 +0100
> "Justin Lecher (jlec)" <jlec@gentoo.org> wrote:
> > On 15/02/16 13:59, Michał Górny wrote:  
> > > On Mon, 15 Feb 2016 09:16:53 +0100
> > > "Justin Lecher (jlec)" <jlec@gentoo.org> wrote:
> > >> _isdp_big-warning() {
> > >> 	debug-print-function ${FUNCNAME} "${@}"
> > >> 	case ${1} in
> > >> 		pre-check )
> > >> 			echo ""    
> > > Don't mix echo with ewarn.    
> > Why?  
> Because they won't go through the same output channels.

That's kinda the point.  You want a blank (unstarred) space
to separate out the "important" messages from the generic
spew put out by the package manager/eclasses/build system
that you have no control over.

If you have several different messages you want a blank space
in between them so you can use e* to create whitespace within
the message to avoid the wall of text syndrome while still
making it clear where it begins and ends.

Let's take openrc for example (not picking on anyone, it's just
the last package with a bunch of different post-install messages
I happened to emerge).

Currently:

 * The OpenRC dependency data has already been migrated.
 * Caching service dependencies ... [ ok ]
 * In this version of OpenRC, the loopback interface no longer
 * satisfies the net virtual.
 * If you have services now which do not start because of this,
 * They can be fixed by adding rc_need="!net"
 * to the /etc/conf.d/<servicename> file.
 * You should also file a bug against the service asking that
 * need net be dropped from the dependencies.
 * The bug you file should block the following tracker:
 * https://bugs.gentoo.org/show_bug.cgi?id=439092
 * 
 * Bug https://bugs.gentoo.org/show_bug.cgi?id=427996 was not
 * fixed correctly in earlier versions of OpenRC.
 * The correct fix is implemented in this version, but that
 * means netmount needs to be added to the default runlevel if
 * you are using nfs file systems.
 * 
 * You should now update all files in /etc, using etc-update
 * or equivalent before restarting any services or this host.

This is pretty much unreadable to me.

Better:

 * The OpenRC dependency data has already been migrated.
 * Caching service dependencies ... [ ok ]
 *
 * In this version of OpenRC, the loopback interface no longer
 * satisfies the net virtual.
 *
 * If you have services now which do not start because of this,
 * They can be fixed by adding rc_need="!net"
 * to the /etc/conf.d/<servicename> file.
 *
 * You should also file a bug against the service asking that
 * need net be dropped from the dependencies.
 *
 * The bug you file should block the following tracker:
 * https://bugs.gentoo.org/show_bug.cgi?id=439092
 * 
 * Bug https://bugs.gentoo.org/show_bug.cgi?id=427996 was not
 * fixed correctly in earlier versions of OpenRC.
 *
 * The correct fix is implemented in this version, but that
 * means netmount needs to be added to the default runlevel if
 * you are using nfs file systems.
 * 
 * You should now update all files in /etc, using etc-update
 * or equivalent before restarting any services or this host.

This is better but you still have to read the whole thing to
make sure you didn't miss anything important.

Even better:

 * The OpenRC dependency data has already been migrated.
 * Caching service dependencies ... [ ok ]

 * In this version of OpenRC, the loopback interface no longer
 * satisfies the net virtual.
 *
 * If you have services now which do not start because of this,
 * They can be fixed by adding rc_need="!net"
 * to the /etc/conf.d/<servicename> file.
 *
 * You should also file a bug against the service asking that
 * need net be dropped from the dependencies.
 * The bug you file should block the following tracker:
 *
 * https://bugs.gentoo.org/show_bug.cgi?id=439092
 
 * Bug https://bugs.gentoo.org/show_bug.cgi?id=427996 was not
 * fixed correctly in earlier versions of OpenRC.
 *
 * The correct fix is implemented in this version, but that
 * means netmount needs to be added to the default runlevel if
 * you are using nfs file systems.
 
 * You should now update all files in /etc, using etc-update
 * or equivalent before restarting any services or this host.

Here I can read the first line of the second block and know I can
skip the next 12 lines without missing anything.  The next block
isn't worded the greatest, but that's beside the point.  And now
I get an important message at the end that I previously never 
noticed because tl;dr.

You're right that using echo means the whitespace doesn't get
saved by the elog system.  A while back someone proposed we
add espace for exactly this reason but IIRC they were laughed
down, which is a shame.

-- 

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [gentoo-dev] Re: RFC: intel-sdp-r1.eclass
  2016-02-17  4:48       ` [gentoo-dev] " Ryan Hill
@ 2016-02-17  6:47         ` Michał Górny
  2016-02-17 10:53           ` Duncan
  0 siblings, 1 reply; 12+ messages in thread
From: Michał Górny @ 2016-02-17  6:47 UTC (permalink / raw
  To: Ryan Hill; +Cc: gentoo-dev

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

On Tue, 16 Feb 2016 22:48:08 -0600
Ryan Hill <rhill@gentoo.org> wrote:

> On Mon, 15 Feb 2016 15:35:12 +0100
> Michał Górny <mgorny@gentoo.org> wrote:
> > On Mon, 15 Feb 2016 14:37:41 +0100
> > "Justin Lecher (jlec)" <jlec@gentoo.org> wrote:  
> > > On 15/02/16 13:59, Michał Górny wrote:    
> > > > On Mon, 15 Feb 2016 09:16:53 +0100
> > > > "Justin Lecher (jlec)" <jlec@gentoo.org> wrote:  
> > > >> _isdp_big-warning() {
> > > >> 	debug-print-function ${FUNCNAME} "${@}"
> > > >> 	case ${1} in
> > > >> 		pre-check )
> > > >> 			echo ""      
> > > > Don't mix echo with ewarn.      
> > > Why?    
> > Because they won't go through the same output channels.  
> 
> That's kinda the point.  You want a blank (unstarred) space
> to separate out the "important" messages from the generic
> spew put out by the package manager/eclasses/build system
> that you have no control over.

This is not just that. Different output channels mean that:

- some of them can be turned off. In this case, you end up with empty
  lines and no message.

- There is no guarantee of correct output order! The empty lines may
  move randomly over the text.

- When the function happens to be used in $(), part of the output will
  be collected (not that it's 100% valid concern right now because
  EAPI 6 doesn't guarantee e* using stderr yet).

> If you have several different messages you want a blank space
> in between them so you can use e* to create whitespace within
> the message to avoid the wall of text syndrome while still
> making it clear where it begins and ends.
> 
> Let's take openrc for example (not picking on anyone, it's just
> the last package with a bunch of different post-install messages
> I happened to emerge).
> 
> Currently:
> 
>  * The OpenRC dependency data has already been migrated.
>  * Caching service dependencies ... [ ok ]
>  * In this version of OpenRC, the loopback interface no longer
>  * satisfies the net virtual.
>  * If you have services now which do not start because of this,
>  * They can be fixed by adding rc_need="!net"
>  * to the /etc/conf.d/<servicename> file.
>  * You should also file a bug against the service asking that
>  * need net be dropped from the dependencies.
>  * The bug you file should block the following tracker:
>  * https://bugs.gentoo.org/show_bug.cgi?id=439092
>  * 
>  * Bug https://bugs.gentoo.org/show_bug.cgi?id=427996 was not
>  * fixed correctly in earlier versions of OpenRC.
>  * The correct fix is implemented in this version, but that
>  * means netmount needs to be added to the default runlevel if
>  * you are using nfs file systems.
>  * 
>  * You should now update all files in /etc, using etc-update
>  * or equivalent before restarting any services or this host.
> 
> This is pretty much unreadable to me.
> 
> Better:
> 
>  * The OpenRC dependency data has already been migrated.
>  * Caching service dependencies ... [ ok ]
>  *
>  * In this version of OpenRC, the loopback interface no longer
>  * satisfies the net virtual.
>  *
>  * If you have services now which do not start because of this,
>  * They can be fixed by adding rc_need="!net"
>  * to the /etc/conf.d/<servicename> file.
>  *
>  * You should also file a bug against the service asking that
>  * need net be dropped from the dependencies.
>  *
>  * The bug you file should block the following tracker:
>  * https://bugs.gentoo.org/show_bug.cgi?id=439092
>  * 
>  * Bug https://bugs.gentoo.org/show_bug.cgi?id=427996 was not
>  * fixed correctly in earlier versions of OpenRC.
>  *
>  * The correct fix is implemented in this version, but that
>  * means netmount needs to be added to the default runlevel if
>  * you are using nfs file systems.
>  * 
>  * You should now update all files in /etc, using etc-update
>  * or equivalent before restarting any services or this host.
> 
> This is better but you still have to read the whole thing to
> make sure you didn't miss anything important.
> 
> Even better:
> 
>  * The OpenRC dependency data has already been migrated.
>  * Caching service dependencies ... [ ok ]
> 
>  * In this version of OpenRC, the loopback interface no longer
>  * satisfies the net virtual.
>  *
>  * If you have services now which do not start because of this,
>  * They can be fixed by adding rc_need="!net"
>  * to the /etc/conf.d/<servicename> file.
>  *
>  * You should also file a bug against the service asking that
>  * need net be dropped from the dependencies.
>  * The bug you file should block the following tracker:
>  *
>  * https://bugs.gentoo.org/show_bug.cgi?id=439092
>  
>  * Bug https://bugs.gentoo.org/show_bug.cgi?id=427996 was not
>  * fixed correctly in earlier versions of OpenRC.
>  *
>  * The correct fix is implemented in this version, but that
>  * means netmount needs to be added to the default runlevel if
>  * you are using nfs file systems.
>  
>  * You should now update all files in /etc, using etc-update
>  * or equivalent before restarting any services or this host.
> 
> Here I can read the first line of the second block and know I can
> skip the next 12 lines without missing anything.  The next block
> isn't worded the greatest, but that's beside the point.  And now
> I get an important message at the end that I previously never 
> noticed because tl;dr.
> 
> You're right that using echo means the whitespace doesn't get
> saved by the elog system.  A while back someone proposed we
> add espace for exactly this reason but IIRC they were laughed
> down, which is a shame.

So... to summarize your point. You shouldn't use the correct function
that is saved in elog which is primary way of getting info because you
find it more convenient to have empty non-'starred' lines that don't
actually get to elog and make elog a mess?

If you really don't like empty 'starred' lines (and I actually like
them since they make separation between packages cleaner), why not
submit a patch for Portage and make 'elog' with no arguments output log
line without a star? That's a trivial solution that doesn't require
extra functions for the sake of inventing elogspace, ewarnspace, ...

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [gentoo-dev] Re: RFC: intel-sdp-r1.eclass
  2016-02-17  6:47         ` Michał Górny
@ 2016-02-17 10:53           ` Duncan
  2016-02-17 11:09             ` David Seifert
  2016-02-17 11:20             ` Michał Górny
  0 siblings, 2 replies; 12+ messages in thread
From: Duncan @ 2016-02-17 10:53 UTC (permalink / raw
  To: gentoo-dev

Michał Górny posted on Wed, 17 Feb 2016 07:47:06 +0100 as excerpted:

> On Tue, 16 Feb 2016 22:48:08 -0600 Ryan Hill <rhill@gentoo.org> wrote:
> 
>> On Mon, 15 Feb 2016 15:35:12 +0100 Michał Górny <mgorny@gentoo.org>
>> wrote:
>> > On Mon, 15 Feb 2016 14:37:41 +0100 "Justin Lecher (jlec)"
>> > <jlec@gentoo.org> wrote:
>> > > On 15/02/16 13:59, Michał Górny wrote:

>> > > > Don't mix echo with ewarn.
>> > > Why?
>> > Because they won't go through the same output channels.
>> 
>> That's kinda the point.  You want a blank (unstarred) space to separate
>> out the "important" messages from the generic spew put out by the
>> package manager/eclasses/build system that you have no control over.
> 
> This is not just that. Different output channels mean that:

> - There is no guarantee of correct output order! The empty lines may
>   move randomly over the text.

Good point!  (Of course the others are too, but this one could be 
particularly damaging to the intended communication.)

>> If you have several different messages you want a blank space in
>> between them so you can use e* to create whitespace within the message
>> to avoid the wall of text syndrome while still making it clear where it
>> begins and ends.

>> You're right that using echo means the whitespace doesn't get saved by
>> the elog system.  A while back someone proposed we add espace for
>> exactly this reason but IIRC they were laughed down, which is a shame.
> 
> So... to summarize your point. You shouldn't use the correct function
> that is saved in elog which is primary way of getting info because you
> find it more convenient to have empty non-'starred' lines that don't
> actually get to elog and make elog a mess?
> 
> If you really don't like empty 'starred' lines (and I actually like them
> since they make separation between packages cleaner), why not submit a
> patch for Portage and make 'elog' with no arguments output log line
> without a star? That's a trivial solution that doesn't require extra
> functions for the sake of inventing elogspace, ewarnspace, ...

It is at least possible to use say blank ewarn between elog lines, or the 
reverse, so while there's no totally blank separator, there's at least a 
different color to the star on the starred-blank-line separator.

Similarly, if there's more than one "topic" to the messages, and they're 
of different severity, the severities can be interspersed to get color 
separation.

I believe I've seen both techniques used to good effect in a few packages 
in the past, but I can't name any off the top of my head.

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [gentoo-dev] Re: RFC: intel-sdp-r1.eclass
  2016-02-17 10:53           ` Duncan
@ 2016-02-17 11:09             ` David Seifert
  2016-02-17 11:20             ` Michał Górny
  1 sibling, 0 replies; 12+ messages in thread
From: David Seifert @ 2016-02-17 11:09 UTC (permalink / raw
  To: gentoo-dev

On Mi, 2016-02-17 at 10:53 +0000, Duncan wrote:
> Michał Górny posted on Wed, 17 Feb 2016 07:47:06 +0100 as excerpted:
> 
> > On Tue, 16 Feb 2016 22:48:08 -0600 Ryan Hill <rhill@gentoo.org>
> > wrote:
> > 
> > > On Mon, 15 Feb 2016 15:35:12 +0100 Michał Górny <mgorny@gentoo.or
> > > g>
> > > wrote:
> > > > On Mon, 15 Feb 2016 14:37:41 +0100 "Justin Lecher (jlec)"
> > > > <jlec@gentoo.org> wrote:
> > > > > On 15/02/16 13:59, Michał Górny wrote:
> 
> > > > > > Don't mix echo with ewarn.
> > > > > Why?
> > > > Because they won't go through the same output channels.
> > > 
> > > That's kinda the point.  You want a blank (unstarred) space to
> > > separate
> > > out the "important" messages from the generic spew put out by the
> > > package manager/eclasses/build system that you have no control
> > > over.
> > 
> > This is not just that. Different output channels mean that:
> 
> > - There is no guarantee of correct output order! The empty lines
> > may
> >   move randomly over the text.
> 
> Good point!  (Of course the others are too, but this one could be 
> particularly damaging to the intended communication.)
> 
> > > If you have several different messages you want a blank space in
> > > between them so you can use e* to create whitespace within the
> > > message
> > > to avoid the wall of text syndrome while still making it clear
> > > where it
> > > begins and ends.
> 
> > > You're right that using echo means the whitespace doesn't get
> > > saved by
> > > the elog system.  A while back someone proposed we add espace for
> > > exactly this reason but IIRC they were laughed down, which is a
> > > shame.
> > 
> > So... to summarize your point. You shouldn't use the correct
> > function
> > that is saved in elog which is primary way of getting info because
> > you
> > find it more convenient to have empty non-'starred' lines that
> > don't
> > actually get to elog and make elog a mess?
> > 
> > If you really don't like empty 'starred' lines (and I actually like
> > them
> > since they make separation between packages cleaner), why not
> > submit a
> > patch for Portage and make 'elog' with no arguments output log line
> > without a star? That's a trivial solution that doesn't require
> > extra
> > functions for the sake of inventing elogspace, ewarnspace, ...
> 
> It is at least possible to use say blank ewarn between elog lines, or
> the 
> reverse, so while there's no totally blank separator, there's at
> least a 
> different color to the star on the starred-blank-line separator.
> 
> Similarly, if there's more than one "topic" to the messages, and
> they're 
> of different severity, the severities can be interspersed to get
> color 
> separation.
> 
> I believe I've seen both techniques used to good effect in a few
> packages 
> in the past, but I can't name any off the top of my head.
> 

For all those who care, I've updated the eclass under:

https://github.com/gentoo-science/sci/pull/588


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [gentoo-dev] Re: RFC: intel-sdp-r1.eclass
  2016-02-17 10:53           ` Duncan
  2016-02-17 11:09             ` David Seifert
@ 2016-02-17 11:20             ` Michał Górny
  1 sibling, 0 replies; 12+ messages in thread
From: Michał Górny @ 2016-02-17 11:20 UTC (permalink / raw
  To: gentoo-dev, Duncan

Dnia 17 lutego 2016 11:53:32 CET, Duncan <1i5t5.duncan@cox.net> napisał(a):
>Michał Górny posted on Wed, 17 Feb 2016 07:47:06 +0100 as excerpted:
>
>> On Tue, 16 Feb 2016 22:48:08 -0600 Ryan Hill <rhill@gentoo.org>
>wrote:
>> 
>>> On Mon, 15 Feb 2016 15:35:12 +0100 Michał Górny <mgorny@gentoo.org>
>>> wrote:
>>> > On Mon, 15 Feb 2016 14:37:41 +0100 "Justin Lecher (jlec)"
>>> > <jlec@gentoo.org> wrote:
>>> > > On 15/02/16 13:59, Michał Górny wrote:
>
>>> > > > Don't mix echo with ewarn.
>>> > > Why?
>>> > Because they won't go through the same output channels.
>>> 
>>> That's kinda the point.  You want a blank (unstarred) space to
>separate
>>> out the "important" messages from the generic spew put out by the
>>> package manager/eclasses/build system that you have no control over.
>> 
>> This is not just that. Different output channels mean that:
>
>> - There is no guarantee of correct output order! The empty lines may
>>   move randomly over the text.
>
>Good point!  (Of course the others are too, but this one could be 
>particularly damaging to the intended communication.)
>
>>> If you have several different messages you want a blank space in
>>> between them so you can use e* to create whitespace within the
>message
>>> to avoid the wall of text syndrome while still making it clear where
>it
>>> begins and ends.
>
>>> You're right that using echo means the whitespace doesn't get saved
>by
>>> the elog system.  A while back someone proposed we add espace for
>>> exactly this reason but IIRC they were laughed down, which is a
>shame.
>> 
>> So... to summarize your point. You shouldn't use the correct function
>> that is saved in elog which is primary way of getting info because
>you
>> find it more convenient to have empty non-'starred' lines that don't
>> actually get to elog and make elog a mess?
>> 
>> If you really don't like empty 'starred' lines (and I actually like
>them
>> since they make separation between packages cleaner), why not submit
>a
>> patch for Portage and make 'elog' with no arguments output log line
>> without a star? That's a trivial solution that doesn't require extra
>> functions for the sake of inventing elogspace, ewarnspace, ...
>
>It is at least possible to use say blank ewarn between elog lines, or
>the 
>reverse, so while there's no totally blank separator, there's at least
>a 
>different color to the star on the starred-blank-line separator.
>
>Similarly, if there's more than one "topic" to the messages, and
>they're 
>of different severity, the severities can be interspersed to get color 
>separation.
>
>I believe I've seen both techniques used to good effect in a few
>packages 
>in the past, but I can't name any off the top of my head.

This is mixing channels again. Someone may decide to output warnings separately from elogs. Or not output elogs at all.


-- 
Best regards,
Michał Górny (by phone)


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-02-17 11:20 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-15  8:16 [gentoo-dev] RFC: intel-sdp-r1.eclass Justin Lecher (jlec)
2016-02-15 12:59 ` Michał Górny
2016-02-15 13:37   ` Justin Lecher (jlec)
2016-02-15 14:35     ` Michał Górny
2016-02-15 14:41       ` Justin Lecher (jlec)
2016-02-15 14:55         ` Michał Górny
2016-02-17  4:48       ` [gentoo-dev] " Ryan Hill
2016-02-17  6:47         ` Michał Górny
2016-02-17 10:53           ` Duncan
2016-02-17 11:09             ` David Seifert
2016-02-17 11:20             ` Michał Górny
2016-02-15 18:48     ` Martin Vaeth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox