[bug#55541] [PATCH] gnu: Add azpainter.

[bug#55541] [PATCH] gnu: Add azpainter.

[Tobias Kortkamp]

* gnu/packages/graphics.scm (azpainter): New variable.
---
 gnu/packages/graphics.scm | 66 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/gnu/packages/graphics.scm b/gnu/packages/graphics.scm
index e966a82dbb..1457cf83fb 100644
--- a/gnu/packages/graphics.scm
+++ b/gnu/packages/graphics.scm
@@ -30,6 +30,7 @@
 ;;; Copyright © 2022 Michael Rohleder <mike@rohleder.de>
 ;;; Copyright © 2022 John Kehayias <john.kehayias@protonmail.com>
 ;;; Copyright © 2022 Zheng Junjie <873216071@qq.com>
+;;; Copyright © 2022 Tobias Kortkamp <tobias.kortkamp@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -2124,3 +2125,68 @@ (define-public monado
 such as VR and AR on mobile, PC/desktop, and any other device.  Monado aims to be
 a complete and conforming implementation of the OpenXR API made by Khronos.")
     (license license:boost1.0)))
+
+(define-public azpainter
+  (package
+    (name "azpainter")
+    (version "3.0.5")
+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://gitlab.com/azelpg/azpainter")
+                    (commit (string-append "v" version))))
+              (file-name (git-file-name name version))
+              (sha256
+               (base32
+                "1iplp3p8pw9q44kb43hrk89sv2aff6bdy9fk58j2v6k5lqbk6kvf"))))
+    (build-system gnu-build-system) ;actually a home grown build system
+    (arguments
+     (list #:tests? #f
+           #:phases
+           #~(modify-phases %standard-phases
+               (replace 'configure
+                 (lambda _
+                   (invoke "./configure"
+                           (string-append "--prefix="
+                                          #$output))))
+               (replace 'build
+                 (lambda* (#:key parallel-build? #:allow-other-keys)
+                   (let ((job-count (if parallel-build?
+                                        (number->string (parallel-job-count))
+                                        "1")))
+                     (invoke "ninja" "-j" job-count "-C" "build"))))
+               (add-before 'install 'disable-cache-generation
+                 (lambda _
+                   (setenv "DESTDIR" "/") #t))
+               (replace 'install
+                 (lambda _
+                   (invoke "ninja" "-C" "build" "install"))))))
+    (inputs (list fontconfig
+                  freetype
+                  libjpeg-turbo
+                  libpng
+                  libtiff
+                  libwebp
+                  libx11
+                  libxcursor
+                  libxext
+                  libxi
+                  zlib))
+    (native-inputs (list ninja pkg-config))
+    (home-page "http://azsky2.html.xdomain.jp/soft/azpainter.html")
+    (synopsis "Paint software for editing illustrations and images")
+    (description
+     "AzPainter is a lightweight full color painting application for editing
+illustrations and images.
+
+Features include:
+@itemize
+@item Layers
+@item Many artistic filters
+@item Good range of selection tools
+@item Pen pressure support with automatic brush size adjustment
+@item Support for 16-bit color images with transparency (RGBA)
+@item Support for image formats like PSD, PNG, JPEG, TIFF, WebP
+@end itemize
+")
+    (license license:gpl3+)))

base-commit: 2f170893719e6e9fc8e19cc5f0568e20a95d92b4
-- 
2.36.0

[bug#55541] [PATCH] gnu: Add azpainter.

[Maxime Devos]

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

Tobias Kortkamp schreef op vr 20-05-2022 om 13:18 [+0200]:
> +    (build-system gnu-build-system) ;actually a home grown build system
> +    (arguments
> +     (list #:tests? #f
> +           #:phases
> +           #~(modify-phases %standard-phases
> +               (replace 'configure
> +                 (lambda _
> +                   (invoke "./configure"
> +                           (string-append "--prefix="
> +                                          #$output))))

As-is, this home-grown build system is broken when cross-compiling:

  * When cross-compiling, TARGET-gcc needs to be used instead of gcc.
    Maybe do (setenv "CC" #$(cc-for-target)) first?

  * Likewise, TARGET-pkg-config instead of pkg-config (not 100% sure)

  * It tries to run binaries during ./configure.  When cross-compiling,
    ./conftest will always fail (unless using emulation) and hence
    always detect ‘little endian’ but this is incorrect when
    cross-compiling for big-endian architectures.

(Needs some fixes or work-arounds.)  You can test with "guix build
azpainter --target=aarch64-linux-gnu" or such.

Also, some other problems.  From mlk_studio.c

int mFILEreadBE32(FILE *fp,void *buf)
{
	uint8_t v[4];

	if(fread(v, 1, 4, fp) < 4)
		return 1;
	else
	{
		*((uint32_t *)buf) = ((uint32_t)v[0] << 24) | (v[1] <<
16) | (v[2] << 8) | v[3];
		return 0;
	}
}

looks like a potential strict-aliasing violation to me, resulting in
undefined behaviour -- what if buf is a pointer to an array of, say,
doubles?  Also a potential alignment problem, though maybe it's only
called for sufficiently aligned 'buf'.  The strict-aliasing problem
can be worked around with -fno-strict-aliasing or maybe just -fno-ipa-
strict-aliasing , though I don't know if that's sufficient.

Greetings,
Maxime.

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

bug#55541: [PATCH] gnu: Add azpainter.

[Ludovic Courtès]

Hi Tobias,

Tobias Kortkamp <tobias.kortkamp@gmail.com> skribis:

> * gnu/packages/graphics.scm (azpainter): New variable.

Applied, thanks!

Maxime Devos <maximedevos@telenet.be> skribis:

> As-is, this home-grown build system is broken when cross-compiling:
>
>   * When cross-compiling, TARGET-gcc needs to be used instead of gcc.
>     Maybe do (setenv "CC" #$(cc-for-target)) first?
>
>   * Likewise, TARGET-pkg-config instead of pkg-config (not 100% sure)
>
>   * It tries to run binaries during ./configure.  When cross-compiling,
>     ./conftest will always fail (unless using emulation) and hence
>     always detect ‘little endian’ but this is incorrect when
>     cross-compiling for big-endian architectures.
>
> (Needs some fixes or work-arounds.)  You can test with "guix build
> azpainter --target=aarch64-linux-gnu" or such.
>
> Also, some other problems.  From mlk_studio.c
>
> int mFILEreadBE32(FILE *fp,void *buf)
> {
> 	uint8_t v[4];
>
> 	if(fread(v, 1, 4, fp) < 4)
> 		return 1;
> 	else
> 	{
> 		*((uint32_t *)buf) = ((uint32_t)v[0] << 24) | (v[1] <<
> 16) | (v[2] << 8) | v[3];
> 		return 0;
> 	}
> }
>
> looks like a potential strict-aliasing violation to me, resulting in
> undefined behaviour -- what if buf is a pointer to an array of, say,
> doubles?  Also a potential alignment problem, though maybe it's only
> called for sufficiently aligned 'buf'.  The strict-aliasing problem
> can be worked around with -fno-strict-aliasing or maybe just -fno-ipa-
> strict-aliasing , though I don't know if that's sufficient.

These are all good points and I appreciate that you did such a thorough
review (audit?) of the package!

That said, I think it’s a bit too much to ask of a downstream packager
or user to address these issues.  As I see it, these issues should be
reported upstream and addressed upstream.

I hope that makes sense!

Thanks,
Ludo’.

[bug#55541] [PATCH] gnu: Add azpainter.

[Maxime Devos]

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

reopen 55541

Ludovic Courtès schreef op vr 24-06-2022 om 22:56 [+0200]:
> These are all good points and I appreciate that you did such a thorough
> review (audit?) of the package!

I looked through the code a bit, didn't check every file, so I wouldn't
call it an audit.

> 
> That said, I think it’s a bit too much to ask of a downstream packager
> or user to address these issues.  As I see it, these issues should be
> reported upstream and addressed upstream.
> 
> I hope that makes sense!

AFAICT the issues have not been reported upstream yet, so I don't think
we can close this entry on debbugs yet.  While I'd like for downstream
packaging to be trivial, the sad reality is that sometimes is not the
case, the issues are still there and need to be resolved somehow (fixed
downstream or upstream, or reported upstream).

If not by the new downstream packager that submitted the patch, then by
the the one committing the patch, or by a reviewer, or by some more
neboluous role of a random Guix contributor, or in some exceptional
cases the issue could be considered ‘too difficult and not too bad’
with some corresponding reasoning.  (It's most efficient if the
reporting or fixing is done directly by the submitter, but if the
submitter can't do it for whatever reason, then surely something can
eventually be worked out by other people, albeit more slowly.)

However, AFAICT, none of that has happened yet.

More generally, I don't think we should have an ‘packages included in
Guix should be good, unless submitted by a newbie’ exception.  Also,
potentially the new submitter would _like_ to learn more about Guix
(and have time for it, etc.) and learn how to improve things?

In the future, if someone submits a patch and I notice it has some
complicated problems, should I just ignore the complicated problems and
just LGTM?  This seems contrary to the concept of reviewing to me. 
(This is probably not what you meant, but to me, this is implied by
your response.)

Greetings,
Maxime.

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

Dealing with upstream issues

[Ludovic Courtès]

Hi Maxime,

(Moving to guix-devel; starting point at
<https://issues.guix.gnu.org/55541#3>.)

Maxime Devos <maximedevos@telenet.be> skribis:

>> That said, I think it’s a bit too much to ask of a downstream packager
>> or user to address these issues.  As I see it, these issues should be
>> reported upstream and addressed upstream.
>> 
>> I hope that makes sense!
>
> AFAICT the issues have not been reported upstream yet, so I don't think
> we can close this entry on debbugs yet.  While I'd like for downstream
> packaging to be trivial, the sad reality is that sometimes is not the
> case, the issues are still there and need to be resolved somehow (fixed
> downstream or upstream, or reported upstream).
>
> If not by the new downstream packager that submitted the patch, then by
> the the one committing the patch, or by a reviewer, or by some more
> neboluous role of a random Guix contributor, or in some exceptional
> cases the issue could be considered ‘too difficult and not too bad’
> with some corresponding reasoning.  (It's most efficient if the
> reporting or fixing is done directly by the submitter, but if the
> submitter can't do it for whatever reason, then surely something can
> eventually be worked out by other people, albeit more slowly.)
>
> However, AFAICT, none of that has happened yet.
>
> More generally, I don't think we should have an ‘packages included in
> Guix should be good, unless submitted by a newbie’ exception.  Also,
> potentially the new submitter would _like_ to learn more about Guix
> (and have time for it, etc.) and learn how to improve things?
>
> In the future, if someone submits a patch and I notice it has some
> complicated problems, should I just ignore the complicated problems and
> just LGTM?  This seems contrary to the concept of reviewing to me. 
> (This is probably not what you meant, but to me, this is implied by
> your response.)

You did thorough review work and pointed at valid issues, thanks for
doing that.

Those issues are upstream issues, in that they’re not Guix-specific.
For instance, that ./configure runs binaries effectively prevents
cross-compilation, whether in Guix or not; that code potentially
violates C99 strict-aliasing rules is also not Guix-specific.

My view is that such issues should be reported upstream but cannot alone
block package adoption in Guix.

Regarding the review process, I think we should strive for a predictable
process—not requesting work from the submitter beyond what they can
expect.  Submitters can be expected to follow the written rules¹ and
perhaps a few more rules (for example, I don’t think we’ve documented
the fact that #:tests? #f is a last resort and should come with a
comment).  However, following that principle, we reviewers cannot
reasonably ask for work beyond that.  Upholding this principle makes
sure submitters can have a good picture of what it will take for their
work to be included.

Related to that, I think it’s important to have a consistent review
process.  In other words, we should be equally demanding for all patches
to avoid bad surprises or a feeling of double standard.

I hope this clarifies my view!

Thanks,
Ludo’.

¹ https://guix.gnu.org/manual/devel/en/html_node/Submitting-Patches.html

Re: Dealing with upstream issues

[Maxime Devos]

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

Ludovic Courtès schreef op ma 27-06-2022 om 12:10 [+0200]:
> Regarding the review process, I think we should strive for a predictable
> process—not requesting work from the submitter beyond what they can
> expect.  Submitters can be expected to follow the written rules¹ and
> perhaps a few more rules (for example, I don’t think we’ve documented
> the fact that #:tests? #f is a last resort and should come with a
> comment). 
> 
> However, following that principle, we reviewers cannot
> reasonably ask for work beyond that. [...]

We can ask to do send the notice upstream, if it's in the rules (I
consider this to be part of the unwritten rules).  And I don't often
have time for addressing the noticed issues and I have other things to
do as well -- I usually limit myself to just reviewing.  I do not
intend to take up work beyond that.

I also consider them to not be rules as in ‘if they are followed, it
WILL be accepted’ but more guidelines like ‘these things are important,
they usually need to be followed, but it's not exhaustive, at times it
might be discovered the list is incomplete’.

I don't think that patch submitters can reasonably expect reviewers to
do all the work.

Also, previously in discussions about the review process, weren't there
points about a reviewer not having to do everything all at once, they
could choose to review parts they know how to review and have time for
and leave the rest for others?

> Related to that, I think it’s important to have a consistent review
> process.  In other words, we should be equally demanding for all
> patches to avoid bad surprises or a feeling of double standard.

I think I've been consistent in asking to inform upstream of the issues
(*), with no distinction of whether it's a new submitter or an
established one or whatever.

(*) Except for when it's really basic downstream changes, like #:make-
flags #~(list ... #$(cc-for-target)) or a substitute* cc -> TARGET-gcc.

Greetings,
Maxime.

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

Re: Dealing with upstream issues

[Maxime Devos]

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

Maxime Devos schreef op ma 27-06-2022 om 12:30 [+0200]:
> We can ask to do send the notice upstream, if it's in the rules (I
> consider this to be part of the unwritten rules). [...]
> I also consider them to not be rules as in ‘if they are followed, it
> WILL be accepted’ but more guidelines like ‘these things are
> important, they usually need to be followed, but it's not exhaustive,
> at times it might be discovered the list is incomplete’.

Also, some of those rules are incorrect -- "guix style" occasionally
makes things wrong and patch submitters had to be told to not follow
it.

Greetings,
Maxime.


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

Re: Dealing with upstream issues

[zimoun]

Hi Maxime,

On Mon, 27 Jun 2022 at 12:37, Maxime Devos <maximedevos@telenet.be> wrote:

> Also, some of those rules are incorrect -- "guix style" occasionally
> makes things wrong and patch submitters had to be told to not follow
> it.

Do you have concrete examples in mind?


Cheers,
simon

Re: Dealing with upstream issues

[zimoun]

Hi,

Well, from my understanding, there is mismatches between “review
process”, “adoption in Guix” and “fix upstream”.


On Mon, 27 Jun 2022 at 12:10, Ludovic Courtès <ludo@gnu.org> wrote:

>> AFAICT the issues have not been reported upstream yet, so I don't think
>> we can close this entry on debbugs yet.  While I'd like for downstream
>> packaging to be trivial, the sad reality is that sometimes is not the
>> case, the issues are still there and need to be resolved somehow (fixed
>> downstream or upstream, or reported upstream).

Maybe I misunderstand the point.  To me, the aim of the package
submission is the inclusion in Guix.  AFAIK, the Guix project is not
applying any standard audit on the upstream code before inclusion.

Therefore, if the upstream code is poor in some areas, then it is not
blocking for the package adoption in Guix…

>> If not by the new downstream packager that submitted the patch, then by
>> the the one committing the patch, or by a reviewer, or by some more
>> neboluous role of a random Guix contributor, or in some exceptional
>> cases the issue could be considered ‘too difficult and not too bad’
>> with some corresponding reasoning.  (It's most efficient if the
>> reporting or fixing is done directly by the submitter, but if the
>> submitter can't do it for whatever reason, then surely something can
>> eventually be worked out by other people, albeit more slowly.)
>>
>> However, AFAICT, none of that has happened yet.

…and such adoption does not mean the upstream quality cannot be
improved, by reporting or add a patch.


> My view is that such issues should be reported upstream but cannot alone
> block package adoption in Guix.

I agree; we cannot fix the world. ;-) In the case of patch#55541, the
issues of cross-compilation can be reported directly to upstream and
another Debbugs number could be open.


Cheers,
simon

Re: Dealing with upstream issues

[Maxime Devos]

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

zimoun schreef op ma 27-06-2022 om 14:32 [+0200]:
> Hi Maxime,
> 
> On Mon, 27 Jun 2022 at 12:37, Maxime Devos <maximedevos@telenet.be> wrote:
> 
> > Also, some of those rules are incorrect -- "guix style" occasionally
> > makes things wrong and patch submitters had to be told to not follow
> > it.
> 
> Do you have concrete examples in mind?

E.g.: https://issues.guix.gnu.org/55606#16

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

Re: Dealing with upstream issues

[Maxime Devos]

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

zimoun schreef op ma 27-06-2022 om 14:53 [+0200]:
> Maybe I misunderstand the point.  To me, the aim of the package
> submission is the inclusion in Guix.  AFAIK, the Guix project is not
> applying any standard audit on the upstream code before inclusion.
> 
> Therefore, if the upstream code is poor in some areas, then it is not
> blocking for the package adoption in Guix…

I think there should be some degree of standards (where I mean
standards in the non-shodyness sense of the word, not in the sense of
specs, though specs would be nice too) and of audit (bundling, malware,
non-free, bugs) -- some of which are blocking (bundling, malware, non-
free, some bugs), some of which aren't (some other bugs).

E.g., see removal of unmaintained Python, of some old SSL libraries

>>> However, AFAICT, none of that has happened yet.
> …and such adoption does not mean the upstream quality cannot be
> improved, by reporting or add a patch.

The problem is that this sometimes seems to be neglected unless people
are practically forced to make the issue be reported upstream.

> > My view is that such issues should be reported upstream but cannot
> alone
> > block package adoption in Guix.
> 
> I agree; we cannot fix the world. ;-) In the case of patch#55541, the
> issues of cross-compilation can be reported directly to upstream

Agreed -- I did not ask that explicitely in #55541, but the implied
question was to report it upstream (or fix local, that could be done
too).  But my point is that this should have been done _before_ merging
the patch.

> and another Debbugs number could be open.

Would be pointless.  Standard policy seems to be to leave the debbugs
issue unresolved forever, then someone does some cleanup in debbugs to
close the debbugs number due to lack of activity or such.  Things would
be delayed forever.

Greetings,
Maxime.

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

Re: Dealing with upstream issues

[zimoun]

On Mon, 27 Jun 2022 at 16:20, Maxime Devos <maximedevos@telenet.be> wrote:

> E.g.: https://issues.guix.gnu.org/55606#16

Do you mean

--8<---------------cut here---------------start------------->8---
> +(define-public harec
> +  (let ((commit "bbabe09bddf74bd699f8ad2224fdd6e2eefbd35e")
> (revision "0"))
Despite what (guix style) may tell you, revision goes to an extra line.
--8<---------------cut here---------------end--------------->8---

? <https://issues.guix.gnu.org/55606#16-lineno56>


Cheers,
simon

Re: Dealing with upstream issues

[zimoun]

Hi,

On Mon, 27 Jun 2022 at 16:32, Maxime Devos <maximedevos@telenet.be> wrote:

>> Maybe I misunderstand the point.  To me, the aim of the package
>> submission is the inclusion in Guix.  AFAIK, the Guix project is not
>> applying any standard audit on the upstream code before inclusion.
>> 
>> Therefore, if the upstream code is poor in some areas, then it is not
>> blocking for the package adoption in Guix…
>
> I think there should be some degree of standards (where I mean
> standards in the non-shodyness sense of the word, not in the sense of
> specs, though specs would be nice too) and of audit (bundling, malware,
> non-free, bugs) -- some of which are blocking (bundling, malware, non-
> free, some bugs), some of which aren't (some other bugs).
>
> E.g., see removal of unmaintained Python, of some old SSL libraries

You are mixing unrelated topics, IMHO.

We have policies, not standard.

«A policy is a set of ideas or plans that is used as a basis for making
decisions, especially in politics, economics, or business.» 

«A standard is a level of quality or achievement, especially a level
that is thought to be acceptable.»


>> > My view is that such issues should be reported upstream but cannot
>> > alone
>> > block package adoption in Guix.
>> 
>> I agree; we cannot fix the world. ;-) In the case of patch#55541, the
>> issues of cross-compilation can be reported directly to upstream
>
> Agreed -- I did not ask that explicitely in #55541, but the implied
> question was to report it upstream (or fix local, that could be done
> too).  But my point is that this should have been done _before_ merging
> the patch.

So, what are you explicitly asking? :-)


>> and another Debbugs number could be open.
>
> Would be pointless.  Standard policy seems to be to leave the debbugs
> issue unresolved forever, then someone does some cleanup in debbugs to
> close the debbugs number due to lack of activity or such.  Things would
> be delayed forever.

Old unsolved bugs are still open.  The cross-compilation of one package is
an issue for sure, but:

 1. it is not an issue for inclusion in Guix
 2. it has to be solved by people interested by cross-compilation

Other said, it cannot be asked to submitter to fix unrelated-to-Guix
issue on upstream code.  Although cross-compilation issue is somehow
related to Guix. ;-)


Cheers,
simon

Re: Dealing with upstream issues

[Maxime Devos]

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

zimoun schreef op ma 27-06-2022 om 17:06 [+0200]:
> Do you mean
> 
> --8<---------------cut here---------------start------------->8---
> > +(define-public harec
> > +  (let ((commit "bbabe09bddf74bd699f8ad2224fdd6e2eefbd35e")
> > (revision "0"))
> Despite what (guix style) may tell you, revision goes to an extra line.
> --8<---------------cut here---------------end--------------->8---
> 
> ? <https://issues.guix.gnu.org/55606#16-lineno56>

Yes, that comment (and some others, but that's the one I could find
quickly).

Greetings,
Mxaime.

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

Re: Dealing with upstream issues

[Maxime Devos]

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

zimoun schreef op ma 27-06-2022 om 17:23 [+0200]:
> You are mixing unrelated topics, IMHO.
> 
> We have policies, not standard.
> 
> «A policy is a set of ideas or plans that is used as a basis for making
> decisions, especially in politics, economics, or business.» 
> 
> «A standard is a level of quality or achievement, especially a level
> that is thought to be acceptable.»

I think it's a reasonable policy to have standards, and that it's a bad
policy to not have standards.  Seems related to me.  Also, we do have
some standards -- e.g., bundling, malware, non-freeness and security
problems is not considered quality.

> > Agreed -- I did not ask that explicitely in #55541, but the implied
> > question was to report it upstream (or fix local, that could be
> done
> > too).  But my point is that this should have been done _before_
> merging
> > the patch.
> 
> So, what are you explicitly asking? :-)

Nothing, it was implicit.

> Other said, it cannot be asked to submitter to fix unrelated-to-Guix
> issue on upstream code.  Although cross-compilation issue is somehow
> related to Guix. ;-)

I never asked this.  All I asked for is to report the issue upstream,
such that upstream can fix it.  (Though fixing it would be nice bonus.)

Greetings,
Maxime.

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

Re: Dealing with upstream issues

[Maxime Devos]

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

zimoun schreef op ma 27-06-2022 om 17:23 [+0200]:
> Old unsolved bugs are still open

Sometimes they aren't:

 * https://issues.guix.gnu.org/45828
 * https://issues.guix.gnu.org/26705
 * https://issues.guix.gnu.org/25719 (exception handling hasn't been cleaned up before closing)
 * https://issues.guix.gnu.org/44199 (it's a WIP, not completed yet, but still closed!)

Greetings,
Maxime.

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

Re: Dealing with upstream issues

[Maxime Devos]

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

zimoun schreef op ma 27-06-2022 om 17:23 [+0200]:
> Old unsolved bugs are still open.  The cross-compilation of one package is
> an issue for sure, but:
> 
>  1. it is not an issue for inclusion in Guix
>  2. it has to be solved by people interested by cross-compilation

Unfortunately, systematic cross-compilation fixes tend to be ignored,
while individual new package patches can go through, so it is necessary
to do the fixes in the individual new package patches.

Greetings,
Maxime.

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

Re: Dealing with upstream issues

[zimoun]

Hi Maxime,

I am confused by your replies in the thread.  Maybe, I miss the topic of
the comment by Ludo.

Well, from my understanding, the question is: should a perfectly working
and fine submission be delayed because unrelated-to-Guix issues are in
upstream code?

Ludo said these unrelated-to-Guix issues are not blocker, from my
understandings.  And I agree.  Do you disagree?

Reading you, I am missing what you are suggesting.

You are commenting on “standard” which somehow asks about explicit
criteria.  And, you are implicitly commenting on blocking while issues
from upstream are not fixed.  Instead of trying to deduce myself (and
probably the wrong way), could you please explicitly write down your
arguments?  Do you think that patch#55541 should be delayed while
upstream is not supported by cross-compilation?

I agree that fixing as much as possible and earlier is the best.
However, there is a tension between being perfect and doing with the
resources at hand.  For sure, it would be better if submitter also
report (or fix) upstream some issues, but I am not convinced the Guix
project should make this as mandatory requirements for inclusion of
submitted packages in Guix.  Do you think that patch#55541 should be
delayed while submitter has not open an upstream issue?


Last, I miss these comments about old bugs and what you are implicitly
suggesting with them.  Are you suggesting that old unsolved bugs are
closed without valid motivation?

>> Old unsolved bugs are still open
>
> Sometimes they aren't:

> * https://issues.guix.gnu.org/45828

Closed because:

        This can happen if guix-daemon was restarted but ‘guix publish’ wasn’t:
        ‘guix publish’ opens only one connection to the store at startup time,
        and then never tries to re-open it.  There was an old bug on this topic:

        https://issues.guix.gnu.org/26705

        Back then I marked it as ‘wontfix’ because:

          1. Losing a connection to the daemon Does Not Happen™ in normal
             conditions.  Namely, upon ‘herd restart guix-daemon’, ‘guix
             publish’ is automatically restarted.  One situation where ‘guix
             publish’ is not restarted is if one does “killall guix-daemon” or
             similar.  (Perhaps that’s something to fix in the Shepherd?)

> * https://issues.guix.gnu.org/26705

Closed because:

        For now I’m closing this bug as “wontfix” because I’ve never seen any
        occurrence of #2, and because #1 cannot happen on GuixSD (if ‘guix-daemon’
        is restarted, the shepherd will also restart ‘guix-publish’.

> * https://issues.guix.gnu.org/25719 (exception handling hasn't been cleaned up before closing)

Closed because:

        I haven't seen this particular exception in a long time.  I cannot tell whether
        the actual usability has been fixed, though--it could be that only the servers
        are more reliable and this code path is thus not currently being entered.

> * https://issues.guix.gnu.org/44199 (it's a WIP, not completed yet, but still closed!)

This history is:

Maxime Devos    wrote on 24 Oct 2020 21:47
zimoun          wrote on 27 Oct 2020 14:39
Maxime Devos    wrote on 27 Oct 2020 19:50
Maxime Devos    wrote on 1  Nov 2020 01:05
Ludovic Courtès wrote on 15 Nov 2020 22:13

        > This patch defines a `gnunet-fetch' method, allowing for downloading
        > files from GNUnet by their GNUnet chk-URI.

        While I think this is a laudable goal, I’m reluctant to including GNUnet
        support just yet because, as stated in recent release announcements,
        GNUnet is still in flux and not considered “production ready”.

        So I think we should keep it around and revisit this issue when GNUnet
        is considered “stable”.  WDYT?

zimoun          wrote on 16 Nov 2020 01:35
Maxime Devos    wrote on 18 Nov 2020 20:14

        > So I think we should keep it around and revisit this issue when
        > GNUnet
        > is considered “stable”.  WDYT?

        Sounds reasonable to me. There are also a lot of missing parts: a
        service definition for Guix System, findings substitutes, finding
        sources by hash (the one Guix uses, not the GNUnet hash) ..., so it
        isn't like my rudimentary patch was usable on large scale anyway.

Ludovic Courtès wrote on 18 Nov 2020 23:12

        tags 44199 wontfix
        close 44199
        quit


Therefore, if you have more details for one of these reports, feel free
to comment, provide more info or fix; for sure it will help.


Cheers,
simon

Re: Dealing with upstream issues

[Maxime Devos]

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

zimoun schreef op di 28-06-2022 om 13:01 [+0200]:
> Well, from my understanding, the question is: should a perfectly working
> and fine submission be delayed because unrelated-to-Guix issues are in
> upstream code?

This is not the question.  The dispute is about:

Maxime Devos: https://issues.guix.gnu.org/55541#3
> AFAICT the issues have not been reported upstream yet, so I don't
> think we can close this entry on debbugs yet. 

zimoun:
> Ludo said these unrelated-to-Guix issues are not blocker, from my
> understandings.  And I agree.  Do you disagree?

I agree too.  What I disagree with, is ignoring the bug.  The blocker
for me is: appropriate parties need to be at least informed of bug if
it isn't fixed.

> You are commenting on “standard” which somehow asks about explicit
> criteria.  And, you are implicitly commenting on blocking while
> issues from upstream are not fixed.  Instead of trying to deduce
> myself (and probably the wrong way), could you please explicitly
> write down your arguments?

Reviewer noticed a $bug.  This kind of $bug has two accepted and
standard methods for addressing it:

 (1) fix it (by replacing the configure script or patching it or
     sufficient substitute*).
     (a) in Guix (often work-around-ish, though often a work-around is
         sufficient for these kind of cross-compilation problems)
     (b) upstream (more work, sometimes more fulfilling, sometimes not 
         worth it
 (2) report it upstream (because it's more complicated than a simple
     'substitute*'.

Why?  It's a bug, needs to be fixed somehow, and for (2): we can't
solve everything ourselves.

What happened:

  Committer pushed changed, ignoring (1) and (2).

/me: What?  Why ignore the bugs?

Greetings,
Maxime.

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

Re: Dealing with upstream issues

[Maxime Devos]

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

zimoun schreef op di 28-06-2022 om 13:01 [+0200]:
>  Do you think that patch#55541 should be
> delayed while submitter has not open an upstream issue?

Yes, as mentioned in <https://issues.guix.gnu.org/55541#3>.

Greetings,
Maxime.

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

Re: Dealing with upstream issues

[Maxime Devos]

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

zimount:
> Last, I miss these comments about old bugs and what you are implicitly
> suggesting with them.  Are you suggesting that old unsolved bugs are
> closed without valid motivation?

You often close bugs with as rationale: ‘no response since X months,
hence closing’, so it seems to me that you would simply close bug
reports if the bug reporter is gone.

> > > Old unsolved bugs are still open
> > 
> > Sometimes they aren't:
> 
> > * https://issues.guix.gnu.org/45828
> 
> Closed because:
> 
>         This can happen if guix-daemon was restarted but ‘guix publish’ wasn’t:
>         ‘guix publish’ opens only one connection to the store at startup time,
>         and then never tries to re-open it.  There was an old bug on this topic:
> 
>         https://issues.guix.gnu.org/26705
> 
>         Back then I marked it as ‘wontfix’ because:
> 
>           1. Losing a connection to the daemon Does Not Happen™ in normal
>              conditions.  Namely, upon ‘herd restart guix-daemon’, ‘guix
>              publish’ is automatically restarted.  One situation where ‘guix
>              publish’ is not restarted is if one does “killall guix-daemon” or
>              similar.  (Perhaps that’s something to fix in the Shepherd?)
> 
> > * https://issues.guix.gnu.org/26705
> 
> Closed because:
> 
>         For now I’m closing this bug as “wontfix” because I’ve never seen any
>         occurrence of #2, and because #1 cannot happen on GuixSD (if ‘guix-daemon’
>         is restarted, the shepherd will also restart ‘guix-publish’.

It's a bug marked "wontfix" -- sure, I suppose #1 cannot happen on Guix
System, but there are foreign distros too.

> > * https://issues.guix.gnu.org/25719 (exception handling hasn't been cleaned up before closing)
> 
> Closed because:
> 
>         I haven't seen this particular exception in a long time.  I cannot tell whether
>         the actual usability has been fixed, though--it could be that only the servers
>         are more reliable and this code path is thus not currently being entered.

These kind of things are still bugs -- occassionally we see these kind
of bug reports pop up, so likely the underlying issue is still there
and error handlings is still loosy.

> > * https://issues.guix.gnu.org/44199 (it's a WIP, not completed yet, but still closed!)
> 
> This history is:
> 
> Maxime Devos    wrote on 24 Oct 2020 21:47
> zimoun          wrote on 27 Oct 2020 14:39
> Maxime Devos    wrote on 27 Oct 2020 19:50
> Maxime Devos    wrote on 1  Nov 2020 01:05
> Ludovic Courtès wrote on 15 Nov 2020 22:13
> 
>         > This patch defines a `gnunet-fetch' method, allowing for downloading
>         > files from GNUnet by their GNUnet chk-URI.
> 
>         While I think this is a laudable goal, I’m reluctant to including GNUnet
>         support just yet because, as stated in recent release announcements,
>         GNUnet is still in flux and not considered “production ready”.
> 
>         So I think we should keep it around and revisit this issue when GNUnet
>         is considered “stable”.  WDYT?
> 
> zimoun          wrote on 16 Nov 2020 01:35
> Maxime Devos    wrote on 18 Nov 2020 20:14
> 
>         > So I think we should keep it around and revisit this issue when
>         > GNUnet
>         > is considered “stable”.  WDYT?
> 
>         Sounds reasonable to me. There are also a lot of missing parts: a
>         service definition for Guix System, findings substitutes, finding
>         sources by hash (the one Guix uses, not the GNUnet hash) ..., so it
>         isn't like my rudimentary patch was usable on large scale anyway.

Oh right that was a bad example, the approach is broken (no http/https
fallbacks, bootstrap problems, etc); current idea is to extend
(guix download) with gnunet://fs/... instead.



> Therefore, if you have more details for one of these reports, feel free
> to comment, provide more info or fix; for sure it will help.

That's the issue I wanted to highlight -- issues are closed before
being fixed when the the reporter disappears (and hence, cannot provide
"more info", or has other things to do than provide a fix by
theirselves), even if the bug is understood.

Greetings,
Maxime.

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

Re: Dealing with upstream issues

[zimoun]

Hi,

On Tue, 28 Jun 2022 at 14:21, Maxime Devos <maximedevos@telenet.be> wrote:
> zimoun schreef op di 28-06-2022 om 13:01 [+0200]:
>> Well, from my understanding, the question is: should a perfectly working
>> and fine submission be delayed because unrelated-to-Guix issues are in
>> upstream code?
>
> This is not the question.  The dispute is about:
>
> Maxime Devos: https://issues.guix.gnu.org/55541#3
>> AFAICT the issues have not been reported upstream yet, so I don't
>> think we can close this entry on debbugs yet. 
>
> zimoun:
>> Ludo said these unrelated-to-Guix issues are not blocker, from my
>> understandings.  And I agree.  Do you disagree?
>
> I agree too.  What I disagree with, is ignoring the bug.  The blocker
> for me is: appropriate parties need to be at least informed of bug if
> it isn't fixed.

And… as I wrote [1]:

        I agree; we cannot fix the world. ;-) In the case of patch#55541, the
        issues of cross-compilation can be reported directly to upstream and
        another Debbugs number could be open.

1: <https://yhetil.org/guix/87wnd2w9mm.fsf@gmail.com>


The bug is not ignored, to the contrary.  But it is not a blocker for
patch inclusion and so patch#55541 can be closed.

You are free, as the reviewer, to open a report for Guix pointing the
issue of cross-compilation of ’azpainter’; bonus point for you if you
open an issue upstream.  Extra point for the one who fixes the package.

Well, you and me spend more time in discussing that than in just
reporting the issue. ;-)


Cheers,
simon

Re: Dealing with upstream issues

[zimoun]

Hi,

On Tue, 28 Jun 2022 at 14:31, Maxime Devos <maximedevos@telenet.be> wrote:

> You often close bugs with as rationale: ‘no response since X months,
> hence closing’, so it seems to me that you would simply close bug
> reports if the bug reporter is gone.

[...]

> That's the issue I wanted to highlight -- issues are closed before
> being fixed when the the reporter disappears (and hence, cannot provide
> "more info", or has other things to do than provide a fix by
> theirselves), even if the bug is understood.

These claims are inaccurate.  And it appears to me unfair considering
all the amount of time I personally spend on old bug triage; instead of
doing other (funner?) things.

My workflow dealing with old bugs is: pick one and read the report (and
the complete thread, if any), then,

 1. the report provides enough information for reproducing; I try to
    reproduce myself and report more info, and then I try to collaborate
    for fixing or closing.

 2. the report does not provide enough information to understand what
    the bug is about or to find a way to reproduce; then I ask more info
    – sometimes my reply is even the first one, then,

    a) an answer is back so it is similar as #1.
    
    b) no answer after 2 or more weeks, so I try to determine if the
       report is actionable and if the next action is fine-grained
       enough to be doable.  After 2 or more weeks, I ask again.

       Therefore, if a bug report after 2 or 3 years is not commented,
       especially after 2 or more attempts to understand and ask for the
       next steps without an answer back by the whole community, what
       could be the action other than just close the report.


Ultimately, nothing is perfect and people are doing their best with
their resource at hand; at least, I do my best with the resource at my
hands.  I would be more than happy if more people would try to sort,
classify or fix the old bugs.  Maybe, you will join the effort ?

I stop now the discussion in this thread since I do not see what we are
discussing.

Cheers,
simon

Re: Dealing with upstream issues

[Maxime Devos]

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

zimoun schreef op di 28-06-2022 om 18:21 [+0200]:
> And… as I wrote [1]:
> 
>         I agree; we cannot fix the world. ;-) In the case of patch#55541, the
>         issues of cross-compilation can be reported directly to upstream and
>         another Debbugs number could be open.
> 
> 1: <https://yhetil.org/guix/87wnd2w9mm.fsf@gmail.com>
> 
> 
> The bug is not ignored, to the contrary.

It is -- where's the bug report upstream or a fix?

> But it is not a blocker for patch inclusion and so patch#55541 can be closed.

It is a blocker -- as previousy mentioned, upstream doesn't even know
about the bug.

> You are free, as the reviewer, to open a report for Guix pointing the
> issue of cross-compilation of ’azpainter’;

As mentioned previously, that is not reviewing, and I don't have time
to fix the world.  I have other things to do than only acting on guix-
patches@.  Also, as mentioned previously, can't a reviewer do a partial
review?  Why the insistence on the reviewer -- isn't making a proper
patch the submitter's job, not the reviewer's?

> Well, you and me spend more time in discussing that than in just
> reporting the issue. ;-)

Yes, but if I just give in and report the issue after other insist I do
it, then I create the expectation that I'll just do everything.  I
refuse, I insist on being allowed to quit doing something and start
doing other things.

(So TBC, I won't be doing that.)

> Ultimately, nothing is perfect and people are doing their best with
> their resource at hand; at least, I do my best with the resource at
> my hands.  I would be more than happy if more people would try to
> sort, classify or fix the old bugs.  Maybe, you will join the effort
> ?

I would have more time for that if we refused patches with issues like
this one and did not insist on letting the reviewer do the submittter's
job.

Greetings,
Maxime.



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

Re: Dealing with upstream issues

[Maxime Devos]

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

zimoun schreef op di 28-06-2022 om 18:25 [+0200]:
> My workflow dealing with old bugs is: pick one and read the report (and
> the complete thread, if any), then,
> 
>  1. the report provides enough information for reproducing; I try to
>     reproduce myself and report more info, and then I try to collaborate
>     for fixing or closing.
> 
>  2. the report does not provide enough information to understand what
>     the bug is about or to find a way to reproduce; then I ask more info
>     – sometimes my reply is even the first one, then, [...]

This procedure looks reasonable to me!

Greetings,
Maxime.


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

Re: Dealing with upstream issues

[zimoun]

Hi,

On Tue, 28 Jun 2022 at 18:47, Maxime Devos <maximedevos@telenet.be> wrote:

> It is -- where's the bug report upstream or a fix?

Upstream [1] does not have a bug tracker, or I am missing it.  See
bug#56285 for tracking the issue in Guix.


1: <https://gitlab.com/azelpg/azpainter>
2: <http://issues.guix.gnu.org/issue/56285>


Cheers,
simon

Re: Dealing with upstream issues

[Maxime Devos]

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

zimoun schreef op di 28-06-2022 om 19:36 [+0200]:
> Hi,
> 
> On Tue, 28 Jun 2022 at 18:47, Maxime Devos <maximedevos@telenet.be> wrote:
> 
> > It is -- where's the bug report upstream or a fix?
> 
> Upstream [1] does not have a bug tracker, or I am missing it.  See
> bug#56285 for tracking the issue in Guix.
> 
> 

O right I forgot about that.  In that case, it's a lot more complicated
than I thought ...

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

Re: Dealing with upstream issues

[bokr]

Hi zimoun, et al,

On +2022-06-28 18:25:05 +0200, zimoun wrote:
> Hi,
> 
> On Tue, 28 Jun 2022 at 14:31, Maxime Devos <maximedevos@telenet.be> wrote:
> 
> > You often close bugs with as rationale: ‘no response since X months,
> > hence closing’, so it seems to me that you would simply close bug
> > reports if the bug reporter is gone.
> 
> [...]
> 
> > That's the issue I wanted to highlight -- issues are closed before
> > being fixed when the the reporter disappears (and hence, cannot provide
> > "more info", or has other things to do than provide a fix by
> > theirselves), even if the bug is understood.
> 
> These claims are inaccurate.  And it appears to me unfair considering
> all the amount of time I personally spend on old bug triage; instead of
> doing other (funner?) things.
> 
> My workflow dealing with old bugs is: pick one and read the report (and
> the complete thread, if any), then,
> 
>  1. the report provides enough information for reproducing; I try to
>     reproduce myself and report more info, and then I try to collaborate
>     for fixing or closing.
> 
>  2. the report does not provide enough information to understand what
>     the bug is about or to find a way to reproduce; then I ask more info
>     – sometimes my reply is even the first one, then,
> 
>     a) an answer is back so it is similar as #1.
>     
>     b) no answer after 2 or more weeks, so I try to determine if the
>        report is actionable and if the next action is fine-grained
>        enough to be doable.  After 2 or more weeks, I ask again.
> 
>        Therefore, if a bug report after 2 or 3 years is not commented,
>        especially after 2 or more attempts to understand and ask for the
>        next steps without an answer back by the whole community, what
>        could be the action other than just close the report.
>

Nothing, except maybe special archiving, or tagging for indexing?

By bug closing time, you have typically produced the best summary
of the bug chase, with clues and tips and examples for reproducing
and links where to find more info, such as links to Ludo's
(particularly, but others too) debugging examles. ( Kudos and thanks :)

So it's a matter of making sure your valuable work is archived for
easy use in future bug chases, ISTM.

Of course, your posts /are/ archived in the mailing lists.
(I like the POP3 mbox-format archives, where it's easy
to grep the headers. I do wget -c <archive-url>
to make myself copies of selected mail list months,
so I can search offline and view with mutt.

What I'd like is something in the Subject: line
to make it greppable for /both/ what the bug is about
and how it was closed, i.e. bug status.

Maybe if a post that says "closing" could have
"[closing: <status>]" in the Subject: line, where
<status> could say "fixed upstream" or "unresolved"
or whatever (bikeshed for dev ideas?).

Then you could use "[closing: unresolved]" in the closing
post Subject: line for a case that withered from inattention,
(your 2b) but still looks suspicious (if you think so).

E.g. suspicious like an fseg that went away because
new linkage for an update made the bad write
clobber something still, but without fseg:
It would be misleading to mark that "fixed" IMO.
Maybe "disappeared" :)

Anyway, the idea is to make the Subject: line greppable for both
what the bug is about and its status when it was closed.

> Ultimately, nothing is perfect and people are doing their best with
> their resource at hand; at least, I do my best with the resource at my
> hands.  I would be more than happy if more people would try to sort,
> classify or fix the old bugs.  Maybe, you will join the effort ?
> 
> I stop now the discussion in this thread since I do not see what we are
> discussing.
> 
> Cheers,
> simon
> 
--
Regards,
Bengt Richter

Missing tags in Debbugs?

[zimoun]

Hi,

Thanks for the feed back

On Wed, 29 Jun 2022 at 08:07, bokr@bokr.com wrote:

> Anyway, the idea is to make the Subject: line greppable for both
> what the bug is about and its status when it was closed.

I agree that it is hard to work with the Debbugs archive.  What you are
asking seems possible with Debbugs but the GNU instance is not
supporting many tags [1].  Using the ’Subject:’ line as tagging system
could be a workaround but I am not convinced the ratio effort /
usefulness is worth because it is too error-prone, IMHO.

Arun has presented nice ideas about improving Mumi and it appears to me
the direction to go.


1: <https://debbugs.gnu.org/Developer.html#tags>


Cheers,
simon

Re: Missing tags in Debbugs?

[Bengt Richter]

On +2022-06-29 09:29:20 +0200, zimoun wrote:
> Hi,
> 
> Thanks for the feed back
> 
> On Wed, 29 Jun 2022 at 08:07, bokr@bokr.com wrote:
> 
> > Anyway, the idea is to make the Subject: line greppable for both
> > what the bug is about and its status when it was closed.
> 
> I agree that it is hard to work with the Debbugs archive.  What you are
> asking seems possible with Debbugs but the GNU instance is not
> supporting many tags [1].  Using the ’Subject:’ line as tagging system
> could be a workaround but I am not convinced the ratio effort /
> usefulness is worth because it is too error-prone, IMHO.
> 
> Arun has presented nice ideas about improving Mumi and it appears to me
> the direction to go.
> 
> 
> 1: <https://debbugs.gnu.org/Developer.html#tags>

Thanks, that LGTM: Looks like easy-to-parse html :)

This looks interesting too, that I just found:

2: <https://debbugs.gnu.org/server-request.html#introduction>

Will have to try it. Maybe emacs already has a mode for that?
(I need to refresh my emacs-fu ;/ )
> 
> 
> Cheers,
> simon
--
Regards,
Bengt Richter

Re: Dealing with upstream issues

[Ludovic Courtès]

Hi!

Just to be clear, I started this thread as discussion on the kind of
interaction we reviewers should offer to submitters.  I am not
suggesting changes in our “quality standards”, nor am I suggesting that
one group of people do more work.

Maxime Devos <maximedevos@telenet.be> skribis:

> Ludovic Courtès schreef op ma 27-06-2022 om 12:10 [+0200]:
>> Regarding the review process, I think we should strive for a predictable
>> process—not requesting work from the submitter beyond what they can
>> expect.  Submitters can be expected to follow the written rules¹ and
>> perhaps a few more rules (for example, I don’t think we’ve documented
>> the fact that #:tests? #f is a last resort and should come with a
>> comment). 
>> 
>> However, following that principle, we reviewers cannot
>> reasonably ask for work beyond that. [...]
>
> We can ask to do send the notice upstream, if it's in the rules (I
> consider this to be part of the unwritten rules).

Yes, that’s a reasonable thing to ask for.  However, we can only ask for
it if the submitter identified a problem themselves; if I’m the one
finding a problem, it’s inappropriate to ask someone else to report it
on my behalf.

> And I don't often have time for addressing the noticed issues and I
> have other things to do as well -- I usually limit myself to just
> reviewing.  I do not intend to take up work beyond that.

Of course, and I don’t mean reviewers should do more work!  I think the
few people that dedicate time to patch review already have more than
enough on their plate; the last thing I’d want is to put more pressure
on them.  We have to care for one another, and that starts by making
sure none of us feels a pressure to always do more.

> I also consider them to not be rules as in ‘if they are followed, it
> WILL be accepted’ but more guidelines like ‘these things are important,
> they usually need to be followed, but it's not exhaustive, at times it
> might be discovered the list is incomplete’.

Agreed.

> I don't think that patch submitters can reasonably expect reviewers to
> do all the work.

Agreed.

> Also, previously in discussions about the review process, weren't there
> points about a reviewer not having to do everything all at once, they
> could choose to review parts they know how to review and have time for
> and leave the rest for others?

I don’t remember discussions along these lines.  I think it can be
helpful sometimes, and tricky other times.

For example, for a package, I find it hard to split review work.  But
for a patch series that touches many different things (documentation,
build system, importer, whatever), splitting review work among different
people may work better.

>> Related to that, I think it’s important to have a consistent review
>> process.  In other words, we should be equally demanding for all
>> patches to avoid bad surprises or a feeling of double standard.
>
> I think I've been consistent in asking to inform upstream of the issues
> (*), with no distinction of whether it's a new submitter or an
> established one or whatever.

I think our standards should be the same whether the submitter is a
newcomer or not.

The difference is in how we reviewers reply.  To a newcomer, reviewers
should IMO do the “last kilometer” themselves on behalf of submitters:
tweaking the commit log, synopsis/description, indentation, that kind of
thing.  It’s important because that gives submitters a good experience,
it helps them learn, and it’s also low-friction for the reviewer.
(That’s also the whole point of guix-mentors.)

Naturally, one can be more demanding of seasoned contributors, and I
think it’s OK to leave it up to them to fix such things.

Thanks,
Ludo’.

PS: Sorry for the wall of text!