merge result when repo contains unstaged conflicting changes

[jdblischak]

Currently merge is designed to handle the following situations:

• The current branch is already up-to-date
• Merge conflict
• Fast-forward merge
• Automatic merge commit

This is reflected in the messaging:

git2r/R/merge.R

Lines 112 to 120 in 11ad162

	format.git_merge_result <- function(x, ...) {
	if (isTRUE(x$up_to_date))
	return("Already up-to-date")
	if (isTRUE(x$conflicts))
	return("Merge: Conflicts")
	if (isTRUE(x$fast_forward))
	return("Merge: Fast-forward")
	return("Merge")
	}

However, it does not handle well the situation in which there is an unstaged conflicting change. First, it somehow returns NULL for fast_forward, conflicts, and sha. In PR #321 we decided that these should always return a logical, logical, and a character vector, respectively.

Second, the message is not helpful; it only returns Merge. In contrast, here is the type of message that Git provides:

$ git merge branch
error: Your local changes to the following files would be overwritten by merge:
	test.txt
Please, commit your changes or stash them before you can merge.
Aborting

Could we update merge to handle this case? Please let me know if you'd like me to add tests and update the message returned for git_merge_result. I already tried to figure out how these values were set to NULL, but it looks like they were all properly initialized in git2r_merge.c. I assume we need to also initialize them for the scenario where there are unstaged conflicting changes.

Here is a reproducible example:

library("git2r")

## For debugging
sessionInfo()

## Create a directory in tempdir
path <- tempfile(pattern="git2r-")
dir.create(path)

## Initialize a repository
repo <- init(path)
config(repo, user.name="Alice", user.email="[email protected]")

## Create a file, add and commit
writeLines("Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do",
           con = file.path(path, "test.txt"))
add(repo, "test.txt")
commit_1 <- commit(repo, "Commit message 1")

## Create branch, checkout, change file and commit
b <- checkout(repo, branch = "branch", create = TRUE)
writeLines(c("Lorem ipsum dolor amet sit, consectetur adipisicing elit, sed do",
             "eiusmod tempor incididunt ut labore et dolore magna aliqua."),
           con = file.path(path, "test.txt"))
add(repo, "test.txt")
commit(repo, "Commit message branch")

## Checkout master and create a conflicting change
b <- branches(repo)
checkout(b[sapply(b, "[", "name") == "master"][[1]], force=TRUE)
writeLines(c("Lorem ipsum dolor sit amet, adipisicing consectetur elit, sed do",
             "eiusmod tempor incididunt ut labore et dolore magna aliqua."),
           con = file.path(path, "test.txt"))

## Attempt to merge with unstaged conflicting change
status(repo)
m_2 <- merge(repo, "branch")
str(m_2)
format(m_2)

## Cleanup
unlink(path, recursive=TRUE)

And here is what I get when I run the example using git2r built from the current master branch:

library("git2r")

## For debugging
sessionInfo()
#> R version 3.3.3 (2017-03-06)
#> Platform: x86_64-apple-darwin13.4.0 (64-bit)
#> Running under: OS X Yosemite 10.10.5
#> 
#> locale:
#> [1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8
#> 
#> attached base packages:
#> [1] stats     graphics  grDevices utils     datasets  methods   base     
#> 
#> other attached packages:
#> [1] git2r_0.25.2.9000
#> 
#> loaded via a namespace (and not attached):
#>  [1] magrittr_1.5      htmltools_0.3.6   tools_3.3.3      
#>  [4] yaml_2.2.0        Rcpp_1.0.0        stringi_1.3.1    
#>  [7] rmarkdown_1.10.14 highr_0.7         knitr_1.21       
#> [10] htmldeps_0.1.1    xfun_0.3          digest_0.6.13    
#> [13] stringr_1.4.0     evaluate_0.10.1

## Create a directory in tempdir
path <- tempfile(pattern="git2r-")
dir.create(path)

## Initialize a repository
repo <- init(path)
config(repo, user.name="Alice", user.email="[email protected]")

## Create a file, add and commit
writeLines("Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do",
           con = file.path(path, "test.txt"))
add(repo, "test.txt")
commit_1 <- commit(repo, "Commit message 1")

## Create branch, checkout, change file and commit
b <- checkout(repo, branch = "branch", create = TRUE)
writeLines(c("Lorem ipsum dolor amet sit, consectetur adipisicing elit, sed do",
             "eiusmod tempor incididunt ut labore et dolore magna aliqua."),
           con = file.path(path, "test.txt"))
add(repo, "test.txt")
commit(repo, "Commit message branch")
#> [c06de7b] 2019-05-23: Commit message branch

## Checkout master and create a conflicting change
b <- branches(repo)
checkout(b[sapply(b, "[", "name") == "master"][[1]], force=TRUE)
writeLines(c("Lorem ipsum dolor sit amet, adipisicing consectetur elit, sed do",
             "eiusmod tempor incididunt ut labore et dolore magna aliqua."),
           con = file.path(path, "test.txt"))

## Attempt to merge with unstaged conflicting change
status(repo)
#> Unstaged changes:
#>  Modified:   test.txt
m_2 <- merge(repo, "branch")
str(m_2)
#> List of 4
#>  $ up_to_date  : logi FALSE
#>  $ fast_forward: NULL
#>  $ conflicts   : NULL
#>  $ sha         : NULL
#>  - attr(*, "class")= chr "git_merge_result"
format(m_2)
#> [1] "Merge"

## Cleanup
unlink(path, recursive=TRUE)

^{Created on 2019-05-23 by the reprex package (v0.2.1.9000)}

[stewid]

Thanks for this suggestion. Yes, I agree that we should improve the handling of an unstaged conflicting change. It would be great if you can add tests and update the message returned for git_merge_result.

[jdblischak]

I've been investigating this issue further so that I can handle it properly with my workflowr package. A few updates:

1. The problem does not only occur with unstaged changes. Conflicting changes that are staged generate the same issue. Thus only conflicting changes that are committed cause git2r to report a merge conflict.
2. The argument fail = TRUE only has an effect for committed changes

Here is the example code that additionally includes staged conflicting changes, committed conflicting changes, and the effect of fail = TRUE.

library("git2r")

## For debugging
sessionInfo()

## Create a directory in tempdir
path <- tempfile(pattern="git2r-")
dir.create(path)

## Initialize a repository
repo <- init(path)
config(repo, user.name="Alice", user.email="[email protected]")

## Create a file, add and commit
writeLines("Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do",
           con = file.path(path, "test.txt"))
add(repo, "test.txt")
commit_1 <- commit(repo, "Commit message 1")

## Create branch, checkout, change file and commit
b <- checkout(repo, branch = "branch", create = TRUE)
writeLines(c("Lorem ipsum dolor amet sit, consectetur adipisicing elit, sed do",
             "eiusmod tempor incididunt ut labore et dolore magna aliqua."),
           con = file.path(path, "test.txt"))
add(repo, "test.txt")
commit(repo, "Commit message branch")

## Checkout master and create a conflicting change
b <- branches(repo)
checkout(b[sapply(b, "[", "name") == "master"][[1]], force=TRUE)
writeLines(c("Lorem ipsum dolor sit amet, adipisicing consectetur elit, sed do",
             "eiusmod tempor incididunt ut labore et dolore magna aliqua."),
           con = file.path(path, "test.txt"))

## Attempt to merge with unstaged conflicting change
status(repo)
m <- merge(repo, "branch")
str(m)
format(m)

## Attempt to merge with unstaged conflicting change, fail=TRUE
m <- merge(repo, "branch", fail = TRUE)
str(m)
format(m)

## Attempt to merge with staged conflicting change
add(repo, "test.txt")
status(repo)
m <- merge(repo, "branch")
str(m)
format(m)

## Attempt to merge with staged conflicting change, fail=TRUE
m <- merge(repo, "branch", fail = TRUE)
str(m)
format(m)

## Attempt to merge with committed conflicting change, fail=TRUE
commit(repo, "commit conflicting change")
status(repo)
m <- merge(repo, "branch", fail = TRUE)
status(repo)
readLines(file.path(path, "test.txt"))

## Attempt to merge with committed conflicting change
m <- merge(repo, "branch")
status(repo)
readLines(file.path(path, "test.txt"))

## Cleanup
unlink(path, recursive=TRUE)

And here are the results of running this example on Ubuntu 18.04 with the latest version of the master branch 0.26.1.9000:

Click to see results

> library("git2r")

> ## For debugging
> sessionInfo()
R version 3.6.1 (2019-07-05)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 18.04.3 LTS

Matrix products: default
BLAS:   /usr/lib/x86_64-linux-gnu/atlas/libblas.so.3.10.3
LAPACK: /usr/lib/x86_64-linux-gnu/atlas/liblapack.so.3.10.3

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C               LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8    LC_PAPER=en_US.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C             LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] git2r_0.26.1.9000

loaded via a namespace (and not attached):
[1] compiler_3.6.1 tools_3.6.1   

> ## Create a directory in tempdir
> path <- tempfile(pattern="git2r-")

> dir.create(path)

> ## Initialize a repository
> repo <- init(path)

> config(repo, user.name="Alice", user.email="[email protected]")

> ## Create a file, add and commit
> writeLines("Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do",
+            con = file.path(path, .... [TRUNCATED] 

> add(repo, "test.txt")

> commit_1 <- commit(repo, "Commit message 1")

> ## Create branch, checkout, change file and commit
> b <- checkout(repo, branch = "branch", create = TRUE)

> writeLines(c("Lorem ipsum dolor amet sit, consectetur adipisicing elit, sed do",
+              "eiusmod tempor incididunt ut labore et dolore magna ..." ... [TRUNCATED] 

> add(repo, "test.txt")

> commit(repo, "Commit message branch")
[41978c7] 2019-10-07: Commit message branch

> ## Checkout master and create a conflicting change
> b <- branches(repo)

> checkout(b[sapply(b, "[", "name") == "master"][[1]], force=TRUE)

> writeLines(c("Lorem ipsum dolor sit amet, adipisicing consectetur elit, sed do",
+              "eiusmod tempor incididunt ut labore et dolore magna ..." ... [TRUNCATED] 

> ## Attempt to merge with unstaged conflicting change
> status(repo)
Unstaged changes:
	Modified:   test.txt


> m <- merge(repo, "branch")

> str(m)
List of 4
 $ up_to_date  : logi FALSE
 $ fast_forward: NULL
 $ conflicts   : NULL
 $ sha         : NULL
 - attr(*, "class")= chr "git_merge_result"

> format(m)
[1] "Merge"

> ## Attempt to merge with unstaged conflicting change, fail=TRUE
> m <- merge(repo, "branch", fail = TRUE)

> str(m)
List of 4
 $ up_to_date  : logi FALSE
 $ fast_forward: NULL
 $ conflicts   : NULL
 $ sha         : NULL
 - attr(*, "class")= chr "git_merge_result"

> format(m)
[1] "Merge"

> ## Attempt to merge with staged conflicting change
> add(repo, "test.txt")

> status(repo)
Staged changes:
	Modified:   test.txt


> m <- merge(repo, "branch")

> str(m)
List of 4
 $ up_to_date  : logi FALSE
 $ fast_forward: NULL
 $ conflicts   : NULL
 $ sha         : NULL
 - attr(*, "class")= chr "git_merge_result"

> format(m)
[1] "Merge"

> ## Attempt to merge with staged conflicting change, fail=TRUE
> m <- merge(repo, "branch", fail = TRUE)

> str(m)
List of 4
 $ up_to_date  : logi FALSE
 $ fast_forward: NULL
 $ conflicts   : NULL
 $ sha         : NULL
 - attr(*, "class")= chr "git_merge_result"

> format(m)
[1] "Merge"

> ## Attempt to merge with committed conflicting change, fail=TRUE
> commit(repo, "commit conflicting change")
[8af28fb] 2019-10-07: commit conflicting change

> status(repo)
working directory clean

> m <- merge(repo, "branch", fail = TRUE)

> status(repo)
working directory clean

> readLines(file.path(path, "test.txt"))
[1] "Lorem ipsum dolor sit amet, adipisicing consectetur elit, sed do"
[2] "eiusmod tempor incididunt ut labore et dolore magna aliqua."     

> ## Attempt to merge with committed conflicting change
> m <- merge(repo, "branch")

> status(repo)
Unstaged changes:
	Conflicted: test.txt


> readLines(file.path(path, "test.txt"))
[1] "<<<<<<< HEAD"                                                    
[2] "Lorem ipsum dolor sit amet, adipisicing consectetur elit, sed do"
[3] "======="                                                         
[4] "Lorem ipsum dolor amet sit, consectetur adipisicing elit, sed do"
[5] ">>>>>>> branch"                                                  
[6] "eiusmod tempor incididunt ut labore et dolore magna aliqua."     

> ## Cleanup
> unlink(path, recursive=TRUE)

[jdblischak]

However, things get more complicated when there are both unstaged and committed conflicted changes at the time of the merge. The expected behavior should be to alert the user that the unstaged changes need to be handled before the merge is able to complete (i.e. this is what Git does). Instead, this is the current git2r behavior:

1. When fail=FALSE (the default), the merge does not occur. However, it returns NULL for conflicts and the message returned is Merge. Thus it looks like the merge was successful when it wasn't.
2. When fail=TRUE, it warns of conflicts. However, the git_merge_result object returned by merge.git_repository is identical to the situation where the conflicting changes are committed. Thus it isn't possible to explain to the user that the conflicting changes include unstaged changes in addition to the committed changes.

Here is example code to reproduce the behavior described above. It creates the same file on two different branches. After merging the first, it edits the file to create an unstaged change. Then it attempt to merge the second branch which has committed conflicting changes.

library("git2r")

## For debugging
sessionInfo()

## Create a directory in tempdir
path <- tempfile(pattern="git2r-")
dir.create(path)

## Initialize a repository
repo <- init(path)
config(repo, user.name="Alice", user.email="[email protected]")

## Create an initial commit
writeLines("README", con = file.path(path, "README.md"))
add(repo, "README.md")
commit_initial <- commit(repo, "Initial commit")

## Create a file on a branch
b1 <- branch_create(commit_initial, name = "b1")
checkout(b1)
writeLines("Line from branch b1", con = file.path(path, "test.txt"))
add(repo, "test.txt")
commit(repo, "Commit test.txt on branch b1")

## Create the same file on a different branch that will conflict
b2 <- branch_create(commit_initial, name = "b2")
checkout(b2)
writeLines("Line from branch b2", con = file.path(path, "test.txt"))
add(repo, "test.txt")
commit(repo, "Commit test.txt on branch b2")

## Merge branch b1 into master
checkout(repo, "master")
m <- merge(repo, "b1")
str(m)

## Merge branch b2 into master with fail=TRUE (aborts merge due to conflict)
m <- merge(repo, "b2", fail = TRUE)
str(m)

## Create an unstaged change
writeLines("unstaged change", con = file.path(path, "test.txt"))

## Merge branch b2 into master with fail=TRUE and unstaged changes. The
## unstaged conflicting changes are invisible.
m <- merge(repo, "b2", fail = TRUE)
str(m)
format(m)

## Merge branch b2 into master with fail=FALSE and unstaged changes. The
## merge does not proceed, but `conflicts` is `NULL`.
m <- merge(repo, "b2")
str(m)
format(m)
readLines(file.path(path, "test.txt"))

## Cleanup
unlink(path, recursive=TRUE)

Click to see my results

> library("git2r")

> ## For debugging
> sessionInfo()
R version 3.6.1 (2019-07-05)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 18.04.3 LTS

Matrix products: default
BLAS:   /usr/lib/x86_64-linux-gnu/atlas/libblas.so.3.10.3
LAPACK: /usr/lib/x86_64-linux-gnu/atlas/liblapack.so.3.10.3

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C               LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8    LC_PAPER=en_US.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C             LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] git2r_0.26.1.9000

loaded via a namespace (and not attached):
[1] compiler_3.6.1 tools_3.6.1   

> ## Create a directory in tempdir
> path <- tempfile(pattern="git2r-")

> dir.create(path)

> ## Initialize a repository
> repo <- init(path)

> config(repo, user.name="Alice", user.email="[email protected]")

> ## Create an initial commit
> writeLines("README", con = file.path(path, "README.md"))

> add(repo, "README.md")

> commit_initial <- commit(repo, "Initial commit")

> ## Create a file on a branch
> b1 <- branch_create(commit_initial, name = "b1")

> checkout(b1)

> writeLines("Line from branch b1", con = file.path(path, "test.txt"))

> add(repo, "test.txt")

> commit(repo, "Commit test.txt on branch b1")
[c13c711] 2019-10-07: Commit test.txt on branch b1

> ## Create the same file on a different branch that will conflict
> b2 <- branch_create(commit_initial, name = "b2")

> checkout(b2)

> writeLines("Line from branch b2", con = file.path(path, "test.txt"))

> add(repo, "test.txt")

> commit(repo, "Commit test.txt on branch b2")
[61394e4] 2019-10-07: Commit test.txt on branch b2

> ## Merge branch b1 into master
> checkout(repo, "master")

> m <- merge(repo, "b1")

> str(m)
List of 4
 $ up_to_date  : logi FALSE
 $ fast_forward: logi TRUE
 $ conflicts   : logi FALSE
 $ sha         : chr NA
 - attr(*, "class")= chr "git_merge_result"

> ## Merge branch b2 into master with fail=TRUE (aborts merge due to conflict)
> m <- merge(repo, "b2", fail = TRUE)

> str(m)
List of 4
 $ up_to_date  : logi FALSE
 $ fast_forward: logi FALSE
 $ conflicts   : logi TRUE
 $ sha         : chr NA
 - attr(*, "class")= chr "git_merge_result"

> ## Create an unstaged change
> writeLines("unstaged change", con = file.path(path, "test.txt"))

> ## Merge branch b2 into master with fail=TRUE and unstaged changes. The
> ## unstaged conflicting changes are invisible.
> m <- merge(repo, "b2", fail = TRUE)

> str(m)
List of 4
 $ up_to_date  : logi FALSE
 $ fast_forward: logi FALSE
 $ conflicts   : logi TRUE
 $ sha         : chr NA
 - attr(*, "class")= chr "git_merge_result"

> format(m)
[1] "Merge: Conflicts"

> ## Merge branch b2 into master with fail=FALSE and unstaged changes. The
> ## merge does not proceed, but `conflicts` is `NULL`.
> m <- merge(repo, "b2")

> str(m)
List of 4
 $ up_to_date  : logi FALSE
 $ fast_forward: logi FALSE
 $ conflicts   : NULL
 $ sha         : NULL
 - attr(*, "class")= chr "git_merge_result"

> format(m)
[1] "Merge"

> readLines(file.path(path, "test.txt"))
[1] "unstaged change"

> ## Cleanup
> unlink(path, recursive=TRUE)