Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass
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 OpenPGP_0x8CAC2A9678548E35.asc Description: OpenPGP public key OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass
> 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 signature.asc Description: PGP signature
Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass
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 OpenPGP_0x8CAC2A9678548E35.asc Description: OpenPGP public key OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass
> 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 signature.asc Description: PGP signature
Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass
On 10/01/2024 12.04, Sam James wrote: Florian Schmaus 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. OpenPGP_0x8CAC2A9678548E35.asc Description: OpenPGP public key OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass
Florian Schmaus 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
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 OpenPGP_0x8CAC2A9678548E35.asc Description: OpenPGP public key OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass
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 signature.asc Description: This is a digitally signed message part
Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass
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 OpenPGP_0x8CAC2A9678548E35.asc Description: OpenPGP public key OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass
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 signature.asc Description: This is a digitally signed message part
Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass
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 OpenPGP_0x8CAC2A9678548E35.asc Description: OpenPGP public key OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [gentoo-dev] [PATCH 0/1] [RFC] greadme.eclass
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 signature.asc Description: This is a digitally signed message part
