[gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass

[gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass

[Florian Schmaus]

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

[gentoo-dev] [PATCH 1/1] greadme.eclass: new eclass

[Florian Schmaus]

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

Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass

[Michał Górny]

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

On Sat, 2024-01-06 at 18:01 +0100, Florian Schmaus wrote:
> 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.

Are you implying that readme.gentoo-r1 is unfixable and you need to
start over, and have a third generation of eclasses to install a readme
file?

> 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].

Dangling reference here.  In any case, documentation compression is
a standard feature of the package manager.  If it doesn't work for
whatever reason, I'd rather see you focus on find a good solution rather
than working it around via abusing `docompress -x`.  It's basically
a case of "standard feature X doesn't work for me sometimes, so I now
randomly disable X via my eclass, and hope nobody notices".

> 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.

The compression mechanism automatically detects when the file is too
small to be worth compressing.  See PORTAGE_DOCOMPRESS_SIZE_LIMIT.


-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass

[Florian Schmaus]

[-- Attachment #1.1.1: Type: text/plain, Size: 3803 bytes --]

On 06/01/2024 18.21, Michał Górny wrote:
> On Sat, 2024-01-06 at 18:01 +0100, Florian Schmaus wrote:
>> 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.
> 
> Are you implying that readme.gentoo-r1 is unfixable and you need to
> start over, and have a third generation of eclasses to install a readme
> file?

Not at all. See the second item of the TODO list in the eclass' description.

That said, both eclasses are rather small, and "fixing" readme.gentoo-r1 
would consist of copying most of greadme into readme.gentoo-r1, while 
still having the "legacy" readme.gentoo-r1 functionality in it.

Starting over strikes me as sensible in this case.


>> 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].
> 
> Dangling reference here.  In any case, documentation compression is
> a standard feature of the package manager.  If it doesn't work for
> whatever reason, I'd rather see you focus on find a good solution rather
> than working it around via abusing `docompress -x`.  

The problem is the lack of a guarantee that the utilities required to 
decompress the file are available at installation time.

It gets even worse if you consider binpkgs, as you have surely read the 
comment while looking at the code of the greadme.eclass.


 > It's basically
 > a case of "standard feature X doesn't work for me sometimes, so I now
 > randomly disable X via my eclass, and hope nobody notices".

The primary motivation for posting this RFC was to find a solution and 
avoid the "docompress -x" approach. However, I don't think there is one 
that does not require adjusting PMS to provide the guarantee that the 
eclass can decompress the compressed doc file.

Adjusting PMS seems overkill just for avoiding to exclude a single small 
file from compression.

I also tried to make the greadme eclass handle unpacking errors 
gracefully. This turned out to be harder than I hoped because most 
(all?) functions of unpacker.eclass are not nonfatal compatible. The 
same is true for unpack() from PMS, which is used by unpacker.eclass.

I'll send out v2 of greadme.eclass soon.

Looking at the conditional _GREADME_COMPRESS code, you will find that 
adding support for compressing the doc file introduces a lot of 
complexity to the eclass. That would still make me consider it, but I 
can not see any reliable approach to unpacking that does not involve 
adjusting PMS.

Surely you would be able to correct me if I am wrong.


>> 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.
> 
> The compression mechanism automatically detects when the file is too
> small to be worth compressing.  See PORTAGE_DOCOMPRESS_SIZE_LIMIT.

How is this mechanism helpful here?

- 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 --]

[gentoo-dev] [PATCH v2 1/3] greadme.eclass: new eclass

[Florian Schmaus]

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_files[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_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 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
-- 
2.41.0

[gentoo-dev] [PATCH v2 2/3] add UNPACKER_NO_BANNER variable

[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

[gentoo-dev] [PATCH v2 3/3] greadme.eclass: set UNPACKER_NO_BANNER

[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

[gentoo-dev] Re: [PATCH v2 2/3] add UNPACKER_NO_BANNER variable

[Florian Schmaus]

[-- 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 --]

Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass

[Michał Górny]

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

On Tue, 2024-01-09 at 09:30 +0100, Florian Schmaus wrote:
> On 06/01/2024 18.21, Michał Górny wrote:
> > On Sat, 2024-01-06 at 18:01 +0100, Florian Schmaus wrote:
> > > 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.
> > 
> > Are you implying that readme.gentoo-r1 is unfixable and you need to
> > start over, and have a third generation of eclasses to install a readme
> > file?
> 
> Not at all. See the second item of the TODO list in the eclass' description.
> 
> That said, both eclasses are rather small,
> 

Since when, 300 lines to install a README file is "small"?

> > > 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].
> > 
> > Dangling reference here.  In any case, documentation compression is
> > a standard feature of the package manager.  If it doesn't work for
> > whatever reason, I'd rather see you focus on find a good solution rather
> > than working it around via abusing `docompress -x`.  
> 
> The problem is the lack of a guarantee that the utilities required to 
> decompress the file are available at installation time.

It's user's responsibility to ensure that the tools set in their
make.conf are available.

> It gets even worse if you consider binpkgs, as you have surely read the 
> comment while looking at the code of the greadme.eclass.

See FEATURES=-binpkg-docompress.  That's the correct way to distribute
binary packages.


-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass

[Florian Schmaus]

[-- Attachment #1.1.1: Type: text/plain, Size: 3618 bytes --]

On 09/01/2024 10.59, Michał Górny wrote:
> On Tue, 2024-01-09 at 09:30 +0100, Florian Schmaus wrote:
>> On 06/01/2024 18.21, Michał Górny wrote:
>>> On Sat, 2024-01-06 at 18:01 +0100, Florian Schmaus wrote:
>>>> 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.
>>>
>>> Are you implying that readme.gentoo-r1 is unfixable and you need to
>>> start over, and have a third generation of eclasses to install a readme
>>> file?
>>
>> Not at all. See the second item of the TODO list in the eclass' description.
>>
>> That said, both eclasses are rather small,
>>
> 
> Since when, 300 lines to install a README file is "small"?

The eclass becomes very small if you remove the _GREADME_COMPRESS code. 
As I wrote, adding compression support to the eclass makes the code very 
complex.


>>>> 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].
>>>
>>> Dangling reference here.  In any case, documentation compression is
>>> a standard feature of the package manager.  If it doesn't work for
>>> whatever reason, I'd rather see you focus on find a good solution rather
>>> than working it around via abusing `docompress -x`.
>>
>> The problem is the lack of a guarantee that the utilities required to
>> decompress the file are available at installation time.
> 
> It's user's responsibility to ensure that the tools set in their
> make.conf are available.

What if the user obtained a binpkg that was compressed with a different 
algorithm than the one set in their make.conf? Of course, the binhost 
could have set FEATURES=-binpkg-docompress, but what if not?

Even if we say it is the user's fault, then the problem of handling a 
decompressor failure would still exist. The eclass does not gracefully 
continue when decompressing the doc file, but instead it runs into a 
die(). To address this we would need to make unpacker.eclass and 
unpack() aware of nonfatal. The latter would require a PMS change.

Or, I could duplicate unpack logic --- which is probably a lot to 
account for the various compression options --- in the eclass. But I 
suspect be both do not want that.


>> It gets even worse if you consider binpkgs, as you have surely read the
>> comment while looking at the code of the greadme.eclass.
> 
> See FEATURES=-binpkg-docompress.  That's the correct way to distribute
> binary packages.

Is that documented somewhere that this is the right way to distribute 
binary packages?


With the information I have right now, I do not see a better alternative 
than excluding the doc file from compression.

Are you willing a sacrifice a very sensible feature just to be able to 
compress what is usually a file of a few hundred of bytes?



- 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: 614 bytes --]

Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass

[Michał Górny]

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

On Tue, 2024-01-09 at 11:39 +0100, Florian Schmaus wrote:
> Even if we say it is the user's fault, then the problem of handling a 
> decompressor failure would still exist. The eclass does not gracefully 
> continue when decompressing the doc file, but instead it runs into a 
> die(). To address this we would need to make unpacker.eclass and 
> unpack() aware of nonfatal. The latter would require a PMS change.
> 
> Or, I could duplicate unpack logic --- which is probably a lot to 
> account for the various compression options --- in the eclass. But I 
> suspect be both do not want that.

Perhaps this is the moment when you realize that there is no guarantee
that unpacker.eclass and/or unpack will support the compression format
set for docompress.  The two were never intended to interact.

-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass

[Florian Schmaus]

[-- Attachment #1.1.1: Type: text/plain, Size: 1228 bytes --]

On 09/01/2024 11.43, Michał Górny wrote:
> On Tue, 2024-01-09 at 11:39 +0100, Florian Schmaus wrote:
>> Even if we say it is the user's fault, then the problem of handling a
>> decompressor failure would still exist. The eclass does not gracefully
>> continue when decompressing the doc file, but instead it runs into a
>> die(). To address this we would need to make unpacker.eclass and
>> unpack() aware of nonfatal. The latter would require a PMS change.
>>
>> Or, I could duplicate unpack logic --- which is probably a lot to
>> account for the various compression options --- in the eclass. But I
>> suspect be both do not want that.
> 
> Perhaps this is the moment when you realize that there is no guarantee
> that unpacker.eclass and/or unpack will support the compression format
> set for docompress.  The two were never intended to interact.

Hence I wrote about making then nonfatal aware. Then greadme.eclass 
could try to opportunistically decompress the doc file, and simply 
continue with a sensible behavior it it fails.

A look at the greadme code reveals that it tries to do so already, but 
unfortunately unsuccessfully because nonfatal has no effect on 
unpacker/unpack.

- 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 --]

Re: [gentoo-dev] [PATCH v2 1/3] greadme.eclass: new eclass

[David Seifert]

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.

Re: [gentoo-dev] [PATCH v2 1/3] greadme.eclass: new eclass

[Florian Schmaus]

[-- 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 --]

Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass

[Sam James]

Florian Schmaus <flow@gentoo.org> writes:

> 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.

I have a few concerns at the moment, but of varying severity:
1) The name seems odd (why not readme.gentoo-r2)?
2) Why can't the existing eclass be improved?
3) Is the reason for 2) strong enough to introduce a new eclass?
4) The compression deal seems not worth bothering with.

I don't really want to see a bifurcation in our README eclasses
if we can help it. Porting to something new takes ages and it's
a lot of churn.

Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass

[Florian Schmaus]

[-- Attachment #1.1.1: Type: text/plain, Size: 3563 bytes --]

On 10/01/2024 12.04, Sam James wrote:
> 
> Florian Schmaus <flow@gentoo.org> writes:
> 
>> 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.
> 
> I have a few concerns at the moment, but of varying severity:

> 1) The name seems odd (why not readme.gentoo-r2)? > 2) Why can't the existing eclass be improved?

Both points, the name of the eclass and the question if this should be 
added to the existing eclass or as a new eclass, are absolutely *no* 
hill I want to die on.

What I *really* care about is having the functionality that there is a 
readme eclass that *also* shows the elog message if the README's content 
changed (and not just on the first installation of the package).


 > 4) The compression deal seems not worth bothering with.

Just to clarify: you are agreeing that excluding the readme doc from 
being compressed is fine?


> 3) Is the reason for 2) strong enough to introduce a new eclass?
> I don't really want to see a bifurcation in our README eclasses
> if we can help it. Porting to something new takes ages and it's
> a lot of churn.

I think the arguments for introducing a new eclass are strong enough. I 
will elaborate on this in the appendix of this mail, since I don't think 
its what we should focus on at the moment.

However, right now I want clarify that using "docompress -x" in this 
case is agreeable by everyone.

- Flow


# Appendix: Arguments for introducing a new class

readme.gentoo-r1 can be viewed at

https://github.com/gentoo/gentoo/blob/master/eclass/readme.gentoo-r1.eclass

while the simple and short version (203 lines including comments!) of 
the proposed greadme.eclass can be seen at

https://github.com/Flowdalic/gentoo/blob/greadme.eclass-simple/eclass/greadme.eclass

The proposed API of the greadme eclass is already different from 
readme.gentoo-r1. It exports phase functions, which readme.gentoo-r1 
does not. The readme.gentoo-r1 eclass always shoves the full content of 
the readme into an environment variable. Excluding the readme file from 
compression means we do not have to do that in greadme.

Please feel invited take a look at the code of both eclasses and make 
yourself an opinion if it is sensible to incorporate greadme.eclass into 
the existing eclass.

Then maybe also also look a the API of both eclasses. Switching from 
readme.gentoo-r1 to greadme is simple and straightforward in most cases.

Furthermore, the name readme.gentoo-r1 seems odd, it is one of the few 
eclasses that use a dot as separator (although, I can see why this may 
been done). Looking at eclasses/ it appears that readme-gentoo-r1.eclass 
would be more in-line with the existing eclasses. Or, as a matter of 
opinion, maybe better, because shorter: greadme

Adding a new readme eclass hopefully is not a bifurcation, cause we 
ideally would be able to phase out readme.gentoo-r1.




[-- 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 --]

Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass

[Ulrich Mueller]

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

>>>>> On Wed, 10 Jan 2024, Florian Schmaus wrote:

> On 10/01/2024 12.04, Sam James wrote:
>> 1) The name seems odd (why not readme.gentoo-r2)?
>> 2) Why can't the existing eclass be improved?

> Both points, the name of the eclass and the question if this should be
> added to the existing eclass or as a new eclass, are absolutely *no*
> hill I want to die on.

> What I *really* care about is having the functionality that there is a
> readme eclass that *also* shows the elog message if the README's
> content changed (and not just on the first installation of the
> package).

Looks like readme.gentoo-r1 already gives you control over this:

# If you want to show them always, please set FORCE_PRINT_ELOG to a non empty
# value in your ebuild before this function is called.
# This can be useful when, for example, DOC_CONTENTS is modified, then, you can
# rely on specific REPLACING_VERSIONS handling in your ebuild to print messages
# when people update from versions still providing old message.

>> 4) The compression deal seems not worth bothering with.

> Just to clarify: you are agreeing that excluding the readme doc from
> being compressed is fine?

Please respect the user's compression settings there. IMHO overriding
them with docompress -x is a big no-no.

> [...]

> It exports phase functions, which readme.gentoo-r1 does not.

Looking at the history, readme.gentoo[-r0] used to export phase
functions:
https://gitweb.gentoo.org/repo/gentoo.git/tree/eclass/readme.gentoo.eclass?id=1e7b2242de29ec60105df1ef31939aed85a8b0eb#n32
It turned out to be a bad design choice, so -r1 no longer does that.

> The readme.gentoo-r1 eclass always shoves the full content of the
> readme into an environment variable.

Why is this a problem?

Ulrich

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]

Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass

[Florian Schmaus]

[-- Attachment #1.1.1: Type: text/plain, Size: 2473 bytes --]

On 10/01/2024 14.58, Ulrich Mueller wrote:
>>>>>> On Wed, 10 Jan 2024, Florian Schmaus wrote:
> 
>> On 10/01/2024 12.04, Sam James wrote:
>>> 1) The name seems odd (why not readme.gentoo-r2)?
>>> 2) Why can't the existing eclass be improved?
> 
>> Both points, the name of the eclass and the question if this should be
>> added to the existing eclass or as a new eclass, are absolutely *no*
>> hill I want to die on.
> 
>> What I *really* care about is having the functionality that there is a
>> readme eclass that *also* shows the elog message if the README's
>> content changed (and not just on the first installation of the
>> package).
> 
> Looks like readme.gentoo-r1 already gives you control over this:
> 
> # If you want to show them always, please set FORCE_PRINT_ELOG to a non empty
> # value in your ebuild before this function is called.
> # This can be useful when, for example, DOC_CONTENTS is modified, then, you can
> # rely on specific REPLACING_VERSIONS handling in your ebuild to print messages
> # when people update from versions still providing old message.

It is easy to forget setting FORCE_PRINT_ELOG, just as it is easy to 
forget to unset it again.

An automatism is always preferable over a manual solution.


>>> 4) The compression deal seems not worth bothering with.
> 
>> Just to clarify: you are agreeing that excluding the readme doc from
>> being compressed is fine?
> 
> Please respect the user's compression settings there. IMHO overriding
> them with docompress -x is a big no-no.

Then why does "docompress -x" exist at all?

There seems to be a big win-win if we override the compression settingin 
this case.


>> It exports phase functions, which readme.gentoo-r1 does not.
> 
> Looking at the history, readme.gentoo[-r0] used to export phase
> functions:
> https://gitweb.gentoo.org/repo/gentoo.git/tree/eclass/readme.gentoo.eclass?id=1e7b2242de29ec60105df1ef31939aed85a8b0eb#n32
> It turned out to be a bad design choice, so -r1 no longer does that.

Interesting find.

It is not obvious to me why the eclass exporting phase function should 
is a bad design choice.

@pacho: could you shed some light into this?


>> The readme.gentoo-r1 eclass always shoves the full content of the
>> readme into an environment variable.
> 
> Why is this a problem?

Nobody described that as a problem. Not adding stuff into the 
environment is simply nice to have.

- 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 --]

Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass

[Ulrich Mueller]

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

>>>>> On Wed, 10 Jan 2024, Florian Schmaus wrote:

> On 10/01/2024 14.58, Ulrich Mueller wrote:
>> Looks like readme.gentoo-r1 already gives you control over this:
>> # If you want to show them always, please set FORCE_PRINT_ELOG to a non empty
>> # value in your ebuild before this function is called.
>> # This can be useful when, for example, DOC_CONTENTS is modified, then, you can
>> # rely on specific REPLACING_VERSIONS handling in your ebuild to print messages
>> # when people update from versions still providing old message.

> It is easy to forget setting FORCE_PRINT_ELOG, just as it is easy to
> forget to unset it again.

> An automatism is always preferable over a manual solution.

Maybe I want manual control? For example, when I fix a typo in the
README file then I don't want to show it to users again.

>>> Just to clarify: you are agreeing that excluding the readme doc from
>>> being compressed is fine?
>> Please respect the user's compression settings there. IMHO
>> overriding
>> them with docompress -x is a big no-no.

> Then why does "docompress -x" exist at all?

Short answer, because some upstream programs access these directories
and cannot cope with compressed files there.

Long answer, see the previous discussion on this list [1] and in
bug 250077 [2].

> There seems to be a big win-win if we override the compression
> settingin this case.

I tend to disagree. We shouldn't override users' choices unless
absolutely necessary.

Ulrich

[1] https://archives.gentoo.org/gentoo-dev/message/2fd5f58132881ef69219c126a525bce3
[2] https://bugs.gentoo.org/250077

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]

Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass

[Florian Schmaus]

[-- Attachment #1.1.1: Type: text/plain, Size: 1475 bytes --]

On 10/01/2024 16.10, Ulrich Mueller wrote:
>>>>>> On Wed, 10 Jan 2024, Florian Schmaus wrote:
> 
>> On 10/01/2024 14.58, Ulrich Mueller wrote:
>>> Looks like readme.gentoo-r1 already gives you control over this:
>>> # If you want to show them always, please set FORCE_PRINT_ELOG to a non empty
>>> # value in your ebuild before this function is called.
>>> # This can be useful when, for example, DOC_CONTENTS is modified, then, you can
>>> # rely on specific REPLACING_VERSIONS handling in your ebuild to print messages
>>> # when people update from versions still providing old message.
> 
>> It is easy to forget setting FORCE_PRINT_ELOG, just as it is easy to
>> forget to unset it again.
> 
>> An automatism is always preferable over a manual solution.
> 
> Maybe I want manual control? For example, when I fix a typo in the
> README file then I don't want to show it to users again.

An automatism does not exclude an manual override. That can easily be 
added to greadme.eclass.


>> There seems to be a big win-win if we override the compression
>> settingin this case.
> 
> I tend to disagree. We shouldn't override users' choices unless
> absolutely necessary.

Just that there is no misunderstanding: you are aware that it is a file 
of typically just a few lines of text that we are discussing here 
whether or not it should be compressed?

Seems like overshooting the mark to argue with users' choices in this case.

- 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 --]

[gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass

[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

[gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)

[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

[gentoo-dev] [PATCH 2/4] readme.gentoo-r1.eclass: use _GREADME_TMP_FILE in existing code

[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

[gentoo-dev] [PATCH 3/4] readme.gentoo-r1.eclass: add readme.gentoo_stdin()

[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

[gentoo-dev] [PATCH 4/4] readme.gentoo-r1.eclass: add readme.gentoo_file()

[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

Re: [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass

[Ulrich Mueller]

[-- 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 --]

Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)

[Ulrich Mueller]

[-- 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 --]

Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)

[Eli Schwartz]

[-- 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 --]

Re: [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass

[Florian Schmaus]

[-- 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 --]

Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)

[Florian Schmaus]

[-- 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 --]

Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)

[Ulrich Mueller]

[-- 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 --]

Re: [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass

[Ulrich Mueller]

>>>>> 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.

Re: [gentoo-dev] [PATCH 0/4] Improve readme.gentoo-r1.eclass

[Florian Schmaus]

[-- 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 --]

Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)

[Florian Schmaus]

[-- 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 --]

Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)

[Eli Schwartz]

[-- 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 --]

Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)

[Ulrich Mueller]

[-- 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 --]

Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)

[Florian Schmaus]

[-- 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 --]

Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)

[Ulrich Mueller]

[-- 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 --]

Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)

[Florian Schmaus]

[-- 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 --]

Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)

[Eli Schwartz]

[-- 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 --]

Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)

[Ionen Wolkens]

[-- 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 --]

Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)

[Ulrich Mueller]

[-- 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 --]

Re: [gentoo-dev] [PATCH 1/4] readme.gentoo-r1.eclass: display readme if content changed (or fresh install)

[Ionen Wolkens]

[-- 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 --]