Opened 8 years ago

Closed 8 years ago

#3488 closed defect (fixed)

make_dist.sh uses non-POSIX "mv -T"

Reported by: gdt Owned by: strk
Priority: medium Milestone: PostGIS 2.3.0
Component: build Version: master
Keywords: Cc:

Description

mv -T is not POSIX: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/mv.html and perhaps more importantly is not present in BSD-derived systems (at least NetBSD, OS X). Also the non-POSIX —backup appears. patch to follow, as this seems easy to work around.

Change History (6)

comment:1 by gdt, 8 years ago

Note that I have hand-separted this hunk from another. But it's trivial, so hopefully that doesn't hurt.

Index: make_dist.sh
===================================================================
--- make_dist.sh	(revision 14733)
+++ make_dist.sh	(working copy)
@@ -107,8 +116,10 @@
   VREV=`cat "$outdir"/postgis_svn_revision.h | awk '{print $3}'`
   version="${VMAJ}.${VMIN}.${VMIC}-r${VREV}"
   newoutdir=postgis-${version}
-  mv -vT --backup=t "$outdir" "$newoutdir"
-  outdir=${newoutdir}
+  if [ "$newoutdir" != "$outdir" ]; then
+      mv "$outdir" "$newoutdir"
+      outdir=${newoutdir}
+  fi
 fi
 
 package="postgis-$version.tar.gz"

comment:2 by strk, 8 years ago

The -T was needed to avoid moving "postgis-dev" under "postgis-2.3.0dev", when "postgis-2.3.0dev" was available (which is always the case right after running ./make_dist.sh.

I don't see how this patch addresses that case.

Maybe to simplify things we could just create a pid-dependently-named working dir…

comment:3 by gdt, 8 years ago

I see. Maybe always nuke/create EXPORT, and put the directory there, where you will know it will be empty. Or do rm -rf ${newdir} first, which is what -T seems to mean.

comment:4 by strk, 8 years ago

(In [14737]) Always remove any previously existing dist dir on ./make_dist.sh

Simplifies the code and makes it POSIX compliant. See #3488

comment:5 by strk, 8 years ago

-T does not remove the old directory, but simply refuses to consider it a target directory in which to _move_ things:

  -T, --no-target-directory
      treat DEST as a normal file

If the directory exists, it would fail.

But then we add —backup=t, which renames the target directory, if existing. Finding a name which is available, from a pattern.

Anyway, simplified the code by forcing removal of target dir with r14737. Once happy this will need to be copied over the 2.2 branch.

comment:6 by strk, 8 years ago

Milestone: PostGIS FuturePostGIS 2.3.0
Resolution: fixed
Status: newclosed

r14739 copies the trunk version to 2.2 branch. I'm closing this.

Note: See TracTickets for help on using tickets.