On Fri, Feb 15, 2019 at 01:01:27PM -0800, 'Vishal Verma' wrote: > > On Fri, 2019-02-15 at 12:06 -0800, [email protected] wrote: > > From: Ira Weiny <[email protected]> > > > > make-git-snapshot.sh made an assumption of the git tree location. > > > > Furthermore, it assumed the user has an rpmbuild environment directory > > structure set up. > > > > Enhance the script to figure out where in what location it has been > > cloned and setup the rpmbuild tree if the user does not already > > have it. > > > > Signed-off-by: Ira Weiny <[email protected]> > > > > --- > > Changes since V1 > > use rpmdev-setuptree > > > > make-git-snapshot.sh | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/make-git-snapshot.sh b/make-git-snapshot.sh > > index 142419d623fe..513086b1f93d 100755 > > --- a/make-git-snapshot.sh > > +++ b/make-git-snapshot.sh > > @@ -2,10 +2,18 @@ > > set -e > > > > NAME=ndctl > > -REFDIR="$HOME/git/ndctl" # for faster cloning, if available > > + > > +pushd `dirname $0` > > +REFDIR=`pwd` > > We should use the $() syntax for command substitution. > Also quote it - pushd "$(dirname "$0")" > And for setting REFDIR, no need to execute the pwd command - just use > the $PWD variable from the environment.
Probably irrelevant with your comments below. > > Additionally, $0 is not a reliable way to get the location of the > script. It is probably best to make an assumption that the script will > be run from the top level of an ndctl tree, and test for that instead of > trying to deduce where the script is being run from and trying to cd to > it. When can $0 not be the name of the script? > > We only really need the git-version script, and if we can test for its > presence, then we can relatively safely assume we're in the right > location. If it is not a git tree then we can leave it for git-archive > to complain. So something like: > > test -x "./git-version" Ah ok... Then the stuff above is irrelevant. And I would prefer something more explicit than just returning. But the real question is if this is really supposed to be run by a user or not. If not then your way is better. diff --git a/make-git-snapshot.sh b/make-git-snapshot.sh index 513086b1f93d..fccf1da0d61b 100755 --- a/make-git-snapshot.sh +++ b/make-git-snapshot.sh @@ -3,9 +3,12 @@ set -e NAME=ndctl -pushd `dirname $0` -REFDIR=`pwd` -popd +if [ ! -x ./git-version ]; then + echo "$0 : ERROR: Must run from top level of git tree" + exit 1 +fi + +REFDIR=$PWD UPSTREAM=$REFDIR #TODO update once we have a public upstream OUTDIR=$HOME/rpmbuild/SOURCES > > should suffice. > > > +popd > > + > > UPSTREAM=$REFDIR #TODO update once we have a public upstream > > OUTDIR=$HOME/rpmbuild/SOURCES > > > > +if [ ! -d $OUTDIR ]; then > > + rpmdev-setuptree > > +fi > > + > > There was some discussion around this a while back: [1] and [2] > > But we had decided that rpmdev-setuptree was heavy handed in that it can > overwrite user configuration and we shouldn't use it. Instead in [2], we > added a test for the rpmdev tree to exist, and bail if it doesn't. > > The one improvement I can see to this is: > > if [ ! -d $OUTDIR ]; then > mkdir -p $OUTDIR > fi Dan? Ira > > That way if components of OUTDIR are missing, we can make those > directories only, which is harmless. > > [1]: https://lists.01.org/pipermail/linux-nvdimm/2017-November/013378.html > [2]: https://lists.01.org/pipermail/linux-nvdimm/2017-November/013408.html > > > [ -n "$1" ] && HEAD="$1" || HEAD="HEAD" > > > > WORKDIR="$(mktemp -d --tmpdir "$NAME.XXXXXXXXXX")" > > @@ -14,7 +22,7 @@ trap 'rm -rf $WORKDIR' exit > > [ -d "$REFDIR" ] && REFERENCE="--reference $REFDIR" > > git clone $REFERENCE "$UPSTREAM" "$WORKDIR" > > > > -VERSION=$(./git-version) > > +VERSION=$($REFDIR/git-version) > > DIRNAME="ndctl-${VERSION}" > > git archive --remote="$WORKDIR" --format=tar --prefix="$DIRNAME/" HEAD | > > gzip > $OUTDIR/"ndctl-${VERSION}.tar.gz" > > > _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
