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 F3E9815817D for ; Sun, 16 Jun 2024 18:15:41 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id A0FE82BC01B; Sun, 16 Jun 2024 18:15:37 +0000 (UTC) Received: from smtp.gentoo.org (woodpecker.gentoo.org [IPv6:2001:470:ea4a:1:5054:ff:fec7:86e4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 475C92BC014 for ; Sun, 16 Jun 2024 18:15:37 +0000 (UTC) Message-ID: <45bcb8d5-bea7-49a2-81e8-ee2d161872bc@gentoo.org> Date: Sun, 16 Jun 2024 21:15:25 +0300 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 User-Agent: Mozilla Thunderbird Subject: Re: [gentoo-dev] [PATCH v4] greadme.eclass: new eclass To: gentoo-dev@lists.gentoo.org, Florian Schmaus References: <20240616155123.1016092-1-flow@gentoo.org> From: Arthur Zamarin Content-Language: en-US In-Reply-To: <20240616155123.1016092-1-flow@gentoo.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------Lkmy2PRqayEpSM8yqXnX76Cc" X-Archives-Salt: 3f5cb729-92ca-4dbb-b655-4d4fb5de507d X-Archives-Hash: e3d5a114241ca65b2dd060dc13718492 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --------------Lkmy2PRqayEpSM8yqXnX76Cc Content-Type: multipart/mixed; boundary="------------SkMu36smw0B26DDhmDe5GC6T"; protected-headers="v1" From: Arthur Zamarin To: gentoo-dev@lists.gentoo.org, Florian Schmaus Message-ID: <45bcb8d5-bea7-49a2-81e8-ee2d161872bc@gentoo.org> Subject: Re: [gentoo-dev] [PATCH v4] greadme.eclass: new eclass References: <20240616155123.1016092-1-flow@gentoo.org> In-Reply-To: <20240616155123.1016092-1-flow@gentoo.org> --------------SkMu36smw0B26DDhmDe5GC6T Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 16/06/2024 18.51, Florian Schmaus wrote: > This new eclass includes various improvements over the existing > readme.gentoo-r1.eclass. So, some weird question from me - why is it called greadme? I can understand why you don't want to modify existing eclass, but why not call it "readme.gentoo-r2.eclass"? This should make it a little less confusing (cause I imagine folks asking - which to use. With -r2 we all know which one is better). > First, the content of README.gento will only be shown on new > installations or if it has changed between updates. >=20 > Second, it allows the composition of readme via bash's heredoc. Is it common usage you think? I've never felt the need for that. > Third, it exports phase functions, which helps to reduce the boilerplat= e > code in many cases. That actually sounds nice, but there is a caveat - if someone inherits that eclass after other eclass phases, it might shadow those eclass phase functions. There was a Check Request for pkgcheck to catch such shadowed phase functions, but this still wasn't easy to implement, so we still miss it. So I'm afraid this eclass being inherited last, causing shadowed phases. > Finally, unlike readme.gentoo-r1.elcass, this eclass does not need to > store the content of the readme in an environment variable. Not having > to store the content in an environment variable reduces the pollution o= f > the environment (sadly, this only refers to the process environment). I'll be honest, I never felt this is really needed? From looking at the current -r1 eclass, you could define DOC_CONTENTS just before invoking readme.gentoo_create_doc, so you could for example modify as you want the message and use `local DOC_CONTENTS=3D"..."`. I'm not saying directly I'm against, don't understand me incorrectly. What I want to see a current way to use -r1 eclass, which is bad/ugly/boilerplate, and the new way with your eclass which is nicer. A nice thing I took from mgorny's eclass changes, is giving an example diff for a package, showing the new usage, diff, and "win points". > Signed-off-by: Florian Schmaus > --- > eclass/greadme.eclass | 240 ++++++++++++++++++++++++++++++++++++++++++= > 1 file changed, 240 insertions(+) > create mode 100644 eclass/greadme.eclass >=20 > diff --git a/eclass/greadme.eclass b/eclass/greadme.eclass > new file mode 100644 > index 000000000000..ddbcc8e9858d > --- /dev/null > +++ b/eclass/greadme.eclass > @@ -0,0 +1,240 @@ > +# 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 > +# @SUPPORTED_EAPIS: 8 > +# @BLURB: install a doc file, that will be conditionally shown via elo= g messages > +# @DESCRIPTION: > +# An eclass for installing a README.gentoo doc file with important > +# information for the user. The content of README.gentoo will shown b= e > +# via elog messages either on fresh installations or if the contents o= f > +# 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=3Dfoo. > +# 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=3D1 > + > +case ${EAPI} in > + 8) ;; > + *) die "${ECLASS}: EAPI ${EAPI:-0} not supported" ;; > +esac > + > +_GREADME_FILENAME=3D"README.gentoo" > +_GREADME_TMP_FILE=3D"${T}/${_GREADME_FILENAME}" > +_GREADME_REL_PATH=3D"/usr/share/doc/${PF}/${_GREADME_FILENAME}" > + > +# @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. > + > +# @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} =3D=3D --* ]]; do > + case ${1} in > + --append) > + append=3D1 > + shift > + ;; > + esac > + done > + > + 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 > + if [[ -f ${_GREADME_TMP_FILE} ]]; then > + die "Gentoo README already exists while trying to create it" > + fi > + > + 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=3D"${1}" > + if [[ -z ${input_doc_file} ]]; then > + die "No file specified" > + fi > + > + if [[ -f ${_GREADME_TMP_FILE} ]]; then > + die "Gentoo README already exists" > + 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} "${@}" > + > + # Subshell to avoid pollution of calling environment. > + ( > + docinto . > + dodoc "${_GREADME_TMP_FILE}" > + ) > + > + # Exclude the readme file from compression, so that its contents can > + # be easily compared. > + docompress -x "${_GREADME_REL_PATH}" > +} > + > +# @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=3D"fresh-install" > + return > + fi > + > + if [[ -v GREADME_SHOW ]]; then > + case ${GREADME_SHOW} in > + yes) > + _GREADME_SHOW=3D"forced" > + ;; > + no) > + _GREADME_SHOW=3D"" > + ;; > + *) > + die "Invalid argument of GREADME_SHOW: ${GREADME_SHOW}" > + ;; > + esac > + return > + fi > + > + local image_greadme_file=3D"${ED}${_GREADME_REL_PATH}" > + if [[ ! -f ${image_greadme_file} ]]; then > + # No README file was created by the ebuild. > + _GREADME_SHOW=3D"" > + return > + fi > + > + check_live_doc_file() { > + local cur_pvr=3D$1 > + local live_greadme_file=3D"${EROOT}/usr/share/doc/${PN}-${cur_pvr}/$= {_GREADME_FILENAME}" > + > + if [[ ! -f ${live_greadme_file} ]]; then > + _GREADME_SHOW=3D"no-current-greadme" > + return > + fi > + > + cmp -s "${live_greadme_file}" "${image_greadme_file}" > + local ret=3D$? > + case ${ret} in > + 0) > + _GREADME_SHOW=3D"" > + ;; > + 1) > + _GREADME_SHOW=3D"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. > + 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 con= tents. > + return > + fi > + > + local greadme=3D"${EROOT}${_GREADME_REL_PATH}" > + > + if [[ ! -f ${greadme} ]]; then > + ewarn "Unable to show ${_GREADME_FILENAME}, file not installed (FEAT= URES=3Dnodoc enabled?)." > + return > + fi > + > + local line > + while read -r line; do elog "${line}"; done < "${greadme}" > + elog "" > + elog "(Note: Above message is only printed the first time package is"= > + elog "installed or if the message changes on update. Please look at" > + elog "${EPREFIX}${_GREADME_REL_PATH} for future reference)" > +} > + > +fi > + > +EXPORT_FUNCTIONS pkg_preinst pkg_postinst --=20 Arthur Zamarin arthurzam@gentoo.org Gentoo Linux developer (Python, pkgcore stack, QA, Arch Teams, GURU) --------------SkMu36smw0B26DDhmDe5GC6T-- --------------Lkmy2PRqayEpSM8yqXnX76Cc Content-Type: application/pgp-signature; name="OpenPGP_signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="OpenPGP_signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAEBCgAdFiEE/axFlFuH2ptjtO5EAqCvUD0SBQQFAmZvK74ACgkQAqCvUD0S BQRylgf/Tcq/6+8AF1D96Bl/vCblvLWgW7icqqak5Aiqb2fg6s3JMsSVWfLg/TiV JYKkQ035dJfFWzv6OMVHSMNDEYhgIrggNpNpE4/WZrI8KhGZ62IK7xl9VLFxlS9v TBOxfm7i0d+ZPAGTL28OjGn2IiMtmntuby02A+O9FJ2araRJGaeqdEnBKbfvQLka gTptwt+rzeVpKht4A2ni5XDJQl5GqJ7Ktfc3rOQ41Px8bl7g0vGEBDf85IgLSqXe ZAnUq+m451lhUs1YJZLyYxivkAJcIF5uDXAqUSdI3C68XbYiwmN0TCeWfcVWFS7D 6+ZtbZQz1nMQXTGoIS/aG3OsolED5g== =ogDO -----END PGP SIGNATURE----- --------------Lkmy2PRqayEpSM8yqXnX76Cc--