Fix generated merge commit on workspace redirect (#1306)

The :workspace filter compares the workspace mapping
between commits and creates a merge commit with the
history of newly mapped paths.

This did not work correctly in case the parents workspace
mapping was a redirect, in which case it considered all
paths as newly mapped even though they are already part
of the history.

Now the redirect is taken into account when parting the
parents workspace definition and we get correct history.

Change: fix-redirect-merge

Author: christian-schilling

josh-core/src/filter/mod.rs

@@ -384,14 +384,17 @@ fn resolve_workspace_redirect<'a>(
     repo: &'a git2::Repository,
     tree: &'a git2::Tree<'a>,
     path: &Path,
-) -> Option<Filter> {
+) -> Option<(Filter, std::path::PathBuf)> {
     let f = parse::parse(&tree::get_blob(repo, tree, &path.join("workspace.josh")))
         .unwrap_or_else(|_| to_filter(Op::Empty));
 
-    if let Op::Workspace(_) = to_op(f) {
-        Some(chain(
-            to_filter(Op::Exclude(to_filter(Op::File(path.to_owned())))),
-            f,
+    if let Op::Workspace(p) = to_op(f) {
+        Some((
+            chain(
+                to_filter(Op::Exclude(to_filter(Op::File(path.to_owned())))),
+                f,
+            ),
+            p,
         ))
     } else {
         None
@@ -570,7 +573,8 @@ fn apply_to_commit2(
                 .map(|parent| transaction.get(filter, parent))
                 .collect::<Option<Vec<git2::Oid>>>();
 
-            if let Some(redirect) = resolve_workspace_redirect(repo, &commit.tree()?, ws_path) {
+            if let Some((redirect, _)) = resolve_workspace_redirect(repo, &commit.tree()?, ws_path)
+            {
                 if let Some(r) = apply_to_commit2(&to_op(redirect), &commit, transaction)? {
                     transaction.insert(filter, commit.id(), r, true);
                     return Ok(Some(r));
@@ -588,17 +592,22 @@ fn apply_to_commit2(
                 .map(|parent| {
                     rs_tracing::trace_scoped!("parent", "id": parent.id().to_string());
 
+                    let p = if let Some((_, p)) =
+                        resolve_workspace_redirect(repo, &parent.tree()?, ws_path)
+                    {
+                        p
+                    } else {
+                        ws_path.clone()
+                    };
+
                     let pcw = get_workspace(
                         repo,
                         &parent.tree().unwrap_or_else(|_| tree::empty(repo)),
-                        ws_path,
+                        &p,
                     );
+                    let f = opt::optimize(to_filter(Op::Subtract(cw, pcw)));
 
-                    apply_to_commit2(
-                        &to_op(opt::optimize(to_filter(Op::Subtract(cw, pcw)))),
-                        &parent,
-                        transaction,
-                    )
+                    apply_to_commit2(&to_op(f), &parent, transaction)
                 })
                 .collect::<JoshResult<Option<Vec<_>>>>()?;
 
@@ -764,7 +773,7 @@ fn apply2<'a>(
             let base = to_filter(Op::Subdir(path.to_owned()));
             let wsj_file = chain(base, wsj_file);
 
-            if let Some(redirect) = resolve_workspace_redirect(repo, &tree, path) {
+            if let Some((redirect, _)) = resolve_workspace_redirect(repo, &tree, path) {
                 return apply(transaction, redirect, tree);
             }
 

tests/filter/workspace_redirect.t

@@ -24,6 +24,19 @@
   $ git add ws
   $ git commit -m "add ws" 1> /dev/null
 
+  $ mkdir sub3
+  $ echo contents3 > sub3/file4
+  $ git add sub3
+  $ git commit -m "add file4" 1> /dev/null
+
+  $ cat > ws/workspace.josh <<EOF
+  > ::sub2/subsub/
+  > a = :/sub1
+  > b = :/sub3
+  > EOF
+  $ git add ws
+  $ git commit -m "edit ws" 1> /dev/null
+
   $ mkdir ws_new
   $ echo "foobar" > ws_new/extra_file_new
   $ cat > ws_new/workspace.josh <<EOF
@@ -33,25 +46,35 @@
   $ git commit -m "add ws_new" 1> /dev/null
 
   $ josh-filter -s :workspace=ws master --update refs/heads/filtered
+  [1] :prefix=b
+  [2] :/sub3
   [2] :[
       a = :/sub1
       ::sub2/subsub/
   ]
-  [2] :workspace=ws
+  [3] :workspace=ws
   $ josh-filter -s :workspace=ws_new master --update refs/heads/filtered_new
+  [1] :prefix=b
   [1] :workspace=ws_new
+  [2] :/sub3
   [2] :[
       a = :/sub1
       ::sub2/subsub/
   ]
-  [2] :workspace=ws
-  [3] :exclude[::ws_new]
+  [3] :workspace=ws
+  [5] :exclude[::ws_new]
 
   $ git log --graph --pretty=%s refs/heads/filtered
+  *   edit ws
+  |\  
+  | * add file4
   * add ws
   * add file2
   * add file1
   $ git log --graph --pretty=%s refs/heads/filtered_new
+  *   edit ws
+  |\  
+  | * add file4
   * add ws
   * add file2
   * add file1
@@ -71,6 +94,13 @@
   +++ b/a/file4
   @@ -0,0 +1 @@
   +contents4
+  diff --git a/b/file4 b/b/file4
+  new file mode 100644
+  index 0000000..1cb5d64
+  --- /dev/null
+  +++ b/b/file4
+  @@ -0,0 +1 @@
+  +contents3
   diff --git a/extra_file b/extra_file
   new file mode 100644
   index 0000000..323fae0
@@ -87,12 +117,13 @@
   +contents1
   diff --git a/workspace.josh b/workspace.josh
   new file mode 100644
-  index 0000000..68bf391
+  index 0000000..795cb6d
   --- /dev/null
   +++ b/workspace.josh
-  @@ -0,0 +1,2 @@
+  @@ -0,0 +1,3 @@
   +::sub2/subsub/
   +a = :/sub1
+  +b = :/sub3
   $ git diff ${EMPTY_TREE}..refs/heads/filtered_new
   diff --git a/a/file1 b/a/file1
   new file mode 100644
@@ -108,6 +139,13 @@
   +++ b/a/file4
   @@ -0,0 +1 @@
   +contents4
+  diff --git a/b/file4 b/b/file4
+  new file mode 100644
+  index 0000000..1cb5d64
+  --- /dev/null
+  +++ b/b/file4
+  @@ -0,0 +1 @@
+  +contents3
   diff --git a/extra_file b/extra_file
   new file mode 100644
   index 0000000..323fae0
@@ -124,12 +162,13 @@
   +contents1
   diff --git a/workspace.josh b/workspace.josh
   new file mode 100644
-  index 0000000..68bf391
+  index 0000000..795cb6d
   --- /dev/null
   +++ b/workspace.josh
-  @@ -0,0 +1,2 @@
+  @@ -0,0 +1,3 @@
   +::sub2/subsub/
   +a = :/sub1
+  +b = :/sub3
 
 
   $ cat > ws/workspace.josh <<EOF
@@ -139,11 +178,13 @@
   $ git commit -m "add ws recursion" 1> /dev/null
 
   $ josh-filter -s :workspace=ws master --update refs/heads/filtered
+  [1] :prefix=b
+  [2] :/sub3
   [2] :[
       a = :/sub1
       ::sub2/subsub/
   ]
-  [2] :workspace=ws_new
-  [3] :exclude[::ws]
-  [3] :exclude[::ws_new]
-  [3] :workspace=ws
+  [3] :workspace=ws_new
+  [4] :exclude[::ws]
+  [4] :workspace=ws
+  [6] :exclude[::ws_new]