[gentoo-pms] [PATCH] EAPI 6: Specify return status for eapply_user.

[gentoo-pms] [PATCH] EAPI 6: Specify return status for eapply_user.

[Ulrich Mueller]

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

Hi all,

It is late in the EAPI 6 process, but I believe that the patch below
is a good change. We should specify a return status for eapply_user.

Please review.

Ulrich


From cb0ac7d5bc88d70be0b81e17a3fb6b5c34642b50 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
Date: Wed, 28 Oct 2015 23:20:32 +0100
Subject: [PATCH] EAPI 6: Specify return status for eapply_user.

Suggested by dilfridge.
---
 pkg-mgr-commands.tex | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/pkg-mgr-commands.tex b/pkg-mgr-commands.tex
index 51f6ebf..b6164df 100644
--- a/pkg-mgr-commands.tex
+++ b/pkg-mgr-commands.tex
@@ -157,10 +157,12 @@ Ebuilds must not run any of these commands once the current phase function has r
 \item[eapply\_user] \featurelabel{eapply-user} Takes no arguments. Package managers supporting it
     apply user-provided patches to the source tree in the current working directory. Exact behaviour
     is implementation defined and beyond the scope of this specification. Package managers not
-    supporting it must implement the command as a no-op. Only available in EAPIs listed in
-    table~\ref{tab:patch-commands} as supporting \t{eapply\_user}. In EAPIs where it is supported,
-    \t{eapply\_user} must be called once in the \t{src\_prepare} phase; for any subsequent calls
-    the command will do nothing.
+    supporting it must implement the command as a no-op. Returns 0 if patches applied successfully,
+    or 1 if no patches were applied. On failure, returns 2 when called using \t{nonfatal}, otherwise
+    aborts the build process. Only available in EAPIs listed in table~\ref{tab:patch-commands} as
+    supporting \t{eapply\_user}. In EAPIs where it is supported, \t{eapply\_user} must be called
+    once in the \t{src\_prepare} phase; for any subsequent calls the command will do nothing, and
+    it will return the same status as for the first call.
 \end{description}
 
 \begin{algorithm}
-- 
2.6.2

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

[gentoo-pms] [PATCH v2] EAPI 6: Specify return status for eapply_user.

[Ulrich Mueller]

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

>>>>> On Wed, 28 Oct 2015, Ulrich Mueller wrote:

> It is late in the EAPI 6 process, but I believe that the patch below
> is a good change. We should specify a return status for eapply_user.

Patch v2, with logic inverted:
   0 = sources are unmodified
   1 = patches applied successfully
   2 = failure

Rationale: The relevant information is if sources are unmodified or
(potentially) modified, so these two states should correspond to shell
true and false, respectively.

Also seems natural to return 0 in the most common case that there are
no user patches, and in the case that eapply_user() is a no-op.

> Please review.

Ulrich


From ae6782cc84e18c8c39c2583731ffed0e258db209 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
Date: Thu, 29 Oct 2015 08:10:42 +0100
Subject: [PATCH] EAPI 6: Specify return status for eapply_user.

Suggested by dilfridge.
---
 pkg-mgr-commands.tex | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/pkg-mgr-commands.tex b/pkg-mgr-commands.tex
index 51f6ebf..0668686 100644
--- a/pkg-mgr-commands.tex
+++ b/pkg-mgr-commands.tex
@@ -157,10 +157,12 @@ Ebuilds must not run any of these commands once the current phase function has r
 \item[eapply\_user] \featurelabel{eapply-user} Takes no arguments. Package managers supporting it
     apply user-provided patches to the source tree in the current working directory. Exact behaviour
     is implementation defined and beyond the scope of this specification. Package managers not
-    supporting it must implement the command as a no-op. Only available in EAPIs listed in
+    supporting it must implement the command as a no-op. Returns 0 if the sources are unmodified,
+    or 1 if patches applied successfully. On failure, returns 2 when called using \t{nonfatal},
+    otherwise aborts the build process. Only available in EAPIs listed in
     table~\ref{tab:patch-commands} as supporting \t{eapply\_user}. In EAPIs where it is supported,
     \t{eapply\_user} must be called once in the \t{src\_prepare} phase; for any subsequent calls
-    the command will do nothing.
+    the command will do nothing, and it will return the same status as for the first call.
 \end{description}
 
 \begin{algorithm}
-- 
2.6.2

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [gentoo-pms] [PATCH v2] EAPI 6: Specify return status for eapply_user.

[Michał Górny]

Dnia 29 października 2015 09:28:53 CET, Ulrich Mueller <ulm@gentoo.org> napisał(a):
>>>>>> On Wed, 28 Oct 2015, Ulrich Mueller wrote:
>
>> It is late in the EAPI 6 process, but I believe that the patch below
>> is a good change. We should specify a return status for eapply_user.
>
>Patch v2, with logic inverted:
>   0 = sources are unmodified
>   1 = patches applied successfully
>   2 = failure
>
>Rationale: The relevant information is if sources are unmodified or
>(potentially) modified, so these two states should correspond to shell
>true and false, respectively.
>
>Also seems natural to return 0 in the most common case that there are
>no user patches, and in the case that eapply_user() is a no-op.

Nah, that's going to be confusing.

  if ! eapply_user; then
    # patches were applied...?!

I'd go for:

0 - applied
1 - no patches
and die on failure

>
>> Please review.
>
>Ulrich
>
>
>From ae6782cc84e18c8c39c2583731ffed0e258db209 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
>Date: Thu, 29 Oct 2015 08:10:42 +0100
>Subject: [PATCH] EAPI 6: Specify return status for eapply_user.
>
>Suggested by dilfridge.
>---
> pkg-mgr-commands.tex | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/pkg-mgr-commands.tex b/pkg-mgr-commands.tex
>index 51f6ebf..0668686 100644
>--- a/pkg-mgr-commands.tex
>+++ b/pkg-mgr-commands.tex
>@@ -157,10 +157,12 @@ Ebuilds must not run any of these commands once
>the current phase function has r
>\item[eapply\_user] \featurelabel{eapply-user} Takes no arguments.
>Package managers supporting it
>apply user-provided patches to the source tree in the current working
>directory. Exact behaviour
>is implementation defined and beyond the scope of this specification.
>Package managers not
>-    supporting it must implement the command as a no-op. Only
>available in EAPIs listed in
>+    supporting it must implement the command as a no-op. Returns 0 if
>the sources are unmodified,
>+    or 1 if patches applied successfully. On failure, returns 2 when
>called using \t{nonfatal},
>+    otherwise aborts the build process. Only available in EAPIs listed
>in
>table~\ref{tab:patch-commands} as supporting \t{eapply\_user}. In EAPIs
>where it is supported,
>\t{eapply\_user} must be called once in the \t{src\_prepare} phase; for
>any subsequent calls
>-    the command will do nothing.
>+    the command will do nothing, and it will return the same status as
>for the first call.
> \end{description}
> 
> \begin{algorithm}

-- 
Best regards,
Michał Górny

Re: [gentoo-pms] [PATCH v2] EAPI 6: Specify return status for eapply_user.

[Ulrich Mueller]

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

>>>>> On Thu, 29 Oct 2015, Michał Górny wrote:

> Dnia 29 października 2015 09:28:53 CET, Ulrich Mueller <ulm@gentoo.org> napisał(a):
>> Patch v2, with logic inverted:
>> 0 = sources are unmodified
>> 1 = patches applied successfully
>> 2 = failure
>> 
>> Rationale: The relevant information is if sources are unmodified or
>> (potentially) modified, so these two states should correspond to shell
>> true and false, respectively.
>> 
>> Also seems natural to return 0 in the most common case that there are
>> no user patches, and in the case that eapply_user() is a no-op.

> Nah, that's going to be confusing.

Why? We have two successful states, so obviously (at least) one of
them must return shell false. I don't see why one would prefer the
"user modified" case over the "unmodified" case.

>   if ! eapply_user; then
>     # patches were applied...?!

So?

> I'd go for:

> 0 - applied
> 1 - no patches
> and die on failure

No, that would be inconsistent with behaviour of all other commands
under nonfatal.

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

[gentoo-pms] Re: [PATCH v2] EAPI 6: Specify return status for eapply_user.

[Ulrich Mueller]

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

>>>>> On Thu, 29 Oct 2015, Rich Freeman wrote:

> On Thu, Oct 29, 2015 at 4:28 AM, Ulrich Mueller <ulm@gentoo.org> wrote:
>> 
>> Also seems natural to return 0 in the most common case that there
>> are no user patches, and in the case that eapply_user() is a no-op.

> I think the no-op situation requires care.

By "no-op" I meant the case that the package manager would implement
the function as a no-op, which is allowed by the spec.

> The whole reason for having the no-op option is if it gets called in
> one or more eclasses and then in the ebuild itself.

> That would mean that it would become the responsibility of the
> eclass to re-autoconf the package if the patches apply.  As long as
> this is done I'm fine with the approach.  I suppose that makes
> sense, but I just want to make sure everybody is on the same page.
> Otherwise the patches apply in an eclass and then the ebuild gets
> the impression that no patches were applied.

That's a different situation, and I hope it will be taken care of by
the requirement that on subsequent calls to eapply_user the same
return status as before must be returned.

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

[gentoo-pms] [PATCH v3] EAPI 6: Specify return status for eapply_user.

[Ulrich Mueller]

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

>>>>> On Thu, 29 Oct 2015, Ulrich Mueller wrote:

> We have two successful states, so obviously (at least) one of them
> must return shell false.

This is the part which causes the problems. As discussed amongst
council members, we are going to drop this. So, the command will
return true for success and false for failure.

If it should turn out that information about applied user's patches
will be needed, a query function (that could even show a list of
applied patches, like perl -V does) can be added later. This will be
something for EAPI 7, though.

Patch v3 is below.

Ulrich


From cad46e884d84bacbd7763ea7cec69b59f5031acb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
Date: Fri, 30 Oct 2015 01:37:00 +0100
Subject: [PATCH] EAPI 6: Specify return status for eapply_user.

---
 pkg-mgr-commands.tex | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/pkg-mgr-commands.tex b/pkg-mgr-commands.tex
index 51f6ebf..b9d53bd 100644
--- a/pkg-mgr-commands.tex
+++ b/pkg-mgr-commands.tex
@@ -157,10 +157,12 @@ Ebuilds must not run any of these commands once the current phase function has r
 \item[eapply\_user] \featurelabel{eapply-user} Takes no arguments. Package managers supporting it
     apply user-provided patches to the source tree in the current working directory. Exact behaviour
     is implementation defined and beyond the scope of this specification. Package managers not
-    supporting it must implement the command as a no-op. Only available in EAPIs listed in
-    table~\ref{tab:patch-commands} as supporting \t{eapply\_user}. In EAPIs where it is supported,
-    \t{eapply\_user} must be called once in the \t{src\_prepare} phase; for any subsequent calls
-    the command will do nothing.
+    supporting it must implement the command as a no-op. Returns shell true (0) if patches applied
+    successfully, or if no patches were provided. Otherwise, aborts the build process, unless run
+    using \t{nonfatal}, in which case it returns non-zero exit status. Only available in EAPIs
+    listed in table~\ref{tab:patch-commands} as supporting \t{eapply\_user}. In EAPIs where it is
+    supported, \t{eapply\_user} must be called once in the \t{src\_prepare} phase; for any
+    subsequent calls, the command will do nothing and return~0.
 \end{description}
 
 \begin{algorithm}
-- 
2.6.2

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [gentoo-pms] [PATCH v3] EAPI 6: Specify return status for eapply_user.

[Michał Górny]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512



Dnia 30 października 2015 09:17:49 CET, Ulrich Mueller <ulm@gentoo.org> napisał(a):
>>>>>> On Thu, 29 Oct 2015, Ulrich Mueller wrote:
>
>> We have two successful states, so obviously (at least) one of them
>> must return shell false.
>
>This is the part which causes the problems. As discussed amongst
>council members, we are going to drop this. So, the command will
>return true for success and false for failure.
>
>If it should turn out that information about applied user's patches
>will be needed, a query function (that could even show a list of
>applied patches, like perl -V does) can be added later. This will be
>something for EAPI 7, though.

Sounds good to me. In fact, we can't really predict what user patches may change and what ebuild changes they may require. And we can't really expect every ebuild to account even for the most common changes.

>
>Patch v3 is below.
>
>Ulrich
>
>
>From cad46e884d84bacbd7763ea7cec69b59f5031acb Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
>Date: Fri, 30 Oct 2015 01:37:00 +0100
>Subject: [PATCH] EAPI 6: Specify return status for eapply_user.
>
>---
> pkg-mgr-commands.tex | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
>diff --git a/pkg-mgr-commands.tex b/pkg-mgr-commands.tex
>index 51f6ebf..b9d53bd 100644
>--- a/pkg-mgr-commands.tex
>+++ b/pkg-mgr-commands.tex
>@@ -157,10 +157,12 @@ Ebuilds must not run any of these commands once
>the current phase function has r
>\item[eapply\_user] \featurelabel{eapply-user} Takes no arguments.
>Package managers supporting it
>apply user-provided patches to the source tree in the current working
>directory. Exact behaviour
>is implementation defined and beyond the scope of this specification.
>Package managers not
>-    supporting it must implement the command as a no-op. Only
>available in EAPIs listed in
>-    table~\ref{tab:patch-commands} as supporting \t{eapply\_user}. In
>EAPIs where it is supported,
>-    \t{eapply\_user} must be called once in the \t{src\_prepare}
>phase; for any subsequent calls
>-    the command will do nothing.
>+    supporting it must implement the command as a no-op. Returns shell
>true (0) if patches applied
>+    successfully, or if no patches were provided. Otherwise, aborts
>the build process, unless run
>+    using \t{nonfatal}, in which case it returns non-zero exit status.
>Only available in EAPIs
>+    listed in table~\ref{tab:patch-commands} as supporting
>\t{eapply\_user}. In EAPIs where it is
>+    supported, \t{eapply\_user} must be called once in the
>\t{src\_prepare} phase; for any
>+    subsequent calls, the command will do nothing and return~0.
> \end{description}
>
> \begin{algorithm}

- --
Best regards,
Michał Górny
-----BEGIN PGP SIGNATURE-----

iQJJBAEBCgAzLBxNaWNoYcWCIEfDs3JueSAoR2VudG9vKSA8bWdvcm55QGdlbnRv
by5vcmc+BQJWMzIYAAoJELB6GurvtEZOQ5kP/2WEgAZH48/PuoLPbdH8fhfYwZXk
AitCVDzVlscN5hvrMxV49e0/UbvUFFQbWszwplH4BGjQ7GW2e2mBd+4IjzktkKiL
RT2YyCl8BdROaNQWx0cxZiC/VajvzEX7GAYgOV/GiBGb5671HU2ByjMlIJZkJafF
oqewZQh1yDhzMhsBMBO6COlJYkCvbUDnoghcqjshCDdIjJY7REvtAh9Ce9FvhGlF
IznytxM9/yXabk6am6TX7caU3eAiH/gYWV9T7q4owYSuiTix850gwxA4WcFh1uW7
45gncBKxSpqtcaagZVsZZOa8eL2Q67K3b+gWNZkPIYLUsDtYiDuSSDmkez0ALWRz
JBE0nAxcG1l1S81P5CsgeL1RejSMJTwjbrPJR1kjPMeSOTCueLMWjWY/2U3gR+wS
w4Q01mEsDH3I7Q4GUA2T10HagUeW1tv9bVLbwz6GrjZcj0hEQDYetijcGauiDjrB
6YG5gSNszRyhQqxeKdZ4Hc0kXjCdk8vt3/aK2pEYil8vBh+O+LlNqLB4VoviKT0x
EjUXBbXVq68FAY6FcdIK8sp57HquXsEPgCsx1p8PuR0s0x7YSbS1C6ztFFwF6v0d
XFbR0D8ePN0yReY9IqHu7lPEmhNcq8mZ/l83PIOeZsnz/T2fDo5i8WpSgseioD8j
m8pwHyHNhbg6lfTE
=1uvJ
-----END PGP SIGNATURE-----