From 8c3da8abdb16ecad007dc60068deb90e151737ea Mon Sep 17 00:00:00 2001
From: Karen Arutyunov <karen@codesynthesis.com>
Date: Mon, 21 Sep 2020 17:52:52 +0300
Subject: Fix submission handler issues arising due to package archive name
 ambiguity

Now we take into account the fact that the foo-bar-1.0.tar.gz archive may
contain either foo-bar/1.0 or foo/bar-1.0 package.
---
 brep/handler/buildfile                 |  1 +
 brep/handler/handler.bash.in           |  7 +++++
 brep/handler/submit/submit-git.bash.in | 17 +++++++-----
 brep/handler/submit/submit-git.in      | 29 +++++---------------
 brep/handler/submit/submit-pub.in      | 41 +++++++++++++++-------------
 manifest                               |  1 +
 repositories.manifest                  |  4 +++
 tests/submit/submit-git.testscript     | 49 ++++++++++++++++++++++++++++++----
 tests/submit/submit-pub.testscript     | 18 ++++++++++---
 9 files changed, 111 insertions(+), 56 deletions(-)

diff --git a/brep/handler/buildfile b/brep/handler/buildfile
index b351ca3..00c072e 100644
--- a/brep/handler/buildfile
+++ b/brep/handler/buildfile
@@ -3,6 +3,7 @@
 
 import mods  = libbutl.bash%bash{manifest-parser}
 import mods += libbutl.bash%bash{manifest-serializer}
+import mods += bpkg-rep%bash{package-archive}
 
 ./: bash{handler} submit/ ci/
 
diff --git a/brep/handler/handler.bash.in b/brep/handler/handler.bash.in
index 1169b99..0aa3f69 100644
--- a/brep/handler/handler.bash.in
+++ b/brep/handler/handler.bash.in
@@ -12,6 +12,9 @@ fi
 @import libbutl/manifest-parser@
 @import libbutl/manifest-serializer@
 
+bpkg_rep_bpkg=bpkg
+@import bpkg-rep/package-archive@
+
 # Diagnostics.
 #
 # We expect the user to set the verbose variable either to true or empty
@@ -148,3 +151,7 @@ function manifest_serialize () # <name> <value>
 #  trace "$1: $2"
   printf "%s:%s\0" "$1" "$2" >&"$manifest_serializer_ifd"
 }
+
+function pkg_verify_archive () { bpkg_rep_pkg_verify_archive "$@"; }
+function pkg_find_archives ()  { bpkg_rep_pkg_find_archives "$@"; }
+function pkg_find_archive ()   { bpkg_rep_pkg_find_archive "$@"; }
diff --git a/brep/handler/submit/submit-git.bash.in b/brep/handler/submit/submit-git.bash.in
index 9f25c28..983d7b5 100644
--- a/brep/handler/submit/submit-git.bash.in
+++ b/brep/handler/submit/submit-git.bash.in
@@ -77,15 +77,20 @@ function check_package_duplicate () # <name> <version> <repo-dir>
   #
   local s
   for s in "${!sections[@]}"; do
-    local d="$rep/${sections[$s]}"
+    local p
+    IFS=$'\n' eval \
+    'p=($(run pkg_find_archive "$nam-$ver.*" "$rep/${sections[$s]}"))'
 
-    if [ -d "$d" ]; then
-      local f
-      f="$(run find "$d" -name "$nam-$ver.*")"
+    if [ "${#p[@]}" -ne 0 ]; then
+      local n="${p[0]}"
+      local v="${p[1]}"
 
-      if [ -n "$f" ]; then
-        trace "found: $f"
+      trace "found: $n/$v in ${p[3]}"
+
+      if [ "$n" == "$nam" ]; then
         exit_with_manifest 422 "duplicate submission"
+      else
+        exit_with_manifest 422 "submission conflicts with $n/$v"
       fi
     fi
   done
diff --git a/brep/handler/submit/submit-git.in b/brep/handler/submit/submit-git.in
index 54cd230..c67c30c 100644
--- a/brep/handler/submit/submit-git.in
+++ b/brep/handler/submit/submit-git.in
@@ -639,28 +639,11 @@ for i in {1..11}; do
     exit_with_manifest 400 "unrecognized section '$section'"
   fi
 
-  # Strips the version revision part, if present.
-  #
-  v="$(sed -n -re 's%^(\+?[^+]+)(\+[0-9]+)?$%\1%p' <<<"$version")"
-
-  # Make sure the section directory exists before we run find in it.
-  #
-  d="$tgt_dir/$s/$project"
-  run mkdir -p "$d" # Create all the parent directories as well.
-
-  # Go through the potentially matching archives (for example, for
-  # foo-1.2.3+2: foo-1.2.3.tar.gz, foo-1.2.3+1.tar.gz, foo-1.2.30.tar.gz, etc)
-  # and remove those that match exactly.
-  #
-  # Change CWD to the section directory to make sure that the found archive
-  # paths don't contain spaces.
-  #
-  fs=($(run cd "$tgt_dir/$s" && run find -name "$name-$v*"))
+  IFS=$'\n' eval \
+  'arcs=($(run pkg_find_archives "$name" "$version*" "$tgt_dir/$s"))'
 
-  for f in "${fs[@]}"; do
-    if [[ "$f" =~ ^\./[^/]+/"$name-$v"(\+[0-9]+)?\.[^/]+$ ]]; then
-      run git -C "$tgt_dir" rm $gqo "$s/$f" >&2
-    fi
+  for f in "${arcs[@]}"; do
+    run git -C "$tgt_dir" rm $gqo "${f#$tgt_dir/}" >&2
   done
 
   # Finally, add the package archive to the target repository.
@@ -669,8 +652,10 @@ for i in {1..11}; do
   # Make sure the project directory exists before we copy the archive into it.
   # Note that it was removed by git-rm if it became empty.
   #
+  d="$tgt_dir/$s/$project"
+  run mkdir -p "$d"        # Create all the parent directories as well.
+
   a="$d/$archive"
-  run mkdir -p "$d"                # Create all the parent directories as well.
   run cp "$data_dir/$archive" "$a"
 
   git_add "$tgt_dir" "${a#$tgt_dir/}"
diff --git a/brep/handler/submit/submit-pub.in b/brep/handler/submit/submit-pub.in
index d262ae9..d5f5c4e 100644
--- a/brep/handler/submit/submit-pub.in
+++ b/brep/handler/submit/submit-pub.in
@@ -294,8 +294,22 @@ trap exit_trap EXIT
 
 # Check for the package duplicate (in all projects).
 #
-if [ -n "$(run find "$repo_old/1" -name "$archive")" ]; then
-  exit_with_manifest 422 "duplicate submission"
+# Use <name>-<version>.* without .tar.gz in case we want to support more
+# archive types later.
+#
+IFS=$'\n' eval 'p=($(run pkg_find_archive "$name-$version.*" "$repo_old/1"))'
+
+if [ "${#p[@]}" -ne 0 ]; then
+  n="${p[0]}"
+  v="${p[1]}"
+
+  trace "found: $n/$v in ${p[3]}"
+
+  if [ "$n" == "$name" ]; then
+    exit_with_manifest 422 "duplicate submission"
+  else
+    exit_with_manifest 422 "submission conflicts with $n/$v"
+  fi
 fi
 
 # Copy the current repository using hardlinks.
@@ -310,25 +324,14 @@ fi
 run rsync -rtO --exclude 'packages.manifest' --link-dest="$repo_old" \
     "$repo_old/" "$repo_new"
 
-# Remove the package version revisions that may exist in the repository.
+# Remove the package version revision archives that may exist in the
+# repository.
 #
-# Strips the version revision part, if present.
-#
-v="$(sed -n -re 's%^(\+?[^+]+)(\+[0-9]+)?$%\1%p' <<<"$version")"
+IFS=$'\n' eval \
+'arcs=($(run pkg_find_archives "$name" "$version*" "$repo_new/1"))'
 
-# Go through the potentially matching archives (for example, for foo-1.2.3+2:
-# foo-1.2.3.tar.gz, foo-1.2.3+1.tar.gz, foo-1.2.30.tar.gz, etc) and remove
-# those that match exactly.
-#
-# Change CWD to the section directory to make sure that the found archive
-# paths don't contain spaces.
-#
-fs=($(run cd "$repo_new/1" && run find -name "$name-$v*"))
-
-for f in "${fs[@]}"; do
-  if [[ "$f" =~ ^\./[^/]+/"$name-$v"(\+[0-9]+)?\.[^/]+$ ]]; then
-    run rm "$repo_new/1/$f" >&2
-  fi
+for f in "${arcs[@]}"; do
+  run rm "$f"
 done
 
 # Copy the archive rather than moving it since we may need it for
diff --git a/manifest b/manifest
index 02dd2f0..f10e646 100644
--- a/manifest
+++ b/manifest
@@ -33,3 +33,4 @@ depends: libbutl [0.14.0-a.0.1 0.14.0-a.1)
 depends: libbpkg [0.14.0-a.0.1 0.14.0-a.1)
 depends: libbbot [0.14.0-a.0.1 0.14.0-a.1)
 depends: libbutl.bash [0.14.0-a.0.1 0.14.0-a.1)
+depends: bpkg-rep [0.14.0-a.0.1 0.14.0-a.1)
diff --git a/repositories.manifest b/repositories.manifest
index f6ba123..dccde57 100644
--- a/repositories.manifest
+++ b/repositories.manifest
@@ -19,6 +19,10 @@ location: ../libbutl.bash.git##HEAD
 
 :
 role: prerequisite
+location: ../bpkg-rep.git##HEAD
+
+:
+role: prerequisite
 location: https://git.build2.org/packaging/libapr/libapr1.git##HEAD
 
 :
diff --git a/tests/submit/submit-git.testscript b/tests/submit/submit-git.testscript
index c0a31fe..d08a47c 100644
--- a/tests/submit/submit-git.testscript
+++ b/tests/submit/submit-git.testscript
@@ -94,10 +94,10 @@ pkg_ctl="$prj_ctl/hello.git"
 : success
 :
 {
-  : ref-unknown-tgt-aquire-prj-pkg
+  : ref-unknown-tgt-acquire-prj-pkg
   :
   : Test that on the first package submission the project and package names
-  : ownership is successfully aquired. Authentication is enabled on both the
+  : ownership is successfully acquired. Authentication is enabled on both the
   : reference and target repos.
   :
   : Note that here we also test that --commiter-* options are picked up
@@ -173,7 +173,7 @@ pkg_ctl="$prj_ctl/hello.git"
   : ref-disabled-tgt-aquire-prj-pkg
   :
   : Test that on the first package submit the project and package names
-  : ownership is successfully aquired. Authentication is disabled for the
+  : ownership is successfully acquired. Authentication is disabled for the
   : reference repo.
   :
   {
@@ -203,10 +203,10 @@ pkg_ctl="$prj_ctl/hello.git"
       EOO
   }
 
-  : ref-absent-tgt-aquire-prj-pkg
+  : ref-absent-tgt-acquire-prj-pkg
   :
   : Test that on the first package submit the project and package names
-  : ownership is successfully aquired. Reference repo is absent.
+  : ownership is successfully acquired. Reference repo is absent.
   :
   : Note that here we also pass the --result-url option.
   :
@@ -436,6 +436,45 @@ pkg_ctl="$prj_ctl/hello.git"
       EOO
   }
 
+  : ref-absent-tgt-pkg-rev
+  :
+  : Test that the package revision is removed.
+  :
+  {
+    $clone_root_data;
+
+    $clone_root_tgt;
+    $g clone tgt.git &tgt/***;
+
+    cat <<EOI >=tgt/submit.config.bash;
+      sections['*']=1/alpha
+      EOI
+
+    # Add the libhello/0.1.0+1 package revision to the target repository.
+    #
+    tar -xf $data_dir/libhello-0.1.0.tar.gz &libhello-0.1.0/***;
+    sed -i -e 's/(version: 0.1.0)/\1+1/' libhello-0.1.0/manifest;
+    mv libhello-0.1.0 libhello-0.1.0+1;
+    mkdir -p tgt/1/alpha/hello/;
+    tar cfz tgt/1/alpha/hello/libhello-0.1.0+1.tar.gz libhello-0.1.0+1;
+    $g -C tgt add 1/;
+
+    $g -C tgt commit -am 'Add config and archive';
+    $g -C tgt push;
+
+    $* "file:///$~/tgt.git" $data_dir >>"EOO" &tgt/1/alpha/hello/libhello-0.1.0.tar.gz;
+      : 1
+      status: 200
+      message: package submission is queued: libhello/0.1.0
+      reference: $checksum
+      EOO
+
+    $g -C tgt pull;
+
+    test -f tgt/1/alpha/hello/libhello-0.1.0+1.tar.gz == 1;
+    test -f tgt/1/alpha/hello/libhello-0.1.0.tar.gz
+  }
+
   : section-fallback
   :
   {
diff --git a/tests/submit/submit-pub.testscript b/tests/submit/submit-pub.testscript
index b73d108..78c42b6 100644
--- a/tests/submit/submit-pub.testscript
+++ b/tests/submit/submit-pub.testscript
@@ -76,7 +76,7 @@ clone_root_rep = cp --no-cleanup -r $root_rep ./ &pkg-1/*** &?pkg.lock
 
   : for-real
   :
-  : Here we create the (fake) package revision which is expected to be removed
+  : Here we also create the package revision which is expected to be removed
   : by the handler.
   :
   {
@@ -84,8 +84,13 @@ clone_root_rep = cp --no-cleanup -r $root_rep ./ &pkg-1/*** &?pkg.lock
     $clone_root_rep;
     ln -s pkg-1 pkg;
 
-    mkdir --no-cleanup pkg-1/1/prj;
-    touch --no-cleanup pkg-1/1/prj/libhello-0.1.0+1.tar.gz;
+    # Add the libhello/0.1.0+1 package revision to the repository.
+    #
+    mkdir --no-cleanup pkg-1/1/hello;
+    tar -xf $~/$data_dir/libhello-0.1.0.tar.gz &libhello-0.1.0/***;
+    sed -i -e 's/(version: 0.1.0)/\1+1/' libhello-0.1.0/manifest;
+    mv libhello-0.1.0 libhello-0.1.0+1;
+    tar cfz pkg-1/1/hello/libhello-0.1.0+1.tar.gz libhello-0.1.0+1;
 
     $* $~/pkg $~/$data_dir &!pkg-1/*** &pkg-*/*** >>"EOO";
       : 1
@@ -94,16 +99,21 @@ clone_root_rep = cp --no-cleanup -r $root_rep ./ &pkg-1/*** &?pkg.lock
       reference: $checksum
       EOO
 
+    test -f pkg/1/hello/libhello-0.1.0+1.tar.gz == 1;
+    test -f pkg/1/hello/libhello-0.1.0.tar.gz;
+
     # While at it, test the duplicate submission.
     #
     $clone_root_data_clean;
 
-    $* $~/pkg $~/$data_dir >>"EOO"
+    $* $~/pkg $~/$data_dir >>"EOO";
       : 1
       status: 422
       message: duplicate submission
       reference: $checksum
       EOO
+
+    test -f pkg/1/hello/libhello-0.1.0.tar.gz
   }
 
   : result-url
-- 
cgit v1.1