aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFrancois Kritzinger <francois@codesynthesis.com>2020-09-04 15:36:18 +0200
committerFrancois Kritzinger <francois@codesynthesis.com>2020-09-11 10:05:12 +0200
commitc36a0b09e93a470de0b7f9d564f5896fe9a4ba96 (patch)
tree824f81b8cf84fd89885172e228fa861812a5b001
parent2d8bd54ef820332f7e6254b2b30f0eade14cfc57 (diff)
Post-review fixesmanage
-rw-r--r--bpkg-rep/manage.in201
1 files changed, 137 insertions, 64 deletions
diff --git a/bpkg-rep/manage.in b/bpkg-rep/manage.in
index e3e824d..eb6ad51 100644
--- a/bpkg-rep/manage.in
+++ b/bpkg-rep/manage.in
@@ -118,7 +118,7 @@ for s in "${src_sections[@]}"; do
done < <(find "$src_dir/$s" -type f -not -name "*.manifest")
done
-if [ -n "$src_owners" ] && [ -d "$src_dir/$src_owners" ]; then
+if [ -n "$src_owners" -a -d "$src_dir/$src_owners" ]; then
while read f; do
src_files+=("${f#$src_dir/}")
done < <(find "$src_dir/$src_owners" -type f)
@@ -257,17 +257,25 @@ declare -A bundle
# All files added by commits in the bundle must be from the same project and,
# in the case of archives, the same repository section.
#
-# Print true if the migration has been successful and empty string if the
-# above assumption is not true, the commit bundle is empty, the required
-# section does not exist in the destination repository, or any file has an
-# invalid path (for example, missing a valid project or section component).
+# @@ Can't go with printing "true" or "" instead of returning 0 or 1 because
+# the $(f) syntax calls the function from a subshell with the result that
+# the modifications it makes to 'bundle' and 'pending_seq' are not seen
+# here, in the parent shell. And there doesn't seem to be any other way of
+# returning a string from a function than the s="$(f)" syntax.
+#
+# Set the global 'migrate_result' to true if the migration has been
+# successful, or to the empty string if the above assumption is not true, the
+# commit bundle is empty, the required section does not exist in the
+# destination repository, or any file has an invalid path (for example,
+# missing a valid project or section component).
#
function migrate ()
{
+ migrate_result=
+
if [ "${#bundle[@]}" -eq 0 ]; then
info "no commits selected"
- echo
- return 0 # @@ TODO other places
+ return
fi
# Check that every commit's added files are in the bundle section and/or
@@ -282,8 +290,8 @@ function migrate ()
#
local sect= # The bundle section.
local proj= # The bundle project.
- local archives=() # The bundle's archive files. @@ pkgs
- local manifests=() # The bundle's ownership manifests. @@ owns
+ local pkgs=() # The bundle's archive files.
+ local owns=() # The bundle's ownership manifests.
for i in "${!bundle[@]}"; do
local h="${pending_seq[i-1]}" # The current commit's abbreviated hash.
@@ -301,20 +309,14 @@ function migrate ()
# associative array which maps to the section directory extracted from
# the file path.
#
- # @@ Is it ok to use builtin regexes ("[[ =~ ]]") instead of sed? Yes
- # Builtin regexes are also used in
- # brep/brep/handler/submit/submit-git.in and
- # private/buildos/hosts.bash so there is at least some precedent.
- #
- # I think let's stick with [ -a ] using () inside.
- #
local fsect= # Current file's section.
local fproj= # Current file's project.
+
if [ -n "$src_owners" ] && [[ "$f" =~ ^"$src_owners"/([^/]+)/.+$ ]]; then
- manifests+=("$f")
+ owns+=("$f")
fproj="${BASH_REMATCH[1]}"
elif [[ "$f" =~ ^(.+)/([^/]+)/[^/]+$ ]]; then # Package archive?
- archives+=("$f")
+ pkgs+=("$f")
local fsect_dir="${BASH_REMATCH[1]}"
fproj="${BASH_REMATCH[2]}"
@@ -337,7 +339,7 @@ function migrate ()
if [ -z "$fsect" ]; then
info "unable to find section name for file '$f'"
- return 1
+ return
fi
# Set the bundle section if unset; otherwise fail if the current file
@@ -347,11 +349,11 @@ function migrate ()
sect="$fsect"
elif [ "$fsect" != "$sect" ]; then
info "cannot include commit $i: '$f' is not in section $sect"
- return 1
+ return
fi
else
info "unrecognized type of file '$f'"
- return 1
+ return
fi
# Set the bundle project if unset; otherwise fail if the current file is
@@ -363,14 +365,14 @@ function migrate ()
proj="$fproj"
elif [ "$fproj" != "$proj" ]; then
info "cannot include commit $i: '$f' is not in project $proj"
- return 1
+ return
fi
done < <(commit_files "$h")
done
# If it exists, 'testing' overrides 'stable' at the destination.
#
- if [ "$sect" == "stable" ] && [ -v dst_sections["testing"] ]; then
+ if [ "$sect" == "stable" -a -v dst_sections["testing"] ]; then
sect="testing"
fi
@@ -378,11 +380,9 @@ function migrate ()
# there are no archive files in the bundle (in which case the bundle section
# name will be empty) and fail otherwise.
#
- # @@ Are we to use [ A ] && [ B ] or [ A -a B ]? A: [ ( A ) -a ( B ) ]
- #
- if [ -n "$sect" ] && [ ! -v dst_sections["$sect"] ]; then
+ if [ -n "$sect" -a ! -v dst_sections["$sect"] ]; then
info "bundle section '$sect' does not exist in the destination repository"
- return 1
+ return
fi
# Migrate the bundle's files.
@@ -392,11 +392,11 @@ function migrate ()
#
# @@ TODO Do the actual migration.
#
- for f in "${archives[@]}"; do
+ for f in "${pkgs[@]}"; do
info "migrating '$f' to section $sect"
done
- for f in "${manifests[@]}"; do
+ for f in "${owns[@]}"; do
if [ -n "$dst_owners" ]; then
info "migrating '$f'"
else
@@ -413,8 +413,7 @@ function migrate ()
pending_seq=("${pending_seq[@]}") # Remove the gaps created by unset.
bundle=()
- echo true
- return 0
+ migrate_result=true
}
# Push local changes to the remote source and/or destination git repositories.
@@ -503,26 +502,13 @@ while true; do
# but not added back (that is, it's no longer in the repository). So we
# mark the re-added file with an exclamation.
#
- # @@ Should we mark files that no longer exist in the repository (that is,
- # deleted by a subsequent commit) with a different symbol than ones that
- # were deleted by a subsequent commit and then added back? For example:
- #
- # commit 1: add foo, add bar
- # commit 2: del foo
- #
- # vs.
- #
- # commit 1: add foo, add bar
- # commit 2: del foo
- # commit 3: add foo
- #
- # Currently we mark foo in both cases with '*' for commit 1.
- #
while read -d '' f; do
if [ "${file_commits[$f]}" == "$h" ]; then
- info " $f"
+ info " $f" # File was last added by the current commit.
+ elif [ -v file_commits["$f"] ]; then
+ info " ! $f" # File was deleted and added back by subsequent commits.
else
- info " * $f"
+ info " * $f" # File was deleted but not added back.
fi
done < <(commit_files "$h")
done
@@ -539,10 +525,15 @@ while true; do
while true; do
# Sort commit bundle in ascending order.
#
- # @@ Let's try to redo with sed.
+ # The '${!bundle[*]}' expression expands to a single word consisting of
+ # each of the 'bundle' array's keys separated by IFS=\n, thus suitable as
+ # input to 'sort', which is newline-based. Then the 'bundle_sorted=()'
+ # expression splits its argument, sort's output, using IFS=\n as
+ # delimiter, into an array which is finally expanded again as
+ # '${bundle_sorted[*]}' after IFS has been unset, resulting in a single
+ # word consisting of the array elements separated by spaces.
#
- IFS=$'\n' bundle_sorted=($(sort - <<<"${!bundle[*]}"))
- unset IFS
+ bundle_sorted=($(sed 's/ /\n/g' <<<"${!bundle[*]}" | sort -))
printf "\n"
echo -n "[${bundle_sorted[*]}][<N>,m,c,p,l,q,?]: "
@@ -553,22 +544,99 @@ while true; do
# Add commit to bundle.
#
[0-9]*)
- if [ ( "$opt" -gt 0 ) -a ( "$opt " -le "${#pending_seq[@]}" ) ]; then
+ # @@ Are we to use [ A ] && [ B ] or [ A -a B ]?
+ # Answer: [ ( A ) -a ( B ) ].
+ #
+ # Though, () has actually to be escaped (see below for details).
+ #
+ # The consensus in the bash community seems to be to prefer && and
+ # || over -a and -o. Some reasons and motivation:
+ #
+ # - [ A ] && [ B ] is commonly recommended by what seems to me to be
+ # reputable sources:
+ # - https://wiki.bash-hackers.org/commands/classictest#the_prefered_way
+ # https://wiki.bash-hackers.org/commands/classictest#why_you_should_avoid_using_-a_and_-o
+ # - http://mywiki.wooledge.org/BashGuide/TestsAndConditionals
+ # "Don't ever use the -a or -o tests of the [ command. Use
+ # multiple [ commands instead (or use [[ if you can). POSIX
+ # doesn't define the behavior of [ with complex sets of tests,
+ # so you never know what you'll get.
+ # - http://mywiki.wooledge.org/BashFAQ/017
+ # - And lots of answers on StackOverflow
+ #
+ # - test's -a and -o options have been marked obsolescent by POSIX:
+ # https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html. By
+ # implication POSIX prefers [ ] && [ ].
+ #
+ # - POSIX does not specify the behaviour of test in cases where
+ # there are more than 4 arguments.
+ #
+ # - && and || are part of the shell language syntax, while -a and
+ # -o are just options of the test command. Using shell language
+ # syntax everywhere is an option, so why not make use of it? For
+ # example:
+ #
+ # - if [ A ] && [ B ]; then
+ # - if command1 && command2; then
+ # - if [[ A && B ]]; then
+ # or, if we want to be 100% consistent with [:
+ # - if [[ A ]] && [[ B ]]; then
+ #
+ # At a syntactic level, [ ] && [ ] is identical to command1 &&
+ # command2, which is idiomatic in all shells, including POSIX.
+ #
+ # - () has to be escaped inside []:
+ # [ \( EXPR \) -a \( EXPR \) ]
+ #
+ # but not inside [[ ]]:
+ #
+ # [[ (EXPR) && (EXPR) ]]
+ #
+ # nor at the topmost level, but there it's better to use {}
+ # because () runs in a subshell:
+ #
+ # if ( [ A ] || [ B ] ) && ( [ C ] || [ D ] ); then # subshell
+ #
+ # if { [ A ] || [ B ]; } && { [ C ] || [ D ]; }; then # this shell
+ #
+ # It is also quite generally recommended to just use [[ all the
+ # time (unless portability is super important, although it is
+ # also supported by KornShell and Zsh), in which case:
+ #
+ # if [[ (A || B) && (C || D) ]]; then
+ #
+ # Shorter and no escapes or semicolons before } required.
+ #
+ # - && and || short-circuit, whereas -a and -o do not.
+ #
+ # - [[ doesn't understand -a or -o, so we'd have to be inconsistent
+ # in cases where we're forced to use [[:
+ # [ A -a B ] and [[ A && B ]] in same script
+ #
+ # Using [[ in the following 'if' condition would become:
+ #
+ # if [[ ("$opt" -gt 0) && ("$opt " -le "${#pending_seq[@]}") ]]
+ #
+ # No escapes and no spaces required after/before ( and ). This is
+ # supported as far back as bash 3.0.
+ #
+ if [ \( "$opt" -gt 0 \) -a \( "$opt " -le "${#pending_seq[@]}" \) ]; then
opt=$(printf "%.3d" "$opt") # Format as in pending commit list.
if [ ! -v bundle["$opt"] ]; then
bundle["$opt"]=true
+ info "commit $opt (${pending_seq[$opt-1]}) added to selected bundle"
else
info "commit $opt is already in the bundle"
fi
else
info "non-existent commit number $opt"
fi
- #@@ TODO: print "commit 1 (deadbeef) added to selected bundle"
;;
# Migrate the commit bundle.
#
m)
- if [ "$(migrate)" ]; then
+ migrate
+ if [ "$migrate_result" ]; then
need_push=true
break
fi
@@ -602,16 +670,21 @@ while true; do
echo -n "push changes? [y/n/(c)ancel]: "
read opt
- # @@ Redo as case (use continue on *).
- #
- if [ "$opt" == "c" ]; then
- break; # Print options menu again.
- elif [ "$opt" == "y" ]; then
- push
- exit 0
- elif [ "$opt" == "n" ]; then
- exit 0
- fi
+ case "$opt" in
+ "c")
+ break # Print options menu again.
+ ;;
+ "y")
+ push
+ exit 0
+ ;;
+ "n")
+ exit 0
+ ;;
+ *)
+ continue
+ ;;
+ esac
done
;;
# ? or invalid option: print menu.