Re: [PATCH v2 02/23] rebase -i: allow squashing empty commits without complaints

2014-08-11 Thread Fabian Ruch
Hi Eric,

Eric Sunshine writes:
 On Wed, Aug 6, 2014 at 7:59 PM, Fabian Ruch baf...@gmail.com wrote:
 The to-do list commands `squash` and `fixup` apply the changes
 introduced by the named commit to the tree but instead of creating
 a new commit on top of the current head it replaces the previous
 commit with a new commit that records the updated tree. If the
 result is an empty commit git-rebase stops with the error message

You asked to amend the most recent commit, but doing so would make
it empty. You can repeat your command with --allow-empty, or you can
remove the commit entirely with git reset HEAD^.

 This message is not very helpful because neither does git-rebase
 support an option `--allow-empty` nor does the messages say how to
 resume the rebase. Firstly, change the error message to

The squash result is empty and --keep-empty was not specified.

You can remove the squash commit now with

  git reset HEAD^

Once you are down, run
 
 I guess you meant: s/down/done
 
 Same issue with the actually message in the code (below).

Fixed.

  git rebase --continue

 If the user wishes to squash a sequence of commits into one
 commit, f. i.

pick A
squash Revert A
squash A'

 , it does not matter for the end result that the first squash
 result, or any sub-sequence in general, is going to be empty. The
 squash message is not affected at all by which commits are created
 and only the commit created by the last line in the sequence will
 end up in the final history. Secondly, print the error message
 only if the whole squash sequence produced an empty commit.

 Lastly, since an empty squash commit is not a failure to rewrite
 the history as planned, issue the message above as a mere warning
 and interrupt the rebase with the return value zero. The
 interruption should be considered as a notification with the
 chance to undo it on the spot. Specifying the `--keep-empty`
 option tells git-rebase to keep empty squash commits in the
 rebased history without notification.

 Add tests.

 Reported-by: Peter Krefting pe...@softwolves.pp.se
 Signed-off-by: Fabian Ruch baf...@gmail.com
 ---
 Hi,

 Peter Krefting is cc'd as the author of the bug report Confusing
 error message in rebase when commit becomes empty discussed on the
 mailing list in June. Phil Hord and Jeff King both participated in
 the problem discussion which ended with two proposals by Jeff.

 Jeff King writes:
   1. Always keep such empty commits. A user who is surprised by them
  being empty can then revisit them. Or drop them by doing another
  rebase without --keep-empty.

   2. Notice ourselves that the end-result of the whole squash is an
  empty commit, and stop to let the user deal with it.

 This patch chooses the second alternative. Either way seems OK. The
 crucial consensus of the discussion was to silently throw away empty
 interim commits.

Fabian

  git-rebase--interactive.sh| 20 +++---
  t/t3404-rebase-interactive.sh | 62 
 +++
  2 files changed, 79 insertions(+), 3 deletions(-)

 diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
 index 3222bf6..8820eac 100644
 --- a/git-rebase--interactive.sh
 +++ b/git-rebase--interactive.sh
 @@ -549,7 +549,7 @@ do_next () {
 squash|s|fixup|f)
 # This is an intermediate commit; its message will 
 only be
 # used in case of trouble.  So use the long version:
 -   do_with_author output git commit 
 --allow-empty-message \
 +   do_with_author output git commit 
 --allow-empty-message --allow-empty \
 --amend --no-verify -F $squash_msg \
 ${gpg_sign_opt:+$gpg_sign_opt} ||
 die_failed_squash $sha1 $rest
 @@ -558,18 +558,32 @@ do_next () {
 # This is the final command of this squash/fixup 
 group
 if test -f $fixup_msg
 then
 -   do_with_author git commit 
 --allow-empty-message \
 +   do_with_author git commit 
 --allow-empty-message --allow-empty \
 --amend --no-verify -F $fixup_msg \
 ${gpg_sign_opt:+$gpg_sign_opt} ||
 die_failed_squash $sha1 $rest
 else
 cp $squash_msg $GIT_DIR/SQUASH_MSG || 
 exit
 rm -f $GIT_DIR/MERGE_MSG
 -   do_with_author git commit --amend 
 --no-verify -F $GIT_DIR/SQUASH_MSG -e \
 +   do_with_author git commit --allow-empty 
 --amend --no-verify -F $GIT_DIR/SQUASH_MSG -e \
 ${gpg_sign_opt:+$gpg_sign_opt} ||

Re: [PATCH v2 04/23] rebase -i: hide interactive command messages in verbose mode

2014-08-11 Thread Fabian Ruch
Hi Thomas,

Thomas Rast writes:
 Fabian Ruch baf...@gmail.com writes:
 @@ -923,6 +923,8 @@ EOF
  ;;
  esac
  
 +mkdir -p $state_dir || die Could not create temporary $state_dir
 +
  git var GIT_COMMITTER_IDENT /dev/null ||
  die You need to set your committer info first
  
 @@ -938,7 +940,6 @@ then
  fi
  
  orig_head=$(git rev-parse --verify HEAD) || die No HEAD?
 -mkdir -p $state_dir || die Could not create temporary $state_dir
  
  :  $state_dir/interactive || die Could not mark as interactive
  write_basic_state
 
 Why this change?  I can't figure out how it relates to the output
 change.

Creating the state directory a few steps earlier into
'git_rebase__interactive' is necessary because the changed definition of
'output' needs it for 'editor.sh'. This change was triggered by a
failing test case that used the branch argument with git-rebase. The
'git checkout branch', which is executed if 'switch_to' is set to
branch, is wrapped into an 'output' line and 'output' failed because
it wasn't able to create 'editor.sh'.

The state directory (of git-rebase--interactive!) is now created
directly after the case expression that handles --continue, --skip and
--edit-todo. They all assume the existence of the state directory and
either jump into 'do_rest' or 'exit' immediately, that is creating the
directory earlier would make the options handling code somewhat
incorrect and would not change anything for the start sequence of
git-rebase--interactive.

The patch message now reads as follows (with the reference to 7725cb5 in
the second paragraph and the complete third paragraph added):

 rebase -i: hide interactive command messages in verbose mode
 
 git-rebase--interactive prints summary messages of the commits it
 creates in the final history only if the `--verbose` option is
 specified by the user and suppresses them otherwise. This behaviour
 is implemented by wrapping git-commit calls in a shell function named
 `output` which redirects stderr to stdout, captures stdout in a shell
 variable and ignores its contents unless the command exits with an
 error status.
 
 The command lines used to implement the to-do list commands `reword`
 and `squash` print diagnostic messages even in non-verbose mode. The
 reason for this inconsistency is that both commands launch the log
 message editor which usually requires a working terminal attached to
 stdin. Wrapping the `reword` and `squash` command lines in `output`
 would seemingly freeze the terminal (see commit 7725cb5, rebase -i:
 fix reword when using a terminal editor). Temporarily redirect the
 editor output to a third file descriptor in order to ship it around
 the capture stream. Wrap the remaining git-commit command lines in
 the new `output`.
 
 In order to temporarily redirect the editor output, the new
 definition of `output` creates a script in the state directory to be
 used as `GIT_EDITOR`. Make sure the state directory exists before
 `output` is called for the first time.
 
 fake_editor prints the to-do list before and after applying the
 `FAKE_LINES` rewrite rules to it. Redirect this debug output to
 stderr so that it does not interfere with the git-rebase status
 output. Add test.

   Fabian
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 08/23] rebase -i: reword executes pre-commit hook on interim commit

2014-08-11 Thread Fabian Ruch
Hi Thomas,

Thomas Rast writes:
 Fabian Ruch baf...@gmail.com writes:
 Subject: Re: [PATCH v2 08/23] rebase -i: reword executes pre-commit hook on 
 interim commit
 
 I think the change makes sense, but can you reword the subjects that it
 describes the state after the commit (i.e. what you are doing), instead
 of before the commit?

The log message subject now reads as follows:

 rebase -i: do not verify reworded patches using pre-commit

   Fabian
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Fixing unclear messages

2014-08-11 Thread Alexander Shopov
 - printf_ln(_(Huh (%s)?), (*ptr)-buf);
 + printf_ln(_(Wrong choice (%s). Choose again.), 
 (*ptr)-buf);
 Why is this an improvement?
Because 'Huh?' without intonation, gesture or a frown provided by a
human being is hard to understand. It does not state that it is the
choice the user provided is wrong and does not prompt the user for the
next action.


 - only_include_assumed = _(Clever... amending the last one with 
 dirty index.);
 + only_include_assumed = _(You are amending the last commit 
 only. There are additional changes in the index.);
 Why is this an improvement?
...
 Besides, amending the last commit only.  implies ...
 ... does not convey any extra information ...
 ...It may be time to remove these messages, by the way. ...
You say that my change does not tangibly improve on an initially
unclear and already obsolete message, am I right?
I prefer the messages to be removed then.

 + die(_(fatal error in function \parse_pack_objects\. This is 
 a bug in Git. Please report it to the developers with an e-mail to 
 git@vger.kernel.org.));
 It probably should be spelled die(BUG: ...).
 + die(_(fatal error in function \conclude_pack\. This 
 is a bug in Git. Please report it to the developers with an e-mail to 
 git@vger.kernel.org.));
 Likewise.
I have no stand on the issue fatal error vs BUG, if you prefer
BUG I can reword.
There was a suggestion to have a separate function that is meant to
echo messages when there are genuine bugs in Git (impossible cases
happening, invariants breaking, etc.) This will allow factoring out
the clause This is a bug in Git. Please report it to the developers
with an e-mail to git@vger.kernel.org. as a single message and I
prefer that for ease of maintenance.

 + die(_(wrong format for the \in-reply-to\ header: %s), 
 msg_id);
 Why is s/insane/wrong format/ an improvement?
Because it is more factual and narrows down what is wrong. Insane
could mean many different things.

 - die(_(Two output directories?));
 + die(_(Maximum one output directory is allowed.));
 Why is it an improvement?
Because the question only implies that the problem is the number of
directories but says nothing how many directories there should be (0,
1, 3... 100?)

 - printf(_(Wonderful.\n));
 + printf(_(The first part of the trivial merge finished successfully
 Huh?
I am not sure what you mean or your objection would be, perhaps I am
misreading the source of Git.
The message appears as a part of sequence during merge when the first
stage passes successfully but there still could be a case that makes
the whole merge impossible.
What does Wonderful mean in a sequence of steps git is doing for you.

You say I would buy s/something/BUG: /;, but I do not think we want
to apply most of the others.
Does this mean the following changes are totally unwelcome or you
(plural, as corresponds to we) want me to resubmit them and
substantiate changes on a message by message base?

-Nope.\n
+Merge was not successful.\n

- ???
+ unknown state

-no tag message?
+missing tag message

-?? what are you talking about?
+unknown command. Only start, good, bad and skip are possible.

-Unprocessed path??? %s
+The path %s was not processed but it had to be. This is a bug in
Git. Please report it to the developers with an e-mail to
git@vger.kernel.org.

Kind regards:
al_shopov
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v8 0/8] Rewrite `git_config()` using config-set API

2014-08-11 Thread Ramsay Jones
On 10/08/14 18:29, Junio C Hamano wrote:
 Ramsay Jones ram...@ramsay1.demon.co.uk writes:
 
 On 08/08/14 15:07, Tanay Abhra wrote:
 ...
 (cc to Ramsay)

 The discussion in both threads (v8 and v9), boils down to this,
 is the `key_value_info` struct really required to be declared public or 
 should be
 just an implementation detail. I will give you the context,

 No, this is not the issue for me. The patch which introduces the
 struct in cache.h does not make use of that struct in any interface.
 It *is* an implementation detail of some code in config.c only.

 I do not know how that structure will be used in future patches. ;-)
 
 It is debatable if it is a good API that tells the users In the
 data I return to you, there is a structure hanging there with these
 two fields. Feel free to peek into it if you need what is recorded
 in them.

There is no debate in my mind. ;-)

In this future patch, the movement of the structure out of config.c would
be plain for everyone to see, and (hopefully) debate the merits of such
an 'interface'. Along with checking that it is properly documented. (which
patch should the documentation be in?) Where should it be documented?
 
  But the contract between the the endgame API and its
 callers can include such a direct access, and then you can say that
 it is a part of the API, not just an implementation detail.

Sure. It's just a question of timing and allowing reviewers to see the
actual change in one patch.

 I think you and Tanay are both right (and I am wrong ;-).

;-)

I don't have *very* strong feelings about this, which is why I didn't
respond to the earlier replies by Tanay and Matthieu, but since I was
cc-ed on this thread ... (It seemed to me that my comments had been
misunderstood).

So yes, I think this API is ugly and could be improved upon, but I
don't care sufficiently to make any further comment. :-D

ATB,
Ramsay Jones


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


How to give permission to another user on my git remote

2014-08-11 Thread Jagan Teki
Hi,

I have created one repository (but I'm not a root user on the server) like
$ git init --bare

And I do push my changes locally to remote repo where I created.
My friend also working the same repo, and he needs to push the changes
on the same.

I tried by adding below line on the remote config file
[config]
sharedRepository = true

Any help, how to do that.

-- 
Jagan.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: How to give permission to another user on my git remote

2014-08-11 Thread Stefan Beller
On 11.08.2014 13:41, Jagan Teki wrote:
 Hi,
 
 I have created one repository (but I'm not a root user on the server) like
 $ git init --bare
 
 And I do push my changes locally to remote repo where I created.
 My friend also working the same repo, and he needs to push the changes
 on the same.
 
 I tried by adding below line on the remote config file
 [config]
 sharedRepository = true
 
 Any help, how to do that.
 

Please see
http://git-scm.com/book/en/Git-on-the-Server-Getting-Git-on-a-Server
to explore different ways how to use git on a server.

Stefan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 17/22] refs.c: add a backend method structure with transaction functions

2014-08-11 Thread Ronnie Sahlberg
On Fri, Aug 8, 2014 at 11:17 AM, David Turner dtur...@twopensource.com wrote:
 On Fri, 2014-08-08 at 09:45 -0700, Ronnie Sahlberg wrote:

 +struct ref_be refs_files = {
 + .transaction_begin  = files_transaction_begin,
 + .transaction_update_sha1= files_transaction_update_sha1,
 + .transaction_create_sha1= files_transaction_create_sha1,
 + .transaction_delete_sha1= files_transaction_delete_sha1,
 + .transaction_update_reflog  = files_transaction_update_reflog,
 + .transaction_commit = files_transaction_commit,
 + .transaction_free   = files_transaction_free,
 +};

 C99 designated initializers are unfortunately forbidden by
 CodingGuidelines.


Right. Fixed it.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/22] refs.c: create a public function for is_refname_available

2014-08-11 Thread Ronnie Sahlberg
Fixed. Thanks!


On Fri, Aug 8, 2014 at 10:27 AM, David Turner dtur...@twopensource.com wrote:
 On Fri, 2014-08-08 at 09:44 -0700, Ronnie Sahlberg wrote:
 + * Check is a particular refname is available for creation. skip
 contains

 s/Check is/Check that/'

 + * a list of refnames to exclude from the refname collission tests.

 collision

 + */
 +int is_refname_available(const char *refname, const char **skip, int
 skipnum);
 +
 +/*


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Fixing unclear messages

2014-08-11 Thread Junio C Hamano
Alexander Shopov a...@kambanaria.org writes:

 - printf_ln(_(Huh (%s)?), (*ptr)-buf);
 + printf_ln(_(Wrong choice (%s). Choose again.), 
 (*ptr)-buf);
 Why is this an improvement?
 Because 'Huh?' without intonation, gesture or a frown provided by a
 human being is hard to understand. It does not state that it is the
 choice the user provided is wrong and does not prompt the user for the
 next action.

This is shown in a human-interactive exchange where a menu has
already given by list_and_choose().  If there were something else
Huh? could mean after you give a response to that prompt, but I do
not think there is.

 You say that my change does not tangibly improve on an initially
 unclear and already obsolete message, am I right?

Not really.  This is not obsolete (it is a good trick even in
today's world order), but is not teaching anything new to the user
because it has to be issued too late (and there is no way to give it
before the user realizes it is necessary), so it is not unclear
per-se, but it does not help much.  If I were asked to say what it
is then, I would say it reassures.

I do not strongly oppose its removal, but if we were to remove it,
we shold add a comment to the condition of the previous if
statement to remind us why no argument with only is allowed if
amend is set.

 There was a suggestion to have a separate function that is meant to
 echo messages when there are genuine bugs in Git (impossible cases
 happening, invariants breaking, etc.) This will allow factoring out
 the clause This is a bug in Git. Please report it to the developers
 with an e-mail to git@vger.kernel.org. as a single message and I
 prefer that for ease of maintenance.

Yes, I see the primary value of this thread was to trigger that
suggestion to classify which die()s are BUG()s.

 - die(_(Two output directories?));
 + die(_(Maximum one output directory is allowed.));
 Why is it an improvement?
 Because the question only implies that the problem is the number of
 directories but says nothing how many directories there should be (0,
 1, 3... 100?)

Because I've never imagined anybody would sensibly expect mv a1
a2... dst1 dst2 dst3... to work (what does that even mean?  Make N
copies of a$m and drop them into each of dst$n?), I never thought of
such a possiblity.  Trying to help more people is good, unless it
needs to be done by bending backwards, and your rewrite here is
definitely a good one in that sense.

 I am not sure what you mean or your objection would be, perhaps I am
 misreading the source of Git.
 ...
 Does this mean the following changes are totally unwelcome or you

FWIW, I see it as a feature to have small number of messages phrased
in colourful ways, especially the ones that do not require reaction
by the users (e.g. trivial merge succeeded does not trigger oops,
I have to ^C and recover immediately) or the required reaction is
obvious (e.g. you gave me no X and expect me to work? can only
mean ah, I have to give X)

We obviously do not want to overdo it, but the ones we have are all
old ones.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 04/23] rebase -i: hide interactive command messages in verbose mode

2014-08-11 Thread Thomas Rast
Fabian Ruch baf...@gmail.com writes:

 Hi Thomas,

 Thomas Rast writes:
 Fabian Ruch baf...@gmail.com writes:
 @@ -923,6 +923,8 @@ EOF
 ;;
  esac
  
 +mkdir -p $state_dir || die Could not create temporary $state_dir
 +
  git var GIT_COMMITTER_IDENT /dev/null ||
 die You need to set your committer info first
  
 @@ -938,7 +940,6 @@ then
  fi
  
  orig_head=$(git rev-parse --verify HEAD) || die No HEAD?
 -mkdir -p $state_dir || die Could not create temporary $state_dir
  
  :  $state_dir/interactive || die Could not mark as interactive
  write_basic_state
 
 Why this change?  I can't figure out how it relates to the output
 change.

 Creating the state directory a few steps earlier into
 'git_rebase__interactive' is necessary because the changed definition of
 'output' needs it for 'editor.sh'. This change was triggered by a
 failing test case that used the branch argument with git-rebase. The
 'git checkout branch', which is executed if 'switch_to' is set to
 branch, is wrapped into an 'output' line and 'output' failed because
 it wasn't able to create 'editor.sh'.
[...]
 In order to temporarily redirect the editor output, the new
 definition of `output` creates a script in the state directory to be
 used as `GIT_EDITOR`. Make sure the state directory exists before
 `output` is called for the first time.

Ah, makes sense.  Thanks for the explanations!

-- 
Thomas Rast
t...@thomasrast.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 08/23] rebase -i: reword executes pre-commit hook on interim commit

2014-08-11 Thread Thomas Rast
Fabian Ruch baf...@gmail.com writes:

 Hi Thomas,

 Thomas Rast writes:
 Fabian Ruch baf...@gmail.com writes:
 Subject: Re: [PATCH v2 08/23] rebase -i: reword executes pre-commit
 hook on interim commit
 
 I think the change makes sense, but can you reword the subjects that it
 describes the state after the commit (i.e. what you are doing), instead
 of before the commit?

 The log message subject now reads as follows:

 rebase -i: do not verify reworded patches using pre-commit

Excellent wording, thanks!

-- 
Thomas Rast
t...@thomasrast.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Sharing merge conflict resolution between multiple developers

2014-08-11 Thread Junio C Hamano
Chris Packham judge.pack...@gmail.com writes:

 Is there any way where we could share the conflict resolution around
 but still end up with a single merge commit.

One idea that immediately comes to me is to use something like
rerere (not its implementation and storage, but the underlying
idea) enhanced with the trick I use to fix-up merges in my daily
integration cycle (look for merge-fix in howto/maintain-git.txt
in Documentation/).

 developer A:
   git merge $upstream
   conflicts

And then commit this immediately, together with conflict markers
(i.e. commit -a), and discard it with reset --hard HEAD^ *after*
storing it somewhere safe.  And then redo the same merge, resolve
the conflicts and commit the usual way.

The difference between the final conflict resolution and the
original conflicted state can be used as a reference for others to
redo the same conflict resolution later elsewhere.  That can most
easily be done by creating a commit that records the final state
whose parent is the one you recorded the initial conflicted state.

So, the recording phase may go something like this:

git checkout $this
git merge $that
git commit -a -m 'merge-fix/$this-$that preimage'
git branch merge-fix/$this-$that
git reset --hard HEAD^
git merge $that
edit
git commit -a -m 'merge $that to $this'
git checkout merge-fix/$this-$that
git read-tree -m -u HEAD $this
git commit -a -m 'merge-fix/$this-$that postimage'

The rough idea is git show merge-fix/$this-$that will show the
patch you can apply on top of the conflicted state other people
would get by running git merge $that while on $this branch.

rerere essentially does the above recording (and replaying)
per-path and it comes with a clever indexing scheme to identify
which previous conflict resolution would apply to the conflicts you
see in your working tree.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 6/8] mv: unindent one level for directory move code

2014-08-11 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  builtin/mv.c | 47 +--
  1 file changed, 21 insertions(+), 26 deletions(-)

 diff --git a/builtin/mv.c b/builtin/mv.c
 index dcfcb11..988945c 100644
 --- a/builtin/mv.c
 +++ b/builtin/mv.c
 @@ -171,42 +171,37 @@ int cmd_mv(int argc, const char **argv, const char 
 *prefix)
lstat(dst, st) == 0)
   bad = _(cannot move directory over file);
   else if (src_is_dir) {
 + int first = cache_name_pos(src, length), last;
   if (first = 0)
   prepare_move_submodule(src, first,
  submodule_gitfile + i);
 + else if (index_range_of_same_dir(src, length,
 +  first, last)  1) {

The function returns (last - first), so (last - first)  1 holds
inside this block, right?

   modes[i] = WORKING_DIRECTORY;
   if (last - first  1)
   bad = _(source directory is empty);

Then do you need this conditional, or it is always bad here?

If it is always bad, then modes[i] do not need to be assigned to,
either, I think.

Am I missing something?

 + } else { /* last - first = 1 */
 + int j, dst_len, n;

 + modes[i] = WORKING_DIRECTORY;
 + n = argc + last - first;
 ...

Otherwise, perhaps squash this in?

diff --git a/builtin/mv.c b/builtin/mv.c
index bf513e0..bf784cb 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -172,15 +172,14 @@ int cmd_mv(int argc, const char **argv, const char 
*prefix)
bad = _(cannot move directory over file);
else if (src_is_dir) {
int first = cache_name_pos(src, length), last;
+
if (first = 0)
prepare_move_submodule(src, first,
   submodule_gitfile + i);
else if (index_range_of_same_dir(src, length,
-first, last)  1) {
-   modes[i] = WORKING_DIRECTORY;
-   if (last - first  1)
-   bad = _(source directory is empty);
-   } else { /* last - first = 1 */
+first, last)  1)
+   bad = _(source directory is empty);
+   else { /* last - first = 1 */
int j, dst_len, n;
 
modes[i] = WORKING_DIRECTORY;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Sharing merge conflict resolution between multiple developers

2014-08-11 Thread Christian Couder
Le 11 août 2014 07:50, Christian Couder christian.cou...@gmail.com a écrit :

 This should be possible using git imerge which is separate tool.

Sorry I sent the above using the gmail app on my mobile phone and
unfortunately I can't make it send plain text emails.
(Emails which are not plain text are rejected by vger.kernel.org.)

Best,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] prepare_revision_walk: Check for return value in all places

2014-08-11 Thread Junio C Hamano
Stefan Beller stefanbel...@gmail.com writes:

 Even the documentation tells us:
   You should check if it
   returns any error (non-zero return code) and if it does not, you can
   start using get_revision() to do the iteration.

 In preparation for this commit, I grepped all occurrences of
 prepare_revision_walk and added error messages, when there were none.

Thanks.  One niggling thing is that I do not seem to find a way for
the function to actually return an error without dying in it, but
these are independently good changes.


 Signed-off-by: Stefan Beller stefanbel...@gmail.com
 ---
  builtin/branch.c | 4 +++-
  builtin/commit.c | 3 ++-
  remote.c | 3 ++-
  3 files changed, 7 insertions(+), 3 deletions(-)

 diff --git a/builtin/branch.c b/builtin/branch.c
 index 0591b22..ced422b 100644
 --- a/builtin/branch.c
 +++ b/builtin/branch.c
 @@ -653,7 +653,9 @@ static int print_ref_list(int kinds, int detached, int 
 verbose, int abbrev, stru
   add_pending_object(ref_list.revs,
  (struct object *) filter, );
   ref_list.revs.limited = 1;
 - prepare_revision_walk(ref_list.revs);
 +
 + if (prepare_revision_walk(ref_list.revs))
 + die(_(revision walk setup failed));
   if (verbose)
   ref_list.maxwidth = calc_maxwidth(ref_list);
   }
 diff --git a/builtin/commit.c b/builtin/commit.c
 index 7867768..bb84e1d 100644
 --- a/builtin/commit.c
 +++ b/builtin/commit.c
 @@ -1055,7 +1055,8 @@ static const char *find_author_by_nickname(const char 
 *name)
   revs.mailmap = mailmap;
   read_mailmap(revs.mailmap, NULL);
  
 - prepare_revision_walk(revs);
 + if (prepare_revision_walk(revs))
 + die(revision walk setup failed);
   commit = get_revision(revs);
   if (commit) {
   struct pretty_print_context ctx = {0};
 diff --git a/remote.c b/remote.c
 index 894db09..112e4d5 100644
 --- a/remote.c
 +++ b/remote.c
 @@ -1893,7 +1893,8 @@ int stat_tracking_info(struct branch *branch, int 
 *num_ours, int *num_theirs)
  
   init_revisions(revs, NULL);
   setup_revisions(rev_argc, rev_argv, revs, NULL);
 - prepare_revision_walk(revs);
 + if (prepare_revision_walk(revs))
 + die(revision walk setup failed);
  
   /* ... and count the commits on each side. */
   *num_ours = 0;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] prepare_revision_walk: Check for return value in all places

2014-08-11 Thread Stefan Beller
On 11.08.2014 21:09, Junio C Hamano wrote:
 Stefan Beller stefanbel...@gmail.com writes:
 
 Even the documentation tells us:
  You should check if it
  returns any error (non-zero return code) and if it does not, you can
  start using get_revision() to do the iteration.

 In preparation for this commit, I grepped all occurrences of
 prepare_revision_walk and added error messages, when there were none.
 
 Thanks.  One niggling thing is that I do not seem to find a way for
 the function to actually return an error without dying in it, but
 these are independently good changes.
 

 Signed-off-by: Stefan Beller stefanbel...@gmail.com
 ---
  builtin/branch.c | 4 +++-
  builtin/commit.c | 3 ++-
  remote.c | 3 ++-
  3 files changed, 7 insertions(+), 3 deletions(-)

 diff --git a/builtin/branch.c b/builtin/branch.c
 index 0591b22..ced422b 100644
 --- a/builtin/branch.c
 +++ b/builtin/branch.c
 @@ -653,7 +653,9 @@ static int print_ref_list(int kinds, int detached, int 
 verbose, int abbrev, stru
  add_pending_object(ref_list.revs,
 (struct object *) filter, );
  ref_list.revs.limited = 1;
 -prepare_revision_walk(ref_list.revs);
 +
 +if (prepare_revision_walk(ref_list.revs))
 +die(_(revision walk setup failed));
  if (verbose)
  ref_list.maxwidth = calc_maxwidth(ref_list);
  }
 diff --git a/builtin/commit.c b/builtin/commit.c
 index 7867768..bb84e1d 100644
 --- a/builtin/commit.c
 +++ b/builtin/commit.c
 @@ -1055,7 +1055,8 @@ static const char *find_author_by_nickname(const char 
 *name)
  revs.mailmap = mailmap;
  read_mailmap(revs.mailmap, NULL);
  
 -prepare_revision_walk(revs);
 +if (prepare_revision_walk(revs))
 +die(revision walk setup failed);

Just reviewed the patch myself and here in commit.c we should have a
localized version, right?
Should I resend the patch with the localisation or could you just amend
that?

  commit = get_revision(revs);
  if (commit) {
  struct pretty_print_context ctx = {0};
 diff --git a/remote.c b/remote.c
 index 894db09..112e4d5 100644
 --- a/remote.c
 +++ b/remote.c
 @@ -1893,7 +1893,8 @@ int stat_tracking_info(struct branch *branch, int 
 *num_ours, int *num_theirs)
  
  init_revisions(revs, NULL);
  setup_revisions(rev_argc, rev_argv, revs, NULL);
 -prepare_revision_walk(revs);
 +if (prepare_revision_walk(revs))
 +die(revision walk setup failed);
  
  /* ... and count the commits on each side. */
  *num_ours = 0;

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Sharing merge conflict resolution between multiple developers

2014-08-11 Thread Nico Williams
IIUC, this might help,

http://www.mail-archive.com/git@vger.kernel.org/msg56418.html
http://www.mail-archive.com/git@vger.kernel.org/msg56468.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] unpack-tree.c: remove dead code

2014-08-11 Thread Stefan Beller
In line 1763 of unpack-tree.c we have a condition on the current tree
if (current) {
...
Within this block of code we can assume current to be non NULL, hence
the code after the statement in line 1796:
if (current)
return ...

cannot be reached.

The proposed patch here changes the order of the current tree and the
newtree part. I'm not sure if that's the right way to handle it.

All referenced lines have been introduced in the same commit
076b0adc (2006-07-30, read-tree: move merge functions to the library),
which was just moving the code around.
The outer condition on the current tree (now in line 1763) was introduced
in c859600954df4c292e, June 2005, [PATCH] read-tree: save more user hassles 
during fast-forward.
The inner condition on the current tree was introduced in
ee6566e8d70da682ac4926d, Sept. 2005, [PATCH] Rewrite read-tree

This issue was found by coverity, Id:290002

Signed-off-by: Stefan Beller stefanbel...@gmail.com
---
 unpack-trees.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index c6aa8fb..e6d37ff 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1793,11 +1793,10 @@ int twoway_merge(const struct cache_entry * const *src,
/* all other failures */
if (oldtree)
return o-gently ? -1 : reject_merge(oldtree, 
o);
-   if (current)
-   return o-gently ? -1 : reject_merge(current, 
o);
if (newtree)
return o-gently ? -1 : reject_merge(newtree, 
o);
-   return -1;
+   /* current is definitely exists here */
+   return o-gently ? -1 : reject_merge(current, o);
}
}
else if (newtree) {
-- 
2.1.0.rc2

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


'git pull' inconsistent messages.

2014-08-11 Thread Sergey Organov
Hello,

$ git pull --rebase=false
Already up-to-date.
$ git pull --rebase=true
Current branch v3.5 is up to date.
$ git pull --rebase=preserve
Successfully rebased and updated refs/heads/v3.5.

The last one being particularly misleading as nothing was actually
changed.

git version 1.9.3

-- Sergey.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Fixing unclear messages

2014-08-11 Thread Alexander Shopov
 If there were something else Huh? could mean after you
 give a response to that prompt, but I do not think there is.
OK, you love your Huhs. Good for you. I cannot find a convincing
argument then.

 If I were asked to say what it is then, I would say it reassures.
It is like a jewel you find in a quest? On the other hand you say the
message is rare enough and shown too late to be useful so there is
little gain to change it. OK, fair enough.

 Yes, I see the primary value of this thread was to trigger that
 suggestion to classify which die()s are BUG()s.
Wonderful.

 Because I've never imagined anybody would sensibly expect mv a1...
... your rewrite here is definitely a good one in that sense.
My experience shows that messages need to be as helpful as possible
even at the cost of some repetition.

 FWIW, I see it as a feature to have small number of messages phrased
 in colourful ways, especially the ones that do not require reaction
I really do not know what to say. People can be color-blind even for
messages plus in-jokes frequently do not travel well across languages.
Sharing my experience: the messages were hard to translate because
they were hard to understand.
I had to follow the code in order to understand their meaning and
usage. Hopefully other users of git will be more clever than me.
I did my best at improving the messages but as you do not perceive it
the same way there would be no sense in continuing the discussion much
longer.

Will you reconsider:
- ???
+ unknown state
Recoding problems with translations, settings of console sometimes
lead to missing or wrongly encoded characters to show as '?'. Three
'?' can be confusing when shown in translation.

 We obviously do not want to overdo it, but the ones we have are all old ones.
You overdid it for me. On the positive side I hope I have listed all
oldies but goldies and next changes will be less touchy.

Do you want me redoing this patch or not at all?

Kind regards:
al_shopov
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

2014-08-11 Thread Sergey Organov
Previous description of -f option was wrong as git rebase does not
require -f to perform rebase when current branch is a descendant of
the commit you are rebasing onto, provided commit(s) to be rebased
contain merge(s).

Signed-off-by: Sergey Organov sorga...@gmail.com
---
 Documentation/git-rebase.txt | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 2a93c64..62dac31 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -316,10 +316,9 @@ which makes little sense.
 
 -f::
 --force-rebase::
-   Force the rebase even if the current branch is a descendant
-   of the commit you are rebasing onto.  Normally non-interactive rebase 
will
-   exit with the message Current branch is up to date in such a
-   situation.
+   Force the rebase even if the result will only change commit
+   timestamps. Normally non-interactive rebase will exit with the
+   message Current branch is up to date in such a situation.
Incompatible with the --interactive option.
 +
 You may find this (or --no-ff with an interactive rebase) helpful after
-- 
1.9.3

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 20/23] rebase -i: parse to-do list command line options

2014-08-11 Thread Fabian Ruch
Hi Thomas,

an updated patch is attached below.

Thomas Rast writes: Fabian Ruch baf...@gmail.com writes:
 [...]
 are not supported at the moment. Neither are options that contain
 spaces because the shell expansion of `args` in `do_next` interprets
 white space characters as argument separator, that is a command line
 like

 pick --author A U Thor fa1afe1 Some change

 is parsed as the pick command

 pick --author

 and the commit hash

 A

 which obviously results in an unknown revision error. For the sake of
 completeness, in the example above the message title variable `rest`
 is assigned the string 'U Thor fa1afe1 Some change' (without the
 single quotes).
 
 You could probably trim down the non-example a bit and instead give an
 example :-)

Done. The example reword --signoff is substituted for the
non-example and used for an error message example as well. I hope
that is not confusing.

 Print an error message for unknown or unsupported command line
 options, which means an error for all specified options at the
 moment.
 
 Can you add a test that verifies we catch an obvious unknown option
 (such as --unknown-option)?

Done. The test checks that git-rebase--interactive fails to execute
'pick --unknown-option' and that the rebase can be resumed after
removing the --unknown-option part.

 Cleanly break the `do_next` loop by assigning the special
 value 'unknown' to the local variable `command`, which triggers the
 unknown command case in `do_cmd`.
 [...]
  do_replay () {
  command=$1
 -sha1=$2
 -rest=$3
 +shift
 +
 +opts=
 +while test $# -gt 0
 +do
 +case $1 in
 +-*)
 +warn Unknown option: $1
 +command=unknown
 +;;
 +*)
 +break
 +;;
 
 This seems a rather hacky solution to me.  Doesn't this now print
 
   warning: Unknown option: --unknown-option
   warning: Unknown command: pick --unknown-option
 
 ?
 
 It shouldn't claim the command is unknown if the command itself was
 valid.

OK. do_replay now first checks for unknown commands and exits
immediately if that is the case, ignoring any options specified.

 Also, you speak of do_cmd above, but the unknown command handling seems
 to be part of do_replay?

Fixed.

   Fabian

-- 8 --
Subject: rebase -i: parse to-do list command line options

Read in to-do list lines as

command args

instead of

command sha1 rest

so that to-do list command lines can specify additional arguments
apart from the commit hash and the log message title, which become
the non-options in `args`. Loop over `args`, put all options (an
argument beginning with a dash) in `opts`, stop the loop on the first
non-option and assign it to `sha1`. For instance, the to-do list

reword --signoff fa1afe1 Some change

is parsed as `command=reword`, `opts= '--signoff'` (including the
single quotes and the space for evaluation and concatenation of
options), `sha1=fa1afel` and `rest=Some change`. The loop does not
know the options it parses so that options that take an argument
themselves are not supported at the moment. Neither are options that
contain spaces because the shell expansion of `args` in `do_next`
interprets white space characters as argument separator.

Print an error message for unknown or unsupported command line
options, which means an error for all specified options at the
moment. Cleanly break the `do_next` loop by assigning a reason to the
local variable `malformed`, which triggers the unknown command code
in `do_replay`. Move the error code to the beginning of `do_replay`
so that unknown commands are reported before bad options are as only
the first error we come across is reported. For instance, the to-do
list from above produces the error

Unknown 'reword' option: --signoff
Please fix this using 'git rebase --edit-todo'.

The to-do list is also parsed when the commit hashes are translated
between long and short format before and after the to-do list is
edited. Apply the same procedure as in `do_replay` with the exception
that we only care about where the options stop and the commit hash
begins. Do not reject any options when transforming the commit
hashes.

Enable the specification of to-do list command line options in
`FAKE_LINES` the same way this is accomplished for command lines
passed to `exec`. Define a new `fake_editor.sh` that edits a static
to-do list instead of the one passed as argument when invoked by
git-rebase. Add a test case that checks that unknown options are
refused and can be corrected using `--edit-todo`.

Signed-off-by: Fabian Ruch baf...@gmail.com
---
 git-rebase--interactive.sh | 80 --
 t/lib-rebase.sh| 20 +--
 t/t3427-rebase-line-options.sh | 26 ++
 3 files changed, 105 insertions(+), 21 deletions(-)
 create mode 100755 t/t3427-rebase-line-options.sh

diff --git 

[PATCH] mailsplit.c: remove dead code

2014-08-11 Thread Stefan Beller
This was found by coverity. (Id: 290001)

the variable 'output' is only assigned to a value inequal to NUL,
after all gotos to the corrupt label.
Therefore we can conclude the two removed lines are actually dead code.

Signed-off-by: Stefan Beller stefanbel...@gmail.com
---
 builtin/mailsplit.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index 06296d4..b499014 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -93,8 +93,6 @@ static int split_one(FILE *mbox, const char *name, int 
allow_bare)
return status;
 
  corrupt:
-   if (output)
-   fclose(output);
unlink(name);
fprintf(stderr, corrupt mailbox\n);
exit(1);
-- 
2.1.0.rc2

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Error fatal: not a git repository after auto packing

2014-08-11 Thread Luke Campagnola
Greetings,

I have been working happily with git for a couple of years, and ran
into a mysterious issue today: after running a git-pull during which I
saw the message Auto packing the repository for optimum performance.
I now receive the error Fatal: not a git repository when running any
git commands, and a little investigation revealed that my .git/refs
directory has gone missing, presumably because the refs were all
combined into .git/packed-refs. To restore access to the repository,
all I needed was to `mkdir .git/refs`. Is this a known bug? It seems
like either git should tolerate the absence of a .git/refs directory,
or the auto packer should not remove it.

I am using git 1.9.1 on kubuntu 14.04. The surrounding console output follows:

raven:/home/luke/vispy (transform-cache)$ git commit -a
[transform-cache 15a0fe3] Optimizations for grid_large
 6 files changed, 91 insertions(+), 52 deletions(-)

raven:/home/luke/vispy (transform-cache)$ git push lcampagn transform-cache
Counting objects: 114, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (31/31), done.
Writing objects: 100% (31/31), 4.00 KiB | 0 bytes/s, done.
Total 31 (delta 25), reused 0 (delta 0)
To g...@github.com:lcampagn/vispy.git
   24b37a6..15a0fe3  transform-cache - transform-cache

raven:/home/luke/vispy (transform-cache)$ git fetch vispy
remote: Counting objects: 78, done.
remote: Compressing objects: 100% (78/78), done.
remote: Total 78 (delta 34), reused 0 (delta 0)
Unpacking objects: 100% (78/78), done.
From https://github.com/vispy/vispy
   ec740af..fd8aa37  master - vispy/master
Auto packing the repository for optimum performance. You may also
run git gc manually. See git help gc for more information.
Counting objects: 5349, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (5315/5315), done.
Writing objects: 100% (5349/5349), done.
Total 5349 (delta 3977), reused 0 (delta 0)
Removing duplicate objects: 100% (256/256), done.

raven:/home/luke/vispy (transform-cache)$ git checkout master
Switched to branch 'master'
Deleted 103 .pyc files
Deleted 11 empty directories

raven:/home/luke/vispy$ git pull vispy master
fatal: Not a git repository (or any of the parent directories): .git

raven:/home/luke/vispy$ git status
fatal: Not a git repository (or any of the parent directories): .git

raven:/home/luke/vispy$ ls -al .git
total 288
drwxr-xr-x   6 luke luke   4096 Aug 11 17:09 .
drwxr-xr-x   9 luke luke   4096 Aug 11 09:25 ..
-rw-r--r--   1 luke luke601 Aug 11 12:22 COMMIT_EDITMSG
-rw-rw-r--   1 luke luke   1088 Aug 11 09:21 config
-rw-r--r--   1 luke luke 73 Mar  2 08:41 description
-rw-r--r--   1 luke luke323 Aug 11 16:56 FETCH_HEAD
-rw-r--r--   1 luke luke337 Mar  3 10:18 GIT_COLA_MSG
-rw-r--r--   1 luke luke 193478 Aug 11 11:14 gitk.cache
-rw-rw-r--   1 luke luke 23 Aug 11 17:09 HEAD
drwxr-xr-x   2 luke luke   4096 Mar  8 07:30 hooks
-rw-rw-r--   1 luke luke  31104 Aug 11 17:09 index
drwxr-xr-x   2 luke luke   4096 Aug 11 16:57 info
drwxr-xr-x   3 luke luke   4096 Aug 11 16:56 logs
drwxr-xr-x 105 luke luke   4096 Aug 11 16:57 objects
-rw-rw-r--   1 luke luke 41 Aug 11 09:25 ORIG_HEAD
-rw-rw-r--   1 luke luke   8210 Aug 11 16:56 packed-refs
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Sharing merge conflict resolution between multiple developers

2014-08-11 Thread Chris Packham
On Tue, Aug 12, 2014 at 6:44 AM, Junio C Hamano gits...@pobox.com wrote:
 Chris Packham judge.pack...@gmail.com writes:

 Is there any way where we could share the conflict resolution around
 but still end up with a single merge commit.

 One idea that immediately comes to me is to use something like
 rerere (not its implementation and storage, but the underlying
 idea) enhanced with the trick I use to fix-up merges in my daily
 integration cycle (look for merge-fix in howto/maintain-git.txt
 in Documentation/).

 developer A:
   git merge $upstream
   conflicts

 And then commit this immediately, together with conflict markers
 (i.e. commit -a), and discard it with reset --hard HEAD^ *after*
 storing it somewhere safe.  And then redo the same merge, resolve
 the conflicts and commit the usual way.

 The difference between the final conflict resolution and the
 original conflicted state can be used as a reference for others to
 redo the same conflict resolution later elsewhere.  That can most
 easily be done by creating a commit that records the final state
 whose parent is the one you recorded the initial conflicted state.

 So, the recording phase may go something like this:

 git checkout $this
 git merge $that
 git commit -a -m 'merge-fix/$this-$that preimage'
 git branch merge-fix/$this-$that
 git reset --hard HEAD^
 git merge $that
 edit
 git commit -a -m 'merge $that to $this'
 git checkout merge-fix/$this-$that
 git read-tree -m -u HEAD $this
 git commit -a -m 'merge-fix/$this-$that postimage'

 The rough idea is git show merge-fix/$this-$that will show the
 patch you can apply on top of the conflicted state other people
 would get by running git merge $that while on $this branch.

So how would someone else pickup that postimage and use it?

  git checkout $this
  git merge $that
  git fetch $remote ':/merge-fix/$this-$that postimage'
  git show ':/merge-fix/$this-$that postimage' | git apply (or patch -p1)
  edit


 rerere essentially does the above recording (and replaying)
 per-path and it comes with a clever indexing scheme to identify
 which previous conflict resolution would apply to the conflicts you
 see in your working tree.

I feel a toy patch coming on.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Problem with Git rev-list output

2014-08-11 Thread Crabtree, Andrew
I'm seeing some oddity in one of my repositories where the root commit is being 
output in 'rev-list' even when I pass in a reference that should exclude it 
from being output.

I've attempted to reproduce the issue in a test environment but so far have 
failed at that.

Problem details below as best as I can capture them.  

Seeing the issue with versions of git from 1.7 to 2.1.

Thanks,
-Andrew

The root commit is here 

me@myvm:/pnb/software/userrepos/cache/t4_ghs.git  (BARE:titan4_v14_a)$ git 
cat-file -p 848de9c422c01c1f724d5c3f615bec476911f59f
tree 5be87811b44f560159d9bb6a10a9fe9bd59147b9
author Migration Script migrat...@gaia.rose.hp.com 1318778853 -0700
committer Gustavo Mora Corrales gustavo.mora.corra...@hp.com 1318778853 -0700

Initial synchronization commit
me@myvm:/pnb/software/userrepos/cache/t4_ghs.git  (BARE:titan4_v14_a)$ git 
--version
git version 2.1.0.rc2.3.g67de23d
me@myvm:/pnb/software/userrepos/cache/t4_ghs.git  (BARE:titan4_v14_a)$ git 
merge-base WALLE-J-14-60-GIT_07-DEC-2011 samrom_t4a
848de9c422c01c1f724d5c3f615bec476911f59f
me@myvm:/pnb/software/userrepos/cache/t4_ghs.git  (BARE:titan4_v14_a)$ git 
rev-list 848de9c422c01c1f724d5c3f615bec476911f59f
848de9c422c01c1f724d5c3f615bec476911f59f
me@myvm:/pnb/software/userrepos/cache/t4_ghs.git  (BARE:titan4_v14_a)$ git 
rev-list WALLE-J-14-60-GIT_07-DEC-2011 | wc -l 
2132
me@myvm:/pnb/software/userrepos/cache/t4_ghs.git  (BARE:titan4_v14_a)$ git 
rev-list samrom_t4a | wc -l
316
me@myvm:/pnb/software/userrepos/cache/t4_ghs.git  (BARE:titan4_v14_a)$ git 
rev-list WALLE-J-14-60-GIT_07-DEC-2011 samrom_t4a | wc -l
2447

# commit is reachable from both references as expected 

me@myvm:/pnb/software/userrepos/cache/t4_ghs.git  (BARE:titan4_v14_a)$ git 
rev-list WALLE-J-14-60-GIT_07-DEC-2011 | grep 
848de9c422c01c1f724d5c3f615bec476911f59f
848de9c422c01c1f724d5c3f615bec476911f59f
me@myvm:/pnb/software/userrepos/cache/t4_ghs.git  (BARE:titan4_v14_a)$ git 
rev-list samrom_t4a | grep 848de9c422c01c1f724d5c3f615bec476911f59f
848de9c422c01c1f724d5c3f615bec476911f59f

# Here -I would have expected --preceding the SHA with -boundary specified
me@myvm:/pnb/software/userrepos/cache/t4_ghs.git  (BARE:titan4_v14_a)$ git 
rev-list --boundary WALLE-J-14-60-GIT_07-DEC-2011 samrom_t4a | grep 
848de9c422c01c1f724d5c3f615bec476911f59f
848de9c422c01c1f724d5c3f615bec476911f59f

# here I don't expect the SHA to show up with either command line.
me@myvm:/pnb/software/userrepos/cache/t4_ghs.git  (BARE:titan4_v14_a)$ git 
rev-list WALLE-J-14-60-GIT_07-DEC-2011 ^samrom_t4a | grep 
848de9c422c01c1f724d5c3f615bec476911f59f
848de9c422c01c1f724d5c3f615bec476911f59f
me@myvm:/pnb/software/userrepos/cache/t4_ghs.git  (BARE:titan4_v14_a)$ git 
rev-list samrom_t4a ^WALLE-J-14-60-GIT_07-DEC-2011 | grep 
848de9c422c01c1f724d5c3f615bec476911f59f
me@myvm:/pnb/software/userrepos/cache/t4_ghs.git  (BARE:titan4_v14_a)$
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Sharing merge conflict resolution between multiple developers

2014-08-11 Thread Junio C Hamano
On Mon, Aug 11, 2014 at 4:29 PM, Chris Packham judge.pack...@gmail.com wrote:

 So, the recording phase may go something like this:
 ...
 git checkout merge-fix/$this-$that
 git read-tree -m -u HEAD $this
 git commit -a -m 'merge-fix/$this-$that postimage'

 The rough idea is git show merge-fix/$this-$that will show the
 patch you can apply on top of the conflicted state other people
 would get by running git merge $that while on $this branch.

 So how would someone else pickup that postimage and use it?

   git checkout $this
   git merge $that
   git fetch $remote ':/merge-fix/$this-$that postimage'
   git show ':/merge-fix/$this-$that postimage' | git apply (or patch -p1)

For a simpler case that would work, but because we are not saving
just a patch but two full trees to compare (i.e. merge-fix/$this-$that
is the postimage, its ^1 is the preimage), you should be able to use
the three-way merge in a similar way cherry-pick works. In fact, that
is how rerere replays the recorded resolution, not with a patch -p1.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] pack-objects: turn off bitmaps when we see --shallow lines

2014-08-11 Thread Jeff King
Reachability bitmaps do not work with shallow operations,
because they cache a view of the object reachability that
represents the true objects. Whereas a shallow repository
(or a shallow operation in a repository) is inherently
cutting off the object graph with a graft.

We explicitly disallow the use of bitmaps in shallow
repositories by checking is_repository_shallow(), and we
should continue to do that. However, we also want to
disallow bitmaps when we are serving a fetch to a shallow
client, since we momentarily take on their grafted view of
the world.

It used to be enough to call is_repository_shallow at the
start of pack-objects.  Upload-pack wrote the other side's
shallow state to a temporary file and pointed the whole
pack-objects process at this state with git --shallow-file,
and from the perspective of pack-objects, we really were
in a shallow repo.  But since b790e0f (upload-pack: send
shallow info over stdin to pack-objects, 2014-03-11), we do
it differently: we send --shallow lines to pack-objects over
stdin, and it registers them itself.

This means that our is_repository_shallow check is way too
early (we have not been told about the shallowness yet), and
that it is insufficient (calling is_repository_shallow is
not enough, as the shallow grafts we register do not change
its return value). Instead, we can just turn off bitmaps
explicitly when we see these lines.

Signed-off-by: Jeff King p...@peff.net
---
Sorry not to catch this earlier. The bug is in v2.0, and I only noticed
the regression because a very small percentage of shallow fetches from
GitHub started failing after we deployed v2.0 a few weeks ago. It took
me a while to figure out the reproduction recipe below. :)

Arguably is_repository_shallow should return 1 if anybody has registered
a shallow graft, but that wouldn't be enough to fix this (we'd still
need to check it again _after_ reading the --shallow lines). So I think
this fix is fine here. I don't know if any other parts of the code would
care, though.

 builtin/pack-objects.c  |  1 +
 t/t5311-pack-bitmaps-shallow.sh | 39 +++
 2 files changed, 40 insertions(+)
 create mode 100755 t/t5311-pack-bitmaps-shallow.sh

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 238b502..b59f5d8 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2494,6 +2494,7 @@ static void get_object_list(int ac, const char **av)
if (get_sha1_hex(line + 10, sha1))
die(not an SHA-1 '%s', line + 10);
register_shallow(sha1);
+   use_bitmap_index = 0;
continue;
}
die(not a rev '%s', line);
diff --git a/t/t5311-pack-bitmaps-shallow.sh b/t/t5311-pack-bitmaps-shallow.sh
new file mode 100755
index 000..872a95d
--- /dev/null
+++ b/t/t5311-pack-bitmaps-shallow.sh
@@ -0,0 +1,39 @@
+#!/bin/sh
+
+test_description='check bitmap operation with shallow repositories'
+. ./test-lib.sh
+
+# We want to create a situation where the shallow, grafted
+# view of reachability does not match reality in a way that
+# might cause us to send insufficient objects.
+#
+# We do this with a history that repeats a state, like:
+#
+#  A--   B--   C
+#file=1file=2file=1
+#
+# and then create a shallow clone to the second commit, B.
+# In a non-shallow clone, that would mean we already have
+# the tree for A. But in a shallow one, we've grafted away
+# A, and fetching A to B requires that the other side send
+# us the tree for file=1.
+test_expect_success 'setup shallow repo' '
+   echo 1 file 
+   git add file 
+   git commit -m orig 
+   echo 2 file 
+   git commit -a -m update 
+   git clone --no-local --bare --depth=1 . shallow.git 
+   echo 1 file 
+   git commit -a -m repeat
+'
+
+test_expect_success 'turn on bitmaps in the parent' '
+   git repack -adb
+'
+
+test_expect_success 'shallow fetch from bitmapped repo' '
+   (cd shallow.git  git fetch)
+'
+
+test_done
-- 
2.1.0.rc0.286.g5c67d74
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html