Hey, 88.0.4324.33.ebuild has multiple issues and is not up to today's ebuild standards. 1: Optional runtime-only dependencies are forbidden by QA policy. I suggest moving xrandr to 'optfeature'. https://projects.gentoo.org/qa/policy-guide/dependencies.html#pg0001 2: There's multiple external commands called without "|| die" added, like cd and popd. https://devmanual.gentoo.org/ebuild-writing/error-handling/index.html#the-die-function 3: In src_install() you should be able to replace 'chmod' with 'fperms'. https://devmanual.gentoo.org/function-reference/install-functions/ 4: There's also QA policy to always install small localization files, so you could clean the ebuild by getting rid of PLOCALES condition. https://projects.gentoo.org/qa/policy-guide/installed-files.html#pg0301 (5: S should be quoted, S="${WORKDIR}") Let me know if you need help!
vapier, we will give you a week to fix these issues, otherwise we will revert your commit. -- QA
(In reply to David Seifert from comment #1) your sad attempts at bullying are just that: sad. maybe your intimidation works on other people, but i'm not interested in your nonsense. stop wasting other developer's time when you have nothing useful to contribute.
(In reply to Joonas Niilola from comment #0) > 1: Optional runtime-only dependencies are forbidden by QA policy. I suggest > moving xrandr to 'optfeature'. > https://projects.gentoo.org/qa/policy-guide/dependencies.html#pg0001 i'll point out the irony in recommending, for QA, an eclass that itself is buggy and suffers from uncontrolled pathname expansion. > 2: There's multiple external commands called without "|| die" added, like cd > and popd. all the codepaths are correctly protected already as written. anything that fails will be caught. > 3: In src_install() you should be able to replace 'chmod' with 'fperms'. i don't see how when fperms doesn't support globs and requires paths as seen inside of $D. > 4: There's also QA policy to always install small localization files, so you > could clean the ebuild by getting rid of PLOCALES condition. > https://projects.gentoo.org/qa/policy-guide/installed-files.html#pg0301 is there a concerted effort to remove l10n usage ? still seems to be pretty prevalent in the tree. > (5: S should be quoted, S="${WORKDIR}") that is unnecessary in bash which safely handles expansions in variable assignments, and always has
(In reply to SpanKY from comment #3) > > 4: There's also QA policy to always install small localization files, so you > > could clean the ebuild by getting rid of PLOCALES condition. > > https://projects.gentoo.org/qa/policy-guide/installed-files.html#pg0301 > > is there a concerted effort to remove l10n usage ? still seems to be pretty > prevalent in the tree. That policy is aimed at small files, and arguably it is their combined size that counts. Not sure how large that is for this package, but looking at my /usr/share/locale I see that it has a total size of 200 MB with only 3 locales enabled. I wouldn't want that to increase by a factor of 10 or 20.
(In reply to Ulrich Müller from comment #4) this package has over 50 locales. each one costs ~20kb. so for the common user who only has one or two locales turned on, it's ~1.1mb unused overhead. while ~1mb in isolation isn't a huge deal, the multiplicative factor in the package & across the tree as you point out is not. so seems like l10n usage here (and in general) is correct. really the l10n eclass is serving the explicit expression that the implicit gettext LINGUAS accomplishes across all GNU gettext users (which is fairly common). so i don't see a problem with chrome-remote-desktop usage here. sounds like if QA wants to come up with a coherent policy around locale handling, they need to get it approved by the developer community, then start concerted efforts in the tree with tooling & deprecation messages & tracking bugs. blindly applying a policy inconsistently helps no one.
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=28af20091a8243d35db232d7d71c41563cba5ee1 commit 28af20091a8243d35db232d7d71c41563cba5ee1 Author: Mike Frysinger <vapier@gentoo.org> AuthorDate: 2021-01-11 11:36:36 +0000 Commit: Mike Frysinger <vapier@gentoo.org> CommitDate: 2021-01-11 12:08:22 +0000 net-misc/chrome-remote-desktop: switch xrandr to optfeature logging Bug: https://bugs.gentoo.org/764626 Signed-off-by: Mike Frysinger <vapier@gentoo.org> .../chrome-remote-desktop-88.0.4324.33.ebuild | 7 ++++--- net-misc/chrome-remote-desktop/metadata.xml | 3 --- 2 files changed, 4 insertions(+), 6 deletions(-)
(In reply to SpanKY from comment #3) > i'll point out the irony in recommending, for QA, an eclass that itself is > buggy and suffers from uncontrolled pathname expansion. > The link provided also lists other possible ways to inform about runtime-only dependencies. Wasn't aware of issues with the eclass, but now the eclass can be fixed! > > i don't see how when fperms doesn't support globs and requires paths as seen > inside of $D. > I'm pretty sure "fperms a+rx /opt/google/${PN}/*" works... Although it was more of an F-Y-I since at least the chmod is properly handled. > > is there a concerted effort to remove l10n usage ? still seems to be pretty > prevalent in the tree. > There's around 100 ebuilds that use l10n.eclass anymore, and the number is reducing by their version bumps. So there's no concentrated hunt against it, but slowly it's being faded away. And as you said, the difference here for installing selected locales vs. all locales is 1 MB, I personally didn't think it's worth it. > > (5: S should be quoted, S="${WORKDIR}") > > that is unnecessary in bash which safely handles expansions in variable > assignments, and always has I see, thanks!
(In reply to Joonas Niilola from comment #7) > (In reply to SpanKY from comment #3) > > i don't see how when fperms doesn't support globs and requires paths as seen > > inside of $D. > > I'm pretty sure "fperms a+rx /opt/google/${PN}/*" works that is incorrect. that is expanding globs against / when they must be expanded against the $ED. which would break the ebuild when the package isn't installed, or incomplete expansion when the set of installed paths. > > is there a concerted effort to remove l10n usage ? still seems to be pretty > > prevalent in the tree. > > There's around 100 ebuilds that use l10n.eclass anymore, and the number is > reducing by their version bumps. So there's no concentrated hunt against it, > but slowly it's being faded away. And as you said, the difference here for > installing selected locales vs. all locales is 1 MB, I personally didn't > think it's worth it. it isn't just l10n usage, it's also gettext/po generation. when you multiply these out, it is not just 1 MB. see Ulrich's examples.
(In reply to Joonas Niilola from comment #7) > There's around 100 ebuilds that use l10n.eclass anymore, and the number is > reducing by their version bumps. Is that so? This is wrong then and not covered by any QA policy.
