On Tue, Jun 25, 2013 at 09:46:30AM +0100, Tiago Gomes wrote:
[snip]
diff --git a/baserock-system-config-sync/baserock-system-config-sync
b/baserock-system-config-sync/baserock-system-config-sync
new file mode 100755
index 0000000..3114f90
--- /dev/null
+++ b/baserock-system-config-sync/baserock-system-config-sync
@@ -0,0 +1,199 @@
+#!/bin/sh
+#
+# Copyright (c) 2013 Codethink Ltd.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License Version 2 as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+set -eu
+
+usage() {
+ echo "Usage: $(basename $0) test V1_DIR VU_DIR V2_DIR VT_DIR" >&2
+ echo " $(basename $0) merge NEW_VERSION_LABEL" >&2
+ exit 1
While usage should be terse, the names of variables make it useless in
my opinion. This usage is also missing the sync option.
+}
+
+
+merge() {
+ local vp_dir="$1" # version beeing processed
Typo, should be `being`.
+ local v1_dir="$2" # factory version
+ local vu_dir="$3" # user modified version
+ local v2_dir="$4" # unmodified new deployed version
+ local vt_dir="$5" # target version where the result of the
+ # merge should be placed
Rather than having short names and a comment describing what it is,
I'd prefer to have the variable names be long and descriptive.
Also technically `local foo=bar` is not portable, but if it works for
both bash and busybox ash I don't mind.
+
+ for f in `find "$vp_dir/"*`; do
Is there any particular reason why you're doing `find $dir/*` instead of
`find $dir`? If so it should be commented in my opinion.
for f in `...` is not always the best way to do a loop. For example,
this will need to wait for the find command to complete, and some shells
can't handle this list being too long.
An alternative iterator is `find "$vp_dir/"* | while read f; do`.
This form has the disadvantage that it will not allow you to alter
variables in the enclosing environment, at which point the less readable
form of `while read f; do ...; done < <(find "$vp_dir/" *)` can be used.
+ # strip first component from file name
+ local stripped_filename=${f#$vp_dir/}
+
+ local vp="$vp_dir/$stripped_filename"
+ local v1="$v1_dir/$stripped_filename"
+ local vu="$vu_dir/$stripped_filename"
+ local v2="$v2_dir/$stripped_filename"
+ local vt="$vt_dir/$stripped_filename"
+
+ if [ -e "$vt" ]; then
+ # If the file already exists in the target,
+ # check if it is of the same kind
+ error_msg="ERROR: '$stripped_filename' does not have the \
+same type in all versions"
+ if [ -f "$vp" ] && [ ! -f "$vt" ]; then
This check can be done in the same test block as `[ -f "$vp" -a ! -f
"$vt"]`,
but this may be the wrong approach given what you want to achieve.
+ echo "$error_msg"
+ exit 1
A common idiom that may be more readable in this case is to define a
die function for this:
die () {
echo $@ >&2
exit 1
}
> + fi
> + if [ -d "$vp" ] && [ ! -d "$vt" ]; then
+ echo "$error_msg"
+ exit 1
> + fi
> + if [ -h "$vp" ] && [ ! -h "$vt" ]; then
+ echo "$error_msg"
+ exit 1
> + fi
If these checks are all for whether they are the same type, then the
following may be easier to read.
if [ "$(stat -c %F "$vp")" != "$(stat -c %F
"$vt")" ]; then
die $error_msg
fi
+ elif [ -d "$vp" ] && [ ! -h
"$vp" ]; then
+ mkdir "$vt"
+ elif [ -h "$vp" ]; then
+ # Discussion: what we want to do about symbolic links?
+ # I chose a symbolic link in this order of preference:
+ # Vuser, V2, V1
+ if [ -h "$vu" ]; then
+ cp -a "$vu" "$vt"
+ elif [ -h "$v2" ]; then
+ cp -a "$v2" "$vt"
+ else
+ cp -a "$v1" "$vt"
+ fi
+ elif [ -f "$vp" ]; then
+ merge_regular_file "$v1" "$vu" "$v2"
"$vt"
+ else
+ echo "ERROR: unexpected error"
+ exit 1
+ fi
+ done
+}
+
+
+merge_regular_file() {
+ local v1="$1"
+ local vu="$2"
+ local v2="$3"
+ local vt="$4"
+ # Reference table for merging a regular file
+ #
+ # V1 Vuser V2 action
+ # ------------------------------------------
+ # none none none inconceivable!
+ # exists none none use V1
+ # none exists none use Vuser
+ # none none exists use V2
+ # exists none exists use V2
+ # exists exists none use V2
+ # none exists exists diff V2 Vuser applied to V2
+ # exists exists exists diff V1 Vuser2 applied to V2
The following conditionals are hard to read.
To be honest, I'd try to follow the table more closely, rather than
v1_exists=$(test -f "$v1" && echo exists || echo none)
v2_exists=$(test -f "$v2" && echo exists || echo none)
vu_exists=$(test -f "$vu" && echo exists || echo none)
case "$v1_exists $v2_exists $vu_exists" in
'exists none none')
cp -a "$v1" "$vt"
;;
'none exists none')
cp -a "$vu" "$vt"
;;
...
*)
die Unexpected error
;;
esac
+ if [ -f "$v1" ] && [ ! -f "$vu" ]
&& [ ! -f "$v2" ]; then
+ cp -a "$v1" "$vt"
+ elif [ ! -f "$v1" ] && [ -f "$vu" ] && [ ! -f
"$v2" ]; then
+ cp -a "$vu" "$vt"
+ elif [ ! -f "$v1" ] && [ ! -f "$vu" ] && [ -f
"$v2" ]; then
+ cp -a "$v2" "$vt"
+ elif [ -f "$v1" ] && [ ! -f "$vu" ] && [ -f
"$v2" ]; then
+ cp -a "$v2" "$vt"
+ elif [ -f "$v1" ] && [ -f "$vu" ] && [ ! -f
"$v2" ]; then
+ # In the relevant mustard node it is specified that
+ # when we have v1 and vu, but not v2, we shouln't
+ # do nothing. I changed to copy the user configuration
+ # instead
+ cp -a "$vu" "$vt"
+ elif [ ! -f "$v1" ] && [ -f "$vu" ] && [ -f
"$v2" ]; then
+ # In the relevant mustard node it is specified a diff
+ # between /dev/null and vu. However this causes a
+ # complait agains reversed patches. The option
+ # "-R" didn't help.
+ if ! (diff "$v2" "$vu" | patch "$v2" -o
"$vt"); then
+ cp -a "$v2" "$vt" # merge failed, use v2
+ fi
+ elif [ -f "$v1" ] && [ -f "$vu" ] && [ -f
"$v2" ]; then
+ if ! (diff "$v1" "$vu" | patch "$v2" -o
"$vt"); then
+ cp -a "$v2" "$vt" # merge failed, use v2
+ fi
+ else
+ echo "ERROR: unexpected error"
+ exit 1
+ fi
+}
+
+
+if [ "$#" = 0 ]; then
+ usage "$0"
+fi
+
+
+mounting_script="$(dirname $0)/../libexec/mount-system-versions-dir"
$0 should be quoted here.
I would put this at the beginning, rather than wait until now to work
this out.
+
+if [ "$1" = "test" ]; then
+ if [ "$#" != 5 ]; then
+ usage "$0"
+ fi
+ mkdir -p "$5"
+ # For every pathname in V1, Vuser, or V2
+ merge "$2" "$2" "$3" "$4" "$5"
+ merge "$3" "$2" "$3" "$4" "$5"
+ merge "$4" "$2" "$3" "$4" "$5"
It's difficult to read this with positional arguments, give them useful
names like you have elsewhere.
+elif [ "$1" = "merge" ]; then
+ if [ "$#" != 2 ]; then
+ usage "$0"
+ fi
+ local new_version="$2"
+ local mouting_point=$(mktemp -d)
+ "$mounting_script" "$mouting_point"
+ if [ ! -d "$mouting_point/systems/$new_version" ]; then
+ echo "Error: version not found - '$new_version'"
+ exit 1
+ fi
+ local v1_dir="$mouting_point/systems/default/orig/etc"
+ local vu_dir="$mouting_point/systems/default/run/etc"
+ local v2_dir="$mouting_point/systems/$new_version/run/etc"
+ local vt_dir="$mouting_point/systems/$new_version/run/etc.new"
+ mkdir "$vt_dir"
+
+ # For every pathname in V1, Vuser, or V2
+ merge "$v1_dir" "$v1_dir" "$vu_dir"
"$v2_dir" "$vt_dir"
+ merge "$vu_dir" "$v1_dir" "$vu_dir"
"$v2_dir" "$vt_dir"
+ merge "$v2_dir" "$v1_dir" "$vu_dir"
"$v2_dir" "$vt_dir"
+
+ rm -rf "$v2_dir"
+ mv "$vt_dir" "$v2_dir"
+ umount "$mouting_point"
+elif [ "$1" = "sync" ]; then
+ local canonical_version="$2"
+ local mouting_point=$(mktemp -d)
+ "$mounting_script" "$mouting_point"
+ if [ ! -d "$mouting_point/systems/$canonical_version" ]; then
+ echo "Error: version not found - '$canonical_version'"
+ exit 1
+ fi
+ for version in $(ls "$mouting_point/systems"); do
ls is for users, not scripts, use "$mounting_point/systems/"*.
+ version_dir="$mouting_point/systems/$version"
+ if [ -d "$version_dir" ] && [ ! -h "$version_dir" ]
\
+ && [ $version != $canonical_version ]; then
+ cp -a "$mouting_point/systems/$canonical_version/run/etc" \
+ "$version_dir/run/etc.new"
+ mv "$version_dir/run/etc" "$version_dir/run/etc.old"
+ mv "$version_dir/run/etc.new" "$version_dir/run/etc"
+ rm -rf "$version_dir/run/etc.old"
+ fi
+ done
+ umount "$mouting_point"
If the script fails then it will have left the disk mounted somewhere
in /tmp. After running "$mounting_script", run `trap 'umount
"$mounting_point"' INT ERR EXIT`, so it will unmount if something fails.
[snip]