Re: [gentoo-dev] bzr.eclass changes, please review

["Rick \"Zero_Chaos\" Farina" <zerochaos@gentoo.org>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 09/14/2012 05:01 AM, Ulrich Mueller wrote:
> In bug 434746 it has been suggested that ${WORKDIR} should look like a
> bzr branch or checkout. Proposed changes for bzr.eclass are included
> below, comprising the following:
> 
> - bzr_fetch can optionally call bzr checkout --lightweight instead of
>   bzr export. The default behaviour won't change, the new behaviour
>   can be enabled with:
> - New variable EBZR_WORKDIR_CHECKOUT, (Or can anyone come up with a
>   better/shorter name?)
> - New variable EBZR_CHECKOUT_CMD.
> - The sandbox environment is now always restored; before it was only
>   restored if ${EBZR_STORE_DIR} didn't exist. This is to prevent the
>   package's build system from writing to ${EBZR_STORE_DIR} when it's
>   calling bzr (which wasn't an issue for an export, but could be for
>   a checkout).
> - Unrelated to the above, some old cleanup code (around line 220) is
>   removed.
> 
> The updated bzr.eclass is available in the emacs overlay, along with
> an app-editors/emacs-vcs ebuild that I've used for testing.
> 
> Ulrich
> 
> 
> --- bzr.eclass	18 Jul 2012 15:12:54 -0000	1.18
> +++ bzr.eclass	14 Sep 2012 08:02:08 -0000
> @@ -61,6 +61,11 @@
>  # The Bazaar command to export a branch.
>  : ${EBZR_EXPORT_CMD:="bzr export"}
>  
> +# @ECLASS-VARIABLE: EBZR_CHECKOUT_CMD
> +# @DESCRIPTION:
> +# The Bazaar command to checkout a branch.
> +: ${EBZR_CHECKOUT_CMD:="bzr checkout --lightweight -q"}
> +
>  # @ECLASS-VARIABLE: EBZR_REVNO_CMD
>  # @DESCRIPTION:
>  # The Bazaar command to list a revision number of the branch.
> @@ -145,6 +150,12 @@
>  # by users.
>  : ${EBZR_OFFLINE=${EVCS_OFFLINE}}
>  
> +# @ECLASS-VARIABLE: EBZR_WORKDIR_CHECKOUT
> +# @DEFAULT_UNSET
> +# @DESCRIPTION:
> +# If this variable is set to a non-empty value, EBZR_CHECKOUT_CMD will
> +# be used instead of EBZR_EXPORT_CMD to copy the sources to WORKDIR.
> +
>  # @FUNCTION: bzr_initial_fetch
>  # @USAGE: <repository URI> <branch directory>
>  # @DESCRIPTION:
> @@ -196,11 +207,11 @@
>  # working copy.
>  bzr_fetch() {
>  	local repo_dir branch_dir
> +	local save_sandbox_write=${SANDBOX_WRITE}
>  
>  	[[ -n ${EBZR_REPO_URI} ]] || die "${EBZR}: EBZR_REPO_URI is empty"
>  
>  	if [[ ! -d ${EBZR_STORE_DIR} ]] ; then
> -		local save_sandbox_write=${SANDBOX_WRITE}
>  		addwrite /
Am I reading this wrong, or is "addwrite /" being more than just a
little lazy?  I know this isn't part of your change set but it has
always bothered me that it needs to unlocking writing on the whole
filesystem to save something in distdir.

Thanks,
Zero
>  		mkdir -p "${EBZR_STORE_DIR}" \
>  			|| die "${EBZR}: can't mkdir ${EBZR_STORE_DIR}"
> @@ -215,19 +226,6 @@
>  
>  	addwrite "${EBZR_STORE_DIR}"
>  
> -	# Clean up if the existing local copy is a checkout (as was the case
> -	# with an older version of bzr.eclass).
> -	# This test can be removed after 1 Mar 2012.
> -	if [[ ${EBZR_FETCH_CMD} != *checkout* && -d ${repo_dir}/.bzr/checkout ]]
> -	then
> -		local tmpname=$(mktemp -u "${repo_dir}._old_.XXXXXX")
> -		ewarn "checkout from old version of ${EBZR} found, moving it to:"
> -		ewarn "${tmpname}"
> -		ewarn "you may manually remove it"
> -		mv "${repo_dir}" "${tmpname}" \
> -			|| die "${EBZR}: can't move old checkout out of the way"
> -	fi
> -
>  	if [[ ! -d ${branch_dir}/.bzr ]]; then
>  		if [[ ${repo_dir} != "${branch_dir}" && ! -d ${repo_dir}/.bzr ]]; then
>  			einfo "creating shared bzr repository: ${repo_dir}"
> @@ -252,14 +250,23 @@
>  		bzr_update "${EBZR_REPO_URI}" "${branch_dir}"
>  	fi
>  
> +	# Restore sandbox environment
> +	SANDBOX_WRITE=${save_sandbox_write}
> +
>  	cd "${branch_dir}" || die "${EBZR}: can't chdir to ${branch_dir}"
>  
>  	# Save revision number in environment. #311101
>  	export EBZR_REVNO=$(${EBZR_REVNO_CMD})
>  
> -	einfo "exporting ..."
> -	${EBZR_EXPORT_CMD} ${EBZR_REVISION:+-r ${EBZR_REVISION}} \
> -		"${WORKDIR}/${P}" . || die "${EBZR}: export failed"
> +	if [[ -n ${EBZR_WORKDIR_CHECKOUT} ]]; then
> +		einfo "checking out ..."
> +		${EBZR_CHECKOUT_CMD} ${EBZR_REVISION:+-r ${EBZR_REVISION}} \
> +			. "${WORKDIR}/${P}" || die "${EBZR}: checkout failed"
> +	else
> +		einfo "exporting ..."
> +		${EBZR_EXPORT_CMD} ${EBZR_REVISION:+-r ${EBZR_REVISION}} \
> +			"${WORKDIR}/${P}" . || die "${EBZR}: export failed"
> +	fi
>  	einfo "revision ${EBZR_REVISION:-${EBZR_REVNO}} is now in ${WORKDIR}/${P}"
>  
>  	popd > /dev/null
> 
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQUy3iAAoJEKXdFCfdEflKGn8P/2nWVsOWor82RWYneDHpxaFU
U0rZn7D3PPNCLhNpIH9/61BlJsarOW7MuFSgTwTRlAnyKN8gLMyOYvr5bBi9URgT
7rC6pKxy+tH9EoyemZks9SoTjSQk6d+jZLx+0nvFUqvqtK+Elp0qs4FyxyWDqq0j
ZL3dunSIcNHhekQYbuaIJQ6bCBTUyqtUTLyzIWRveoHaGPE0Nl5r7y/LJOeOA4JX
my+3UyK6snWvpuUqYjEXjTTRvZivoD1hoX9ALtFOFquzQ6ITHYAWSYgfGP96cTAo
+IiSt2DgSCz1wnGyXPgRgj7I0cijtBZ/ozkIZVfEAdbyzatVkOh+vkepF3Fw8698
1V5SDjqqPCEtHTcLEvTD5Q7yUhx805N78v47GsYAdwul3ZCFCW8nNUdzbCEIQXWD
QXwOZQ9wsYFhedJgbO6Sd68au4OP4L1yL+u/+wdyag5ipnBRcCMuInYmL8n2Orkq
UZR+XI6QURauCqg/MoaiUcbRbE6uUQarke93uU0hR3odZlBDk3Evo9Vvs+veK5Cq
a9VJig3PD+b+EjFfenDlwptI3oBXtd0NMZgkqL3NrOAxsz4osDj7Un9TXqbdHHt7
LAlIgAxWjGZBjGDoj+uFT5DBb+bhI2g0XHik96O5+47oDqw4VORtdOyo0/oo1p14
FdwcD7TA9Xb/WDqCKicg
=dB/J
-----END PGP SIGNATURE-----