Re: [gentoo-dev] [PATCH v4] greadme.eclass: new eclass

[Florian Schmaus <flow@gentoo.org>]

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

Hi Arthur,

thanks for your mail.

On 16/06/2024 20.15, Arthur Zamarin wrote:
> On 16/06/2024 18.51, Florian Schmaus wrote:
>> This new eclass includes various improvements over the existing
>> readme.gentoo-r1.eclass.
> 
> So, some weird question from me - why is it called greadme? I can
> understand why you don't want to modify existing eclass, but why not
> call it "readme.gentoo-r2.eclass"? This should make it a little less
> confusing (cause I imagine folks asking - which to use. With -r2 we all
> know which one is better).


It was already pointed out, but let me quote from my previous mail:

"""
Note that I choose greadme.eclass as the name for the new eclass for
aesthetic reasons. I find readme.gentoo-r2 a mouthful, and it leads to
an ugly combination of dot, hyphen, and underscores. However, if
anyone wants to have function names like
'readme.gentoo-r2_pkg_postinst', then we can go with that.
"""

Personally, I think it is better to have a forward pointer from the old 
elcass to the new eclass via @DEPRECATED, compared to going with 
readme.gentoo-r2. But it is certainly not a hill I want to die on.


>> First, the content of README.gento will only be shown on new
>> installations or if it has changed between updates.
>>
>> Second, it allows the composition of readme via bash's heredoc.
> 
> Is it common usage you think? I've never felt the need for that.

It can't be common usage, because it is not directly supported yet. :)

And yes, I feel the need for that. Short heredocs have the advantage 
over stuff in FILESDIR that one can not forget about cleaning up 
references files in FILESDIR.

Using heredocs also makes it easier to conditionally compose the 
contents of README.gentoo. See the eclass' @CODE example.

But yes, the same would be possible with by composing the content via a 
variable. In the end, it is probably subjective what you prefer.


>> Third, it exports phase functions, which helps to reduce the boilerplate
>> code in many cases.
> 
> That actually sounds nice, but there is a caveat - if someone inherits
> that eclass after other eclass phases, it might shadow those eclass
> phase functions. There was a Check Request for pkgcheck to catch such
> shadowed phase functions, but this still wasn't easy to implement, so we
> still miss it. So I'm afraid this eclass being inherited last, causing
> shadowed phases.

You are right, exported phase functions have their quirks in the 
multi-inherit case you describe.

But I am sorry, I am not sure how this is related to greadme exporting 
phase functions.

A dozen other eclasses also export phase functions. And this is, I 
presume, because even considering those quirks, exported phase functions 
are otherwise a sensible approach to reduce boilerplate.

In any case, if we want to decide that exporting phase functions is now 
an anti-pattern, then this would warrant a larger discussion and ideally 
some democratic vote.


>> Finally, unlike readme.gentoo-r1.elcass, this eclass does not need to
>> store the content of the readme in an environment variable. Not having
>> to store the content in an environment variable reduces the pollution of
>> the environment (sadly, this only refers to the process environment).
> 
> I'll be honest, I never felt this is really needed? From looking at the
> current -r1 eclass, you could define DOC_CONTENTS just before invoking
> readme.gentoo_create_doc, so you could for example modify as you want
> the message and use `local DOC_CONTENTS="..."`.

readme.gentoo-r1.eclass requires DOC_CONTENTS to be part of the 
package's environment to show it later in readme.gentoo_print_elog(), 
which is typically invoked in pkg_postinst(). If DOC_CONTENTS is local 
to readme.gentoo_create_doc(), then it wont be able in pkg_postinst() 
and can potentially not be obtained from the README.gentoo file because 
that file may be compressed.

For greadme.eclass, the file is no longer compressed, therefore 
greadme.eclass does not need to carry a variable in the package's 
environment.


> I'm not saying directly I'm against, don't understand me incorrectly.
> What I want to see a current way to use -r1 eclass, which is
> bad/ugly/boilerplate, and the new way with your eclass which is nicer. A
> nice thing I took from mgorny's eclass changes, is giving an example
> diff for a package, showing the new usage, diff, and "win points".


While for me personally, being able to use heredoc is a major advantage 
in the eclass' API design. This, and the reduced boilerplate due to 
exported phase function are the major improvements API wise.

So one could argue that, while there are improvements in the API, they 
are not that dramatic.

The actual big "win point" of greadme.eclass is that the content of the 
README.gentoo will be shown on new installations or *if the content 
changed*.

That means, if README.gentoo changes, probably due to new, likely 
important, information being added, readme.gentoo-r1 will not show it 
again to the user, while greadme.eclass will.

For cases where the content changes only due to unimportant changes, 
like maybe spelling fixes, greadme.eclass provides a manual override of 
its default behavior.

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