[node-patches] Change in ovirt-node[master]: Fix OVIRT_CACHE_DIR path issue when AUTOBUILD_SOURCE_ROOT is...

mburns at redhat.com mburns at redhat.com
Wed Mar 6 20:52:25 UTC 2013


Michael Burns has posted comments on this change.

Change subject: Fix OVIRT_CACHE_DIR path issue when AUTOBUILD_SOURCE_ROOT is not set in autobuild
......................................................................


Patch Set 1: I would prefer that you didn't submit this

(1 inline comment)

autobuild.sh is not actively used anymore by the core ovirt-node team.  We now rely almost exclusively on jenkins for builds.  It was designed to work with the autobuild framework which will always have that variable set.  

I'm willing to debate the right thing to do, but I don't think setting this to write to / if not set is *not* the right thing.

....................................................
File autobuild.sh
Line 28: fi
Line 29: 
Line 30: test -f Makefile && make -k distclean || :
Line 31: 
Line 32: OVIRT_CACHE_DIR=${AUTOBUILD_SOURCE_ROOT}/ovirt-cache
the way i read this, if AUTOBUILD_SOURCE_ROOT is not defined, the ovirt-cache will be /ovirt-cache which is likely to not be writable to anyone other than root.  

It would be better to do something like

if [ -z "${AUTOBUILD_SOURCE_ROOT}" ]; then
    OVIRT_CACHE_DIR=${HOME}/ovirt-cache
else
    OVIRT_CACHE_DIR=${AUTOBUILD_SOURCE_ROOT-${HOME}}/ovirt-cache
fi
Line 33: OVIRT_LOCAL_REPO=file://${AUTOBUILD_PACKAGE_ROOT}/rpm/RPMS
Line 34: export OVIRT_LOCAL_REPO OVIRT_CACHE_DIR
Line 35: 
Line 36: ./autogen.sh --prefix=$AUTOBUILD_INSTALL_ROOT --with-image-minimizer


--
To view, visit http://gerrit.ovirt.org/12749
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icd47b9db5ad832489a07ff8d649d0a55d903a912
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Wenyi Gao <wenyi at linux.vnet.ibm.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at fedoraproject.org>
Gerrit-Reviewer: Michael Burns <mburns at redhat.com>



More information about the node-patches mailing list