FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Gentoo > Gentoo Development

 
 
LinkBack Thread Tools
 
Old 09-14-2012, 09:01 AM
Ulrich Mueller
 
Default bzr.eclass changes, please review

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 /
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
 
Old 09-14-2012, 01:15 PM
"Rick "Zero_Chaos" Farina"
 
Default bzr.eclass changes, please review

-----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+Elp 0qs4FyxyWDqq0j
ZL3dunSIcNHhekQYbuaIJQ6bCBTUyqtUTLyzIWRveoHaGPE0Nl 5r7y/LJOeOA4JX
my+3UyK6snWvpuUqYjEXjTTRvZivoD1hoX9ALtFOFquzQ6ITHY AWSYgfGP96cTAo
+IiSt2DgSCz1wnGyXPgRgj7I0cijtBZ/ozkIZVfEAdbyzatVkOh+vkepF3Fw8698
1V5SDjqqPCEtHTcLEvTD5Q7yUhx805N78v47GsYAdwul3ZCFCW 8nNUdzbCEIQXWD
QXwOZQ9wsYFhedJgbO6Sd68au4OP4L1yL+u/+wdyag5ipnBRcCMuInYmL8n2Orkq
UZR+XI6QURauCqg/MoaiUcbRbE6uUQarke93uU0hR3odZlBDk3Evo9Vvs+veK5Cq
a9VJig3PD+b+EjFfenDlwptI3oBXtd0NMZgkqL3NrOAxsz4osD j7Un9TXqbdHHt7
LAlIgAxWjGZBjGDoj+uFT5DBb+bhI2g0XHik96O5+47oDqw4VO RtdOyo0/oo1p14
FdwcD7TA9Xb/WDqCKicg
=dB/J
-----END PGP SIGNATURE-----
 
Old 09-14-2012, 03:51 PM
Ulrich Mueller
 
Default bzr.eclass changes, please review

>>>>> On Fri, 14 Sep 2012, Rick "Zero Chaos" Farina wrote:

>> 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.

EBZR_STORE_DIR may be redefined, so you cannot be sure that it's below
DISTDIR. Also, it's not known how many of its path components already
exist. I'm not sure if it would be worth the effort to compute a more
accurate argument for addwrite. (Almost all live eclasses do that
"addwrite /", by the way.)

>> mkdir -p "${EBZR_STORE_DIR}"
>> || die "${EBZR}: can't mkdir ${EBZR_STORE_DIR}"

Ulrich
 
Old 09-14-2012, 04:12 PM
"Rick "Zero_Chaos" Farina"
 
Default bzr.eclass changes, please review

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

On 09/14/2012 11:51 AM, Ulrich Mueller wrote:
>>>>>> On Fri, 14 Sep 2012, Rick "Zero Chaos" Farina wrote:
>
>>> 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.
>
> EBZR_STORE_DIR may be redefined, so you cannot be sure that it's below
> DISTDIR. Also, it's not known how many of its path components already
> exist. I'm not sure if it would be worth the effort to compute a more
> accurate argument for addwrite. (Almost all live eclasses do that
> "addwrite /", by the way.)
I didn't mean to pick on bzr.eclass, I think it's always wrong to do
this. And you picked out the exact reasoning I did "I'm not sure if it
would be worth the effort to compute a more accurate argument for
addwrite." I think it is worth the effort to do it right. I mean
(purposeful exaggeration here) we could save the addwrite entirely by
just "killall sandbox" or we could prevent from reoccurring by
restricting the sandbox feature. Any time you do "addwrite /" you
completely defeat the entire purpose of sandbox. It's not write (get it?).

I'm not saying this is an emergency nor should it hold back any changes
you need to make to argue with me about it. However, if you were to do
it right that would be cool. Otherwise we could all start fixing our
sandbox issues by just doing "addwrite /" at the top of all ebuilds.

Thanks,
Zero
>
>>> mkdir -p "${EBZR_STORE_DIR}"
>>> || die "${EBZR}: can't mkdir ${EBZR_STORE_DIR}"
>
> Ulrich
>
>

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

iQIcBAEBAgAGBQJQU1djAAoJEKXdFCfdEflKlecP/jVjGN76yb/VJ+5/O75lwwhf
7p7pppohVw4gfaiiHOz0uamu37RPPqlmc+LV4TgUxQ+n6VMkdr XG7tU0TTewYEEw
mTorfsF5i2aFXJXMRIT5Xz+P+iEx9m4bhbKuxjL17DKs4F9jKT oao/1ix2lhNKZX
ClnAp+qgKNN2fj/1vnBNkGSIfWF9EEkpfCqC0d2kXLADXJCNqaOmUmVI9+5XXAFw
7X+DW1JMRgg64yQ78bztHmmKqQ211WK3JqqmZPxb4bms5RQPCI LBjc+OQ97KKwBW
woXfV+gZInTrfvgADEFCLeDSKuEojqriu7UrXCBkwNkDEzVdsL WXD25lUzo2bmPP
nuUOjBsIiFJAt8UdpAn+y7hDVy5BfClE24FxVSk+ydthlkGNW6 T0tMEikBhserJ5
bi0qF7qOp6Wu+OVS0a+de2ptcy6z/AVm8ziDSY70mX32GqW0APkft+yvzluibpXZ
a37c+zYoytWb1GK5ijC8I29xi5GDilouaX+DMVz2woZChEhYuZ 8ElKpHzHgI1HZs
dZirnsFjZt4jBBk2iB8NUHyccze4XpmSnwd75w0ltNaAs1IhTA TxIbbkRokQJ0F7
FteSyd/0jzvRGIIgpTpJBtga4UMkRZxpnG/2OIKn0hy+4tvotwUknwzWDGEdCKQq
5k4jnZXA364VLoeYOXeK
=0AWM
-----END PGP SIGNATURE-----
 
Old 09-14-2012, 10:26 PM
Mike Gilbert
 
Default bzr.eclass changes, please review

On Fri, Sep 14, 2012 at 12:12 PM, Rick "Zero_Chaos" Farina
<zerochaos@gentoo.org> wrote:
> I didn't mean to pick on bzr.eclass, I think it's always wrong to do
> this. And you picked out the exact reasoning I did "I'm not sure if it
> would be worth the effort to compute a more accurate argument for
> addwrite." I think it is worth the effort to do it right. I mean
> (purposeful exaggeration here) we could save the addwrite entirely by
> just "killall sandbox" or we could prevent from reoccurring by
> restricting the sandbox feature. Any time you do "addwrite /" you
> completely defeat the entire purpose of sandbox. It's not write (get it?).
>
> I'm not saying this is an emergency nor should it hold back any changes
> you need to make to argue with me about it. However, if you were to do
> it right that would be cool. Otherwise we could all start fixing our
> sandbox issues by just doing "addwrite /" at the top of all ebuilds.
>

The sandbox is mostly useful to prevent build systems from messing
with the live system without the developer's knowledge. It is
perfectly reasonable to disable the sandbox for a single mkdir call
that we have direct control over.
 

Thread Tools




All times are GMT. The time now is 08:43 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org