Re: [gentoo-dev] [PATCH] git-r3.eclass: respect EVCS_UMASK

[Ulrich Mueller]

>>>>> On Sun, 1 Feb 2015, I wrote:

>>>>> On Sun, 1 Feb 2015, Michał Górny wrote:

>>> +           local restore_umask=":"
>>> +           if [[ ${EVCS_UMASK} ]]; then
>>> +                   restore_umask=$(umask -p)
>>> +                   umask "${EVCS_UMASK}" || die "Bad options to umask: 
>>> ${EVCS_UMASK}"
>>> +           fi
>>> mkdir "${GIT_DIR}" || die
>>> git init --bare || die
>>> +           ${restore_umask} || die

>> And this has ugly implicit pattern expansion. Don't do such things.
>> Ever. Even if you want to split commands.

> Please show me how this could possibly cause any problem here.
> restore_umask can only have well-defined values, either ":" or
> the output of "umask -p" which is intended to be used this way.

Anyway, since you don't seem to like the way it is coded, I've
attached an updated patch.

Ulrich


Index: git-r3.eclass
===================================================================
RCS file: /var/cvsroot/gentoo-x86/eclass/git-r3.eclass,v
retrieving revision 1.47
diff -u -B -p -r1.47 git-r3.eclass
--- git-r3.eclass       28 Jul 2014 14:13:50 -0000      1.47
+++ git-r3.eclass       2 Feb 2015 07:34:09 -0000
@@ -131,6 +131,17 @@ fi
 # @DESCRIPTION:
 # If non-empty, this variable prevents any online operations.
 
+# @ECLASS-VARIABLE: EVCS_UMASK
+# @DEFAULT_UNSET
+# @DESCRIPTION:
+# Set this variable to a custom umask. This is intended to be set by
+# users. By setting this to something like 002, it can make life easier
+# for people who do development as non-root (but are in the portage
+# group), and then switch over to building with FEATURES=userpriv.
+# Or vice-versa. Shouldn't be a security issue here as anyone who has
+# portage group write access already can screw the system over in more
+# creative ways.
+
 # @ECLASS-VARIABLE: EGIT_BRANCH
 # @DEFAULT_UNSET
 # @DESCRIPTION:
@@ -309,8 +320,16 @@ _git-r3_set_gitdir() {
 
        addwrite "${EGIT3_STORE_DIR}"
        if [[ ! -d ${GIT_DIR} ]]; then
+               local saved_umask
+               if [[ ${EVCS_UMASK} ]]; then
+                       saved_umask=$(umask)
+                       umask "${EVCS_UMASK}" || die "Bad options to umask: 
${EVCS_UMASK}"
+               fi
                mkdir "${GIT_DIR}" || die
                git init --bare || die
+               if [[ ${saved_umask} ]]; then
+                       umask "${saved_umask}" || die
+               fi
        fi
 }
 
@@ -507,7 +526,11 @@ git-r3_fetch() {
        fi
 
        # try to fetch from the remote
-       local r success
+       local r success saved_umask
+       if [[ ${EVCS_UMASK} ]]; then
+               saved_umask=$(umask)
+               umask "${EVCS_UMASK}" || die "Bad options to umask: 
${EVCS_UMASK}"
+       fi
        for r in "${repos[@]}"; do
                einfo "Fetching ${r} ..."
 
@@ -668,6 +691,9 @@ git-r3_fetch() {
                        break
                fi
        done
+       if [[ ${saved_umask} ]]; then
+               umask "${saved_umask}" || die
+       fi
        [[ ${success} ]] || die "Unable to fetch from any of EGIT_REPO_URI"
 
        # submodules can reference commits in any branch

pgp9EwitgkORW.pgp
Description: PGP signature