Re: [PATCH v2 02/23] rebase -i: allow squashing empty commits without complaints
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
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
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
- 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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.
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
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
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
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
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
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
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
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
