[gentoo-dev] [PATCH] eutils.eclass: Show death notice only when user patches were really applied

[gentoo-dev] [PATCH] eutils.eclass: Show death notice only when user patches were really applied

[Thomas Deutschmann]

As part of the user requested feature from [Gentoo-Bug #543878]
eutils.eclass shows a warning regarding user applied patches in case of an
error [Link 1].

However this warning will always be shown even if no user patch were
applied at all (example: empty /etc/portage/<cat>/<pkg> directory).

This commit adds a new global variable "EPATCH_N_APPLIED_PATCHES" which
tracks the number of applied user patches. This allows us to only show the
notice when user patches were really applied.

Link: https://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/eclass/eutils.eclass?r1=1.443&r2=1.444

Gentoo-Bug: https://bugs.gentoo.org/543878
---
 eclass/eutils.eclass | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/eclass/eutils.eclass b/eclass/eutils.eclass
index dbedffe..025e388 100644
--- a/eclass/eutils.eclass
+++ b/eclass/eutils.eclass
@@ -292,6 +292,10 @@ EPATCH_OPTS=""
 #	-E - automatically remove empty files
 # @CODE
 EPATCH_COMMON_OPTS="-g0 -E --no-backup-if-mismatch"
+# @VARIABLE: EPATCH_N_APPLIED_PATCHES
+# @DESCRIPTION:
+# Counter variable which indicates how many patches were applied.
+EPATCH_N_APPLIED_PATCHES=0
 # @VARIABLE: EPATCH_EXCLUDE
 # @DESCRIPTION:
 # List of patches not to apply.	 Note this is only file names,
@@ -595,6 +599,8 @@ epatch() {
 			: $(( count++ ))
 		done
 
+		: $(( EPATCH_N_APPLIED_PATCHES++ ))
+
 		# if we had to decompress the patch, delete the temp one
 		if [[ -n ${PIPE_CMD} ]] ; then
 			rm -f "${PATCH_TARGET}"
@@ -1736,13 +1742,16 @@ epatch_user() {
 		[[ -r ${EPATCH_SOURCE} ]] || EPATCH_SOURCE=${EPATCH_USER_SOURCE}/${CHOST}/${check}
 		[[ -r ${EPATCH_SOURCE} ]] || EPATCH_SOURCE=${EPATCH_USER_SOURCE}/${check}
 		if [[ -d ${EPATCH_SOURCE} ]] ; then
+			local old_n_applied_patches=${EPATCH_N_APPLIED_PATCHES}
 			EPATCH_SOURCE=${EPATCH_SOURCE} \
 			EPATCH_SUFFIX="patch" \
 			EPATCH_FORCE="yes" \
 			EPATCH_MULTI_MSG="Applying user patches from ${EPATCH_SOURCE} ..." \
 			epatch
 			echo "${EPATCH_SOURCE}" > "${applied}"
-			has epatch_user_death_notice ${EBUILD_DEATH_HOOKS} || EBUILD_DEATH_HOOKS+=" epatch_user_death_notice"
+			if [[ ${old_n_applied_patches} -lt ${EPATCH_N_APPLIED_PATCHES} ]]; then
+				has epatch_user_death_notice ${EBUILD_DEATH_HOOKS} || EBUILD_DEATH_HOOKS+=" epatch_user_death_notice"
+			fi
 			return 0
 		fi
 	done
-- 
2.9.3

Re: [gentoo-dev] [PATCH] eutils.eclass: Show death notice only when user patches were really applied

[Ulrich Mueller]

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

>>>>> On Mon, 22 Aug 2016, Thomas Deutschmann wrote:

> [...]

> This commit adds a new global variable "EPATCH_N_APPLIED_PATCHES"
> which tracks the number of applied user patches. This allows us to
> only show the notice when user patches were really applied.

I wonder if extending an obsolete feature is worth the effort.
In EAPI 6, epatch_user has been replaced by eapply_user.
 
> +		: $(( EPATCH_N_APPLIED_PATCHES++ ))

Why not simply:
		(( EPATCH_N_APPLIED_PATCHES++ ))

> +			if [[ ${old_n_applied_patches} -lt ${EPATCH_N_APPLIED_PATCHES} ]]; then
> +				has epatch_user_death_notice ${EBUILD_DEATH_HOOKS} || EBUILD_DEATH_HOOKS+=" epatch_user_death_notice"

Please keep lines no wider than 80 character positions.

Ulrich

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

Re: [gentoo-dev] [PATCH] eutils.eclass: Show death notice only when user patches were really applied

[Thomas Deutschmann]

[-- Attachment #1.1: Type: text/plain, Size: 1195 bytes --]

On 2016-08-22 09:30, Ulrich Mueller wrote:
> I wonder if extending an obsolete feature is worth the effort.
> In EAPI 6, epatch_user has been replaced by eapply_user.

Well, I created the patch in November 2015 but never submitted it.
Yesterday someone in #gentoo-dev also asked about that false-positive
warning...

Yes, EAPI >=6 doesn't have this problem anymore. But many system
packages won't migrate to EAPI=6 very soon. So this irritating warning
will stay for the next years if we don't fix it. And because it is an
easy fix... isn't it?


>> +		: $(( EPATCH_N_APPLIED_PATCHES++ ))
> 
> Why not simply:
> 		(( EPATCH_N_APPLIED_PATCHES++ ))

When I created the patch I tried to use the same coding style. See

> 			: $(( count++ ))

two lines above.

Can I keep this or should I change?


>> +			if [[ ${old_n_applied_patches} -lt ${EPATCH_N_APPLIED_PATCHES} ]]; then
>> +				has epatch_user_death_notice ${EBUILD_DEATH_HOOKS} || EBUILD_DEATH_HOOKS+=" epatch_user_death_notice"
> 
> Please keep lines no wider than 80 character positions.

OK, I'll split the "has epatch_..." line after the "||".


Thanks for reviewing.


-- 
Regards,
Thomas



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 951 bytes --]

Re: [gentoo-dev] [PATCH] eutils.eclass: Show death notice only when user patches were really applied

[Ulrich Mueller]

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

>>>>> On Mon, 22 Aug 2016, Thomas Deutschmann wrote:

> On 2016-08-22 09:30, Ulrich Mueller wrote:
>> I wonder if extending an obsolete feature is worth the effort.
>> In EAPI 6, epatch_user has been replaced by eapply_user.

> Well, I created the patch in November 2015 but never submitted it.
> Yesterday someone in #gentoo-dev also asked about that
> false-positive warning...

> Yes, EAPI >=6 doesn't have this problem anymore. But many system
> packages won't migrate to EAPI=6 very soon. So this irritating
> warning will stay for the next years if we don't fix it. And because
> it is an easy fix... isn't it?

Sure, it is an easy fix. However, it is not without cost, as it adds
another variable to global scope of all ebuilds inheriting eutils.
Even in EAPI 6 where epatch_user will not be used.

>>> +		: $(( EPATCH_N_APPLIED_PATCHES++ ))
>> 
>> Why not simply:
>> (( EPATCH_N_APPLIED_PATCHES++ ))

> When I created the patch I tried to use the same coding style. See

>> : $(( count++ ))

> two lines above.

git blame point to the following commit:
2975c21ee (Mike Frysinger 2010-01-09 20:06:24 +0000  595) : $(( count++ ))

Looks like this was missed during eclass review back then. (I cannot
find anything in the mailing list archives, though. Can anyone provide
a pointer?)

> Can I keep this or should I change?

*shrug* It's a tiny issue.

Ulrich

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

Re: [gentoo-dev] [PATCH] eutils.eclass: Show death notice only when user patches were really applied

[Michał Górny]

Dnia 22 sierpnia 2016 13:16:28 CEST, Ulrich Mueller <ulm@gentoo.org> napisał(a):
>>>>>> On Mon, 22 Aug 2016, Thomas Deutschmann wrote:
>
>> On 2016-08-22 09:30, Ulrich Mueller wrote:
>>> I wonder if extending an obsolete feature is worth the effort.
>>> In EAPI 6, epatch_user has been replaced by eapply_user.
>
>> Well, I created the patch in November 2015 but never submitted it.
>> Yesterday someone in #gentoo-dev also asked about that
>> false-positive warning...
>
>> Yes, EAPI >=6 doesn't have this problem anymore. But many system
>> packages won't migrate to EAPI=6 very soon. So this irritating
>> warning will stay for the next years if we don't fix it. And because
>> it is an easy fix... isn't it?
>
>Sure, it is an easy fix. However, it is not without cost, as it adds
>another variable to global scope of all ebuilds inheriting eutils.
>Even in EAPI 6 where epatch_user will not be used.

But then, epatch shouldn't be used either.

>
>>>> +		: $(( EPATCH_N_APPLIED_PATCHES++ ))
>>> 
>>> Why not simply:
>>> (( EPATCH_N_APPLIED_PATCHES++ ))
>
>> When I created the patch I tried to use the same coding style. See
>
>>> : $(( count++ ))
>
>> two lines above.
>
>git blame point to the following commit:
>2975c21ee (Mike Frysinger 2010-01-09 20:06:24 +0000  595) : $(( count++
>))
>
>Looks like this was missed during eclass review back then. (I cannot
>find anything in the mailing list archives, though. Can anyone provide
>a pointer?)

vapier and review? Are you asking seriously?

>
>> Can I keep this or should I change?
>
>*shrug* It's a tiny issue.
>
>Ulrich


-- 
Best regards,
Michał Górny (by phone)

[gentoo-dev] [PATCH v2] eutils.eclass: Show death notice only when user patches were really applied

[Thomas Deutschmann]

As part of the user requested feature from [Gentoo-Bug #543878]
eutils.eclass shows a warning regarding user applied patches in case of an
error [Link 1].

However this warning will always be shown even if no user patch were
applied at all (example: empty /etc/portage/<cat>/<pkg> directory).

This commit adds a new global variable "EPATCH_N_APPLIED_PATCHES" which
tracks the number of applied user patches. This allows us to only show the
notice when user patches were really applied.

Link: https://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/eclass/eutils.eclass?r1=1.443&r2=1.444

Gentoo-Bug: https://bugs.gentoo.org/543878
---
 eclass/eutils.eclass | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/eclass/eutils.eclass b/eclass/eutils.eclass
index dbedffe..27b542c 100644
--- a/eclass/eutils.eclass
+++ b/eclass/eutils.eclass
@@ -292,6 +292,10 @@ EPATCH_OPTS=""
 #	-E - automatically remove empty files
 # @CODE
 EPATCH_COMMON_OPTS="-g0 -E --no-backup-if-mismatch"
+# @VARIABLE: EPATCH_N_APPLIED_PATCHES
+# @DESCRIPTION:
+# Counter variable which indicates how many patches were applied.
+EPATCH_N_APPLIED_PATCHES=0
 # @VARIABLE: EPATCH_EXCLUDE
 # @DESCRIPTION:
 # List of patches not to apply.	 Note this is only file names,
@@ -595,6 +599,8 @@ epatch() {
 			: $(( count++ ))
 		done
 
+		(( EPATCH_N_APPLIED_PATCHES++ ))
+
 		# if we had to decompress the patch, delete the temp one
 		if [[ -n ${PIPE_CMD} ]] ; then
 			rm -f "${PATCH_TARGET}"
@@ -1736,13 +1742,17 @@ epatch_user() {
 		[[ -r ${EPATCH_SOURCE} ]] || EPATCH_SOURCE=${EPATCH_USER_SOURCE}/${CHOST}/${check}
 		[[ -r ${EPATCH_SOURCE} ]] || EPATCH_SOURCE=${EPATCH_USER_SOURCE}/${check}
 		if [[ -d ${EPATCH_SOURCE} ]] ; then
+			local old_n_applied_patches=${EPATCH_N_APPLIED_PATCHES}
 			EPATCH_SOURCE=${EPATCH_SOURCE} \
 			EPATCH_SUFFIX="patch" \
 			EPATCH_FORCE="yes" \
 			EPATCH_MULTI_MSG="Applying user patches from ${EPATCH_SOURCE} ..." \
 			epatch
 			echo "${EPATCH_SOURCE}" > "${applied}"
-			has epatch_user_death_notice ${EBUILD_DEATH_HOOKS} || EBUILD_DEATH_HOOKS+=" epatch_user_death_notice"
+			if [[ ${old_n_applied_patches} -lt ${EPATCH_N_APPLIED_PATCHES} ]]; then
+				has epatch_user_death_notice ${EBUILD_DEATH_HOOKS} || \
+					EBUILD_DEATH_HOOKS+=" epatch_user_death_notice"
+			fi
 			return 0
 		fi
 	done
-- 
2.9.3

[gentoo-dev] [PATCH v3] eutils.eclass: Show death notice only when user patches were really applied

[Thomas Deutschmann]

Here's v3. Ulm suggested to get rid of the global EPATCH_N_APPLIED_PATCHES variable
so that EAPI 6' environment stays clean.

Thomas Deutschmann (1):
  eutils.eclass: Show death notice only when user patches were really
    applied

 eclass/eutils.eclass | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

-- 
2.9.3

[gentoo-dev] [PATCH v3] eutils.eclass: Show death notice only when user patches were really applied

[Thomas Deutschmann]

As part of the user requested feature from [Gentoo-Bug #543878]
eutils.eclass shows a warning regarding user applied patches in case of an
error [Link 1].

However this warning will always be shown even if no user patch were
applied at all (example: empty /etc/portage/<cat>/<pkg> directory).

This commit adds a new global variable "EPATCH_N_APPLIED_PATCHES" which
tracks the number of applied user patches. This allows us to only show the
notice when user patches were really applied.

Link: https://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/eclass/eutils.eclass?r1=1.443&r2=1.444

Gentoo-Bug: https://bugs.gentoo.org/543878
---
 eclass/eutils.eclass | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/eclass/eutils.eclass b/eclass/eutils.eclass
index dbedffe..aaf195b 100644
--- a/eclass/eutils.eclass
+++ b/eclass/eutils.eclass
@@ -595,6 +595,8 @@ epatch() {
 			: $(( count++ ))
 		done
 
+		(( EPATCH_N_APPLIED_PATCHES++ ))
+
 		# if we had to decompress the patch, delete the temp one
 		if [[ -n ${PIPE_CMD} ]] ; then
 			rm -f "${PATCH_TARGET}"
@@ -1736,13 +1738,17 @@ epatch_user() {
 		[[ -r ${EPATCH_SOURCE} ]] || EPATCH_SOURCE=${EPATCH_USER_SOURCE}/${CHOST}/${check}
 		[[ -r ${EPATCH_SOURCE} ]] || EPATCH_SOURCE=${EPATCH_USER_SOURCE}/${check}
 		if [[ -d ${EPATCH_SOURCE} ]] ; then
+			local old_n_applied_patches=${EPATCH_N_APPLIED_PATCHES:-0}
 			EPATCH_SOURCE=${EPATCH_SOURCE} \
 			EPATCH_SUFFIX="patch" \
 			EPATCH_FORCE="yes" \
 			EPATCH_MULTI_MSG="Applying user patches from ${EPATCH_SOURCE} ..." \
 			epatch
 			echo "${EPATCH_SOURCE}" > "${applied}"
-			has epatch_user_death_notice ${EBUILD_DEATH_HOOKS} || EBUILD_DEATH_HOOKS+=" epatch_user_death_notice"
+			if [[ ${old_n_applied_patches} -lt ${EPATCH_N_APPLIED_PATCHES} ]]; then
+				has epatch_user_death_notice ${EBUILD_DEATH_HOOKS} || \
+					EBUILD_DEATH_HOOKS+=" epatch_user_death_notice"
+			fi
 			return 0
 		fi
 	done
-- 
2.9.3

Re: [gentoo-dev] [PATCH v3] eutils.eclass: Show death notice only when user patches were really applied

[Thomas Deutschmann]

[-- Attachment #1.1: Type: text/plain, Size: 75 bytes --]

Hi,

now committed, see commit f7d866b62b.


-- 
Regards,
Thomas


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 951 bytes --]