[gentoo-dev] [PATCH] vdr-plugin-2.eclass: make qa warning conditional

[gentoo-dev] [PATCH] vdr-plugin-2.eclass: make qa warning conditional

[Martin Dummer]

Since Agostino's tinderbox tests now report qa warning, the group
vdr@gentoo.org has 101 open bugs assigned, most of them caused by qa
warnings from vdr-plugin-2.eclass.

Many vdr plugins need small adjustments because API or makefile changes
in upstream media-video/vdr which can be easily fixed with small changes.

These warnings are only useful for the vdr plugin maintainers, so I
propose they should (only) be reported as QA-warnings when the global
variable
     VDR_MAINTAINER_MODE="1"
is set in make.conf

This patch is also put to github in
https://github.com/gentoo/gentoo/pull/36504

The PR is lacking many many "Closes: ...." tags, which I will fill in soon.

Any comments?

-- Martin

########################################################################

commit 00a0a3237729ec4886e07d9869fdfd6e0cd5f985
Author: Martin Dummer <martin.dummer@gmx.net>
Date:   Wed May 1 14:49:37 2024 +0200

     vdr-plugin-2.eclass: make qa warning conditional

     many vdr plugins need small adjustments because API or makefile changes
     in upstream media-video/vdr which can be easily fixed with small
changes
     These fixes should (only) be reported as QA-warnings when the
global variable
     VDR_MAINTAINER_MODE="1"
     is set in make.conf

     Signed-off-by: Martin Dummer <martin.dummer@gmx.net>

diff --git a/eclass/vdr-plugin-2.eclass b/eclass/vdr-plugin-2.eclass
index 8f56511032c8..bc9a88db59a5 100644
--- a/eclass/vdr-plugin-2.eclass
+++ b/eclass/vdr-plugin-2.eclass
@@ -160,7 +160,6 @@ vdr_create_header_checksum_file() {
  # Plugins failed on compile with wrong path of libsi includes,
  # this can be fixed by 'function + space separated list of files'
  fix_vdr_libsi_include() {
-    eqawarn "QA Notice: Fixing include of libsi-headers"
      local f
      for f; do
          sed -i "${f}" \
@@ -244,7 +243,7 @@ vdr_gettext_missing() {
      # plugins without converting to gettext

      local GETTEXT_MISSING=$( grep xgettext Makefile )
-    if [[ -z ${GETTEXT_MISSING} ]]; then
+    if [[ -z ${GETTEXT_MISSING} && -n ${VDR_MAINTAINER_MODE} ]]; then
          eqawarn "QA Notice: Plugin isn't converted to gettext handling!"
      fi
  }
@@ -306,11 +305,15 @@ vdr_i18n() {
      if [[ -n ${I18N_OBJECT} ]]; then

          if [[ "${KEEP_I18NOBJECT:-no}" = "yes" ]]; then
-            eqawarn "QA Notice: Forced to keep i18n.o"
+            if [[ -n ${VDR_MAINTAINER_MODE} ]]; then
+                eqawarn "QA Notice: Forced to keep i18n.o"
+            fi
          else
              sed -i "s:i18n.o::g" Makefile \
                  || die "sed failed to remove i18n from Makefile"
-            eqawarn "QA Notice: OBJECT i18n.o found, removed per sed"
+            if [[ -n ${VDR_MAINTAINER_MODE} ]]; then
+                eqawarn "QA Notice: OBJECT i18n.o found, removed per sed"
+            fi
          fi
      fi

@@ -318,7 +321,9 @@ vdr_i18n() {
      if [[ -n ${I18N_STRING} ]]; then
          sed -i
"s:^extern[[:space:]]*const[[:space:]]*tI18nPhrase://static const
tI18nPhrase:" i18n.h \
              || die "sed failed to replace tI18nPhrase"
-        eqawarn "QA Notice: obsolete tI18nPhrase found, disabled per
sed, please recheck"
+        if [[ -n ${VDR_MAINTAINER_MODE} ]]; then
+            eqawarn "QA Notice: obsolete tI18nPhrase found, disabled
per sed, please recheck"
+        fi
      fi
  }

@@ -337,7 +342,9 @@ vdr_remove_i18n_include() {
          || die "sed failed to remove i18n_include"
      done

-    eqawarn "QA Notice: removed i18n.h include in ${@}"
+    if [[ -n ${VDR_MAINTAINER_MODE} ]]; then
+        eqawarn "QA Notice: removed i18n.h include in ${@}"
+    fi
  }

  # @FUNCTION: vdr-plugin-2_print_enable_command
@@ -568,7 +575,9 @@ vdr-plugin-2_src_install() {
          DESTDIR="${D%/}" \
          || die "emake install (makefile target) failed"
      else
-        eqawarn "QA Notice: Plugin use still the old Makefile handling"
+        if [[ -n ${VDR_MAINTAINER_MODE} ]]; then
+            eqawarn "QA Notice: Plugin use still the old Makefile handling"
+        fi
          insinto "${VDR_PLUGIN_DIR}"
          doins libvdr-*.so.*
      fi

Re: [gentoo-dev] [PATCH] vdr-plugin-2.eclass: make qa warning conditional

[Eli Schwartz]

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

On 5/1/24 10:10 AM, Martin Dummer wrote:
> Since Agostino's tinderbox tests now report qa warning, the group
> vdr@gentoo.org has 101 open bugs assigned, most of them caused by qa
> warnings from vdr-plugin-2.eclass.
> 
> Many vdr plugins need small adjustments because API or makefile changes
> in upstream media-video/vdr which can be easily fixed with small changes.
> 
> These warnings are only useful for the vdr plugin maintainers, so I
> propose they should (only) be reported as QA-warnings when the global
> variable
>     VDR_MAINTAINER_MODE="1"
> is set in make.conf
> 
> This patch is also put to github in
> https://github.com/gentoo/gentoo/pull/36504
> 
> The PR is lacking many many "Closes: ...." tags, which I will fill in soon.
> 
> Any comments?


What does "only useful for the vdr plugin maintainers" mean? Why can't
anyone fix them?

There are lots of QA warnings that a package can generate, and lots of
them are "only" relevant to someone editing the upstream source code.
Why should vdr plugins be special?

From a quick glance at the warning messages, my inexpert feeling is that
two of them are a bit "wishy-washy" and could be classified as "a
warning to be picky and do best practices":

- gettext handling
- old Makefile handling

The others seem like worrisome issues that should very much be reported
in tinderboxes and get fixed.

Automatically sed'ing out source code, especially for the one that says
"please recheck", very much looks like the purpose of the qa warning is
that the functionality isn't trusted to be correct, is offered on a
best-effort basis, and needs to be manually reviewed and marked as okay
(by applying as a real patch) in order to squelch the warnings.

In other words, there are "QA issues" and "QA nitpicks".


-- 
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] vdr-plugin-2.eclass: make qa warning conditional

[Sam James]

Eli Schwartz <eschwartz93@gmail.com> writes:

> On 5/1/24 10:10 AM, Martin Dummer wrote:
>> Since Agostino's tinderbox tests now report qa warning, the group
>> vdr@gentoo.org has 101 open bugs assigned, most of them caused by qa
>> warnings from vdr-plugin-2.eclass.
>> 
>> Many vdr plugins need small adjustments because API or makefile changes
>> in upstream media-video/vdr which can be easily fixed with small changes.
>> 
>> These warnings are only useful for the vdr plugin maintainers, so I
>> propose they should (only) be reported as QA-warnings when the global
>> variable
>>     VDR_MAINTAINER_MODE="1"
>> is set in make.conf
>> 
>> This patch is also put to github in
>> https://github.com/gentoo/gentoo/pull/36504
>> 
>> The PR is lacking many many "Closes: ...." tags, which I will fill in soon.
>> 
>> Any comments?
>
>
> What does "only useful for the vdr plugin maintainers" mean? Why can't
> anyone fix them?
>
> There are lots of QA warnings that a package can generate, and lots of
> them are "only" relevant to someone editing the upstream source code.
> Why should vdr plugins be special?
>
> From a quick glance at the warning messages, my inexpert feeling is that
> two of them are a bit "wishy-washy" and could be classified as "a
> warning to be picky and do best practices":
>
> - gettext handling
> - old Makefile handling
>
> The others seem like worrisome issues that should very much be reported
> in tinderboxes and get fixed.

What we really need is:
a) https://bugs.gentoo.org/162450 to avoid scaring users;
b) possibly some level of QA notice to distinguish between "check this
out" (think e.g. qa-vdb LHS where it _might_ be unused, but not
necessarily), and "this is definitely wrong"

I am convinced we need a), I am not-at-all convinced we need b) - at
least not in terms of whether bugs are reported.

>
> Automatically sed'ing out source code, especially for the one that says
> "please recheck", very much looks like the purpose of the qa warning is
> that the functionality isn't trusted to be correct, is offered on a
> best-effort basis, and needs to be manually reviewed and marked as okay
> (by applying as a real patch) in order to squelch the warnings.
>
> In other words, there are "QA issues" and "QA nitpicks".

Re: [gentoo-dev] [PATCH] vdr-plugin-2.eclass: make qa warning conditional

[Martin Dummer]

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


Am 03.05.24 um 06:39 schrieb Sam James:
>
> What we really need is:
> a)https://bugs.gentoo.org/162450  to avoid scaring users;
> b) possibly some level of QA notice to distinguish between "check this
> out" (think e.g. qa-vdb LHS where it _might_ be unused, but not
> necessarily), and "this is definitely wrong"
>
> I am convinced we need a), I am not-at-all convinced we need b) - at
> least not in terms of whether bugs are reported.
>
AFAIS https://bugs.gentoo.org/162450 is not implemented.

Maybe we can agree that the qa-warnings in vdr-eclass make more sense if
i change them to "eawarn" or "einfo"?

In my opinion, most plugins in the vdr context will practically not
develop any further anyway. It is more important to keep the current
status of vdr-software in the ecosystem up to date as well as possible.

So I need a practical useful approach instead of a fundamental
discussion please.

[-- Attachment #2: Type: text/html, Size: 1945 bytes --]

Re: [gentoo-dev] [PATCH] vdr-plugin-2.eclass: make qa warning conditional

[Sam James]

Martin Dummer <martin.dummer@gmx.net> writes:

> Am 03.05.24 um 06:39 schrieb Sam James:
>
>  
> What we really need is:
> a) https://bugs.gentoo.org/162450 to avoid scaring users;
> b) possibly some level of QA notice to distinguish between "check this
> out" (think e.g. qa-vdb LHS where it _might_ be unused, but not
> necessarily), and "this is definitely wrong"
>
> I am convinced we need a), I am not-at-all convinced we need b) - at
> least not in terms of whether bugs are reported.
>
> AFAIS https://bugs.gentoo.org/162450 is not implemented.

Right, that's why I didn't say "we can just use".

>
> Maybe we can agree that the qa-warnings in vdr-eclass make more sense if i change them to "eawarn" or "einfo"?
>

Sure, make them eqawarn.

> In my opinion, most plugins in the vdr context will practically not develop any further anyway. It is more important to
> keep the current status of vdr-software in the ecosystem up to date as well as possible.
>
> So I need a practical useful approach instead of a fundamental discussion please.

My point is that the QA warnings should exist, and you can worry about
making them "developer-only" in future. Right now, they seem useful, and
the things they flag need to be addressed.

Re: [gentoo-dev] [PATCH] vdr-plugin-2.eclass: make qa warning conditional

[Martin Dummer]

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


Am 09.05.24 um 14:13 schrieb Sam James:
> Martin Dummer<martin.dummer@gmx.net>  writes:
>
>> Maybe we can agree that the qa-warnings in vdr-eclass make more sense if i change them to "eawarn" or "einfo"?
>>
> Sure, make them eqawarn.

Hmm - current state of vdr-plugin-2.eclass is: there are many "eqawarn"
in there.



>> In my opinion, most plugins in the vdr context will practically not develop any further anyway. It is more important to
>> keep the current status of vdr-software in the ecosystem up to date as well as possible.
>>
>> So I need a practical useful approach instead of a fundamental discussion please.
> My point is that the QA warnings should exist, and you can worry about
> making them "developer-only" in future. Right now, they seem useful, and
> the things they flag need to be addressed.
>
Basically yes, but here (vdr-plugins) the qawarn are used more in a way
"to remind the plugin developers to adapt their sources to newer vdr
build environment" or "the way i18n implemented has changed"

The eclass fixes these problems with standardized quirks, the "equawarn"
messages show when these quirks are applied.

IMHO its not necessary to tell that to any user, only for interested
packagers when they are bored and look out for some extra work. That's
why I made the warnings conditional, printed out when the variable
"VDR_MAINTAINTER_MODE" is set to a not-empty value.

Finally, I am interested in an opinion whether this is acceptable or
not, otherwise I tend to remove the warnings at all.

[-- Attachment #2: Type: text/html, Size: 2577 bytes --]

Re: [gentoo-dev] [PATCH] vdr-plugin-2.eclass: make qa warning conditional

[Sam James]

Martin Dummer <martin.dummer@gmx.net> writes:

> Am 09.05.24 um 14:13 schrieb Sam James:
>
>  Martin Dummer <martin.dummer@gmx.net> writes:
>
>  
> Maybe we can agree that the qa-warnings in vdr-eclass make more sense if i change them to "eawarn" or "einfo"?
>
>
> Sure, make them eqawarn.
>
> Hmm - current state of vdr-plugin-2.eclass is: there are many "eqawarn" in there.
>
>  
>  In my opinion, most plugins in the vdr context will practically not develop any further anyway. It is more
>  important to
> keep the current status of vdr-software in the ecosystem up to date as well as possible.
>
> So I need a practical useful approach instead of a fundamental discussion please.
>
>
> My point is that the QA warnings should exist, and you can worry about
> making them "developer-only" in future. Right now, they seem useful, and
> the things they flag need to be addressed.
>
> Basically yes, but here (vdr-plugins) the qawarn are used more in a way "to remind the plugin developers to adapt their
> sources to newer vdr build environment" or "the way i18n implemented has changed"
>
> The eclass fixes these problems with standardized quirks, the "equawarn" messages show when these quirks are applied.
>
> IMHO its not necessary to tell that to any user, only for interested packagers when they are bored and look out for some
> extra work. That's why I made the warnings conditional, printed out when the variable "VDR_MAINTAINTER_MODE" is set to a
> not-empty value.
>
> Finally, I am interested in an opinion whether this is acceptable or not, otherwise I tend to remove the warnings at all.

It sounds like maybe you want to turn these into log messages (einfo -
https://devmanual.gentoo.org/ebuild-writing/messages/index.html), and
drop the "QA notice" prefix.

Re: [gentoo-dev] [PATCH] vdr-plugin-2.eclass: make qa warning conditional

[Martin Dummer]

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


Am 09.05.24 um 15:08 schrieb Sam James:
> It sounds like maybe you want to turn these into log messages (einfo -
> https://devmanual.gentoo.org/ebuild-writing/messages/index.html), and
> drop the "QA notice" prefix.
>
I changed https://github.com/gentoo/gentoo/pull/36504 accordingly and
added the "Closes:" tags. Can you please review?

[-- Attachment #2: Type: text/html, Size: 944 bytes --]