I had a situation today where I lost my changes due to (mostly) human error.
Luckily the change was very trivial. But losing uncommitted changes is bad!

It came about like this:

  1 tweak a file to create a follow-up fix for just committed changes
  2 forget to commit these tweaks
  3 run 'got up -c origin/master' to prepare for histedit in order to
    fold the changes together; this created a merge conflict in the file
  4 run 'got he'; realize it errors out because local changes are present
  5 run 'got up' in order to prepare to commit these changes
  6 realize that somehow the changes have entirely disappeared :-O

There are a couple of problems here, but one critical step is the final
'update' in step 5 where got overwrote the conflicted file, eradicating
local changes prepared at step 1. This happened in update_blob() because
when 'status' is GOT_STATUS_CONFLICT we fall through to the final else
clause of the function, which calls install_blob().

To prevent this specific issue we need to handle STATUS_CONFLICT in
update_blob() explicitly. I see several possibilities:

 - just let 'got update' merge into files which contain conflict markers

I expect the merge result would confuse the hell out of people especially
if such a merge happens multiple times. So let's not do that!

 - disallow 'got update' if the work tree contains merge conflicts

This is the safest approach and we already have such checks in place for
some commands (e.g. cherrypick) in order to avoid complicated conflicts 
when we know that people are trying to do potentially complicated merges.
But this implies a full 'got status' crawl before every update operation!
On my laptop that would add about 2 seconds of delay every time an update
is started even on a clean work tree (because stat(2) in combination with
unveil(2) is slow). Is that really worth it to prevent a corner case problem
like this?

 - skip conflicted files during updates

This is what Subversion does as well. Compared to the previous approach
the main downside is that the work tree is left in a partially updated
state, rather than left entirely in its previous state. And the user may
not notice the problem until much later, because people rarely scroll back
up to check the progress output for files which were skipped.
However, if this happens we were already in a state where our code would
not even compile, and the user was careless and updated anyway.
And we still have an advantage over the implementation in got 0.33 because
losing local changes without user intervention has become very unlikely.

The diff below shows how this approach could be implemented.

Does anyone strongly prefer another approach?

diff ceb466a7cccf5ed6424cd7e24839389a42e998c1 /home/stsp/src/got
blob - 6583cc3872bc92bf71d69523894f2b671cdfb368
file + got/got.1
--- got/got.1
+++ got/got.1
@@ -520,6 +520,18 @@ of this commit.
 Preserve any local changes in the work tree and merge them with the
 incoming changes.
 .Pp
+Files which already contain merge conflicts will not be updated to avoid
+further complications.
+Such files will be updated when
+.Cm got update
+is run again after merge conflicts have been resolved.
+If the conflicting changes are no longer needed affected files can be
+reverted with
+.Cm got revert
+before running
+.Cm got update
+again.
+.Pp
 Show the status of each affected file, using the following status codes:
 .Bl -column YXZ description
 .It U Ta file was updated and contained no local changes
@@ -529,6 +541,7 @@ Show the status of each affected file, using the follo
 .It A Ta new file was added
 .It \(a~ Ta versioned file is obstructed by a non-regular file
 .It ! Ta a missing versioned file was restored
+.It # Ta file was not updated because it contains merge conflicts
 .El
 .Pp
 If no
blob - 7ee3eaeaa50ee972174740c861e33c472d33aa9a
file + include/got_worktree.h
--- include/got_worktree.h
+++ include/got_worktree.h
@@ -37,6 +37,7 @@ struct got_fileindex;
 #define GOT_STATUS_CANNOT_DELETE 'd'
 #define GOT_STATUS_BUMP_BASE	'b'
 #define GOT_STATUS_BASE_REF_ERR	'B'
+#define GOT_STATUS_CANNOT_UPDATE '#'
 
 /*
  * Attempt to initialize a new work tree on disk.
blob - b4197e4d8c33edc1a60f7d33a519c558ed3726a8
file + lib/worktree.c
--- lib/worktree.c
+++ lib/worktree.c
@@ -1303,6 +1303,11 @@ update_blob(struct got_worktree *worktree,
 		err = (*progress_cb)(progress_arg, status, path);
 		goto done;
 	}
+	if (status == GOT_STATUS_CONFLICT) {
+		err = (*progress_cb)(progress_arg, GOT_STATUS_CANNOT_UPDATE,
+		    path);
+		goto done;
+	}
 
 	if (ie && status != GOT_STATUS_MISSING &&
 	    (te->mode & S_IXUSR) == (sb.st_mode & S_IXUSR)) {
blob - 2225b5ae82ab9273a4760df29891aa221c5898b3
file + regress/cmdline/update.sh
--- regress/cmdline/update.sh
+++ regress/cmdline/update.sh
@@ -1647,6 +1647,61 @@ function test_update_toggles_xbit {
 	test_done "$testroot" "$ret"
 }
 
+function test_update_preserves_conflicted_file {
+	local testroot=`test_init update_preserves_conflicted_file`
+	local commit_id0=`git_show_head $testroot/repo`
+
+	echo "modified alpha" > $testroot/repo/alpha
+	git_commit $testroot/repo -m "modified alpha"
+	local commit_id1=`git_show_head $testroot/repo`
+
+	got checkout -c $commit_id0 $testroot/repo $testroot/wt > /dev/null
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# fake a merge conflict
+	echo '<<<<<<<' > $testroot/wt/alpha
+	echo 'alpha' >> $testroot/wt/alpha
+	echo '=======' >> $testroot/wt/alpha
+	echo 'alpha, too' >> $testroot/wt/alpha
+	echo '>>>>>>>' >> $testroot/wt/alpha
+	cp $testroot/wt/alpha $testroot/content.expected
+
+	echo "C  alpha" > $testroot/stdout.expected
+	(cd $testroot/wt && got status  > $testroot/stdout)
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "#  alpha" > $testroot/stdout.expected
+	echo -n "Updated to commit " >> $testroot/stdout.expected
+	git_show_head $testroot/repo >> $testroot/stdout.expected
+	echo >> $testroot/stdout.expected
+	(cd $testroot/wt && got update > $testroot/stdout)
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	cmp -s $testroot/content.expected $testroot/wt/alpha
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/content.expected $testroot/wt/alpha
+	fi
+	test_done "$testroot" "$ret"
+}
+
 run_test test_update_basic
 run_test test_update_adds_file
 run_test test_update_deletes_file
@@ -1678,3 +1733,4 @@ run_test test_update_to_commit_on_wrong_branch
 run_test test_update_bumps_base_commit_id
 run_test test_update_tag
 run_test test_update_toggles_xbit
+run_test test_update_preserves_conflicted_file