[gentoo-dev] [PATCH] bzr.eclass: Add --overwrite-tags option to pull command.

[gentoo-dev] [PATCH] bzr.eclass: Add --overwrite-tags option to pull command.

[Ulrich Müller]

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

Fixes: https://bugs.gentoo.org/446422
---
 eclass/bzr.eclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/eclass/bzr.eclass b/eclass/bzr.eclass
index ef1b47936867..792cc3bec36f 100644
--- a/eclass/bzr.eclass
+++ b/eclass/bzr.eclass
@@ -27,7 +27,7 @@ case "${EAPI:-0}" in
 	*)   EXPORT_FUNCTIONS src_unpack src_prepare ;;
 esac
 
-DEPEND=">=dev-vcs/bzr-2.0.1"
+DEPEND=">=dev-vcs/bzr-2.6.0"
 case "${EAPI:-0}" in
 	0|1) ;;
 	*) [[ ${EBZR_REPO_URI%%:*} = sftp ]] \
@@ -57,7 +57,7 @@ esac
 # @ECLASS-VARIABLE: EBZR_UPDATE_CMD
 # @DESCRIPTION:
 # The Bazaar command to update the sources.
-: ${EBZR_UPDATE_CMD:="bzr pull"}
+: ${EBZR_UPDATE_CMD:="bzr pull --overwrite-tags"}
 
 # @ECLASS-VARIABLE: EBZR_EXPORT_CMD
 # @DESCRIPTION:
-- 
2.16.1

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

Re: [gentoo-dev] [PATCH] bzr.eclass: Add --overwrite-tags option to pull command.

[Kristian Fiskerstrand]

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

On 02/06/2018 02:57 PM, Ulrich Müller wrote:
> Fixes: https://bugs.gentoo.org/446422

Bug or Closes, Fixes would be referencing a commit-id c.f GLEP 66.

More generally though, should we start requiring more verbose commit
messages for eclasses to make it easier to trace changes in our git repo
directly without reaching out to bugs? At least including summaries of
the respective bugs as a short description?

-- 
Kristian Fiskerstrand
OpenPGP keyblock reachable at hkp://pool.sks-keyservers.net
fpr:94CB AFDD 3034 5109 5618 35AA 0B7F 8B60 E3ED FAE3


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

Re: [gentoo-dev] [PATCH] bzr.eclass: Add --overwrite-tags option to pull command.

[Ulrich Mueller]

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

>>>>> On Tue, 6 Feb 2018, Kristian Fiskerstrand wrote:

>> Fixes: https://bugs.gentoo.org/446422

> Bug or Closes, Fixes would be referencing a commit-id c.f GLEP 66.

Updated, thanks for catching this.

> More generally though, should we start requiring more verbose commit
> messages for eclasses to make it easier to trace changes in our git
> repo directly without reaching out to bugs? At least including
> summaries of the respective bugs as a short description?

In the concrete example, the bug's summary is "bzr.eclass might need
to use bzr pull's --overwrite-tags flag" which is not much different
from the git commit message.

Ulrich

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

eclass committ message verbosity (Was: Re: [gentoo-dev] [PATCH] bzr.eclass: Add --overwrite-tags option to pull command.)

[Kristian Fiskerstrand]

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

On 02/06/2018 03:36 PM, Ulrich Mueller wrote:
>>>>>> On Tue, 6 Feb 2018, Kristian Fiskerstrand wrote:


>> More generally though, should we start requiring more verbose commit
>> messages for eclasses to make it easier to trace changes in our git
>> repo directly without reaching out to bugs? At least including
>> summaries of the respective bugs as a short description?
> 
> In the concrete example, the bug's summary is "bzr.eclass might need
> to use bzr pull's --overwrite-tags flag" which is not much different
> from the git commit message.

Right, this specific commit likely has little more to gain given the
summary line. But could easily benefit from some text in body still :)

But I generally think we can benefit from some more verbosity in our
commit messages, in particular for eclasses.

-- 
Kristian Fiskerstrand
OpenPGP keyblock reachable at hkp://pool.sks-keyservers.net
fpr:94CB AFDD 3034 5109 5618 35AA 0B7F 8B60 E3ED FAE3


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

Re: [gentoo-dev] [PATCH] bzr.eclass: Add --overwrite-tags option to pull command.

[Alec Warner]

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

On Tue, Feb 6, 2018 at 6:12 AM, Kristian Fiskerstrand <k_f@gentoo.org>
wrote:

> On 02/06/2018 02:57 PM, Ulrich Müller wrote:
> > Fixes: https://bugs.gentoo.org/446422
>
> Bug or Closes, Fixes would be referencing a commit-id c.f GLEP 66.
>
> More generally though, should we start requiring more verbose commit
> messages for eclasses to make it easier to trace changes in our git repo
> directly without reaching out to bugs? At least including summaries of
> the respective bugs as a short description?
>

Nope, people can go read the bug.

-A


>
> --
> Kristian Fiskerstrand
> OpenPGP keyblock reachable at hkp://pool.sks-keyservers.net
> fpr:94CB AFDD 3034 5109 5618 35AA 0B7F 8B60 E3ED FAE3
>
>

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

Re: [gentoo-dev] [PATCH] bzr.eclass: Add --overwrite-tags option to pull command.

[Michał Górny]

W dniu wto, 06.02.2018 o godzinie 09∶07 -0800, użytkownik Alec Warner
napisał:
> On Tue, Feb 6, 2018 at 6:12 AM, Kristian Fiskerstrand <k_f@gentoo.org>
> wrote:
> 
> > On 02/06/2018 02:57 PM, Ulrich Müller wrote:
> > > Fixes: https://bugs.gentoo.org/446422
> > 
> > Bug or Closes, Fixes would be referencing a commit-id c.f GLEP 66.
> > 
> > More generally though, should we start requiring more verbose commit
> > messages for eclasses to make it easier to trace changes in our git repo
> > directly without reaching out to bugs? At least including summaries of
> > the respective bugs as a short description?
> > 
> 
> Nope, people can go read the bug.
> 

That's very nice of you. I do really enjoy when I have a log of like 30
different commits that might be relevant and whose descriptions boil
down to 'go see 30 bugs, and hope you won't have to read all comments'.

-- 
Best regards,
Michał Górny

[gentoo-dev] [PATCH] bzr.eclass: Drop bzr_bootstrap and bzr_src_prepare.

[Ulrich Müller]

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

No ebuild in the Gentoo repository uses the bzr_bootstrap
functionality. Therefore drop the function along with bzr_src_prepare
(which would not have worked in EAPI 6 anyway, due to missing call to
eapply_user). After this change, inheriting eutils is not needed any
more.

Drop support for EAPIs 0 and 1 for further simplification.
---

This is to be applied on top of the patch posted earlier today.

We could go for bzr-r1.eclass instead. However, the functionality
associated with EBZR_PATCHES and EBZR_BOOTSTRAP is not used in the
tree, and overall the eclass is little used. Therefore I think that
applying these changes in-place is acceptable here.

 eclass/bzr.eclass | 99 ++++++++-----------------------------------------------
 1 file changed, 13 insertions(+), 86 deletions(-)

diff --git a/eclass/bzr.eclass b/eclass/bzr.eclass
index 9b3e0b6cf4dc..710b64db28c0 100644
--- a/eclass/bzr.eclass
+++ b/eclass/bzr.eclass
@@ -18,21 +18,20 @@
 # Note: Just set EBZR_REPO_URI to the URI of the branch and src_unpack()
 # of this eclass will export the branch to ${WORKDIR}/${P}.
 
-inherit eutils
-
 EBZR="bzr.eclass"
 
-case "${EAPI:-0}" in
-	0|1) EXPORT_FUNCTIONS src_unpack ;;
-	*)   EXPORT_FUNCTIONS src_unpack src_prepare ;;
+case ${EAPI:-0} in
+	2|3|4|5|6) ;;
+	*) die "${EBZR}: EAPI ${EAPI:-0} is not supported" ;;
 esac
 
-DEPEND=">=dev-vcs/bzr-2.6.0"
-case "${EAPI:-0}" in
-	0|1) ;;
-	*) [[ ${EBZR_REPO_URI%%:*} = sftp ]] \
-		&& DEPEND=">=dev-vcs/bzr-2.6.0[sftp]" ;;
-esac
+EXPORT_FUNCTIONS src_unpack
+
+if [[ ${EBZR_REPO_URI%%:*} = sftp ]]; then
+	DEPEND=">=dev-vcs/bzr-2.6.0[sftp]"
+else
+	DEPEND=">=dev-vcs/bzr-2.6.0"
+fi
 
 # @ECLASS-VARIABLE: EBZR_STORE_DIR
 # @DESCRIPTION:
@@ -85,9 +84,8 @@ esac
 # @DESCRIPTION:
 # The repository URI for the source package.
 #
-# Note: If the ebuild uses an sftp:// URI, then in EAPI 0 or 1 it must
-# make sure that dev-vcs/bzr was built with USE="sftp".  In EAPI 2 or
-# later, the eclass will depend on dev-vcs/bzr[sftp].
+# Note: If the ebuild uses an sftp:// URI, then the eclass will depend
+# on dev-vcs/bzr[sftp].
 
 # @ECLASS-VARIABLE: EBZR_INITIAL_URI
 # @DEFAULT_UNSET
@@ -100,21 +98,6 @@ esac
 #
 # Normally, this variable needs not be set.
 
-# @ECLASS-VARIABLE: EBZR_BOOTSTRAP
-# @DEFAULT_UNSET
-# @DESCRIPTION:
-# Bootstrap script or command like autogen.sh or etc.
-
-# @ECLASS-VARIABLE: EBZR_PATCHES
-# @DEFAULT_UNSET
-# @DESCRIPTION:
-# bzr.eclass can apply patches in bzr_bootstrap().  You can use regular
-# expressions in this variable like *.diff or *.patch and the like.
-# Note: These patches will be applied before EBZR_BOOTSTRAP is processed.
-#
-# Patches are searched both in ${PWD} and ${FILESDIR}.  If not found in
-# either location, the installation dies.
-
 # @ECLASS-VARIABLE: EBZR_PROJECT
 # @DESCRIPTION:
 # The project name of your ebuild.  Normally, the branch will be stored
@@ -276,65 +259,9 @@ bzr_fetch() {
 	popd > /dev/null
 }
 
-# @FUNCTION: bzr_bootstrap
-# @DESCRIPTION:
-# Apply patches in ${EBZR_PATCHES} and run ${EBZR_BOOTSTRAP} if specified.
-bzr_bootstrap() {
-	local patch lpatch
-
-	pushd "${S}" > /dev/null || die "${EBZR}: can't chdir to ${S}"
-
-	if [[ -n ${EBZR_PATCHES} ]] ; then
-		einfo "apply patches -->"
-
-		for patch in ${EBZR_PATCHES} ; do
-			if [[ -f ${patch} ]] ; then
-				epatch "${patch}"
-			else
-				# This loop takes care of wildcarded patches given via
-				# EBZR_PATCHES in an ebuild
-				for lpatch in "${FILESDIR}"/${patch} ; do
-					if [[ -f ${lpatch} ]] ; then
-						epatch "${lpatch}"
-					else
-						die "${EBZR}: ${patch} is not found"
-					fi
-				done
-			fi
-		done
-	fi
-
-	if [[ -n ${EBZR_BOOTSTRAP} ]] ; then
-		einfo "begin bootstrap -->"
-
-		if [[ -f ${EBZR_BOOTSTRAP} ]] && [[ -x ${EBZR_BOOTSTRAP} ]] ; then
-			einfo "   bootstrap with a file: ${EBZR_BOOTSTRAP}"
-			"./${EBZR_BOOTSTRAP}" \
-				|| die "${EBZR}: can't execute EBZR_BOOTSTRAP"
-		else
-			einfo "   bootstrap with commands: ${EBZR_BOOTSTRAP}"
-			"${EBZR_BOOTSTRAP}" \
-				|| die "${EBZR}: can't eval EBZR_BOOTSTRAP"
-		fi
-	fi
-
-	popd > /dev/null
-}
-
 # @FUNCTION: bzr_src_unpack
 # @DESCRIPTION:
-# Default src_unpack(), calls bzr_fetch.  For EAPIs 0 and 1, also calls
-# bzr_src_prepare.
+# Default src_unpack(), calls bzr_fetch.
 bzr_src_unpack() {
 	bzr_fetch
-	case "${EAPI:-0}" in
-		0|1) bzr_src_prepare ;;
-	esac
-}
-
-# @FUNCTION: bzr_src_prepare
-# @DESCRIPTION:
-# Default src_prepare(), calls bzr_bootstrap.
-bzr_src_prepare() {
-	bzr_bootstrap
 }
-- 
2.16.1

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

Re: [gentoo-dev] [PATCH] bzr.eclass: Drop bzr_bootstrap and bzr_src_prepare.

[Michał Górny]

W dniu śro, 07.02.2018 o godzinie 00∶24 +0100, użytkownik Ulrich Müller
napisał:
> No ebuild in the Gentoo repository uses the bzr_bootstrap
> functionality. Therefore drop the function along with bzr_src_prepare
> (which would not have worked in EAPI 6 anyway, due to missing call to
> eapply_user). After this change, inheriting eutils is not needed any
> more.
> 
> Drop support for EAPIs 0 and 1 for further simplification.
> ---
> 
> This is to be applied on top of the patch posted earlier today.
> 
> We could go for bzr-r1.eclass instead. However, the functionality
> associated with EBZR_PATCHES and EBZR_BOOTSTRAP is not used in the
> tree, and overall the eclass is little used. Therefore I think that
> applying these changes in-place is acceptable here.
> 
>  eclass/bzr.eclass | 99 ++++++++-----------------------------------------------
>  1 file changed, 13 insertions(+), 86 deletions(-)
> 
> diff --git a/eclass/bzr.eclass b/eclass/bzr.eclass
> index 9b3e0b6cf4dc..710b64db28c0 100644
> --- a/eclass/bzr.eclass
> +++ b/eclass/bzr.eclass
> @@ -18,21 +18,20 @@
>  # Note: Just set EBZR_REPO_URI to the URI of the branch and src_unpack()
>  # of this eclass will export the branch to ${WORKDIR}/${P}.
>  
> -inherit eutils
> -
>  EBZR="bzr.eclass"
>  
> -case "${EAPI:-0}" in
> -	0|1) EXPORT_FUNCTIONS src_unpack ;;
> -	*)   EXPORT_FUNCTIONS src_unpack src_prepare ;;
> +case ${EAPI:-0} in
> +	2|3|4|5|6) ;;
> +	*) die "${EBZR}: EAPI ${EAPI:-0} is not supported" ;;
>  esac
>  
> -DEPEND=">=dev-vcs/bzr-2.6.0"
> -case "${EAPI:-0}" in
> -	0|1) ;;
> -	*) [[ ${EBZR_REPO_URI%%:*} = sftp ]] \
> -		&& DEPEND=">=dev-vcs/bzr-2.6.0[sftp]" ;;
> -esac
> +EXPORT_FUNCTIONS src_unpack
> +
> +if [[ ${EBZR_REPO_URI%%:*} = sftp ]]; then
> +	DEPEND=">=dev-vcs/bzr-2.6.0[sftp]"
> +else
> +	DEPEND=">=dev-vcs/bzr-2.6.0"
> +fi
>  
>  # @ECLASS-VARIABLE: EBZR_STORE_DIR
>  # @DESCRIPTION:
> @@ -85,9 +84,8 @@ esac
>  # @DESCRIPTION:
>  # The repository URI for the source package.
>  #
> -# Note: If the ebuild uses an sftp:// URI, then in EAPI 0 or 1 it must
> -# make sure that dev-vcs/bzr was built with USE="sftp".  In EAPI 2 or
> -# later, the eclass will depend on dev-vcs/bzr[sftp].
> +# Note: If the ebuild uses an sftp:// URI, then the eclass will depend
> +# on dev-vcs/bzr[sftp].
>  
>  # @ECLASS-VARIABLE: EBZR_INITIAL_URI
>  # @DEFAULT_UNSET
> @@ -100,21 +98,6 @@ esac
>  #
>  # Normally, this variable needs not be set.
>  
> -# @ECLASS-VARIABLE: EBZR_BOOTSTRAP
> -# @DEFAULT_UNSET
> -# @DESCRIPTION:
> -# Bootstrap script or command like autogen.sh or etc.
> -
> -# @ECLASS-VARIABLE: EBZR_PATCHES
> -# @DEFAULT_UNSET
> -# @DESCRIPTION:
> -# bzr.eclass can apply patches in bzr_bootstrap().  You can use regular
> -# expressions in this variable like *.diff or *.patch and the like.
> -# Note: These patches will be applied before EBZR_BOOTSTRAP is processed.
> -#
> -# Patches are searched both in ${PWD} and ${FILESDIR}.  If not found in
> -# either location, the installation dies.
> -
>  # @ECLASS-VARIABLE: EBZR_PROJECT
>  # @DESCRIPTION:
>  # The project name of your ebuild.  Normally, the branch will be stored
> @@ -276,65 +259,9 @@ bzr_fetch() {
>  	popd > /dev/null
>  }
>  
> -# @FUNCTION: bzr_bootstrap
> -# @DESCRIPTION:
> -# Apply patches in ${EBZR_PATCHES} and run ${EBZR_BOOTSTRAP} if specified.
> -bzr_bootstrap() {
> -	local patch lpatch
> -
> -	pushd "${S}" > /dev/null || die "${EBZR}: can't chdir to ${S}"
> -
> -	if [[ -n ${EBZR_PATCHES} ]] ; then
> -		einfo "apply patches -->"
> -
> -		for patch in ${EBZR_PATCHES} ; do
> -			if [[ -f ${patch} ]] ; then
> -				epatch "${patch}"
> -			else
> -				# This loop takes care of wildcarded patches given via
> -				# EBZR_PATCHES in an ebuild
> -				for lpatch in "${FILESDIR}"/${patch} ; do
> -					if [[ -f ${lpatch} ]] ; then
> -						epatch "${lpatch}"
> -					else
> -						die "${EBZR}: ${patch} is not found"
> -					fi
> -				done
> -			fi
> -		done
> -	fi
> -
> -	if [[ -n ${EBZR_BOOTSTRAP} ]] ; then
> -		einfo "begin bootstrap -->"
> -
> -		if [[ -f ${EBZR_BOOTSTRAP} ]] && [[ -x ${EBZR_BOOTSTRAP} ]] ; then
> -			einfo "   bootstrap with a file: ${EBZR_BOOTSTRAP}"
> -			"./${EBZR_BOOTSTRAP}" \
> -				|| die "${EBZR}: can't execute EBZR_BOOTSTRAP"
> -		else
> -			einfo "   bootstrap with commands: ${EBZR_BOOTSTRAP}"
> -			"${EBZR_BOOTSTRAP}" \
> -				|| die "${EBZR}: can't eval EBZR_BOOTSTRAP"
> -		fi
> -	fi
> -
> -	popd > /dev/null
> -}
> -
>  # @FUNCTION: bzr_src_unpack
>  # @DESCRIPTION:
> -# Default src_unpack(), calls bzr_fetch.  For EAPIs 0 and 1, also calls
> -# bzr_src_prepare.
> +# Default src_unpack(), calls bzr_fetch.
>  bzr_src_unpack() {
>  	bzr_fetch
> -	case "${EAPI:-0}" in
> -		0|1) bzr_src_prepare ;;
> -	esac
> -}
> -
> -# @FUNCTION: bzr_src_prepare
> -# @DESCRIPTION:
> -# Default src_prepare(), calls bzr_bootstrap.
> -bzr_src_prepare() {
> -	bzr_bootstrap
>  }

Hmm, unless I'm mistaken, this can cause another definition
of src_prepare to start applying to ebuilds. You can submit a PR with
this change to get md5-cache with exported phase data suitable for
comparison.

Or... given the popularity of the eclass, you can check by hand ;-P.

-- 
Best regards,
Michał Górny

Re: [gentoo-dev] [PATCH] bzr.eclass: Drop bzr_bootstrap and bzr_src_prepare.

[Ulrich Mueller]

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

>>>>> On Wed, 07 Feb 2018, Michał Górny wrote:

>> -# @FUNCTION: bzr_src_prepare
>> -# @DESCRIPTION:
>> -# Default src_prepare(), calls bzr_bootstrap.
>> -bzr_src_prepare() {
>> -	bzr_bootstrap
>> }

> Hmm, unless I'm mistaken, this can cause another definition
> of src_prepare to start applying to ebuilds.

That's right, but wasn't relying on inherit order considered a QA
violation? In other words, shouldn't an ebuild define an explicit
phase function if it inherits more than one eclass exporting that
function?

> You can submit a PR with this change to get md5-cache with exported
> phase data suitable for comparison.

> Or... given the popularity of the eclass, you can check by hand ;-P.

Done so for the Gentoo repo (before posting the patch). Or rather,
I have checked that WORKDIR has identical contents after the prepare
phase.

Ulrich

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

Re: [gentoo-dev] [PATCH] bzr.eclass: Drop bzr_bootstrap and bzr_src_prepare.

[Michał Górny]

W dniu śro, 07.02.2018 o godzinie 14∶18 +0100, użytkownik Ulrich Mueller
napisał:
> > > > > > On Wed, 07 Feb 2018, Michał Górny wrote:
> > > -# @FUNCTION: bzr_src_prepare
> > > -# @DESCRIPTION:
> > > -# Default src_prepare(), calls bzr_bootstrap.
> > > -bzr_src_prepare() {
> > > -	bzr_bootstrap
> > > }
> > Hmm, unless I'm mistaken, this can cause another definition
> > of src_prepare to start applying to ebuilds.
> 
> That's right, but wasn't relying on inherit order considered a QA
> violation? In other words, shouldn't an ebuild define an explicit
> phase function if it inherits more than one eclass exporting that
> function?

Not that I know of. However, I was actually more worried about ebuilds
whose authors didn't even notice it.

> > You can submit a PR with this change to get md5-cache with exported
> > phase data suitable for comparison.
> > Or... given the popularity of the eclass, you can check by hand ;-P.
> 
> Done so for the Gentoo repo (before posting the patch). Or rather,
> I have checked that WORKDIR has identical contents after the prepare
> phase.

Good enough for me.

-- 
Best regards,
Michał Górny