threads / rfc / 21689

RFC patch, 2 partsExplain newbies the "commit before pull" flow

Subject: [RFC/PATCH 0/2] Explain newbies the "commit before pull" flow

## tl;dr

12 messages between Nov 20, 2009 and Nov 23, 2009. Diffs are folded; open one to read it.

replies: 11people: 3as markdown or json

Matthieu Moy· Nov 20, 2009, 15:59 UTC · lore

A colleague of mine asked me "why do I need to commit before I can pull", and I was about to send a link to the user manual, when I noticed I couldn't find any explanation about that there.

This little patch serie propose a few more sentence in the user manual, and a little more verbose error message for those who don't read the docs.

I'm not sure the wording is the best we can do. Native-speakers and Git gurus are welcome to improve it ;-).

Matthieu Moy (2):
  merge-recursive: point the user to commit when file would be
    overwritten.
  user-manual: Document that "git merge" doesn't like uncommited
    changes.
 Documentation/user-manual.txt |   14 +++++++++++---
 merge-recursive.c             |    6 ++++--
 2 files changed, 15 insertions(+), 5 deletions(-)
Matthieu Moy· Nov 20, 2009, 15:59 UTC · re: Matthieu Moy · lore

[PATCH 1/2] merge-recursive: point the user to commit when file would be overwritten.

The commit-before-pull is well accepted in the DVCS community, but is confusing some new users. This should get them back in the right way when the problem occurs.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 merge-recursive.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
Show changes to merge-recursive.c +4 −2
diff --git a/merge-recursive.c b/merge-recursive.c
index f55b7eb..d5e0819 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -172,9 +172,11 @@ static int git_merge_trees(int index_only,
 	struct unpack_trees_options opts;
 	static const struct unpack_trees_error_msgs msgs = {
 		/* would_overwrite */
-		"Your local changes to '%s' would be overwritten by merge.  Aborting.",
+		"Your local changes to '%s' would be overwritten by merge.  Aborting:\n"
+		"Please, commit your changes or stash them before you can merge.",
 		/* not_uptodate_file */
-		"Your local changes to '%s' would be overwritten by merge.  Aborting.",
+		"Your local changes to '%s' would be overwritten by merge.  Aborting:\n"
+		"Please, commit your changes or stash them before you can merge.",
 		/* not_uptodate_dir */
 		"Updating '%s' would lose untracked files in it.  Aborting.",
 		/* would_lose_untracked */

-- 
1.6.5.3.435.g5f2e3.dirty
Junio C Hamano· Nov 20, 2009, 18:05 UTC · re: Matthieu Moy · lore

Re: [PATCH 1/2] merge-recursive: point the user to commit when file would be overwritten.

Matthieu Moy <Matthieu.Moy@imag.fr> writes:
Show 5 quoted lines
> The commit-before-pull is well accepted in the DVCS community, but is
> confusing some new users. This should get them back in the right way when
> the problem occurs.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Don't these extra lines warrant "advice.*" option?
Show 24 quoted lines
> ---
>  merge-recursive.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index f55b7eb..d5e0819 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -172,9 +172,11 @@ static int git_merge_trees(int index_only,
>  	struct unpack_trees_options opts;
>  	static const struct unpack_trees_error_msgs msgs = {
>  		/* would_overwrite */
> -		"Your local changes to '%s' would be overwritten by merge.  Aborting.",
> +		"Your local changes to '%s' would be overwritten by merge.  Aborting:\n"
> +		"Please, commit your changes or stash them before you can merge.",
>  		/* not_uptodate_file */
> -		"Your local changes to '%s' would be overwritten by merge.  Aborting.",
> +		"Your local changes to '%s' would be overwritten by merge.  Aborting:\n"
> +		"Please, commit your changes or stash them before you can merge.",
>  		/* not_uptodate_dir */
>  		"Updating '%s' would lose untracked files in it.  Aborting.",
>  		/* would_lose_untracked */
> -- 
> 1.6.5.3.435.g5f2e3.dirty
Matthieu Moy· Nov 22, 2009, 22:09 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/2] merge-recursive: point the user to commit when file would be overwritten.

Junio C Hamano <gitster@pobox.com> writes:
Show 9 quoted lines
> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> The commit-before-pull is well accepted in the DVCS community, but is
>> confusing some new users. This should get them back in the right way when
>> the problem occurs.
>>
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>
> Don't these extra lines warrant "advice.*" option?

I was thinking this would be overkill for a one-line advice, but it doesn't harm to provide the option. I'll resend the patch with it (it's not _as_ straightforward as it seems to be since the initial structure is static in the function, but still reasonable).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Matthieu Moy· Nov 20, 2009, 15:59 UTC · re: Matthieu Moy · lore

[PATCH 2/2] user-manual: Document that "git merge" doesn't like uncommited changes.

We explain the user why uncommited changes can be problematic with merge, and point to "commit" and "stash" for the solution.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/user-manual.txt |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)
Show changes to Documentation/user-manual.txt +11 −3
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 269ec47..6b4a037 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -1176,14 +1176,22 @@ How to merge
 ------------
 
 You can rejoin two diverging branches of development using
-linkgit:git-merge[1]:
+linkgit:git-merge[1].
 
 -------------------------------------------------
 $ git merge branchname
 -------------------------------------------------
 
 merges the development in the branch "branchname" into the current
-branch.  If there are conflicts--for example, if the same file is
+branch.  Note that Git merges committed changes, but not the working
+tree itself.  Therefore, if you have uncommitted changes touching the
+same files as the ones impacted by the merge, Git will refuse to
+proceed.  Most of the time, you will want to commit your changes before
+you can merge, and if you don't, then linkgit:git-stash[1] can take
+these changes away while you're doing the merge, and reapply them
+afterwards.
+
+If there are conflicts--for example, if the same file is
 modified in two different ways in the remote branch and the local
 branch--then you are warned; the output may look something like this:
 
@@ -1679,7 +1687,7 @@ Sharing development with others
 Getting updates with git pull
 -----------------------------
 
-After you clone a repository and make a few changes of your own, you
+After you clone a repository and commit a few changes of your own, you
 may wish to check the original repository for updates and merge them
 into your own work.
 

-- 
1.6.5.3.435.g5f2e3.dirty
Junio C Hamano· Nov 20, 2009, 20:19 UTC · re: Matthieu Moy · lore

Re: [PATCH 2/2] user-manual: Document that "git merge" doesn't like uncommited changes.

Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> We explain the user why uncommited changes can be problematic with merge,
> and point to "commit" and "stash" for the solution.

It is a shame that our existing documentation did not address this, and the patch certainly takes us in the right direction.

I haven't seen a clear description of distinction between two vastly different modes of failure from "git merge"---one that stops before even touching anything, and one that results in conflicts to be resolved---and how to tell them apart. As the course of action after these two modes are quite different, it helps users to teach it early.

I attempted to summarize it some time ago:
    http://gitster.livejournal.com/25801.html
    (Completing a merge)
    http://gitster.livejournal.com/29060.html
    (Fun with keeping local changes around)
I am not very satisfied with them, but it might be a good start.
Show 27 quoted lines
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>  Documentation/user-manual.txt |   14 +++++++++++---
>  1 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> index 269ec47..6b4a037 100644
> --- a/Documentation/user-manual.txt
> +++ b/Documentation/user-manual.txt
> @@ -1176,14 +1176,22 @@ How to merge
>  ------------
>  
>  You can rejoin two diverging branches of development using
> -linkgit:git-merge[1]:
> +linkgit:git-merge[1].
>  
>  -------------------------------------------------
>  $ git merge branchname
>  -------------------------------------------------
>  
>  merges the development in the branch "branchname" into the current
> -branch.  If there are conflicts--for example, if the same file is
> +branch.  Note that Git merges committed changes, but not the working
> +tree itself.  Therefore, if you have uncommitted changes touching the
> +same files as the ones impacted by the merge, Git will refuse to
> +proceed.

"but not the working tree itself" is not incorrect per-se, but misses the point. How about...

	A merge is made by combining the changes made in "branchname" and
        the changes made up to the latest commit in your current branch
        since their histories forke.  The work tree is overwritten by the
        result of the merge when this combining is done cleanly, or
        overwritten by a half-merged results when this combining results
        in conflicts.  Therefore, ...
Show 20 quoted lines
> +proceed.  Most of the time, you will want to commit your changes before
> +you can merge, and if you don't, then linkgit:git-stash[1] can take
> +these changes away while you're doing the merge, and reapply them
> +afterwards.
> +
> +If there are conflicts--for example, if the same file is
>  modified in two different ways in the remote branch and the local
>  branch--then you are warned; the output may look something like this:
>  
> @@ -1679,7 +1687,7 @@ Sharing development with others
>  Getting updates with git pull
>  -----------------------------
>  
> -After you clone a repository and make a few changes of your own, you
> +After you clone a repository and commit a few changes of your own, you
>  may wish to check the original repository for updates and merge them
>  into your own work.
>  
> -- 
> 1.6.5.3.435.g5f2e3.dirty
Matthieu Moy· Nov 22, 2009, 22:18 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] user-manual: Document that "git merge" doesn't like uncommited changes.

Junio C Hamano <gitster@pobox.com> writes:
Show 9 quoted lines
> "but not the working tree itself" is not incorrect per-se, but misses the
> point.  How about...
>
> 	A merge is made by combining the changes made in "branchname" and
>         the changes made up to the latest commit in your current branch
>         since their histories forke.  The work tree is overwritten by the
>         result of the merge when this combining is done cleanly, or
>         overwritten by a half-merged results when this combining results
>         in conflicts.  Therefore, ...

Maybe better. OTOH, it reveals another problem: Your "the work tree is overwritten by ..." tend to imply that the result is not commited, while the normal case is indeed to create a merge commit automatically.

In its current state, the doc jumps to conflict resolution and how to commit after resolving them without saying "oh, normally this is over, Git did the commit for you". And for people comming from SVN, thinking "merge = update", it's not obvious at all that a merge should result in a commit.

So, I'll resend with one more paragraph talking about autocommit.
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Junio C Hamano· Nov 22, 2009, 23:57 UTC · re: Matthieu Moy · lore

Re: [PATCH 2/2] user-manual: Document that "git merge" doesn't like uncommited changes.

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
Show 16 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
>> "but not the working tree itself" is not incorrect per-se, but misses the
>> point.  How about...
>>
>> 	A merge is made by combining the changes made in "branchname" and
>>         the changes made up to the latest commit in your current branch
>>         since their histories forke.  The work tree is overwritten by the
>>         result of the merge when this combining is done cleanly, or
>>         overwritten by a half-merged results when this combining results
>>         in conflicts.  Therefore, ...
>
> Maybe better. OTOH, it reveals another problem: Your "the work tree is
> overwritten by ..." tend to imply that the result is not commited,
> while the normal case is indeed to create a merge commit
> automatically.
Fair enough.  How about changing this part
>>         since their histories forke.  The work tree is overwritten by the
>>         result of the merge when this combining is done cleanly, or
>>         overwritten by a half-merged results when this combining results
to
	The work tree is overwritten by the result of the
        merge when this combining is done cleanly, and the result is
        committed. Otherwise it is
	overwritten by a half-merged results when this combining results
?
Matthieu Moy· Nov 23, 2009, 07:51 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] user-manual: Document that "git merge" doesn't like uncommited changes.

Junio C Hamano <gitster@pobox.com> writes:
> 	The work tree is overwritten by the result of the
>         merge when this combining is done cleanly, and the result is
>         committed. Otherwise it is
> 	overwritten by a half-merged results when this combining results

I thought of something like this, but this is slightly incorrect in case of fast-forward (Git doesn't "commit", but "reuses" a commit), so I prefered making a separate paragraph.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Matthieu Moy· Nov 22, 2009, 22:26 UTC · re: Matthieu Moy · lore

[PATCH 1/2 v2] merge-recursive: point the user to commit when file would be overwritten.

The commit-before-pull is well accepted in the DVCS community, but is confusing some new users. This should get them back in the right way when the problem occurs.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/config.txt |    4 ++++
 advice.c                 |    2 ++
 advice.h                 |    1 +
 merge-recursive.c        |    8 +++++++-
 4 files changed, 14 insertions(+), 1 deletions(-)
Show changes to 4 files +14 −1

Documentation/config.txt, advice.c, advice.h, merge-recursive.c

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 13871a6..f481f8d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -126,6 +126,10 @@ advice.*::
 		Directions on how to stage/unstage/add shown in the
 		output of linkgit:git-status[1] and the template shown
 		when writing commit messages. Default: true.
+	commitBeforeMerge::
+		Advice shown when linkgit:git-merge[1] refuses to
+		merge to avoid overwritting local changes.
+		Default: true.
 --
 
 core.fileMode::
diff --git a/advice.c b/advice.c
index ae4b1e8..cb666ac 100644
--- a/advice.c
+++ b/advice.c
@@ -2,6 +2,7 @@
 
 int advice_push_nonfastforward = 1;
 int advice_status_hints = 1;
+int advice_commit_before_merge = 1;
 
 static struct {
 	const char *name;
@@ -9,6 +10,7 @@ static struct {
 } advice_config[] = {
 	{ "pushnonfastforward", &advice_push_nonfastforward },
 	{ "statushints", &advice_status_hints },
+	{ "commitbeforemerge", &advice_commit_before_merge },
 };
 
 int git_default_advice_config(const char *var, const char *value)
diff --git a/advice.h b/advice.h
index e9df8e0..3de5000 100644
--- a/advice.h
+++ b/advice.h
@@ -3,6 +3,7 @@
 
 extern int advice_push_nonfastforward;
 extern int advice_status_hints;
+extern int advice_commit_before_merge;
 
 int git_default_advice_config(const char *var, const char *value);
 
diff --git a/merge-recursive.c b/merge-recursive.c
index f55b7eb..1870448 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -3,6 +3,7 @@
  * Fredrik Kuivinen.
  * The thieves were Alex Riesen and Johannes Schindelin, in June/July 2006
  */
+#include "advice.h"
 #include "cache.h"
 #include "cache-tree.h"
 #include "commit.h"
@@ -170,7 +171,7 @@ static int git_merge_trees(int index_only,
 	int rc;
 	struct tree_desc t[3];
 	struct unpack_trees_options opts;
-	static const struct unpack_trees_error_msgs msgs = {
+	struct unpack_trees_error_msgs msgs = {
 		/* would_overwrite */
 		"Your local changes to '%s' would be overwritten by merge.  Aborting.",
 		/* not_uptodate_file */
@@ -182,6 +183,11 @@ static int git_merge_trees(int index_only,
 		/* bind_overlap -- will not happen here */
 		NULL,
 	};
+	if (advice_commit_before_merge) {
+		msgs.would_overwrite = msgs.not_uptodate_file =
+			"Your local changes to '%s' would be overwritten by merge.  Aborting.\n"
+			"Please, commit your changes or stash them before you can merge.";
+	}
 
 	memset(&opts, 0, sizeof(opts));
 	if (index_only)

-- 
1.6.5.3.435.g5f2e3.dirty
Matthieu Moy· Nov 22, 2009, 22:26 UTC · re: Matthieu Moy · lore

[PATCH 2/2 v2] user-manual: Document that "git merge" doesn't like uncommited changes.

We explain the user why uncommited changes can be problematic with merge, and point to "commit" and "stash" for the solution. While talking about commited Vs uncommited changes, we also make it clear that the result of a merge is normally commited.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/user-manual.txt |   20 ++++++++++++++++++--
 1 files changed, 18 insertions(+), 2 deletions(-)
Show changes to Documentation/user-manual.txt +18 −2
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 269ec47..c09f4b2 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -1183,7 +1183,23 @@ $ git merge branchname
 -------------------------------------------------
 
 merges the development in the branch "branchname" into the current
-branch.  If there are conflicts--for example, if the same file is
+branch.
+
+A merge is made by combining the changes made in "branchname" and the
+changes made up to the latest commit in your current branch since
+their histories forked. The work tree is overwritten by the result of
+the merge when this combining is done cleanly, or overwritten by a
+half-merged results when this combining results in conflicts.
+Therefore, if you have uncommitted changes touching the same files as
+the ones impacted by the merge, Git will refuse to proceed. Most of
+the time, you will want to commit your changes before you can merge,
+and if you don't, then linkgit:git-stash[1] can take these changes
+away while you're doing the merge, and reapply them afterwards.
+
+If the changes are independant enough, Git will automatically complete
+the merge and commit the result (or reuse an existing commit in case
+of <<fast-forwards,fast-forward>>, see below). On the other hand,
+if there are conflicts--for example, if the same file is
 modified in two different ways in the remote branch and the local
 branch--then you are warned; the output may look something like this:
 
@@ -1679,7 +1695,7 @@ Sharing development with others
 Getting updates with git pull
 -----------------------------
 
-After you clone a repository and make a few changes of your own, you
+After you clone a repository and commit a few changes of your own, you
 may wish to check the original repository for updates and merge them
 into your own work.
 

-- 
1.6.5.3.435.g5f2e3.dirty

← back to recent threads