From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by finch.gentoo.org (Postfix) with ESMTPS id 88950158170 for ; Sun, 21 Jul 2024 08:27:06 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id AB51DE2A69; Sun, 21 Jul 2024 08:27:02 +0000 (UTC) Received: from smtp.gentoo.org (woodpecker.gentoo.org [140.211.166.183]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 576DEE2A65 for ; Sun, 21 Jul 2024 08:27:02 +0000 (UTC) From: Ulrich Mueller To: Florian Schmaus Cc: gentoo-dev@lists.gentoo.org Subject: Re: [gentoo-dev] [PATCH v5] greadme.eclass: new eclass In-Reply-To: <20240716092515.285042-1-flow@gentoo.org> (Florian Schmaus's message of "Tue, 16 Jul 2024 11:25:15 +0200") References: <20240716092515.285042-1-flow@gentoo.org> Date: Sun, 21 Jul 2024 10:26:50 +0200 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-dev@lists.gentoo.org Reply-to: gentoo-dev@lists.gentoo.org X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" X-Archives-Salt: 35d5b3c5-37fc-4546-984c-d074483d18c6 X-Archives-Hash: 9e6734cfb56a1d2d82d94947ec7656aa --=-=-= Content-Type: text/plain >>>>> On Tue, 16 Jul 2024, Florian Schmaus wrote: > Notes: > - also show readme contents if FEATURES=nodoc Good. :) > --- /dev/null > +++ b/eclass/greadme.eclass > @@ -0,0 +1,257 @@ > +# Copyright 1999-2024 Gentoo Authors > +# Distributed under the terms of the GNU General Public License v2 > + > +# @ECLASS: greadme.eclass > +# @MAINTAINER: > +# Florian Schmaus > +# @AUTHOR: > +# Author: Florian Schmaus "Author:" is redundant when there's only a single author. > +# @SUPPORTED_EAPIS: 8 > +# @BLURB: install a doc file, that will be conditionally shown via elog messages > +# @DESCRIPTION: > +# An eclass for installing a README.gentoo doc file with important > +# information for the user. The content of README.gentoo will shown be > +# via elog messages either on fresh installations or if the contents of > +# the file have changed. Furthermore, the README.gentoo file will be > +# installed under /usr/share/doc/${PF} for later consultation. > +# > +# This eclass was inspired by readme.gentoo-r1.eclass. The main > +# differences are as follows. Firstly, it only displays the doc file > +# contents if they have changed (unless GREADME_SHOW is set). > +# Secondly, it provides a convenient API to install the doc file via > +# stdin. > +# > +# @CODE > +# inherit greadme > +# > +# src_install() { > +# ... > +# greadme_stdin <<-EOF > +# This is the content of the created readme doc file. > +# EOF > +# ... > +# if use foo; then > +# greadme_stdin --append <<-EOF > +# This is conditional readme content, based on USE=foo. > +# EOF > +# fi > +# } > +# @CODE > +# > +# If the ebuild overrides the default pkg_preinst or respectively > +# pkg_postinst, then it must call greadme_pkg_preinst and > +# greadme_pkg_postinst explicitly. > + > +if [[ -z ${_GREADME_ECLASS} ]]; then > +_GREADME_ECLASS=1 > + > +case ${EAPI} in > + 8) ;; > + *) die "${ECLASS}: EAPI ${EAPI:-0} not supported" ;; > +esac > + > +_GREADME_FILENAME="README.gentoo" > +_GREADME_TMP_FILE="${T}/${_GREADME_FILENAME}" > +_GREADME_REL_PATH="/usr/share/doc/${PF}/${_GREADME_FILENAME}" These are internal variables and they're never set to anything else. I also think that it is unlikely that "README.gentoo" would ever be changed to anything else. IMHO hardcoding it would improve readability. ("README.gentoo" makes it immediately clear what is happening, while with ${_GREADME_FILENAME} one must look up what the variable is.) > + > +# @ECLASS_VARIABLE: GREADME_SHOW > +# @DEFAULT_UNSET > +# @DESCRIPTION: > +# If set to "yes" then unconditionally show the contents of the readme > +# file in pkg_postinst via elog. If set to "no", then do not show the > +# contents of the readme file, even if they have changed. > + > +# @ECLASS_VARIABLE: GREADME_AUTOFORMATTING_DISABLED It is an input variable, so make it a (grammatical) command, e.g. GREADME_DISABLE_AUTOFORMAT? > +# @DEFAULT_UNSET > +# @DESCRIPTION: > +# If non-empty, the readme file will not be automatically formatted. > + > +# @FUNCTION: greadme_stdin > +# @USAGE: [--append] > +# @DESCRIPTION: > +# Create the readme doc via stdin. You can use --append to append to an > +# existing readme doc. > +greadme_stdin() { > + debug-print-function ${FUNCNAME} "${@}" > + > + local append > + while [[ -n ${1} ]] && [[ ${1} == --* ]]; do > + case ${1} in > + --append) > + append=1 > + shift > + ;; > + esac > + done Simply check for either no option, or one option that must be exactly "--append". Everything else should be an error, i.e. not silent ignoring of parameters. Something like this: [[ ${1} = --append ]] && { append=1; shift; } [[ $# -eq 0 ]] || die "${FUNCNAME[0]}: Bad parameters: $*" > + > + if [[ -n ${append} ]]; then > + if [[ ! -f ${_GREADME_TMP_FILE} ]]; then > + die "Gentoo README does not exist when trying to append to it" > + fi > + > + cat >> "${_GREADME_TMP_FILE}" || die > + else > + cat > "${_GREADME_TMP_FILE}" || die > + fi > + > + _greadme_install_doc > +} > + > +# @FUNCTION: greadme_file > +# @USAGE: > +# @DESCRIPTION: > +# Installs the provided file as readme doc. > +greadme_file() { > + debug-print-function ${FUNCNAME} "${@}" > + > + local input_doc_file="${1}" > + if [[ -z ${input_doc_file} ]]; then > + die "No file specified" > + fi > + > + cp "${input_doc_file}" "${_GREADME_TMP_FILE}" || die > + > + _greadme_install_doc > +} > + > +# @FUNCTION: _greadme_install_doc > +# @INTERNAL > +# @DESCRIPTION: > +# Installs the readme file from the temp directory into the image. > +_greadme_install_doc() { > + debug-print-function ${FUNCNAME} "${@}" > + > + local greadme="${_GREADME_TMP_FILE}" > + if [[ ! "${GREADME_AUTOFORMATTING_DISABLED}" ]]; then Quotation marks are not necessary here. > + greadme="${_GREADME_TMP_FILE}".formatted > + > + # Use fold, followed by a sed to strip trailing whitespace. > + # https://bugs.gentoo.org/460050#c7 > + fold -s -w 70 "${_GREADME_TMP_FILE}" | > + sed 's/[[:space:]]*$//' > "${greadme}" > + assert "failed to autoformat ${_GREADME_FILENAME}" > + fi > + > + # Subshell to avoid pollution of calling environment. > + ( > + docinto . > + newdoc "${greadme}" "${_GREADME_FILENAME}" > + ) > + > + # Exclude the readme file from compression, so that its contents can > + # be easily compared. > + docompress -x "${_GREADME_REL_PATH}" > + > + # Save the contents of the of the readme. Unfortunately we have to > + # do this in src_* phase, because FEATURES=nodoc is applied right > + # after src_install and not after pkg_preinst. > + _GREADME_CONTENT=$(< "${greadme}") > +} > + > +# @FUNCTION: greadme_pkg_preinst > +# @DESCRIPTION: > +# Performs checks like comparing the readme doc from the image with a > +# potentially existing one in the live system. > +greadme_pkg_preinst() { > + debug-print-function ${FUNCNAME} "${@}" > + > + if [[ -z ${REPLACING_VERSIONS} ]]; then > + _GREADME_SHOW="fresh-install" > + return > + fi > + > + if [[ -v GREADME_SHOW ]]; then > + case ${GREADME_SHOW} in > + yes) > + _GREADME_SHOW="forced" > + ;; > + no) > + _GREADME_SHOW="" > + ;; > + *) > + die "Invalid argument of GREADME_SHOW: ${GREADME_SHOW}" > + ;; > + esac > + return > + fi > + > + local image_greadme_file="${ED}${_GREADME_REL_PATH}" > + if [[ ! -f ${image_greadme_file} ]]; then > + if [[ -v _GREADME_CONTENT ]]; then > + # There is no greadme in the image but the ebuild created > + # one. This is likely because FEATURES=nodoc is active. > + # Unconditionally show greadme's contents. > + _GREADME_SHOW="nodoc-active" > + else > + # No README file was created by the ebuild. > + _GREADME_SHOW="" > + fi > + > + return > + fi > + > + check_live_doc_file() { > + local cur_pvr=$1 > + local live_greadme_file="${EROOT}/usr/share/doc/${PN}-${cur_pvr}/${_GREADME_FILENAME}" > + > + if [[ ! -f ${live_greadme_file} ]]; then > + _GREADME_SHOW="no-current-greadme" > + return > + fi > + > + cmp -s "${live_greadme_file}" "${image_greadme_file}" > + local ret=$? > + case ${ret} in You could check for $? directly here, without the need for the intermediate "ret" variable. > + 0) > + _GREADME_SHOW="" > + ;; > + 1) > + _GREADME_SHOW="content-differs" > + ;; > + *) > + die "cmp failed with ${ret}" > + ;; > + esac > + } > + > + local replaced_version > + for replaced_version in ${REPLACING_VERSIONS}; do > + check_live_doc_file ${replaced_version} > + > + # Once _GREADME_SHOW is non empty, we found a reason to show the > + # readme and we can abort the loop. Having more than one element in REPLACING_VERSIONS is certainly a corner case, but I still wonder about the logic. Shouldn't it be the other way around, i.e. if there is _any_ empty ${_GREADME_SHOW} then there is an identical file installed that has been shown before, i.e. the file should _not_ be shown again? > + if [[ -n ${_GREADME_SHOW} ]]; then > + break > + fi > + done > +} > + > +# @FUNCTION: greadme_pkg_postinst > +# @DESCRIPTION: > +# Conditionally shows the contents of the readme doc via elog. > +greadme_pkg_postinst() { > + debug-print-function ${FUNCNAME} "${@}" > + > + if [[ ! -v _GREADME_SHOW ]]; then > + die "_GREADME_SHOW not set. Did you call greadme_pkg_preinst?" > + fi > + > + if [[ -z ${_GREADME_SHOW} ]]; then > + # If _GREADME_SHOW is empty, then there is no reason to show the contents. > + return > + fi > + > + local line > + printf '%s\n' "${_GREADME_CONTENT}" | while read -r line; do > + elog "${line}" > + done > + elog "" > + elog "NOTE: Above message is only printed the first time package is" > + elog "installed or if the message changed. Please look at" > + elog "${EPREFIX}${_GREADME_REL_PATH}" > + elog "for future reference." > +} > + > +fi > + > +EXPORT_FUNCTIONS pkg_preinst pkg_postinst --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQFDBAEBCAAtFiEEtDnZ1O9xIP68rzDbUYgzUIhBXi4FAmacxkoPHHVsbUBnZW50 b28ub3JnAAoJEFGIM1CIQV4u2ikIAMLQZxkI/1nOaeTD+RuolJLwGTfWWunq1IK/ nfV/YOLlEtU1o6HtFcfIuxay3rZAtuXRo+ym2OkxA3frKdsJdEKNrmL2PospPNVe h/FRkJLEYDaJa/ffGwbVKyziwDugoAi2dTsXQEjp0M9WiWnDXx4A7Cl7wVz6Q1gb Jzxd7vBF48kSZCbsuLE5p3SPCzVQiaanlLuK2igiggufhfDY9dMqOjzTQRTlCzvv /nFX6/7rC784uDlXPknamCv9pj+DsPkbY2dyyX8bK06kC2d2aB/o82k5TbklhbYF 5EwOCUTqgwJVeUrxTdsDzCnP4IKcvzdXScDTyJOy87VF9Ymmo/Y= =fySV -----END PGP SIGNATURE----- --=-=-=--