* [gentoo-dev] [PATCH v2 2/3] add UNPACKER_NO_BANNER variable
2024-01-09 8:39 ` [gentoo-dev] [PATCH v2 1/3] greadme.eclass: new eclass Florian Schmaus
@ 2024-01-09 8:39 ` Florian Schmaus
2024-01-09 8:45 ` [gentoo-dev] " Florian Schmaus
2024-01-09 8:39 ` [gentoo-dev] [PATCH v2 3/3] greadme.eclass: set UNPACKER_NO_BANNER Florian Schmaus
` (2 subsequent siblings)
3 siblings, 1 reply; 43+ messages in thread
From: Florian Schmaus @ 2024-01-09 8:39 UTC (permalink / raw)
To: gentoo-dev; +Cc: Florian Schmaus
Signed-off-by: Florian Schmaus <flow@gentoo.org>
---
eclass/0000-cover-letter.patch | 49 ++++
eclass/0001-greadme.eclass-new-eclass.patch | 305 ++++++++++++++++++++
eclass/unpacker.eclass | 7 +
3 files changed, 361 insertions(+)
create mode 100644 eclass/0000-cover-letter.patch
create mode 100644 eclass/0001-greadme.eclass-new-eclass.patch
diff --git a/eclass/0000-cover-letter.patch b/eclass/0000-cover-letter.patch
new file mode 100644
index 000000000000..2e162d419a44
--- /dev/null
+++ b/eclass/0000-cover-letter.patch
@@ -0,0 +1,49 @@
+From edff06160e33b361c9d6a7e273fc7808337c4518 Mon Sep 17 00:00:00 2001
+From: Florian Schmaus <flow@gentoo.org>
+Date: Sat, 6 Jan 2024 17:44:44 +0100
+Subject: [PATCH 0/1] [RFC] greadme.eclass
+
+I really like the functionality of readme.gentoo-r1.eclass, as it
+allows to communicate Gentoo-specific information about a package to
+the user. Especially as it improves the signal-to-noise ratio of
+messages arriving to our users.
+
+However, readme.gentoo-r1.eclass will only show this information on
+new installs, but not if the information changed. This is a major
+drawback. Furthermore, readme.gentoo-r1.eclass does not provide an API
+to assemble the information via heredoc.
+
+The following patch includes a new eclass named "greadme", that
+addresses those shortcomings of readme.gentoo-r1.eclass and hopefully
+can be seen as a general overhaul.
+
+This is a first draft of the eclass and therefore I'd like to ask for
+feedback.
+
+The greadme.eclass contains some TODO items at the end of its
+@DESCRIPTION.
+
+The main item is doc compression. Right now, greadme.eclass defaults
+to add the readme doc to the compression exclusion list via
+"docompress -x". A mode where the readme doc is compressed, just as
+readme.gentoo-r1.eclass does, can be activated by setting
+_GREADME_COMPRESS. However, I believe this mode is fragile as it can
+not be guaranteed that a binary for the used compression algorithms is
+installed on the host [1].
+
+I believe it is reasonable to simply install the readme doc
+uncompressed, since they are typically only a few lines long. However,
+if anyone can point out a way to achieve the desired functionality with
+a compressed readme doc, then please let me know.
+
+Thanks for reviewing the eclass.
+
+Florian Schmaus (1):
+ greadme.eclass: new eclass
+
+ eclass/greadme.eclass | 281 ++++++++++++++++++++++++++++++++++++++++++
+ 1 file changed, 281 insertions(+)
+ create mode 100644 eclass/greadme.eclass
+
+--
+2.41.0
diff --git a/eclass/0001-greadme.eclass-new-eclass.patch b/eclass/0001-greadme.eclass-new-eclass.patch
new file mode 100644
index 000000000000..52f4d9b6ff4d
--- /dev/null
+++ b/eclass/0001-greadme.eclass-new-eclass.patch
@@ -0,0 +1,305 @@
+From edff06160e33b361c9d6a7e273fc7808337c4518 Mon Sep 17 00:00:00 2001
+From: Florian Schmaus <flow@gentoo.org>
+Date: Sat, 6 Jan 2024 17:39:45 +0100
+Subject: [PATCH 1/1] greadme.eclass: new eclass
+
+This new eclass is similar to readme.gentoo-r1.eclass. The main
+differences are as follows. Firstly, it also displays the doc file
+contents if they have changed. Secondly, it provides a convenient API to
+install the doc file via stdin.
+
+Signed-off-by: Florian Schmaus <flow@gentoo.org>
+---
+ eclass/greadme.eclass | 281 ++++++++++++++++++++++++++++++++++++++++++
+ 1 file changed, 281 insertions(+)
+ create mode 100644 eclass/greadme.eclass
+
+diff --git a/eclass/greadme.eclass b/eclass/greadme.eclass
+new file mode 100644
+index 000000000000..ac83c36983ef
+--- /dev/null
++++ b/eclass/greadme.eclass
+@@ -0,0 +1,281 @@
++# Copyright 1999-2024 Gentoo Authors
++# Distributed under the terms of the GNU General Public License v2
++
++# @ECLASS: greadme.eclass
++# @MAINTAINER:
++# Florian Schmaus <flow@gentoo.org>
++# @AUTHOR:
++# Author: Florian Schmaus <flow@gentoo.org>
++# @SUPPORTED_EAPIS: 6 7 8
++# @BLURB: install a doc file, that will be conditionally shown via elog messages
++# @DESCRIPTION:
++# An eclass for installing a README.gentoo doc file recording tips
++# shown via elog messages. With this eclass, those elog messages will only be
++# shown at first package installation or if the contents of the file have changed.
++# Furthermore, a file for later reviewing will be installed under
++# /usr/share/doc/${PF}
++#
++# This eclass is similar to readme.gentoo-r1.eclass. The main
++# differences are as follows. Firstly, it also displays the doc file
++# contents if they have changed. 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 --apend <<-EOF
++# This is conditional readme content, based on USE=foo.
++# EOF
++# fi
++# }
++# @CODE
++#
++# You must call greadme_pkg_preinst and greadme_pkg_postinst explicitly, if
++# you override the default pkg_preinst or respectively pkg_postinst.
++#
++# TODO:
++# - Should this be named README.Distribution instead of README.Gentoo?
++# Would that make things easier for Gentoo derivates?
++# Similary, (g → d)readme, (G → D)README?
++# - Incooperate changes into readme.gentoo-r1.elcass?
++# - Compressing the readme doc file is probably fragile, as it is not
++# guaranteed that the required binaries for decompression are installed
++# in pkg_preinst/pkg_postinst. Note that it is even possible that two
++# different compression algorithms are used, in case of binpkgs.
++
++if [[ -z ${_README_GENTOO_ECLASS} ]]; then
++_README_GENTOO_ECLASS=1
++
++case ${EAPI} in
++ 6|7|8) ;;
++ *) die "${ECLASS}: EAPI ${EAPI:-0} not supported" ;;
++esac
++
++if [[ ${_GREADME_COMPRESS} ]]; then
++ inherit unpacker
++fi
++
++_GREADME_FILENAME="README.gentoo"
++_GREADME_TMP_FILE="${T}/${_GREADME_FILENAME}"
++_GREADME_REL_PATH="usr/share/doc/${PF}/${_GREADME_FILENAME}"
++
++# @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=false
++ while [[ -n ${1} ]] && [[ ${1} =~ --* ]]; do
++ case ${1} in
++ --append)
++ append=true
++ shift
++ ;;
++ esac
++ done
++
++ if $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"
++ fi
++
++ cat > "${_GREADME_TMP_FILE}" || die
++ fi
++
++ _greadme_install_doc
++}
++
++# @FUNCTION: greadme_file
++# @USAGE: <file>
++# @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
++
++ 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} "${@}"
++
++ if [[ ! -f "${_GREADME_TMP_FILE}" ]]; then
++ die "Gentoo README does not exist"
++ fi
++
++ if ! [[ ${_GREADME_COMPRESS} ]]; then
++ docompress -x "${_GREADME_REL_PATH}"
++ fi
++
++ ( # subshell to avoid pollution of calling environment
++ docinto .
++ dodoc "${_GREADME_TMP_FILE}"
++ ) || die
++
++}
++
++# @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() {
++ if [[ -z ${REPLACING_VERSIONS} ]]; then
++ _GREADME_SHOW="fresh-install"
++ return
++ fi
++
++ check_live_doc_file() {
++ local cur_pvr=$1
++ local live_doc_file="${EROOT}/usr/share/doc/${PN}-${cur_pvr}/${_GREADME_FILENAME}"
++
++ if [[ ${_GREADME_COMPRESS} ]]; then
++ local live_doc_files=( $(ls -1 ${live_doc_file}*) )
++ case ${#live_doc_files[@]} in
++ 0)
++ _GREADME_SHOW="no-current-greadme"
++ return
++ ;;
++ 1)
++ live_doc_file="${live_doc_files[0]}"
++ ;;
++ *)
++ die "unpexpected number of Gentoo README files found"
++ ;;
++ esac
++
++ local image_doc_files=( $(ls -1 ${image_doc_file}*) )
++ case ${#image_doc_files[@]} in
++ 0)
++ die "No Gentoo README found in image"
++ ;;
++ 1)
++ image_doc_file="${image_doc_files[0]}"
++ ;;
++ *)
++ die "unpexpected number of Gentoo README files found"
++ ;;
++ esac
++
++ mkdir "${T}/greadme"
++
++ mkdir "${T}/greadme/live"
++ pushd "${T}/greadme/live" > /dev/null
++ local live_doc_file_basename="$(basename "${live_doc_file}")"
++ if [[ "${live_doc_file_basename}" == "${_GREADME_FILENAME}" ]]; then
++ cp "${live_doc_file}" .
++ else
++ unpacker "${live_doc_file}"
++ fi
++ popd > /dev/null
++
++ mkdir "${T}/greadme/image"
++ pushd "${T}/greadme/image" > /dev/null
++ local image_doc_file_basename="$(basename "${image_doc_file}")"
++ if [[ "${image_doc_file_basename}" == "${_GREADME_FILENAME}" ]]; then
++ cp "${image_doc_file}" .
++ else
++ unpacker "${image_doc_file}"
++ fi
++ popd > /dev/null
++
++ live_doc_file="${T}/greadme/live/${_GREADME_FILENAME}"
++ image_doc_file="${T}/greadme/image/${_GREADME_FILENAME}"
++ # Store the unpacked greadme in a global variable so that it can
++ # be used in greadme_pkg_postinst.
++ _GREADME_UNPACKED="${image_doc_file}"
++ else
++ if [[ ! -f ${live_doc_file} ]]; then
++ _GREADME_SHOW="no-current-greadme"
++ return
++ fi
++ fi
++
++ cmp -s "${live_doc_file}" "${image_doc_file}"
++ local ret=$?
++ case ${ret} in
++ 0)
++ _GREADME_SHOW=""
++ ;;
++ 1)
++ _GREADME_SHOW="content-differs"
++ ;;
++ *)
++ die "cmp failed with ${ret}"
++ ;;
++ esac
++ }
++
++ local image_doc_file="${ED}/${_GREADME_REL_PATH}"
++
++ local replaced_version
++ for replaced_version in ${REPLACING_VERSIONS}; do
++ check_live_doc_file ${replaced_version}
++ 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 greadme_path
++ if [[ ${_GREADME_COMPRESS} ]]; then
++ greadme_path="${_GREADME_UNPACKED}"
++ else
++ greadme_path="${EROOT}/${_GREADME_REL_PATH}"
++ fi
++
++ local greadme_line
++ while read -r greadme_line; do elog "${greadme_line}"; done < "${EROOT}/${_GREADME_REL_PATH}"
++ elog ""
++ elog "(Note: Above message is only printed the first time package is"
++ elog "installed or if the the message changed. Please look at"
++ elog "${EPREFIX}/${_GREADME_REL_PATH} for future reference)"
++}
++
++EXPORT_FUNCTIONS pkg_preinst pkg_postinst
++
++fi
+--
+2.41.0
diff --git a/eclass/unpacker.eclass b/eclass/unpacker.eclass
index 2957ca02d3f4..73b763ca2018 100644
--- a/eclass/unpacker.eclass
+++ b/eclass/unpacker.eclass
@@ -42,6 +42,11 @@ inherit multiprocessing toolchain-funcs
# `xz`, `plzip`, `pdlzip`, and `lzip`. Make sure your choice accepts the "-dc" options.
# Note: this is meant for users to set, not ebuilds.
+# @ECLASS_VARIABLE: UNPACKER_NO_BANNER
+# @DEFAULT_UNSET
+# @DESCRIPTION:
+# If set, do not display the unpack banner with information what got unpacked where.
+
# for internal use only (unpack_pdv and unpack_makeself)
find_unpackable_file() {
local src=$1
@@ -63,6 +68,8 @@ find_unpackable_file() {
}
unpack_banner() {
+ [[ ${UNPACKER_NO_BANNER} ]] && return
+
echo ">>> Unpacking ${1##*/} to ${PWD}"
}
--
2.41.0
^ permalink raw reply related [flat|nested] 43+ messages in thread* [gentoo-dev] Re: [PATCH v2 2/3] add UNPACKER_NO_BANNER variable
2024-01-09 8:39 ` [gentoo-dev] [PATCH v2 2/3] add UNPACKER_NO_BANNER variable Florian Schmaus
@ 2024-01-09 8:45 ` Florian Schmaus
0 siblings, 0 replies; 43+ messages in thread
From: Florian Schmaus @ 2024-01-09 8:45 UTC (permalink / raw)
To: gentoo-dev
[-- Attachment #1.1.1: Type: text/plain, Size: 1388 bytes --]
On 09/01/2024 09.39, Florian Schmaus wrote:
> Signed-off-by: Florian Schmaus <flow@gentoo.org>
> ---
> eclass/0000-cover-letter.patch | 49 ++++
> eclass/0001-greadme.eclass-new-eclass.patch | 305 ++++++++++++++++++++
Ignore those to patch files. They are accidentally added to the commit.
> diff --git a/eclass/unpacker.eclass b/eclass/unpacker.eclass
> index 2957ca02d3f4..73b763ca2018 100644
> --- a/eclass/unpacker.eclass
> +++ b/eclass/unpacker.eclass
> @@ -42,6 +42,11 @@ inherit multiprocessing toolchain-funcs
> # `xz`, `plzip`, `pdlzip`, and `lzip`. Make sure your choice accepts the "-dc" options.
> # Note: this is meant for users to set, not ebuilds.
>
> +# @ECLASS_VARIABLE: UNPACKER_NO_BANNER
> +# @DEFAULT_UNSET
> +# @DESCRIPTION:
> +# If set, do not display the unpack banner with information what got unpacked where.
> +
> # for internal use only (unpack_pdv and unpack_makeself)
> find_unpackable_file() {
> local src=$1
> @@ -63,6 +68,8 @@ find_unpackable_file() {
> }
>
> unpack_banner() {
> + [[ ${UNPACKER_NO_BANNER} ]] && return
> +
> echo ">>> Unpacking ${1##*/} to ${PWD}"
> }
>
Also note that this only prevents the banner emitted by the
unpacker.eclass. The eclass also invokes unpack() from PMS/portage,
emitting a banner which can not be disabled via the eclass.
- Flow
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 17273 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 618 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* [gentoo-dev] [PATCH v2 3/3] greadme.eclass: set UNPACKER_NO_BANNER
2024-01-09 8:39 ` [gentoo-dev] [PATCH v2 1/3] greadme.eclass: new eclass Florian Schmaus
2024-01-09 8:39 ` [gentoo-dev] [PATCH v2 2/3] add UNPACKER_NO_BANNER variable Florian Schmaus
@ 2024-01-09 8:39 ` Florian Schmaus
2024-01-09 11:23 ` [gentoo-dev] [PATCH v2 1/3] greadme.eclass: new eclass David Seifert
2024-06-02 13:57 ` [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass Florian Schmaus
3 siblings, 0 replies; 43+ messages in thread
From: Florian Schmaus @ 2024-01-09 8:39 UTC (permalink / raw)
To: gentoo-dev; +Cc: Florian Schmaus
Signed-off-by: Florian Schmaus <flow@gentoo.org>
---
eclass/greadme.eclass | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/eclass/greadme.eclass b/eclass/greadme.eclass
index 25e0210406c1..ec01d090cc10 100644
--- a/eclass/greadme.eclass
+++ b/eclass/greadme.eclass
@@ -174,7 +174,7 @@ greadme_pkg_preinst() {
if [[ "${image_doc_file_basename}" == "${_GREADME_FILENAME}" ]]; then
cp "${image_doc_file}" . || die
else
- nonfatal unpacker "${image_doc_file}"
+ UNPACKER_NO_BANNER=1 nonfatal unpacker "${image_doc_file}"
if [[ $? -gt 0 ]]; then
# We failed to unpack the readme doc from the
# image, therefore, we can't show it (unless we
@@ -222,7 +222,7 @@ greadme_pkg_preinst() {
if [[ "${live_doc_file_basename}" == "${_GREADME_FILENAME}" ]]; then
cp "${live_doc_file}" .
else
- nonfatal unpacker "${live_doc_file}"
+ UNPACKER_NO_BANNER=1 nonfatal unpacker "${live_doc_file}"
if [[ $? -gt 0 ]]; then
# We failed to unpack the live readme doc, fallback
# to show the new readme contents.
--
2.41.0
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [gentoo-dev] [PATCH v2 1/3] greadme.eclass: new eclass
2024-01-09 8:39 ` [gentoo-dev] [PATCH v2 1/3] greadme.eclass: new eclass Florian Schmaus
2024-01-09 8:39 ` [gentoo-dev] [PATCH v2 2/3] add UNPACKER_NO_BANNER variable Florian Schmaus
2024-01-09 8:39 ` [gentoo-dev] [PATCH v2 3/3] greadme.eclass: set UNPACKER_NO_BANNER Florian Schmaus
@ 2024-01-09 11:23 ` David Seifert
2024-01-09 11:30 ` Florian Schmaus
2024-06-02 13:57 ` [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass Florian Schmaus
3 siblings, 1 reply; 43+ messages in thread
From: David Seifert @ 2024-01-09 11:23 UTC (permalink / raw)
To: gentoo-dev; +Cc: Florian Schmaus
On Tue, 2024-01-09 at 09:39 +0100, Florian Schmaus wrote:
> This new eclass is similar to readme.gentoo-r1.eclass. The main
> differences are as follows. Firstly, it also displays the doc file
> contents if they have changed. Secondly, it provides a convenient API
> to
> install the doc file via stdin.
>
> Furthermore, this eclass dos not store the doc file's contents in an
> environment variable, which helps to keep the environment size of
> ebuilds using the eclass small.
>
> Signed-off-by: Florian Schmaus <flow@gentoo.org>
> ---
> eclass/greadme.eclass | 307
> ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 307 insertions(+)
> create mode 100644 eclass/greadme.eclass
>
> diff --git a/eclass/greadme.eclass b/eclass/greadme.eclass
> new file mode 100644
> index 000000000000..25e0210406c1
> --- /dev/null
> +++ b/eclass/greadme.eclass
> @@ -0,0 +1,307 @@
> +# Copyright 1999-2024 Gentoo Authors
> +# Distributed under the terms of the GNU General Public License v2
> +
> +# @ECLASS: greadme.eclass
> +# @MAINTAINER:
> +# Florian Schmaus <flow@gentoo.org>
> +# @AUTHOR:
> +# Author: Florian Schmaus <flow@gentoo.org>
> +# @SUPPORTED_EAPIS: 6 7 8
> +# @BLURB: install a doc file, that will be conditionally shown via
> elog messages
> +# @DESCRIPTION:
> +# An eclass for installing a README.gentoo doc file recording tips
> +# shown via elog messages. With this eclass, those elog messages
> will only be
> +# shown at first package installation or if the contents of the file
> have changed.
> +# Furthermore, a file for later reviewing will be installed under
> +# /usr/share/doc/${PF}
> +#
> +# This eclass is similar to readme.gentoo-r1.eclass. The main
> +# differences are as follows. Firstly, it also displays the doc file
> +# contents if they have changed. 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 --apend <<-EOF
> +# This is conditional readme content, based on USE=foo.
> +# EOF
> +# fi
> +# }
> +# @CODE
> +#
> +# You must call greadme_pkg_preinst and greadme_pkg_postinst
> explicitly, if
> +# you override the default pkg_preinst or respectively pkg_postinst.
> +#
> +# TODO:
> +# - Should this be named README.Distribution instead of
> README.Gentoo?
> +# Would that make things easier for Gentoo derivates?
> +# Similary, (g → d)readme, (G → D)README?
> +# - Incooperate changes into readme.gentoo-r1.elcass?
> +# - Compressing the readme doc file is probably fragile, as it is not
> +# guaranteed that the required binaries for decompression are
> installed
> +# in pkg_preinst/pkg_postinst. Note that it is even possible that
> two
> +# different compression algorithms are used, in case of binpkgs.
> +
> +if [[ -z ${_README_GENTOO_ECLASS} ]]; then
> +_README_GENTOO_ECLASS=1
> +
> +case ${EAPI} in
> + 6|7|8) ;;
> + *) die "${ECLASS}: EAPI ${EAPI:-0} not supported" ;;
> +esac
> +
> +if [[ ${_GREADME_COMPRESS} ]]; then
> + inherit unpacker
> +fi
> +
> +_GREADME_FILENAME="README.gentoo"
> +_GREADME_TMP_FILE="${T}/${_GREADME_FILENAME}"
> +_GREADME_REL_PATH="usr/share/doc/${PF}/${_GREADME_FILENAME}"
> +
> +# @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=false
> + while [[ -n ${1} ]] && [[ ${1} =~ --* ]]; do
> + case ${1} in
> + --append)
> + append=true
> + shift
> + ;;
> + esac
> + done
> +
> + if $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: <file>
> +# @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
> +
> + 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} "${@}"
> +
> + if [[ ! -f "${_GREADME_TMP_FILE}" ]]; then
> + die "Gentoo README does not exist"
> + fi
> +
> + if ! [[ ${_GREADME_COMPRESS} ]]; then
> + docompress -x "${_GREADME_REL_PATH}"
> + fi
> +
> + ( # subshell to avoid pollution of calling environment
> + docinto .
> + dodoc "${_GREADME_TMP_FILE}"
> + ) || die
> +
> +}
> +
> +# @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() {
> + local image_doc_file="${ED}/${_GREADME_REL_PATH}"
> +
> + if [[ ${_GREADME_COMPRESS} ]]; then
> + local greadme_tmpdir="${T}/greadme"
> +
> + mkdir -p "${greadme_tmpdir}/image" || die
> +
> + local image_doc_files=( $(ls -1 ${image_doc_file}*) )
> + case ${#image_doc_files[@]} in
> + 0)
> + die "No Gentoo README found in image"
> + ;;
> + 1)
> + image_doc_file="${image_doc_files[0]}
> "
> + ;;
> + *)
> + die "unpexpected number of Gentoo
> README files found"
> + ;;
> + esac
> +
> + pushd "${T}/greadme/image" > /dev/null
> + local image_doc_file_basename="$(basename
> "${image_doc_file}")"
> + if [[ "${image_doc_file_basename}" ==
> "${_GREADME_FILENAME}" ]]; then
> + cp "${image_doc_file}" . || die
> + else
> + nonfatal unpacker "${image_doc_file}"
> + if [[ $? -gt 0 ]]; then
> + # We failed to unpack the readme doc
> from the
> + # image, therefore, we can't show it
> (unless we
> + # would save it's content in a env
> variable like
> + # gentoo.readme-r1 does).
> + _GREADME_SHOW=""
> + return
> + fi
> + fi
> + popd > /dev/null
> + fi
> +
> + if [[ -z ${REPLACING_VERSIONS} ]]; then
> + _GREADME_SHOW="fresh-install"
> + return
> + fi
> +
> + check_live_doc_file() {
> + local cur_pvr=$1
> + local live_doc_file="${EROOT}/usr/share/doc/${PN}-
> ${cur_pvr}/${_GREADME_FILENAME}"
> +
> + if [[ ${_GREADME_COMPRESS} ]]; then
> + local live_doc_files=( $(ls -1
> ${live_doc_file}*) )
> + case ${#live_doc_files[@]} in
> + 0)
> + _GREADME_SHOW="no-current-
> greadme"
> + return
> + ;;
> + 1)
> + live_doc_file="${live_doc_fil
> es[0]}"
> + ;;
> + *)
> + die "unpexpected number of
> Gentoo README files found"
> + ;;
> + esac
> +
> + if [[ -d "${greadme_tmpdir}/live" ]]; then
> + rm -rf "${greadme_tmpdir}"/live/* ||
> die
> + else
> + mkdir "${T}/greadme/live"
> + fi
> +
> + pushd "${T}/greadme/live" > /dev/null
> + local live_doc_file_basename="$(basename
> "${live_doc_file}")"
> + if [[ "${live_doc_file_basename}" ==
> "${_GREADME_FILENAME}" ]]; then
> + cp "${live_doc_file}" .
> + else
> + nonfatal unpacker "${live_doc_file}"
> + if [[ $? -gt 0 ]]; then
> + # We failed to unpack the
> live readme doc, fallback
> + # to show the new readme
> contents.
> + _GREADME_SHOW="failed-to-
> unpack-live-readme-doc"
> + return
> + fi
> + fi
> + popd > /dev/null
> +
> + live_doc_file="${T}/greadme/live/${_GREADME_F
> ILENAME}"
> + image_doc_file="${T}/greadme/image/${_GREADME
> _FILENAME}"
> + # Store the unpacked greadme in a global
> variable so that it can
> + # be used in greadme_pkg_postinst.
> + _GREADME_UNPACKED="${image_doc_file}"
> + else
> + if [[ ! -f ${live_doc_file} ]]; then
> + _GREADME_SHOW="no-current-greadme"
> + return
> + fi
> + fi
> +
> + cmp -s "${live_doc_file}" "${image_doc_file}"
> + local ret=$?
> + case ${ret} in
> + 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}
> + 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 greadme_path
> + if [[ ${_GREADME_COMPRESS} ]]; then
> + if [[ -z ${_GREADME_UNPACKED} ]]; then
> + # We failed to decompress the readme doc from
> the image.
> + return
> + fi
> + greadme_path="${_GREADME_UNPACKED}"
> + else
> + greadme_path="${EROOT}/${_GREADME_REL_PATH}"
> + fi
> +
> + local line
> + while read -r line; do elog "${line}"; done <
> "${greadme_path}"
> + elog ""
> + elog "(Note: Above message is only printed the first time
> package is"
> + elog "installed or if the the message changed. Please look
> at"
> + elog "${EPREFIX}/${_GREADME_REL_PATH} for future reference)"
> +}
> +
> +EXPORT_FUNCTIONS pkg_preinst pkg_postinst
> +
> +fi
Sorry, but this is definitely too much complexity for installing a
README file. As is, I will block merging this to the tree.
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [gentoo-dev] [PATCH v2 1/3] greadme.eclass: new eclass
2024-01-09 11:23 ` [gentoo-dev] [PATCH v2 1/3] greadme.eclass: new eclass David Seifert
@ 2024-01-09 11:30 ` Florian Schmaus
0 siblings, 0 replies; 43+ messages in thread
From: Florian Schmaus @ 2024-01-09 11:30 UTC (permalink / raw)
To: David Seifert, gentoo-dev
[-- Attachment #1.1.1: Type: text/plain, Size: 11168 bytes --]
On 09/01/2024 12.23, David Seifert wrote:
> On Tue, 2024-01-09 at 09:39 +0100, Florian Schmaus wrote:
>> This new eclass is similar to readme.gentoo-r1.eclass. The main
>> differences are as follows. Firstly, it also displays the doc file
>> contents if they have changed. Secondly, it provides a convenient API
>> to
>> install the doc file via stdin.
>>
>> Furthermore, this eclass dos not store the doc file's contents in an
>> environment variable, which helps to keep the environment size of
>> ebuilds using the eclass small.
>>
>> Signed-off-by: Florian Schmaus <flow@gentoo.org>
>> ---
>> eclass/greadme.eclass | 307
>> ++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 307 insertions(+)
>> create mode 100644 eclass/greadme.eclass
>>
>> diff --git a/eclass/greadme.eclass b/eclass/greadme.eclass
>> new file mode 100644
>> index 000000000000..25e0210406c1
>> --- /dev/null
>> +++ b/eclass/greadme.eclass
>> @@ -0,0 +1,307 @@
>> +# Copyright 1999-2024 Gentoo Authors
>> +# Distributed under the terms of the GNU General Public License v2
>> +
>> +# @ECLASS: greadme.eclass
>> +# @MAINTAINER:
>> +# Florian Schmaus <flow@gentoo.org>
>> +# @AUTHOR:
>> +# Author: Florian Schmaus <flow@gentoo.org>
>> +# @SUPPORTED_EAPIS: 6 7 8
>> +# @BLURB: install a doc file, that will be conditionally shown via
>> elog messages
>> +# @DESCRIPTION:
>> +# An eclass for installing a README.gentoo doc file recording tips
>> +# shown via elog messages. With this eclass, those elog messages
>> will only be
>> +# shown at first package installation or if the contents of the file
>> have changed.
>> +# Furthermore, a file for later reviewing will be installed under
>> +# /usr/share/doc/${PF}
>> +#
>> +# This eclass is similar to readme.gentoo-r1.eclass. The main
>> +# differences are as follows. Firstly, it also displays the doc file
>> +# contents if they have changed. 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 --apend <<-EOF
>> +# This is conditional readme content, based on USE=foo.
>> +# EOF
>> +# fi
>> +# }
>> +# @CODE
>> +#
>> +# You must call greadme_pkg_preinst and greadme_pkg_postinst
>> explicitly, if
>> +# you override the default pkg_preinst or respectively pkg_postinst.
>> +#
>> +# TODO:
>> +# - Should this be named README.Distribution instead of
>> README.Gentoo?
>> +# Would that make things easier for Gentoo derivates?
>> +# Similary, (g → d)readme, (G → D)README?
>> +# - Incooperate changes into readme.gentoo-r1.elcass?
>> +# - Compressing the readme doc file is probably fragile, as it is not
>> +# guaranteed that the required binaries for decompression are
>> installed
>> +# in pkg_preinst/pkg_postinst. Note that it is even possible that
>> two
>> +# different compression algorithms are used, in case of binpkgs.
>> +
>> +if [[ -z ${_README_GENTOO_ECLASS} ]]; then
>> +_README_GENTOO_ECLASS=1
>> +
>> +case ${EAPI} in
>> + 6|7|8) ;;
>> + *) die "${ECLASS}: EAPI ${EAPI:-0} not supported" ;;
>> +esac
>> +
>> +if [[ ${_GREADME_COMPRESS} ]]; then
>> + inherit unpacker
>> +fi
>> +
>> +_GREADME_FILENAME="README.gentoo"
>> +_GREADME_TMP_FILE="${T}/${_GREADME_FILENAME}"
>> +_GREADME_REL_PATH="usr/share/doc/${PF}/${_GREADME_FILENAME}"
>> +
>> +# @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=false
>> + while [[ -n ${1} ]] && [[ ${1} =~ --* ]]; do
>> + case ${1} in
>> + --append)
>> + append=true
>> + shift
>> + ;;
>> + esac
>> + done
>> +
>> + if $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: <file>
>> +# @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
>> +
>> + 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} "${@}"
>> +
>> + if [[ ! -f "${_GREADME_TMP_FILE}" ]]; then
>> + die "Gentoo README does not exist"
>> + fi
>> +
>> + if ! [[ ${_GREADME_COMPRESS} ]]; then
>> + docompress -x "${_GREADME_REL_PATH}"
>> + fi
>> +
>> + ( # subshell to avoid pollution of calling environment
>> + docinto .
>> + dodoc "${_GREADME_TMP_FILE}"
>> + ) || die
>> +
>> +}
>> +
>> +# @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() {
>> + local image_doc_file="${ED}/${_GREADME_REL_PATH}"
>> +
>> + if [[ ${_GREADME_COMPRESS} ]]; then
>> + local greadme_tmpdir="${T}/greadme"
>> +
>> + mkdir -p "${greadme_tmpdir}/image" || die
>> +
>> + local image_doc_files=( $(ls -1 ${image_doc_file}*) )
>> + case ${#image_doc_files[@]} in
>> + 0)
>> + die "No Gentoo README found in image"
>> + ;;
>> + 1)
>> + image_doc_file="${image_doc_files[0]}
>> "
>> + ;;
>> + *)
>> + die "unpexpected number of Gentoo
>> README files found"
>> + ;;
>> + esac
>> +
>> + pushd "${T}/greadme/image" > /dev/null
>> + local image_doc_file_basename="$(basename
>> "${image_doc_file}")"
>> + if [[ "${image_doc_file_basename}" ==
>> "${_GREADME_FILENAME}" ]]; then
>> + cp "${image_doc_file}" . || die
>> + else
>> + nonfatal unpacker "${image_doc_file}"
>> + if [[ $? -gt 0 ]]; then
>> + # We failed to unpack the readme doc
>> from the
>> + # image, therefore, we can't show it
>> (unless we
>> + # would save it's content in a env
>> variable like
>> + # gentoo.readme-r1 does).
>> + _GREADME_SHOW=""
>> + return
>> + fi
>> + fi
>> + popd > /dev/null
>> + fi
>> +
>> + if [[ -z ${REPLACING_VERSIONS} ]]; then
>> + _GREADME_SHOW="fresh-install"
>> + return
>> + fi
>> +
>> + check_live_doc_file() {
>> + local cur_pvr=$1
>> + local live_doc_file="${EROOT}/usr/share/doc/${PN}-
>> ${cur_pvr}/${_GREADME_FILENAME}"
>> +
>> + if [[ ${_GREADME_COMPRESS} ]]; then
>> + local live_doc_files=( $(ls -1
>> ${live_doc_file}*) )
>> + case ${#live_doc_files[@]} in
>> + 0)
>> + _GREADME_SHOW="no-current-
>> greadme"
>> + return
>> + ;;
>> + 1)
>> + live_doc_file="${live_doc_fil
>> es[0]}"
>> + ;;
>> + *)
>> + die "unpexpected number of
>> Gentoo README files found"
>> + ;;
>> + esac
>> +
>> + if [[ -d "${greadme_tmpdir}/live" ]]; then
>> + rm -rf "${greadme_tmpdir}"/live/* ||
>> die
>> + else
>> + mkdir "${T}/greadme/live"
>> + fi
>> +
>> + pushd "${T}/greadme/live" > /dev/null
>> + local live_doc_file_basename="$(basename
>> "${live_doc_file}")"
>> + if [[ "${live_doc_file_basename}" ==
>> "${_GREADME_FILENAME}" ]]; then
>> + cp "${live_doc_file}" .
>> + else
>> + nonfatal unpacker "${live_doc_file}"
>> + if [[ $? -gt 0 ]]; then
>> + # We failed to unpack the
>> live readme doc, fallback
>> + # to show the new readme
>> contents.
>> + _GREADME_SHOW="failed-to-
>> unpack-live-readme-doc"
>> + return
>> + fi
>> + fi
>> + popd > /dev/null
>> +
>> + live_doc_file="${T}/greadme/live/${_GREADME_F
>> ILENAME}"
>> + image_doc_file="${T}/greadme/image/${_GREADME
>> _FILENAME}"
>> + # Store the unpacked greadme in a global
>> variable so that it can
>> + # be used in greadme_pkg_postinst.
>> + _GREADME_UNPACKED="${image_doc_file}"
>> + else
>> + if [[ ! -f ${live_doc_file} ]]; then
>> + _GREADME_SHOW="no-current-greadme"
>> + return
>> + fi
>> + fi
>> +
>> + cmp -s "${live_doc_file}" "${image_doc_file}"
>> + local ret=$?
>> + case ${ret} in
>> + 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}
>> + 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 greadme_path
>> + if [[ ${_GREADME_COMPRESS} ]]; then
>> + if [[ -z ${_GREADME_UNPACKED} ]]; then
>> + # We failed to decompress the readme doc from
>> the image.
>> + return
>> + fi
>> + greadme_path="${_GREADME_UNPACKED}"
>> + else
>> + greadme_path="${EROOT}/${_GREADME_REL_PATH}"
>> + fi
>> +
>> + local line
>> + while read -r line; do elog "${line}"; done <
>> "${greadme_path}"
>> + elog ""
>> + elog "(Note: Above message is only printed the first time
>> package is"
>> + elog "installed or if the the message changed. Please look
>> at"
>> + elog "${EPREFIX}/${_GREADME_REL_PATH} for future reference)"
>> +}
>> +
>> +EXPORT_FUNCTIONS pkg_preinst pkg_postinst
>> +
>> +fi
>
> Sorry, but this is definitely too much complexity for installing a
> README file. As is, I will block merging this to the tree.
As I wrote, I do not want this too. I like to merge greadme.eclass
without _GREADME_COMPRESS, with which removes large parts of the
complexity of the eclass, I would even say it would be nearly trivial then.
Would you also block a significantly simpler variant of the greadme.eclass?
- Flow
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 17273 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 618 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass
2024-01-09 8:39 ` [gentoo-dev] [PATCH v2 1/3] greadme.eclass: new eclass Florian Schmaus
` (2 preceding siblings ...)
2024-01-09 11:23 ` [gentoo-dev] [PATCH v2 1/3] greadme.eclass: new eclass David Seifert
@ 2024-06-02 13:57 ` Florian Schmaus
2024-06-02 13:57 ` [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install) Florian Schmaus
` (4 more replies)
3 siblings, 5 replies; 43+ messages in thread
From: Florian Schmaus @ 2024-06-02 13:57 UTC (permalink / raw)
To: gentoo-dev; +Cc: pacho, Florian Schmaus
Following up on the comments of the last patchset, this revision
incorporates the functionality of the initially proposed
greadme.eclass into the existing readme.gentoo-r1.eclass. While this
misses the chance to get rid of some ballast of the existing eclass,
people asked to extend the existing eclass instead of starting fresh.
As a quick reminder, those changes aim to improve the user experience
of readme.gentoo-r1.eclass by reducing the amount of messages
presented to the user upon emerge. This is achieved by only showing
the contents of README.gentoo *if they have changed* (or on fresh
installations).
The previous revision was criticized for using "docompress -x
README.gentoo", i.e., excluding README.gentoo from being
compressed. However, exclusion from compression is necessary to
reliably detect if the contents have changed.
After discussing this with others, we came up with the following
compromise that should be acceptable by everyone.
Instead of excluding the whole README.eclass from compression, we use
chksum to create the 4-byte CRC value of the file. The eclass then
uses the existing 4-byte value and compares it with the 4-byte value
in the package's image directory to determine if the contents of
README.gentoo have changed.
I would like to thank Sam for the helpful comments that led to the
resulting design.
Note that this changes readme.gentoo-r1.eclass to export phase
functions when it previously did not.
Finally, I'd like to (restate) my concern that updating the existing
eclass, instead of starting fresh with a new eclass, misses the chance
to eliminate some ballast.
Florian Schmaus (4):
readme.gentoo-r1.eclass: display readme if content changed (or fresh
install)
readme.gentoo-r1.eclass: use _GREADME_TMP_FILE in existing code
readme.gentoo-r1.eclass: add readme.gentoo_stdin()
readme.gentoo-r1.eclass: add readme.gentoo_file()
eclass/readme.gentoo-r1.eclass | 206 +++++++++++++++++++++++++++++----
1 file changed, 185 insertions(+), 21 deletions(-)
--
2.44.1
^ permalink raw reply [flat|nested] 43+ messages in thread* [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)
2024-06-02 13:57 ` [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass Florian Schmaus
@ 2024-06-02 13:57 ` Florian Schmaus
2024-06-02 15:34 ` Ulrich Mueller
2024-06-02 13:57 ` [gentoo-dev] [PATCH 2/4] readme.gentoo-r1.eclass: use _GREADME_TMP_FILE in existing code Florian Schmaus
` (3 subsequent siblings)
4 siblings, 1 reply; 43+ messages in thread
From: Florian Schmaus @ 2024-06-02 13:57 UTC (permalink / raw)
To: gentoo-dev; +Cc: pacho, Florian Schmaus
Improve the user experience of readme.gentoo-r1.eclass by reducing the
number of messages presented to the user upon emerge. We reduce the
number of messages by only showing the contents of README.gentoo *if
they have changed* (or on fresh installations).
To be able to detect if the content has changed, the eclass now records
the 4-byte CRC value of README.gentoo. Upon installation, this 4-byte
value is compared. It is necessary to indirectly compare the content, as
README.gentoo is installed via dodoc, which is subject to docdir
compression, and there is no guarantee that we will be able to
uncompress the live system's version of README.gentoo (let alone that it
is not trivial to find a suitable decompressor candidate).
Signed-off-by: Florian Schmaus <flow@gentoo.org>
---
eclass/readme.gentoo-r1.eclass | 124 ++++++++++++++++++++++++++++-----
1 file changed, 108 insertions(+), 16 deletions(-)
diff --git a/eclass/readme.gentoo-r1.eclass b/eclass/readme.gentoo-r1.eclass
index 202ba31f4f70..2a79911cafbf 100644
--- a/eclass/readme.gentoo-r1.eclass
+++ b/eclass/readme.gentoo-r1.eclass
@@ -1,4 +1,4 @@
-# Copyright 1999-2023 Gentoo Authors
+# Copyright 1999-2024 Gentoo Authors
# Distributed under the terms of the GNU General Public License v2
# @ECLASS: readme.gentoo-r1.eclass
@@ -14,8 +14,11 @@
# shown at first package installation and a file for later reviewing will be
# installed under /usr/share/doc/${PF}
#
-# You need to call readme.gentoo_create_doc in src_install phase and
-# readme.gentoo_print_elog in pkg_postinst
+#
+# You need to call readme.gentoo_create_doc in src_install phase if you
+# use DOC_CONTENTS or obtain the readme from FILESIDR.
+#
+# Note that this eclass exports pkg_preinst and pkg_postinst functions.
if [[ -z ${_README_GENTOO_ECLASS} ]]; then
_README_GENTOO_ECLASS=1
@@ -47,6 +50,15 @@ esac
# If you want to specify a suffix for README.gentoo file please export it.
: "${README_GENTOO_SUFFIX:=""}"
+_GREADME_FILENAME="README.gentoo"
+_GREADME_HASH_FILENAME=".${_GREADME_FILENAME}.hash"
+
+_GREADME_TMP_FILE="${T}/${_GREADME_FILENAME}"
+
+_GREADME_DOC_DIR="usr/share/doc/${PF}"
+_GREADME_REL_PATH="${_GREADME_DOC_DIR}/${_GREADME_FILENAME}"
+_GREADME_HASH_REL_PATH="${_GREADME_DOC_DIR}/${_GREADME_HASH_FILENAME}"
+
# @FUNCTION: readme.gentoo_create_doc
# @DESCRIPTION:
# Create doc file with ${DOC_CONTENTS} variable (preferred) and, if not set,
@@ -75,11 +87,20 @@ readme.gentoo_create_doc() {
die "You are not specifying README.gentoo contents!"
fi
- ( # subshell to avoid pollution of calling environment
- docinto .
- dodoc "${T}"/README.gentoo
- ) || die
- README_GENTOO_DOC_VALUE=$(< "${T}/README.gentoo")
+ # subshell to avoid pollution of calling environment
+ (
+ insinto "${_GREADME_DOC_DIR}"
+
+ doins "${_GREADME_TMP_FILE}"
+ cksum --raw "${_GREADME_TMP_FILE}" | newins - "${_GREADME_HASH_FILENAME}"
+ assert
+ )
+
+ # Save the readme contents in an variable, so that it can be shown ins pkg_postinst().
+ _GREADME_CONTENTS=$(< "${_GREADME_TMP_FILE}")
+
+ # Exclude the 4-byte hash file from compression.
+ docompress -x "${_GREADME_HASH_REL_PATH}"
}
# @FUNCTION: readme.gentoo_print_elog
@@ -96,15 +117,86 @@ readme.gentoo_create_doc() {
readme.gentoo_print_elog() {
debug-print-function ${FUNCNAME} "${@}"
- if [[ -z "${README_GENTOO_DOC_VALUE}" ]]; then
- die "readme.gentoo_print_elog invoked without matching readme.gentoo_create_doc call!"
- elif ! [[ -n "${REPLACING_VERSIONS}" ]] || [[ -n "${FORCE_PRINT_ELOG}" ]]; then
- echo -e "${README_GENTOO_DOC_VALUE}" | while read -r ELINE; do elog "${ELINE}"; done
- elog ""
- elog "(Note: Above message is only printed the first time package is"
- elog "installed. Please look at ${EPREFIX}/usr/share/doc/${PF}/README.gentoo*"
- elog "for future reference)"
+ if ! [[ -n "${REPLACING_VERSIONS}" ]] || [[ -n "${FORCE_PRINT_ELOG}" ]]; then
+ _GREADME_SHOW="force"
+ readme.gentoo-r1_pkg_postinst
fi
}
+# @FUNCTION: readme.gentoo_pkg_preinst
+# @DESCRIPTION:
+# Performs checks like comparing the readme doc from the image with a
+# potentially existing one in the live system.
+readme.gentoo-r1_pkg_preinst() {
+ debug-print-function ${FUNCNAME} "${@}"
+
+ if [[ -z ${REPLACING_VERSIONS} ]]; then
+ _GREADME_SHOW="fresh-install"
+ return
+ fi
+
+ local image_hash_file="${ED}/${_GREADME_HASH_REL_PATH}"
+
+ check_live_doc_file() {
+ local cur_pvr=$1
+ local live_hash_file="${EROOT}/usr/share/doc/${PN}-${cur_pvr}/${_GREADME_HASH_FILENAME}"
+
+ if [[ ! -f ${live_hash_file} ]]; then
+ _GREADME_SHOW="no-current-greadme"
+ return
+ fi
+
+ cmp -s "${live_hash_file}" "${image_hash_file}"
+ local ret=$?
+ case ${ret} in
+ 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.
+ if [[ -n ${_GREADME_SHOW} ]]; then
+ break
+ fi
+ done
+}
+
+# @FUNCTION: readme.gentoo_pkg_postinst
+# @DESCRIPTION:
+# Conditionally shows the contents of the readme doc via elog.
+readme.gentoo-r1_pkg_postinst() {
+ debug-print-function ${FUNCNAME} "${@}"
+
+ if [[ ! -v _GREADME_SHOW ]]; then
+ ewarn "_GREADME_SHOW not set. Missing call of greadme_pkg_preinst?"
+ return
+ fi
+
+ if [[ -z "${_GREADME_SHOW}" ]]; then
+ # If _GREADME_SHOW is empty, then there is no reason to show the contents.
+ return
+ fi
+
+ local line
+ echo -e "${_GREADME_CONTENTS}" | 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 the message changed. Please look at"
+ elog "${EPREFIX}/${_GREADME_REL_PATH} for future reference)"
+}
+
fi
+
+EXPORT_FUNCTIONS pkg_preinst pkg_postinst
--
2.44.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)
2024-06-02 13:57 ` [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install) Florian Schmaus
@ 2024-06-02 15:34 ` Ulrich Mueller
2024-06-02 15:48 ` Eli Schwartz
2024-06-02 16:16 ` Florian Schmaus
0 siblings, 2 replies; 43+ messages in thread
From: Ulrich Mueller @ 2024-06-02 15:34 UTC (permalink / raw)
To: Florian Schmaus; +Cc: gentoo-dev, pacho
[-- Attachment #1: Type: text/plain, Size: 352 bytes --]
>>>>> On Sun, 02 Jun 2024, Florian Schmaus wrote:
> + (
> + insinto "${_GREADME_DOC_DIR}"
> +
> + doins "${_GREADME_TMP_FILE}"
> + cksum --raw "${_GREADME_TMP_FILE}" | newins - "${_GREADME_HASH_FILENAME}"
> + assert
> + )
Why do you need that hash file? The old README file exists on the
system, so couldn't you just compare the new one with it?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)
2024-06-02 15:34 ` Ulrich Mueller
@ 2024-06-02 15:48 ` Eli Schwartz
2024-06-02 16:28 ` Ulrich Mueller
2024-06-02 16:16 ` Florian Schmaus
1 sibling, 1 reply; 43+ messages in thread
From: Eli Schwartz @ 2024-06-02 15:48 UTC (permalink / raw)
To: gentoo-dev
[-- Attachment #1.1.1: Type: text/plain, Size: 837 bytes --]
On 6/2/24 11:34 AM, Ulrich Mueller wrote:
>>>>>> On Sun, 02 Jun 2024, Florian Schmaus wrote:
>
>> + (
>> + insinto "${_GREADME_DOC_DIR}"
>> +
>> + doins "${_GREADME_TMP_FILE}"
>> + cksum --raw "${_GREADME_TMP_FILE}" | newins - "${_GREADME_HASH_FILENAME}"
>> + assert
>> + )
>
> Why do you need that hash file? The old README file exists on the
> system, so couldn't you just compare the new one with it?
Per the commit message, the old readme and the new readme can have the
same contents, but be compressed by different compressors on the live
system vs the image, and/or a compressor with unstable algorithms,
and/or one that isn't installed at the time of merging.
Given you've explicitly rejected disabling compression, I don't quite
see how you can have your cake and eat it too.
--
Eli Schwartz
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 18399 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)
2024-06-02 15:48 ` Eli Schwartz
@ 2024-06-02 16:28 ` Ulrich Mueller
2024-06-02 17:48 ` Florian Schmaus
2024-06-02 17:51 ` Eli Schwartz
0 siblings, 2 replies; 43+ messages in thread
From: Ulrich Mueller @ 2024-06-02 16:28 UTC (permalink / raw)
To: Eli Schwartz; +Cc: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 571 bytes --]
>>>>> On Sun, 02 Jun 2024, Eli Schwartz wrote:
> Per the commit message, the old readme and the new readme can have the
> same contents, but be compressed by different compressors on the live
> system vs the image, and/or a compressor with unstable algorithms,
> and/or one that isn't installed at the time of merging.
> Given you've explicitly rejected disabling compression, I don't quite
> see how you can have your cake and eat it too.
How is installing another file (4 KiB on many file systems) an
improvement over disabling compression for README.gentoo itself?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)
2024-06-02 16:28 ` Ulrich Mueller
@ 2024-06-02 17:48 ` Florian Schmaus
2024-06-02 17:51 ` Eli Schwartz
1 sibling, 0 replies; 43+ messages in thread
From: Florian Schmaus @ 2024-06-02 17:48 UTC (permalink / raw)
To: gentoo-dev, Ulrich Mueller
[-- Attachment #1.1.1: Type: text/plain, Size: 1992 bytes --]
On 02/06/2024 18.28, Ulrich Mueller wrote:
>>>>>> On Sun, 02 Jun 2024, Eli Schwartz wrote:
>
>> Per the commit message, the old readme and the new readme can have the
>> same contents, but be compressed by different compressors on the live
>> system vs the image, and/or a compressor with unstable algorithms,
>> and/or one that isn't installed at the time of merging.
>
>> Given you've explicitly rejected disabling compression, I don't quite
>> see how you can have your cake and eat it too.
>
> How is installing another file (4 KiB on many file systems) an
> improvement over disabling compression for README.gentoo itself?
The hash file's size is constant, unlike README.gentoo, which even could
theoretically be bigger than 4 KiB. And, as you indicated, some
filesystems will even inline the 4-bytes into the inode.
But actually, I would rather simply exclude README.gentoo from
decompression. This would make the eclass simpler. Many (most?)
README.gentoo files are below 4 KiB uncompressed, hence the same
argument could be made that compressing those files does not buy us much
usable disk space.
However, you argued against using "docompress -x" in your mail from
2024-10-01.
Frankly, I don't care much. But I am a little bit tired of reading the
same elog output over and over again. Nearly daily I go through all elog
messages on my machines, and a large fraction is just repetitive, mainly
caused by readme.gentoo-r1.eclass (and others who should be using
readme.gentoo-r1.eclass but do not).
I have a hard time not limiting myself to ewarn and higher messages.
Quite likely other Gentoo users are in the same situation.
Unfortunately, it seems likely that I would then miss valuable information.
Therefore we should take action. Which is either not showing
GENTOO.readme upon updates *or* only showing it only if it changes when
updating a package.
The latter is what this patchset tries to achieve.
- Flow
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 17797 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 614 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)
2024-06-02 16:28 ` Ulrich Mueller
2024-06-02 17:48 ` Florian Schmaus
@ 2024-06-02 17:51 ` Eli Schwartz
2024-06-02 18:24 ` Ulrich Mueller
1 sibling, 1 reply; 43+ messages in thread
From: Eli Schwartz @ 2024-06-02 17:51 UTC (permalink / raw)
To: Ulrich Mueller; +Cc: gentoo-dev
[-- Attachment #1.1.1: Type: text/plain, Size: 1845 bytes --]
On 6/2/24 12:28 PM, Ulrich Mueller wrote:
>>>>>> On Sun, 02 Jun 2024, Eli Schwartz wrote:
>
>> Per the commit message, the old readme and the new readme can have the
>> same contents, but be compressed by different compressors on the live
>> system vs the image, and/or a compressor with unstable algorithms,
>> and/or one that isn't installed at the time of merging.
>
>> Given you've explicitly rejected disabling compression, I don't quite
>> see how you can have your cake and eat it too.
>
> How is installing another file (4 KiB on many file systems) an
> improvement over disabling compression for README.gentoo itself?
I didn't say it was.
The improvement is to disable compression for README.gentoo itself.
*You* are the one insisting on another approach.
Remember the original argument against disabling compression:
"""
> Then why does "docompress -x" exist at all?
Short answer, because some upstream programs access these directories
and cannot cope with compressed files there.
"""
readme.gentoo-r1.eclass as proposed in this thread is exactly such an
upstream program (gentoo is the upstream, and gentoo cannot cope with
compressed files there).
It strikes me as rules lawyering to use this as an argument against the
eclass, but whatever. The alternative to `docompress -x` is to *not have
to decompress* when comparing the compressed contents of two files,
which means recording persistent state.
There is one approach that I can think of for this:
- recording state in a second file (additional to README.gentoo itself)
If you dislike this solution, and are unwilling to back down on
"docompress -x", then my personal feeling is that it is *your*
responsibility to come up with an alternative mechanism for implementing
the functionality.
--
Eli Schwartz
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 18399 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)
2024-06-02 17:51 ` Eli Schwartz
@ 2024-06-02 18:24 ` Ulrich Mueller
2024-06-04 15:15 ` Florian Schmaus
0 siblings, 1 reply; 43+ messages in thread
From: Ulrich Mueller @ 2024-06-02 18:24 UTC (permalink / raw)
To: Eli Schwartz; +Cc: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 1304 bytes --]
>>>>> On Sun, 02 Jun 2024, Eli Schwartz wrote:
> readme.gentoo-r1.eclass as proposed in this thread is exactly such an
> upstream program (gentoo is the upstream, and gentoo cannot cope with
> compressed files there).
> It strikes me as rules lawyering to use this as an argument against
> the eclass, but whatever. The alternative to `docompress -x` is to
> *not have to decompress* when comparing the compressed contents of two
> files, which means recording persistent state.
> There is one approach that I can think of for this:
> - recording state in a second file (additional to README.gentoo itself)
> If you dislike this solution, and are unwilling to back down on
> "docompress -x", then my personal feeling is that it is *your*
> responsibility to come up with an alternative mechanism for
> implementing the functionality.
Installing another file just for the sake of avoiding "docompress -x"
doesn't solve the problem but makes it worse, IMHO. Rather don't
compress README.gentoo then.
(Also, I still don't understand the argument about different compress
programs. That's not likely to happen very often. You could go for best
effort there, and if it fails, consider the files as different. There's
no need for exact science when the only thing that can happen is
additional output.)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)
2024-06-02 18:24 ` Ulrich Mueller
@ 2024-06-04 15:15 ` Florian Schmaus
2024-06-04 17:45 ` Ulrich Mueller
0 siblings, 1 reply; 43+ messages in thread
From: Florian Schmaus @ 2024-06-04 15:15 UTC (permalink / raw)
To: gentoo-dev, Ulrich Mueller
[-- Attachment #1.1.1: Type: text/plain, Size: 1512 bytes --]
On 02/06/2024 20.24, Ulrich Mueller wrote:
> Installing another file just for the sake of avoiding "docompress -x"
> doesn't solve the problem but makes it worse, IMHO. Rather don't
> compress README.gentoo then.
Both is fine with me.
That said, many filesystem support inline data. If I am not mistaken,
then its even enabled by default for xfs (which we recommend in the
handbook) and btrfs. Also some README.gentoo files become suitable for
inlining after compression (btrfs' limit is 2048 bytes).
Considering this, the 4-byte hash file is superior under the right
circumstances when compared to excluding README.gentoo from compression.
And I could imagine that the circumstances are right for many of our users.
> (Also, I still don't understand the argument about different compress
> programs. That's not likely to happen very often. You could go for best
> effort there, and if it fails, consider the files as different. There's
> no need for exact science when the only thing that can happen is
> additional output.)
I fear this would open a can of worms. Which compressions algorithms do
you support? What if someone wants to add another compression algorithm?
What if there are multiple candidates that can be used to decompress the
file? How large do we want the opportunistic decompression function
allow to grow?
Last but not least, opportunistic approaches are fine if you have no
other choice. But we have the option to make it reliable.
- Flow
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 17797 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 618 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)
2024-06-04 15:15 ` Florian Schmaus
@ 2024-06-04 17:45 ` Ulrich Mueller
2024-06-04 18:28 ` Florian Schmaus
2024-06-04 18:37 ` Ionen Wolkens
0 siblings, 2 replies; 43+ messages in thread
From: Ulrich Mueller @ 2024-06-04 17:45 UTC (permalink / raw)
To: Florian Schmaus; +Cc: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 1387 bytes --]
>>>>> On Tue, 04 Jun 2024, Florian Schmaus wrote:
> Both is fine with me.
> That said, many filesystem support inline data. If I am not mistaken,
> then its even enabled by default for xfs (which we recommend in the
> handbook) and btrfs. Also some README.gentoo files become suitable for
> inlining after compression (btrfs' limit is 2048 bytes).
I see 48 README.gentoo* files on my system here, and the _uncompressed_
size of the largest of them (belonging to www-client/firefox) is 1238
bytes. So, by your metric all of them could be inlined even without
compressing them. 14 of the 48 files aren't even compressed because
their size is below Portage's size limit (which is 128 bytes IIRC).
Also, it's not surprising that these files are very small. If they were
large, they wouldn't be suitable as output in pkg_postinst.
OTOH, if the filesystem is ext4, README.gentoo and the hash file will
each use up one 4 KiB block and one inode.
> Considering this, the 4-byte hash file is superior under the right
> circumstances when compared to excluding README.gentoo from
> compression. And I could imagine that the circumstances are right for
> many of our users.
I very much doubt this.
In any case, the above size considerations aren't important. My main
point is that the code is getting way too complicated for the simple
task of printing a few lines in pkg_postinst.
Ulrich
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)
2024-06-04 17:45 ` Ulrich Mueller
@ 2024-06-04 18:28 ` Florian Schmaus
2024-06-04 18:33 ` Eli Schwartz
2024-06-04 18:40 ` Ulrich Mueller
2024-06-04 18:37 ` Ionen Wolkens
1 sibling, 2 replies; 43+ messages in thread
From: Florian Schmaus @ 2024-06-04 18:28 UTC (permalink / raw)
To: Ulrich Mueller, gentoo-dev
[-- Attachment #1.1.1: Type: text/plain, Size: 2253 bytes --]
On 04/06/2024 19.45, Ulrich Mueller wrote:
>>>>>> On Tue, 04 Jun 2024, Florian Schmaus wrote:
>
>> Both is fine with me.
>
>> That said, many filesystem support inline data. If I am not mistaken,
>> then its even enabled by default for xfs (which we recommend in the
>> handbook) and btrfs. Also some README.gentoo files become suitable for
>> inlining after compression (btrfs' limit is 2048 bytes).
>
> I see 48 README.gentoo* files on my system here, and the _uncompressed_
> size of the largest of them (belonging to www-client/firefox) is 1238
> bytes. So, by your metric all of them could be inlined even without
> compressing them. 14 of the 48 files aren't even compressed because
> their size is below Portage's size limit (which is 128 bytes IIRC).
Fair point. On your system all README.gentoo could be inlined by btrfs.
You also say that compression does not make a difference for all files
on your system, as a whole 4 KiB block will likely be used anyway. We
agree in the point that most README.gentoo files are small, because they
should be suitable as output in pkg_postinst.
Doesn't this tell us that excluding README.gentoo from compression we be
ok-ish?
>> Considering this, the 4-byte hash file is superior under the right
>> circumstances when compared to excluding README.gentoo from
>> compression. And I could imagine that the circumstances are right for
>> many of our users.
>
> I very much doubt this.
>
> In any case, the above size considerations aren't important. My main
> point is that the code is getting way too complicated for the simple
> task of printing a few lines in pkg_postinst.
Agreed. Even though you oversimplify the task. It's not just printing
and installing a readme file. It's also comparing the contents of the
current file with the new one.
And yes, "docompress -x README.gentoo" would make the code muuuuuuuuch
simpler. And, as additional benefit, would help us get rid of storing
the whole content of the readme in a environment variable (which is what
readme.gentoo-r1.elcass currently does). Hence this is what was
previously suggested, until people complained about it.
It seems like we are going in circles… :(
- Flow
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 17797 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 618 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)
2024-06-04 18:28 ` Florian Schmaus
@ 2024-06-04 18:33 ` Eli Schwartz
2024-06-04 18:40 ` Ulrich Mueller
1 sibling, 0 replies; 43+ messages in thread
From: Eli Schwartz @ 2024-06-04 18:33 UTC (permalink / raw)
To: gentoo-dev
[-- Attachment #1.1.1: Type: text/plain, Size: 843 bytes --]
On 6/4/24 2:28 PM, Florian Schmaus wrote:
> And yes, "docompress -x README.gentoo" would make the code muuuuuuuuch
> simpler. And, as additional benefit, would help us get rid of storing
> the whole content of the readme in a environment variable (which is what
> readme.gentoo-r1.elcass currently does). Hence this is what was
> previously suggested, until people complained about it.
>
> It seems like we are going in circles… :(
I think it would be fantastic if we could drop the crude workaround of
adding the text of the readme.gentoo file into environment to sidestep
the fact that the original design was buggy and stored it as an optional
documentation file vulnerable to corruption as a side effect of default
`docompress` handling, when its purpose was to be a pkg_postinst
metadata file.
--
Eli Schwartz
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 18399 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)
2024-06-04 18:28 ` Florian Schmaus
2024-06-04 18:33 ` Eli Schwartz
@ 2024-06-04 18:40 ` Ulrich Mueller
1 sibling, 0 replies; 43+ messages in thread
From: Ulrich Mueller @ 2024-06-04 18:40 UTC (permalink / raw)
To: Florian Schmaus; +Cc: gentoo-dev
[-- Attachment #1: Type: text/plain, Size: 186 bytes --]
>>>>> On Tue, 04 Jun 2024, Florian Schmaus wrote:
> And yes, "docompress -x README.gentoo" would make the code muuuuuuuuch
> simpler.
Go for it then. Apparently it is the lesser evil.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)
2024-06-04 17:45 ` Ulrich Mueller
2024-06-04 18:28 ` Florian Schmaus
@ 2024-06-04 18:37 ` Ionen Wolkens
2024-06-04 18:59 ` Ionen Wolkens
1 sibling, 1 reply; 43+ messages in thread
From: Ionen Wolkens @ 2024-06-04 18:37 UTC (permalink / raw)
To: gentoo-dev; +Cc: Florian Schmaus
[-- Attachment #1: Type: text/plain, Size: 1435 bytes --]
On Tue, Jun 04, 2024 at 07:45:39PM +0200, Ulrich Mueller wrote:
> In any case, the above size considerations aren't important. My main
> point is that the code is getting way too complicated for the simple
> task of printing a few lines in pkg_postinst.
Have to say that this is mostly how I feel as well. Not that I followed
this whole conversation in full.
That aside, with all this talk of using the installed README.gentoo,
note that the file may not even be there because of FEATURES="nodoc".
Albeit could just assume it's unchanged in these cases.
Don't know if idea came up in this thread before but, if *really* had
to implement a mechanic to display the README.gentoo again on changes,
think I'd personally add an optional version variable/argument that
could be bumped by the ebuild maintainer whenever the README is
changed. Then if the version it's replacing is older than that it'll
display it again with a notice explaining that it changed. There are
some limitations to this approach but well, e.g.
- won't work without a bump/revbump to compare with
- maintainer might forget to set the version after changes
- version won't mean as much if update the README in all ebuild
versions at once, and can't tell what's actually been seen
(might cause occasional see-it-again when stabilizing)
- can't display a diff if wanted one, not that the hash approach
could do that either
--
ionen
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)
2024-06-04 18:37 ` Ionen Wolkens
@ 2024-06-04 18:59 ` Ionen Wolkens
0 siblings, 0 replies; 43+ messages in thread
From: Ionen Wolkens @ 2024-06-04 18:59 UTC (permalink / raw)
To: gentoo-dev, Florian Schmaus
[-- Attachment #1: Type: text/plain, Size: 2181 bytes --]
On Tue, Jun 04, 2024 at 02:37:56PM -0400, Ionen Wolkens wrote:
> On Tue, Jun 04, 2024 at 07:45:39PM +0200, Ulrich Mueller wrote:
> > In any case, the above size considerations aren't important. My main
> > point is that the code is getting way too complicated for the simple
> > task of printing a few lines in pkg_postinst.
>
> Have to say that this is mostly how I feel as well. Not that I followed
> this whole conversation in full.
>
> That aside, with all this talk of using the installed README.gentoo,
> note that the file may not even be there because of FEATURES="nodoc".
> Albeit could just assume it's unchanged in these cases.
>
> Don't know if idea came up in this thread before but, if *really* had
> to implement a mechanic to display the README.gentoo again on changes,
> think I'd personally add an optional version variable/argument that
> could be bumped by the ebuild maintainer whenever the README is
> changed. Then if the version it's replacing is older than that it'll
> display it again with a notice explaining that it changed. There are
> some limitations to this approach but well, e.g.
> - won't work without a bump/revbump to compare with
> - maintainer might forget to set the version after changes
> - version won't mean as much if update the README in all ebuild
> versions at once, and can't tell what's actually been seen
> (might cause occasional see-it-again when stabilizing)
> - can't display a diff if wanted one, not that the hash approach
> could do that either
Forgot to say, one perk is that maintainer can choose when the readme
is worth showing again. Hash would display it for minor style or typo
fixes.
Also in case what I was talking about is unclear, I'm talking about
ver_test using ${REPLACING_VERSIONS} like we often do in pkg_postinst
to show information only once on a bump.
e.g.
pkg-1.0.0: initial readme (no version)
pkg-1.0.1: modified readme, set readme's version to 1.0.1 in ebuild
pkg-1.0.2: readme is the same, keep readme's 1.0.1 unchanged
first install (any version) = display
1.0.0 -> 1.0.1 bump = display again
1.0.1 -> 1.0.2 bump = won't display
--
ionen
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)
2024-06-02 15:34 ` Ulrich Mueller
2024-06-02 15:48 ` Eli Schwartz
@ 2024-06-02 16:16 ` Florian Schmaus
1 sibling, 0 replies; 43+ messages in thread
From: Florian Schmaus @ 2024-06-02 16:16 UTC (permalink / raw)
To: Ulrich Mueller, gentoo-dev; +Cc: pacho
[-- Attachment #1.1.1: Type: text/plain, Size: 1156 bytes --]
On 02/06/2024 17.34, Ulrich Mueller wrote:
>>>>>> On Sun, 02 Jun 2024, Florian Schmaus wrote:
>
>> + (
>> + insinto "${_GREADME_DOC_DIR}"
>> +
>> + doins "${_GREADME_TMP_FILE}"
>> + cksum --raw "${_GREADME_TMP_FILE}" | newins - "${_GREADME_HASH_FILENAME}"
>> + assert
>> + )
>
> Why do you need that hash file? The old README file exists on the
> system, so couldn't you just compare the new one with it?
As Eli wrote and as I tried to make clear in the commit message (but
probably failed), there is unfortunately no reliable way to decompress
the two, most likely, compressed README.gentoo files in pkg_preinst.
Nothing guarantees that the decompressor is available at the time
pkg_preinst runs.
And even if we would ignore that, and try to opportunistically
decompress the files, then it is far from trivial to select the right
decompressor. I've looked in unpacker.eclass, which has some magic for
that, but only for the algorithms not required by PMS.
Hence I am glad we came up (I think it was mostly Sam) with the
hash-based approach, as it avoids having to deal with decompressing the
files.
- Flow
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 17797 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 618 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* [gentoo-dev] [PATCH 2/4] readme.gentoo-r1.eclass: use _GREADME_TMP_FILE in existing code
2024-06-02 13:57 ` [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass Florian Schmaus
2024-06-02 13:57 ` [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install) Florian Schmaus
@ 2024-06-02 13:57 ` Florian Schmaus
2024-06-02 13:57 ` [gentoo-dev] [PATCH 3/4] readme.gentoo-r1.eclass: add readme.gentoo_stdin() Florian Schmaus
` (2 subsequent siblings)
4 siblings, 0 replies; 43+ messages in thread
From: Florian Schmaus @ 2024-06-02 13:57 UTC (permalink / raw)
To: gentoo-dev; +Cc: pacho, Florian Schmaus
The previous commit introduced _GREADME_TMP_FILE. This commit changes
the existing code to use this variable.
Signed-off-by: Florian Schmaus <flow@gentoo.org>
---
eclass/readme.gentoo-r1.eclass | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/eclass/readme.gentoo-r1.eclass b/eclass/readme.gentoo-r1.eclass
index 2a79911cafbf..078077241944 100644
--- a/eclass/readme.gentoo-r1.eclass
+++ b/eclass/readme.gentoo-r1.eclass
@@ -70,20 +70,20 @@ readme.gentoo_create_doc() {
if [[ -n "${DOC_CONTENTS}" ]]; then
if [[ -n "${DISABLE_AUTOFORMATTING}" ]]; then
- echo "${DOC_CONTENTS}" > "${T}"/README.gentoo || die
+ echo "${DOC_CONTENTS}" > "${_GREADME_TMP_FILE}" || die
else
local saved_flags=$-
set -f # disable filename expansion in echo arguments
echo -e ${DOC_CONTENTS} | fold -s -w 70 \
- | sed 's/[[:space:]]*$//' > "${T}"/README.gentoo
+ | sed 's/[[:space:]]*$//' > "${_GREADME_TMP_FILE}"
assert
set +f -${saved_flags}
fi
elif [[ -f "${FILESDIR}/README.gentoo-${SLOT%/*}" ]]; then
- cp "${FILESDIR}/README.gentoo-${SLOT%/*}" "${T}"/README.gentoo || die
+ cp "${FILESDIR}/README.gentoo-${SLOT%/*}" "${_GREADME_TMP_FILE}" || die
elif [[ -f "${FILESDIR}/README.gentoo${README_GENTOO_SUFFIX}" ]]; then
- cp "${FILESDIR}/README.gentoo${README_GENTOO_SUFFIX}" "${T}"/README.gentoo || die
- else
+ cp "${FILESDIR}/README.gentoo${README_GENTOO_SUFFIX}" "${_GREADME_TMP_FILE}" || die
+ elif [[ ! -f "${_GREADME_TMP_FILE}" ]]; then
die "You are not specifying README.gentoo contents!"
fi
--
2.44.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [gentoo-dev] [PATCH 3/4] readme.gentoo-r1.eclass: add readme.gentoo_stdin()
2024-06-02 13:57 ` [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass Florian Schmaus
2024-06-02 13:57 ` [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install) Florian Schmaus
2024-06-02 13:57 ` [gentoo-dev] [PATCH 2/4] readme.gentoo-r1.eclass: use _GREADME_TMP_FILE in existing code Florian Schmaus
@ 2024-06-02 13:57 ` Florian Schmaus
2024-06-02 13:57 ` [gentoo-dev] [PATCH 4/4] readme.gentoo-r1.eclass: add readme.gentoo_file() Florian Schmaus
2024-06-02 15:25 ` [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass Ulrich Mueller
4 siblings, 0 replies; 43+ messages in thread
From: Florian Schmaus @ 2024-06-02 13:57 UTC (permalink / raw)
To: gentoo-dev; +Cc: pacho, Florian Schmaus
Add a new function readme.gentoo_stdin() that consumes the content of
README.gentoo from stdin. In many cases, this is a supperiour method to
construct readme, compared to the eclass' DOC_CONTENTS approach.
Signed-off-by: Florian Schmaus <flow@gentoo.org>
---
eclass/readme.gentoo-r1.eclass | 51 ++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/eclass/readme.gentoo-r1.eclass b/eclass/readme.gentoo-r1.eclass
index 078077241944..db7fa0c47077 100644
--- a/eclass/readme.gentoo-r1.eclass
+++ b/eclass/readme.gentoo-r1.eclass
@@ -14,6 +14,22 @@
# shown at first package installation and a file for later reviewing will be
# installed under /usr/share/doc/${PF}
#
+# @CODE
+# inherit readme.gentoo-r1
+#
+# src_install() {
+# …
+# readme.gentoo_stdin <<-EOF
+# This is the content of the created readme doc file.
+# EOF
+# …
+# if use foo; then
+# readme.gentoo_stdin --apend <<-EOF
+# This is conditional readme content, based on USE=foo.
+# EOF
+# fi
+# }
+# @CODE
#
# You need to call readme.gentoo_create_doc in src_install phase if you
# use DOC_CONTENTS or obtain the readme from FILESIDR.
@@ -59,6 +75,41 @@ _GREADME_DOC_DIR="usr/share/doc/${PF}"
_GREADME_REL_PATH="${_GREADME_DOC_DIR}/${_GREADME_FILENAME}"
_GREADME_HASH_REL_PATH="${_GREADME_DOC_DIR}/${_GREADME_HASH_FILENAME}"
+# @FUNCTION: readme.gentoo_stdin
+# @USAGE: [--append]
+# @DESCRIPTION:
+# Create the readme doc via stdin. You can use --append to append to an
+# existing readme doc.
+readme.gentoo_stdin() {
+ debug-print-function ${FUNCNAME} "${@}"
+
+ local append=false
+ while [[ -n ${1} ]] && [[ ${1} =~ --* ]]; do
+ case ${1} in
+ --append)
+ append=true
+ shift
+ ;;
+ esac
+ done
+
+ if $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
+
+ readme.gentoo_create_doc
+}
+
# @FUNCTION: readme.gentoo_create_doc
# @DESCRIPTION:
# Create doc file with ${DOC_CONTENTS} variable (preferred) and, if not set,
--
2.44.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [gentoo-dev] [PATCH 4/4] readme.gentoo-r1.eclass: add readme.gentoo_file()
2024-06-02 13:57 ` [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass Florian Schmaus
` (2 preceding siblings ...)
2024-06-02 13:57 ` [gentoo-dev] [PATCH 3/4] readme.gentoo-r1.eclass: add readme.gentoo_stdin() Florian Schmaus
@ 2024-06-02 13:57 ` Florian Schmaus
2024-06-02 15:25 ` [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass Ulrich Mueller
4 siblings, 0 replies; 43+ messages in thread
From: Florian Schmaus @ 2024-06-02 13:57 UTC (permalink / raw)
To: gentoo-dev; +Cc: pacho, Florian Schmaus
The new readme.gentoo_file() function provided more flexibility from
where the readme file is obtained, compared to the existing methods.
Signed-off-by: Florian Schmaus <flow@gentoo.org>
---
eclass/readme.gentoo-r1.eclass | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/eclass/readme.gentoo-r1.eclass b/eclass/readme.gentoo-r1.eclass
index db7fa0c47077..619f3eaa8fbf 100644
--- a/eclass/readme.gentoo-r1.eclass
+++ b/eclass/readme.gentoo-r1.eclass
@@ -110,6 +110,27 @@ readme.gentoo_stdin() {
readme.gentoo_create_doc
}
+# @FUNCTION: readme.gentoo_file
+# @USAGE: <file>
+# @DESCRIPTION:
+# Installs the provided file as readme doc.
+readme.gentoo_file() {
+ debug-print-function ${FUNCNAME} "${@}"
+
+ local input_doc_file="${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 "Failed to copy ${input_doc_file}"
+
+ readme.gentoo_create_doc
+}
+
# @FUNCTION: readme.gentoo_create_doc
# @DESCRIPTION:
# Create doc file with ${DOC_CONTENTS} variable (preferred) and, if not set,
--
2.44.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass
2024-06-02 13:57 ` [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass Florian Schmaus
` (3 preceding siblings ...)
2024-06-02 13:57 ` [gentoo-dev] [PATCH 4/4] readme.gentoo-r1.eclass: add readme.gentoo_file() Florian Schmaus
@ 2024-06-02 15:25 ` Ulrich Mueller
2024-06-02 16:12 ` Florian Schmaus
4 siblings, 1 reply; 43+ messages in thread
From: Ulrich Mueller @ 2024-06-02 15:25 UTC (permalink / raw)
To: Florian Schmaus; +Cc: gentoo-dev, pacho
[-- Attachment #1: Type: text/plain, Size: 397 bytes --]
>>>>> On Sun, 02 Jun 2024, Florian Schmaus wrote:
> Note that this changes readme.gentoo-r1.eclass to export phase
> functions when it previously did not.
IMHO that's a very bad idea and will probably break ebuilds that rely
on the current behaviour.
(Also, readme.gentoo.eclass used to export phase functions, which was
removed in -r1. So you should have a good rationale for reverting this.)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass
2024-06-02 15:25 ` [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass Ulrich Mueller
@ 2024-06-02 16:12 ` Florian Schmaus
2024-06-02 16:40 ` Ulrich Mueller
0 siblings, 1 reply; 43+ messages in thread
From: Florian Schmaus @ 2024-06-02 16:12 UTC (permalink / raw)
To: Ulrich Mueller, gentoo-dev; +Cc: pacho
[-- Attachment #1.1.1: Type: text/plain, Size: 1597 bytes --]
On 02/06/2024 17.25, Ulrich Mueller wrote:
>>>>>> On Sun, 02 Jun 2024, Florian Schmaus wrote:
>
>> Note that this changes readme.gentoo-r1.eclass to export phase
>> functions when it previously did not.
>
> IMHO that's a very bad idea and will probably break ebuilds that rely
> on the current behaviour.
I pondered about this and its one of the reasons I'd rather start with a
fresh eclass.
That said, worst case scenario I could came up with is that the elog
message is printed twice. And this is also only the case if the ebuild
has readme.gentoo_print_elog *not* in pkg_postinst. However, the
readme.gentoo-r1.eclass documentation suggests you to put it in
pkg_postinst.
And if an ebuild has readme.gentoo_print_elog *in* pkg_postinst, which
should be true for nearly all ebuilds currently inheriting
readme.gentoo-r1, then you don't use the newly exported pkg_postinst
function (and the outcome of the exproted pkg_preinst is simply ignored).
Bottom line is: exporting the functions should do no harm.
> (Also, readme.gentoo.eclass used to export phase functions, which was
> removed in -r1. So you should have a good rationale for reverting this.)
I talked with pacho about this. The background back then was
https://bugs.gentoo.org/520094, but this about src_install. At the end
of the discussion with pacho, we both came to the conclusion that
exporting pkg_preinst and pkg_postinst is not a problem.
Bottom line here is: eclasses exporting phase functions is not uncommon.
I do not see any issue in this case, do you?
- Flow
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 17797 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 618 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass
2024-06-02 16:12 ` Florian Schmaus
@ 2024-06-02 16:40 ` Ulrich Mueller
2024-06-02 17:34 ` Florian Schmaus
0 siblings, 1 reply; 43+ messages in thread
From: Ulrich Mueller @ 2024-06-02 16:40 UTC (permalink / raw)
To: Florian Schmaus; +Cc: gentoo-dev, pacho
>>>>> On Sun, 02 Jun 2024, Florian Schmaus wrote:
>> IMHO that's a very bad idea and will probably break ebuilds that rely
>> on the current behaviour.
> I pondered about this and its one of the reasons I'd rather start with
> a fresh eclass.
> That said, worst case scenario I could came up with is that the elog
> message is printed twice. And this is also only the case if the ebuild
> has readme.gentoo_print_elog *not* in pkg_postinst. However, the
> readme.gentoo-r1.eclass documentation suggests you to put it in
> pkg_postinst.
> And if an ebuild has readme.gentoo_print_elog *in* pkg_postinst, which
> should be true for nearly all ebuilds currently inheriting
> readme.gentoo-r1, then you don't use the newly exported pkg_postinst
> function (and the outcome of the exproted pkg_preinst is simply
> ignored).
> Bottom line is: exporting the functions should do no harm.
It would break most ebuilds that inherit elisp and readme.gentoo-r1,
because elisp_pkg_postinst would no longer be called.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass
2024-06-02 16:40 ` Ulrich Mueller
@ 2024-06-02 17:34 ` Florian Schmaus
0 siblings, 0 replies; 43+ messages in thread
From: Florian Schmaus @ 2024-06-02 17:34 UTC (permalink / raw)
To: Ulrich Mueller; +Cc: gentoo-dev, pacho
[-- Attachment #1.1.1: Type: text/plain, Size: 1546 bytes --]
On 02/06/2024 18.40, Ulrich Mueller wrote:
>>>>>> On Sun, 02 Jun 2024, Florian Schmaus wrote:
>
>>> IMHO that's a very bad idea and will probably break ebuilds that rely
>>> on the current behaviour.
>
>> I pondered about this and its one of the reasons I'd rather start with
>> a fresh eclass.
>
>> That said, worst case scenario I could came up with is that the elog
>> message is printed twice. And this is also only the case if the ebuild
>> has readme.gentoo_print_elog *not* in pkg_postinst. However, the
>> readme.gentoo-r1.eclass documentation suggests you to put it in
>> pkg_postinst.
>
>> And if an ebuild has readme.gentoo_print_elog *in* pkg_postinst, which
>> should be true for nearly all ebuilds currently inheriting
>> readme.gentoo-r1, then you don't use the newly exported pkg_postinst
>> function (and the outcome of the exproted pkg_preinst is simply
>> ignored).
>
>> Bottom line is: exporting the functions should do no harm.
>
> It would break most ebuilds that inherit elisp and readme.gentoo-r1,
> because elisp_pkg_postinst would no longer be called.
You are right. I had not considered this case. Interesting.
We could maybe define a pre-inherit variable
README_GENTOO_EXPORT_PHASE_FUNCS to opt-in to the new behavior. Is there
any prior art regarding this?
However, it feels like my hunch was right and this is just another
argument why it should be a new eclass.
But if you have any suggestions on how to best deal with this, then
please let me know.
- Flow
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 17797 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 618 bytes --]
^ permalink raw reply [flat|nested] 43+ messages in thread