Re: [PATCH 1/2] t3404: fix another typo
Hey Johannes, Very minor nits. On Wed, Jun 29, 2016 at 8:01 PM, Johannes Schindelin wrote: > The past tense of "to run" is "run", not "ran". Past tense of "to run" is "ran" while past participle is "to run". Past tense: He ran. Past Participle: He has to run. > Signed-off-by: Johannes Schindelin > --- > t/t3404-rebase-interactive.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh > index 66348f1..c7ea8ba 100755 > --- a/t/t3404-rebase-interactive.sh > +++ b/t/t3404-rebase-interactive.sh > @@ -60,7 +60,7 @@ test_expect_success 'setup' ' > test_commit P fileP > ' > > -# "exec" commands are ran with the user shell by default, but this may This sentence seems to be in present tense if I am not wrong (though I am not a native English speaker). > +# "exec" commands are run with the user shell by default, but this may > # be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work > # to create a file. Unsetting SHELL avoids such non-portable behavior > # in tests. It must be exported for it to take effect where needed. The change introduces fixed the grammo properly but the commit message probably reports it incorrectly. Would it be better if the commit message is without changing anything else: The present tense of "to run" is "run", not "ran". Regards, Pranit Bauva -- 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 --rebase should use fast forward merge if possible
Hi, since I have added "pull.rebase=preserve" to my global configuration I wonder why "git pull" also trys to rebase if a fast forward merge is possible. A fast forward merge would speed up every pull if your local branch contains no new commits and the remote branch is ahead. The result would be the same. Is it possible to change the behavior of "git pull --rebase=true|preserve|interactive" to use a fast forward merge if the remote branch is ahead and the local branch contains no new commits? Regards, Mattias -- 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: git pull --rebase should use fast forward merge if possible
neul...@dakosy.de writes: > since I have added "pull.rebase=preserve" to my global configuration I > wonder why "git pull" also trys to rebase if a fast forward merge is > possible. > > A fast forward merge would speed up every pull if your local branch > contains no new commits and the remote branch is ahead. The result would > be the same. > > Is it possible to change the behavior of "git pull > --rebase=true|preserve|interactive" to use a fast forward merge if the > remote branch is ahead and the local branch contains no new commits? Interesting. I do not think of a reason why we shouldn't. If we were still working in scripted Porcelain, it would have been a five minute hack, perhaps like this. contrib/examples/git-pull.sh | 8 1 file changed, 8 insertions(+) diff --git a/contrib/examples/git-pull.sh b/contrib/examples/git-pull.sh index 6b3a03f..3648040 100755 --- a/contrib/examples/git-pull.sh +++ b/contrib/examples/git-pull.sh @@ -358,6 +358,14 @@ fi if test true = "$rebase" then + if git merge-base --is-ancestor "$orig_head" "$merge_head" + then + # We can just fast-forward, as "git rebase --no-ff" + # is nonsense. + git merge --ff-only "$merge_head" + exit + fi + o=$(git show-branch --merge-base $curr_branch $merge_head $oldremoteref) if test "$oldremoteref" = "$o" then -- 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 v4] Refactor recv_sideband()
Nicolas Pitre writes:
> To make it clearer, here's a patch on top of pu that fixes all the
> issues I think are remaining. All tests pass now.
>
> diff --git a/sideband.c b/sideband.c
> index 36a032f..0e6c6df 100644
> --- a/sideband.c
> +++ b/sideband.c
> @@ -23,10 +23,8 @@ int recv_sideband(const char *me, int in_stream, int out)
> const char *term, *suffix;
> char buf[LARGE_PACKET_MAX + 1];
> struct strbuf outbuf = STRBUF_INIT;
> - const char *b, *brk;
> int retval = 0;
>
> - strbuf_addf(&outbuf, "%s", PREFIX);
> term = getenv("TERM");
> if (isatty(2) && term && strcmp(term, "dumb"))
> suffix = ANSI_SUFFIX;
> @@ -34,14 +32,15 @@ int recv_sideband(const char *me, int in_stream, int out)
> suffix = DUMB_SUFFIX;
>
> while (!retval) {
> + const char *b, *brk;
> int band, len;
> len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX,
> 0);
> if (len == 0)
> break;
> if (len < 1) {
> strbuf_addf(&outbuf,
> - "\n%s: protocol error: no band
> designator\n",
> - me);
> + "%s%s: protocol error: no band designator",
> + outbuf.len ? "\n" : "", me);
OK, because you no longer put PREFIX in outbuf, outbuf.len becomes
an easy way to tell if we have anything accumulated, and if there
already is something, we separate our message from it with a LF.
I like the simplicity and clarity of the logic.
> @@ -50,7 +49,8 @@ int recv_sideband(const char *me, int in_stream, int out)
> len--;
> switch (band) {
> case 3:
> - strbuf_addf(&outbuf, "\n%s%s\n", PREFIX, buf + 1);
> + strbuf_addf(&outbuf, "%s%s%s", outbuf.len ? "\n" : "",
> + PREFIX, buf + 1);
Likewise for all other changes.
> - if (outbuf.len)
> + if (outbuf.len) {
> + strbuf_addf(&outbuf, "\n");
> fwrite(outbuf.buf, 1, outbuf.len, stderr);
> + }
... and this does make sense.
Lukas, can you see what is in 'pu' after I push out today's
integration result in several hours and tell us if you like the
result of the SQUASH??? change?
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: preview: What's cooking in git.git (Jun 2016, #10; Tue, 28)
Johannes Schindelin writes: > On Tue, 28 Jun 2016, Junio C Hamano wrote: > >> * jk/ansi-color (2016-06-23) 7 commits >> (merged to 'next' on 2016-06-28 at 354989c) >> + color: support strike-through attribute >> + color: support "italic" attribute >> + color: allow "no-" for negating attributes >> + color: refactor parse_attr >> + add skip_prefix_mem helper >> + doc: refactor description of color format >> + color: fix max-size comment >> >> The output coloring scheme learned two new attributes, italic and >> strike, in addition to existing bold, reverse, etc. >> >> Will merge to 'master'. > > Please note that those "colors" do not work on Windows, at least as far as > I know, I only skimmed the code in set_attr(): > > https://github.com/git/git/blob/v2.9.0/compat/winansi.c#L175-L314 > > ... and it looks as if italic is plainly unsupported, and strike-through > is not handled. This hopefully is a low-hanging-fruit for aspiring new developers in the Windows land, perhaps? We do not use italic/strike as a built-in default style for anything, so we do not have to wait for Windows support of these two attributes to appear to include this topic in the next release. After all, users on "screen", or anything that translates these ANSI colors via termcap/terminfo, do not get them, either. A user may try using these once, notices that her terminal lacks support, and would move on. 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] t/Makefile: add a rule to re-run previously-failed tests
Johannes Schindelin writes: > While developing patch series, it is a good practice to run the test > suite from time to time, just to make sure that obvious bugs are caught > early. With complex patch series, it is common to run `make -j15 -k > test`, i.e. run the tests in parallel and not stop at the first failing > test but continue. Hmmm, my tests run in parallel and do not stop at the first one without '-k'. What are we doing differently? > It is the most convenient way to determine which tests failed after > running the entire test suite, in parallel, to look for left-over "trash > directory.t*" subdirectories in the t/ subdirectory. Good idea, but I'd drop "in the t/ subdirectory" from the description. > +failed: $(patsubst trash,,$(patsubst directory.%,%.sh,$(wildcard trash\ > directory.t[0-9]*))) > + This would not work if you use --root= in GIT_TEST_OPTS, I am afraid. -- 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: git pull --rebase should use fast forward merge if possible
Junio C Hamano writes:
>> Is it possible to change the behavior of "git pull
>> --rebase=true|preserve|interactive" to use a fast forward merge if the
>> remote branch is ahead and the local branch contains no new commits?
>
> Interesting. I do not think of a reason why we shouldn't.
>
> If we were still working in scripted Porcelain, it would have been a
> five minute hack, perhaps like this.
>
> contrib/examples/git-pull.sh | 8
> 1 file changed, 8 insertions(+)
... and if we have to work with built-ins, it becomes a lot larger
than a five-minute hack, unfortunately.
Something like this may have a chance of working ;-)
builtin/pull.c | 22 ++
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index bf3fd3f..777ae56 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -878,10 +878,24 @@ int cmd_pull(int argc, const char **argv, const char
*prefix)
if (merge_heads.nr > 1)
die(_("Cannot merge multiple branches into empty
head."));
return pull_into_void(*merge_heads.sha1, curr_head);
- } else if (opt_rebase) {
- if (merge_heads.nr > 1)
- die(_("Cannot rebase onto multiple branches."));
+ }
+ if (opt_rebase && merge_heads.nr > 1)
+ die(_("Cannot rebase onto multiple branches."));
+
+ if (opt_rebase) {
+ struct commit_list *list = NULL;
+ struct commit *merge_head, *head;
+
+ head = lookup_commit_reference(orig_head);
+ commit_list_insert(head, &list);
+ merge_head = lookup_commit_reference(merge_heads.sha1[0]);
+ if (is_descendant_of(merge_head, list)) {
+ /* we can fast-forward this without invoking rebase */
+ opt_ff = "--ff-only";
+ return run_merge();
+ }
return run_rebase(curr_head, *merge_heads.sha1,
rebase_fork_point);
- } else
+ }
+ else
return run_merge();
}
--
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 9/9] am: make a direct call to merge_recursive
Johannes Schindelin writes:
> From: Junio C Hamano
Did I write this thing?
Having two sets of numbers that illustrated that this is not really
a useful optimization in the bigger picture looks vaguely familiar
(e.g. $gmane/279417), but the numbers are different.
> It feels *slightly* wrong to submit your own patch to review,
> however, please keep in mind that
>
> 1) I changed the patch (o.gently does not exist anymore, so I do
> not set it), and
>
> 2) I added my own timings performed on Windows.
It probably is much less confusing if you take the authorship,
possibly with a passing reference to whatever I wrote as the source
of inspiration in the log message, or something. I do not think I
deserve a credit in this 9-patch series.
I haven't read the other 8 patches, so I cannot comment on it yet.
> builtin/am.c | 27 ++-
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/builtin/am.c b/builtin/am.c
> index 3dfe70b..dd41154 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -1587,25 +1587,26 @@ static int run_fallback_merge_recursive(const struct
> am_state *state,
> unsigned char *our_tree,
> unsigned char *his_tree)
> {
> - struct child_process cp = CHILD_PROCESS_INIT;
> + const unsigned char *bases[1] = {orig_tree};
> + struct merge_options o;
> + struct commit *result;
> + char *his_tree_name;
> int status;
>
> - cp.git_cmd = 1;
> + init_merge_options(&o);
> +
> + o.branch1 = "HEAD";
> + his_tree_name = xstrfmt("%.*s", linelen(state->msg), state->msg);
> + o.branch2 = his_tree_name;
>
> - argv_array_pushf(&cp.env_array, "GITHEAD_%s=%.*s",
> - sha1_to_hex(his_tree), linelen(state->msg),
> state->msg);
> if (state->quiet)
> - argv_array_push(&cp.env_array, "GIT_MERGE_VERBOSITY=0");
> + o.verbosity = 0;
>
> - argv_array_push(&cp.args, "merge-recursive");
> - argv_array_push(&cp.args, sha1_to_hex(orig_tree));
> - argv_array_push(&cp.args, "--");
> - argv_array_push(&cp.args, sha1_to_hex(our_tree));
> - argv_array_push(&cp.args, sha1_to_hex(his_tree));
> + status = merge_recursive_generic(&o, our_tree, his_tree, 1, bases,
> &result);
> + if (status < 0)
> + exit(128);
> + free(his_tree_name);
>
> - status = run_command(&cp) ? (-1) : 0;
> - discard_cache();
> - read_cache();
> return status;
> }
--
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 0/5] Use skip_blank_lines() for more commit messages
Johannes Schindelin writes: > It occurred to me that there might be more code locations where finding > the commit object's body is hand-rolled, and could benefit from using > the skip_blank_lines() function to handle not-quite-standard but > still-allowed objects from being processed correctly. Thanks for following up on this one. We could have left them as low-hanging-fruit for new people, but doing these ourselves while we still remember is a good idea. -- 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 1/2] t3404: fix another typo
Johannes Schindelin writes: > On Wed, 29 Jun 2016, Junio C Hamano wrote: > >> Johannes Schindelin writes: >> >> > The past tense of "to run" is "run", not "ran". >> >> Actually, past tense of the verb "to run" is "ran" ;-) The reason >> why this patch is still correct is because this is writing in >> passive voice, where you want "be + past participle", and the past >> participle of the verb "to run" is "run". > > Embarrassing! ;-) > > Well, I shall fix up the commit message thusly: > > t3404: fix another typo > > The passive form of "to run" is "run", not "ran". If your convention of referring to a verb is to show its infinitive form, i.e. "to run", then you would probably want to say The passive from of "to run" is "to be run", not "to be ran". But I think we do not need any of this if we just retitled it to Subject: t3404: fix a grammo (commands are ran -> commands are run) without any body. -- 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 1/9] Report bugs consistently
On Wed, Jun 29, 2016 at 7:36 AM, Johannes Schindelin
wrote:
> The vast majority of error messages in Git's source code which report a
> bug use the convention to prefix the message with "BUG:".
> [...]
> Signed-off-by: Johannes Schindelin
> ---
> diff --git a/merge-recursive.c b/merge-recursive.c
> @@ -1853,7 +1852,7 @@ int merge_trees(struct merge_options *o,
> - die(_("Unprocessed path??? %s"),
> + die(_("BUG: unprocessed path??? %s"),
This and others downcase the first word (which is consistent with
modern practice)...
> diff --git a/sha1_file.c b/sha1_file.c
> @@ -795,7 +795,7 @@ void close_all_packs(void)
> - die("BUG! Want to close pack marked 'do-not-close'");
> + die("BUG: Want to close pack marked 'do-not-close'");
...but this one neglects to.
--
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] t3404: add a test for the --gpg-sign option
Johannes Schindelin writes: > To keep the time t3404 requires short (in this developer's Windows > setup, this single test already takes a painful 8 minutes to pass), > we avoid a full-blown GPG test and cop out by verifying the message > displayed to the user upon an 'edit' command. This is a tangent, but I wonder if we should be solving it by parallelizing the tests even more. If the script for example can be split into part1 and part2 that share the same set-up that is prepared by the very first test, we could split this into three files (common one that is dot-sourced by two actual test scripts that have part1 and part2). > > Signed-off-by: Johannes Schindelin > --- > t/t3404-rebase-interactive.sh | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh > index c7ea8ba..4c96075 100755 > --- a/t/t3404-rebase-interactive.sh > +++ b/t/t3404-rebase-interactive.sh > @@ -1281,4 +1281,11 @@ test_expect_success 'editor saves as CR/LF' ' > ) > ' > > +EPIPHANY="'" Why? If you really need a variable, SQ is probably the way this codebase typically spell it. > +test_expect_success 'rebase -i --gpg-sign=' ' > + set_fake_editor && > + FAKE_LINES="edit 1" git rebase -i --gpg-sign=\" HEAD^ >out 2>err && > + grep "$EPIPHANY-S\"$EPIPHANY" err I am not sure what is going on here. You are asking to sign using the keyid of a single double-quote, and expecting the message that says You can amend the commit now, with git commit --amend '-S"' ... that has a substring '-S"' in it to ensure that the codepath to parse --gpg-sign= on the command line of "rebase", and to the message we see here are working correctly, without actually checking if GPG is invoked at all, or if it is invoked the key given by the option is correctly passed to the invocation? If so, can't you do that without confusing users by using keyid "? IOW, wouldn't using --gpg-sign=me work equally well? For example: diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 4c96075..2dd3644 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1281,11 +1281,10 @@ test_expect_success 'editor saves as CR/LF' ' ) ' -EPIPHANY="'" test_expect_success 'rebase -i --gpg-sign=' ' set_fake_editor && - FAKE_LINES="edit 1" git rebase -i --gpg-sign=\" HEAD^ >out 2>err && - grep "$EPIPHANY-S\"$EPIPHANY" err + FAKE_LINES="edit 1" git rebase -i --gpg-sign=me HEAD^ >out 2>err && + grep -e "commit --amend '\''-Sme'\''" err ' test_done -- 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/9] merge-recursive: clarify code in was_tracked()
Johannes Schindelin writes:
> It can be puzzling to see that was_tracked() tries to match an index
> entry by name even if cache_name_pos() returned a negative value. Let's
> clarify that cache_name_pos() implicitly looks for stage 0, while we are
> also okay with finding other stages.
>
> Signed-off-by: Johannes Schindelin
> ---
> merge-recursive.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index 98f4632..bcb53f0 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -658,6 +658,7 @@ static int was_tracked(const char *path)
> {
> int pos = cache_name_pos(path, strlen(path));
>
> + /* cache_name_pos() looks for stage == 0, so pos may be < 0 */
It returns >= if found at stage #0, or a negative (counting from -1)
to indicate where the path would be inserted if it were to be added
at stage #0.
The new comment does not explain how "pos may be < 0" leads to
"hence pos = -1 - pos is the right thing to do here". It is
misleading and we probably are better off without.
> if (pos < 0)
> pos = -1 - pos;
> while (pos < active_nr &&
--
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
topological index field for commit objects
This is no RFE but rather recurring thoughts whenever I'm working with commit graphs: a topological index attribute for commit objects would be incredible useful. By "topological index" I mean a simple integer for which following condition holds true: if commit C is part of the history of commit D, then C's topological index is smaller than D's index This would allow topological sorting of commits (e.g. in queues) on the fly and quickly give a "no" answer on the question whether D is part of C's history. -Marc -- 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: topological index field for commit objects
On Wed, Jun 29, 2016 at 11:31 AM, Marc Strapetz wrote: > This is no RFE but rather recurring thoughts whenever I'm working with > commit graphs: a topological index attribute for commit objects would be > incredible useful. By "topological index" I mean a simple integer for which > following condition holds true: Look for "generation numbers" in the list archive, perhaps? -- 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 3/9] Prepare the builtins for a libified merge_recursive()
Johannes Schindelin writes:
> A truly libified function does not die() just for fun.
The sentence is wasting bits. After all, a helper function in
run-once-and-exit program does not die() just for fun, either.
So what's more interesting to know for the readers?
> As such, the
> recursive merge will convert all die() calls to return -1 instead in the
> next commits, giving the caller a chance at least to print some helpful
> message.
>
> Let's prepare the builtins for this fatal error condition, even if we do
> not really do more than imitating the previous die()'s exit(128): this is
> what callers of e.g. `git merge` have come to expect.
One thing missing is your design decision and justification.
For example, the above explanation hints that the original code in
this hunk expected merge_trees() to die() with some useful message
when there is an error condition, but merge_trees() is going to be
improved not to die() itself and return -1 instead to signal an
error, so that the caller can react more flexibly, and this is a
step to prepare for the version of merge_trees() that no longer
dies.
> - merge_trees(&o, new->commit->tree, work,
> + ret = merge_trees(&o, new->commit->tree, work,
> old->commit->tree, &result);
> + if (ret < 0)
> + exit(128);
The postimage of the patch tells us that the caller is now
responsible for exiting with status 128, but neither the proposed
log message nor the above hunk tells us where the message the
original code must have given to the end user from die() inside
merge_trees(). The updated caller just exits, so a natural guess is
that the calls to die() have been changed to fprintf(stderr) with
the patch.
But that does not mesh very well with the stated objective of the
patch. The callers want flexibility to do their own error handling,
including giving their own message, so letting merge_trees() to
still write the same message to the standard error stream would not
work well for them. A caller may want to do merge_trees() just to
see if it succeeds, without wanting to give _any_ indication of that
is happening to the user, because it has an alternate/fallback code
if merge_trees() fails, for example (analogy: "am -3" first tries a
straight patch application before fallking back to 3-way merge; it
may not want to show the error from the first attempt).
The reader _can_ guess that this step ignores the error-message
issue, and improving it later (or keep ignoring that issue) might be
OK in the context of this patch series, but it is necessary to be
upfront to the readers what the design choices were and which one of
those choices the proposed patch adopted as its design for them to
be able to evaluate the patch series correctly.
One design alternative we've seen used in our code to help callers
who want no default messages is to pass struct strbuf &err down the
callchain and collect the default messages without emitting them
directly to the standard error stream when an error is diagnosed. I
do not know if that pattern is applicable or should be applied to
this codepath.
> Note that the callers of the sequencer (revert and cherry-pick) already
> fail fast even for the return value -1; The only difference is that they
> now get a chance to say " failed".
>
> Signed-off-by: Johannes Schindelin
> ---
> builtin/checkout.c | 4 +++-
> builtin/merge.c| 4
> sequencer.c| 4
> 3 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index c3486bd..14312f7 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -567,8 +567,10 @@ static int merge_working_tree(const struct checkout_opts
> *opts,
> o.ancestor = old->name;
> o.branch1 = new->name;
> o.branch2 = "local";
> - merge_trees(&o, new->commit->tree, work,
> + ret = merge_trees(&o, new->commit->tree, work,
> old->commit->tree, &result);
> + if (ret < 0)
> + exit(128);
> ret = reset_tree(new->commit->tree, opts, 0,
>writeout_error);
> if (ret)
> diff --git a/builtin/merge.c b/builtin/merge.c
> index b555a1b..133b853 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -682,6 +682,8 @@ static int try_merge_strategy(const char *strategy,
> struct commit_list *common,
> hold_locked_index(&lock, 1);
> clean = merge_recursive(&o, head,
> remoteheads->item, reversed, &result);
> + if (clean < 0)
> + exit(128);
> if (active_cache_changed &&
> write_locked_index(&the_index, &lock, COMMIT_LOCK))
> die (_("unable t
Re: Fwd: what is a snapshot?
W dniu 2016-06-19 o 16:15, Ovatta Bianca pisze: > I read in every comparison of git vs other version control systems, > that git does not record differences but takes "snapshots" > I was wondering what a "snapshot" is ? Does git store at every commit > the entire files which have been modified even if only a few bytes > were changed out of a large file? There are two things: the conceptual level, and actual storage. On the conceptual level, object representing revisions (commit) refer to object representing top directory (tree) of a project, that is a snapshot of a project state at given revision. On the storage level, Git has two types of object storage. In "loose" format (used for new objects), each object is stored as a separate file. This is not as wasteful as you think: first, there is deduplication, that is each version of a file is stored only once. Second, contents (usually text) is stored compressed. In "packed" format (nowadays Git automatically repacks from "loose" to "packed" when it looks like it is needed) there is additional libxdiff-like deltafication. In this format Git stores differences (well, it also ensures that delta chain doesn't gets too long). HTH -- Jakub Narębski -- 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
[RFD] Place to document magic pathspecs like ":/" and pathspec handling
Hello, 123456789012345678901234567890123456789012345678901234567890123456789012345| I have noticed that the magic pathspec ":/" is described only in RelNotes for revision 1.7.6: |* A magic pathspec ":/" tells a command that limits its operation to the current directory when ran from a subdirectory to work on the entire working tree. In general, ":/path/to/file" would be relative to the root of the working tree hierarchy. After "git reset --hard; edit Makefile; cd t/", "git add -u" would be a no-op, but "git add -u :/" would add the updated contents of the Makefile at the top level. If you want to name a path in the current subdirectory whose unusual name begins with ":/", you can name it by "./:/that/path" or by "\:/that/path".| || |I think the reason might be that there was no good place to put that information in. Nowadays we have gitcli(7) manual page, but perhaps it would be better to create a separate manpage for issues related to pathspec handling (of which ":/" is only one part)... but then what should it be named? What do you think? -- Jakub Narębski ||| -- 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: [RFD] Place to document magic pathspecs like ":/" and pathspec handling
On Wed, Jun 29, 2016 at 12:47 PM, Jakub Narębski wrote: > > I have noticed that the magic pathspec ":/" is described only in RelNotes > for revision 1.7.6: > |I think the reason might be that there was no good place to put that > information in. Nowadays we have gitcli(7) manual page, but perhaps $ git help glossary look for pathspec. -- 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: [RFC/PATCH v2 00/10] Add initial experimental external ODB support
Christian Couder wrote:
> Design discussion about performance
> ~~~
>
> Yeah, it is not efficient to fork/exec a command to just read or write
> one object to or from the external ODB. Batch calls and/or using a
> daemon and/or RPC should be used instead to be able to store regular
> objects in an external ODB. But for now the external ODB would be all
> about really big files, where the cost of a fork+exec should not
> matter much. If we later want to extend usage of external ODBs, yeah
> we will probably need to design other mechanisms.
I would also investigate switching run_command to use vfork+exec
or posix_spawn for performance (keeping in mind vfork
caveats documented at https://ewontfix.com/7/ )
posix_spawn in future glibc (probably 2.24) will use CLONE_VFORK
in all cases under Linux, and posix_spawn may help with
portability, too. I think the only thing we can't support
with posix_spawn which run_command supports is chdir;
all the redirections/closing FDs should be fine.
With only 10MB malloc-ed, the following shows vfork performance
being noticeably faster than plain fork:
/* gcc -o vfork-test -Wall -O2 vfork-test.c */
#include
#include
#include
#include
#include
int main(int argc, char *argv[], char *envp[])
{
int i;
int do_vfork = argc > 1 && !strcmp(argv[1], "vfork");
char * const cmd[] = { "/bin/true", 0 };
size_t n = 1024 * 1024 * 10;
char *mem = malloc(n);
memset(mem, 'a', n); /* make sure it's really allocated */
for (i = 0; i < 1; i++) {
pid_t pid = do_vfork ? vfork() : fork();
if (pid == 0) {
execve(cmd[0], cmd, envp);
write(2, "exec error\n", 11);
_exit(1);
}
waitpid(pid, 0, 0);
}
return 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 4/9] merge_recursive: abort properly upon errors
Johannes Schindelin writes:
> There are a couple of places where return values indicating errors
> are ignored. Let's teach them manners.
>
> Signed-off-by: Johannes Schindelin
> ---
> merge-recursive.c | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index bcb53f0..c4ece96 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1944,8 +1944,9 @@ int merge_recursive(struct merge_options *o,
> saved_b2 = o->branch2;
> o->branch1 = "Temporary merge branch 1";
> o->branch2 = "Temporary merge branch 2";
> - merge_recursive(o, merged_common_ancestors, iter->item,
> - NULL, &merged_common_ancestors);
> + if (merge_recursive(o, merged_common_ancestors, iter->item,
> + NULL, &merged_common_ancestors) < 0)
> + return -1;
> o->branch1 = saved_b1;
> o->branch2 = saved_b2;
> o->call_depth--;
OK, this early return (and others in this patch) are only for
negative (i.e. error) cases, and "attempted a merge, resulted in
conflicts" cases are handled as before.
Which is good.
I wonder if o->branch[12] need to be restored, though. The only
sensible thing the caller can do is to punt, but would it expect to
be able to do some error reporting based on these fields, e.g.
printf("merge of %s and %s failed", o->branch1, o->branch2) or
something? In addition to that kind of "state restoration", we may
need to watch out for resource leaks, but I think there is none at
least these three early returns.
Thanks.
> @@ -1961,6 +1962,8 @@ int merge_recursive(struct merge_options *o,
> o->ancestor = "merged common ancestors";
> clean = merge_trees(o, h1->tree, h2->tree,
> merged_common_ancestors->tree,
> &mrtree);
> + if (clean < 0)
> + return clean;
>
> if (o->call_depth) {
> *result = make_virtual_commit(mrtree, "merged tree");
> @@ -2017,6 +2020,9 @@ int merge_recursive_generic(struct merge_options *o,
> hold_locked_index(lock, 1);
> clean = merge_recursive(o, head_commit, next_commit, ca,
> result);
> + if (clean < 0)
> + return clean;
> +
> if (active_cache_changed &&
> write_locked_index(&the_index, lock, COMMIT_LOCK))
> return error(_("Unable to write index."));
--
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: topological index field for commit objects
On Wed, Jun 29, 2016 at 11:59 AM, Junio C Hamano wrote: > On Wed, Jun 29, 2016 at 11:31 AM, Marc Strapetz > wrote: >> This is no RFE but rather recurring thoughts whenever I'm working with >> commit graphs: a topological index attribute for commit objects would be >> incredible useful. By "topological index" I mean a simple integer for which >> following condition holds true: > > Look for "generation numbers" in the list archive, perhaps? Thanks for the pointer to the interesting discussions. In http://www.spinics.net/lists/git/msg161363.html Linus wrote in a discussion with Jeff: > Right now, we do *have* a "generation number". It's just that it's > very easy to corrupt even by mistake. It's called "committer date". We > could improve on it. Would it make sense to refuse creating commits that have a commit date prior to its parents commit date (except when the user gives a `--dammit-I-know-I-break-a-wildy-used-heuristic`)? With the proposal to refuse an earlier committer date we would avoid the corruption by mistake and only allow for corruption by actual users intention. And then going forward we could rely more on the committer date than we do today (i.e. some algorithms can go faster)? For that we'd * need to document, what stuff actually breaks if the user overwrites the committer date to be earlier * and add a switch `--go-slow-because-dates-are-broken` This seems to be the easiest fix in 2016 to me as that would require to make just a very small change in commit creation, and we can postpone the second part (relying even more on dates) until someone wants to fix it? Thanks, 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 -- 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 5/9] merge-recursive: avoid returning a wholesale struct
Johannes Schindelin writes:
> It is technically allowed, as per C89, for functions' return type to
> be complete structs (i.e. *not* just pointers to structs), but it is
> bad practice.
Not necessarily.
> This is a very late attempt to contain the damage done by this developer
> in 6d297f8 (Status update on merge-recursive in C, 2006-07-08) which
> introduced such a return type.
>
> It will also help the current effort to libify merge-recursive.c, as
> it will allow us to return proper error codes later.
But this part of the motivation does make sense. Having to pass an
extra int &error_code field, only because the return value is
already used for something else, is a lot more weird.
> Signed-off-by: Johannes Schindelin
> ---
> merge-recursive.c | 93
> ++-
> 1 file changed, 50 insertions(+), 43 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index c4ece96..d56651c9 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -888,47 +888,47 @@ static int merge_3way(struct merge_options *o,
> return merge_status;
> }
>
> -static struct merge_file_info merge_file_1(struct merge_options *o,
> +static int merge_file_1(struct merge_options *o,
> const struct diff_filespec *one,
> const struct diff_filespec *a,
> const struct diff_filespec *b,
> const char *branch1,
> -const char *branch2)
> +const char *branch2,
> +struct merge_file_info *result)
> {
> - struct merge_file_info result;
> - result.merge = 0;
> - result.clean = 1;
> + result->merge = 0;
> + result->clean = 1;
>
> if ((S_IFMT & a->mode) != (S_IFMT & b->mode)) {
> - result.clean = 0;
> + result->clean = 0;
> if (S_ISREG(a->mode)) {
> - result.mode = a->mode;
> - hashcpy(result.sha, a->sha1);
> + result->mode = a->mode;
> + hashcpy(result->sha, a->sha1);
> } else {
> - result.mode = b->mode;
> - hashcpy(result.sha, b->sha1);
> + result->mode = b->mode;
> + hashcpy(result->sha, b->sha1);
> }
> } else {
> if (!sha_eq(a->sha1, one->sha1) && !sha_eq(b->sha1, one->sha1))
> - result.merge = 1;
> + result->merge = 1;
>
> /*
>* Merge modes
>*/
> if (a->mode == b->mode || a->mode == one->mode)
> - result.mode = b->mode;
> + result->mode = b->mode;
> else {
> - result.mode = a->mode;
> + result->mode = a->mode;
> if (b->mode != one->mode) {
> - result.clean = 0;
> - result.merge = 1;
> + result->clean = 0;
> + result->merge = 1;
> }
> }
>
> if (sha_eq(a->sha1, b->sha1) || sha_eq(a->sha1, one->sha1))
> - hashcpy(result.sha, b->sha1);
> + hashcpy(result->sha, b->sha1);
> else if (sha_eq(b->sha1, one->sha1))
> - hashcpy(result.sha, a->sha1);
> + hashcpy(result->sha, a->sha1);
> else if (S_ISREG(a->mode)) {
> mmbuffer_t result_buf;
> int merge_status;
> @@ -940,62 +940,63 @@ static struct merge_file_info merge_file_1(struct
> merge_options *o,
> die(_("Failed to execute internal merge"));
>
> if (write_sha1_file(result_buf.ptr, result_buf.size,
> - blob_type, result.sha))
> + blob_type, result->sha))
> die(_("Unable to add %s to database"),
> a->path);
>
> free(result_buf.ptr);
> - result.clean = (merge_status == 0);
> + result->clean = (merge_status == 0);
> } else if (S_ISGITLINK(a->mode)) {
> - result.clean = merge_submodule(result.sha,
> + result->clean = merge_submodule(result->sha,
> one->path, one->sha1,
> a->sha1, b->sha1,
> !o->call_depth);
> } else if (S_ISLNK(a->mode)) {
> - hashcpy(res
Re: git pull --rebase should use fast forward merge if possible
On Wed, Jun 29, 2016 at 10:23 AM, Junio C Hamano wrote:
> Junio C Hamano writes:
>
>>> Is it possible to change the behavior of "git pull
>>> --rebase=true|preserve|interactive" to use a fast forward merge if the
>>> remote branch is ahead and the local branch contains no new commits?
>>
>> Interesting. I do not think of a reason why we shouldn't.
Me neither.
>>
>> If we were still working in scripted Porcelain, it would have been a
>> five minute hack, perhaps like this.
>>
>> contrib/examples/git-pull.sh | 8
>> 1 file changed, 8 insertions(+)
>
> ... and if we have to work with built-ins, it becomes a lot larger
> than a five-minute hack, unfortunately.
>
> Something like this may have a chance of working ;-)
>
> builtin/pull.c | 22 ++
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index bf3fd3f..777ae56 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -878,10 +878,24 @@ int cmd_pull(int argc, const char **argv, const char
> *prefix)
> if (merge_heads.nr > 1)
> die(_("Cannot merge multiple branches into empty
> head."));
> return pull_into_void(*merge_heads.sha1, curr_head);
> - } else if (opt_rebase) {
> - if (merge_heads.nr > 1)
> - die(_("Cannot rebase onto multiple branches."));
> + }
> + if (opt_rebase && merge_heads.nr > 1)
> + die(_("Cannot rebase onto multiple branches."));
> +
> + if (opt_rebase) {
> + struct commit_list *list = NULL;
> + struct commit *merge_head, *head;
> +
> + head = lookup_commit_reference(orig_head);
> + commit_list_insert(head, &list);
> + merge_head = lookup_commit_reference(merge_heads.sha1[0]);
The crashes for merge_heads.nr == 0.
(I did not inspect code further up if this is caught before, I think
it would trigger if
you and the remote are on an initial commit with no parents?)
> + if (is_descendant_of(merge_head, list)) {
> + /* we can fast-forward this without invoking rebase */
> + opt_ff = "--ff-only";
> + return run_merge();
> + }
> return run_rebase(curr_head, *merge_heads.sha1,
> rebase_fork_point);
> - } else
> + }
> + else
Keep the style ?
> return run_merge();
> }
> --
> 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
--
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: git pull --rebase should use fast forward merge if possible
On Wed, Jun 29, 2016 at 1:40 PM, Stefan Beller wrote: >> + merge_head = lookup_commit_reference(merge_heads.sha1[0]); > > The crashes for merge_heads.nr == 0. > (I did not inspect code further up if this is caught before, I think > it would trigger if > you and the remote are on an initial commit with no parents?) Perhaps you can inspect code before you start typing? die_no_merge_candidates() would have triggered, I would imagine. Note that I won't be applying this without test updates and proper log message, so no need to worry about the style yet ;-) -- 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: topological index field for commit objects
Stefan Beller writes: > On Wed, Jun 29, 2016 at 11:59 AM, Junio C Hamano wrote: >> On Wed, Jun 29, 2016 at 11:31 AM, Marc Strapetz >> wrote: >>> This is no RFE but rather recurring thoughts whenever I'm working with >>> commit graphs: a topological index attribute for commit objects would be >>> incredible useful. By "topological index" I mean a simple integer for which >>> following condition holds true: >> >> Look for "generation numbers" in the list archive, perhaps? > > Thanks for the pointer to the interesting discussions. > > In http://www.spinics.net/lists/git/msg161363.html > Linus wrote in a discussion with Jeff: > >> Right now, we do *have* a "generation number". It's just that it's >> very easy to corrupt even by mistake. It's called "committer date". We >> could improve on it. > > Would it make sense to refuse creating commits that have a commit date > prior to its parents commit date (except when the user gives a > `--dammit-I-know-I-break-a-wildy-used-heuristic`)? I think that has also been discussed in the past. I do not think it would help very much in practice, as projects already have up to 10 years (and the ones migrated from CVS, even more) worth of commits they cannot rewrite that may record incorrect committer dates. You'd need something like "you can trust committer dates that are newer that this date" per project to switch between slow path and fast path, with an updated fsck that knows how to compute that number after you pulled from somebody who used that overriding option. If the use of generation number can somehow be limited narrowly, we may be able to incrementally introduce it only for new commits, but I haven't thought things through, so let me do so aloud here ;-) Suppose we use it only for this purpose: * When we have two commits, C1 and C2, with generation numbers G1 and G2, we can say "C1 cannot possibly be an ancestor of C2" if G1 > G2. We cannot say anything else based on generation numbers (or lack thereof). then I think we could just say "A newly created commit must record generation number G that is larger than generation numbers of its parent commits; ignore parents that lack generation number for the purpose of this sentence". I am not sure if that limited use is all that useful, though. -- 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 1/9] Report bugs consistently
Johannes Schindelin writes:
> The vast majority of error messages in Git's source code which report a
> bug use the convention to prefix the message with "BUG:".
Good thing to do.
But if we were to review and apply a 200+ line patch, I wonder if we
want to go one step further to allow us to write
BUG("killed-file %s not found", name);
instead.
--
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: topological index field for commit objects
On Wed, Jun 29, 2016 at 01:39:17PM -0700, Junio C Hamano wrote: > > Would it make sense to refuse creating commits that have a commit date > > prior to its parents commit date (except when the user gives a > > `--dammit-I-know-I-break-a-wildy-used-heuristic`)? > > I think that has also been discussed in the past. I do not think it > would help very much in practice, as projects already have up to 10 > years (and the ones migrated from CVS, even more) worth of commits > they cannot rewrite that may record incorrect committer dates. Yep, it has been discussed and I agree it runs into a lot of corner cases. > If the use of generation number can somehow be limited narrowly, we > may be able to incrementally introduce it only for new commits, but > I haven't thought things through, so let me do so aloud here ;-) I think the problem is that you really _do_ want generation numbers for old commits. One of the most obvious cases is something like "tag --contains HEAD", because it has to examine older tags. So your history looks something like: A -- B -- ... Z \\ v1.0 HEAD Without generation numbers (or some proxy), you have to walk the history between B..Z to find the answer. With generation numbers, it is immediately obvious. So this is the ideal case for generation numbers (the worst cases are when the things you are looking for are in branchy, close history where the generation numbers don't tell you much; but in such cases the walking is usually not too bad). So I think you really do want to be able to generate and store generation numbers after the fact. That has an added bonus that you do not have to worry about baking incorrect values into your objects; you do the topological walk once, and you _know_ it is correct (at least as correct as the parent links, but that is our source of truth). I have patches that generate and store the numbers at pack time, similar to the way we do the reachability bitmaps. They're not production ready, but they could probably be made so without too much effort. You wouldn't have ready-made generation numbers for commits since the last full repack, but you can compute them incrementally based on what you do have at a cost linear to the unpacked commits (this is the same for bitmaps). -Peff -- 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: topological index field for commit objects
W dniu 2016-06-29 o 20:59, Junio C Hamano pisze: > On Wed, Jun 29, 2016 at 11:31 AM, Marc Strapetz > wrote: > >> This is no RFE but rather recurring thoughts whenever I'm working with >> commit graphs: a topological index attribute for commit objects would be >> incredible useful. By "topological index" I mean a simple integer for which >> following condition holds true: >> >> if commit C is part of the history of commit D, >>then C's topological index is smaller than D's index >> >> This would allow topological sorting of commits (e.g. in queues) on the fly >> and quickly give a "no" answer on the question whether D is part of >> C's history. > > Look for "generation numbers" in the list archive, perhaps? If I remember correctly the discussion, the problem by adding generation number to the commit object format was twofold (at least). First there was a problem of backward compatibility, namely what to do with existing repositories, where commit objects do not have generation number. Objects in Git are immutable (and I think replacements mechanism wasn't invented yet - anyway too many replacements would slow down operations I guess). Second objection was of philosophical nature: generation numbers duplicate (cache) information that is stored in the graph of revisions. Also, what if they get out of sync? That is, if I remember the summary of that discussion correctly. Also, generation numbers (graph level) only help with topological sorting; for finding of two commits are connected (two nodes are connected) people play with different ideas, for example FELINE index: http://openproceedings.org/EDBT/2014/paper_166.pdf Nowadays there is also [compressed] bitmap index (if enabled), though I am not sure if it is yet used to speed-up reachability queries http://githubengineering.com/counting-objects/ https://www.eclipsecon.org/2013/sites/eclipsecon.org.2013/files/Scaling%20Up%20JGit%20-%20EclipseCon%202013.pdf -- Jakub Narębski -- 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: topological index field for commit objects
On Wed, Jun 29, 2016 at 1:39 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> On Wed, Jun 29, 2016 at 11:59 AM, Junio C Hamano wrote: >>> On Wed, Jun 29, 2016 at 11:31 AM, Marc Strapetz >>> wrote: This is no RFE but rather recurring thoughts whenever I'm working with commit graphs: a topological index attribute for commit objects would be incredible useful. By "topological index" I mean a simple integer for which following condition holds true: >>> >>> Look for "generation numbers" in the list archive, perhaps? >> >> Thanks for the pointer to the interesting discussions. >> >> In http://www.spinics.net/lists/git/msg161363.html >> Linus wrote in a discussion with Jeff: >> >>> Right now, we do *have* a "generation number". It's just that it's >>> very easy to corrupt even by mistake. It's called "committer date". We >>> could improve on it. >> >> Would it make sense to refuse creating commits that have a commit date >> prior to its parents commit date (except when the user gives a >> `--dammit-I-know-I-break-a-wildy-used-heuristic`)? > > I think that has also been discussed in the past. I should have guessed that and tried to find it. > I do not think it > would help very much in practice, as projects already have up to 10 > years (and the ones migrated from CVS, even more) worth of commits > they cannot rewrite that may record incorrect committer dates. Chances are that the 10 years of history may be correct time wise as long as people don't introduce a bad date malevolently. > You'd need something like "you can trust committer dates that are > newer that this date" per project and git version as old versions of git can still be used later. > to switch between slow path and > fast path, with an updated fsck that knows how to compute that > number after you pulled from somebody who used that overriding > option. Well you could have a project setting (`config.sortedDates`) that is automatically computed once when cloning a project and depending on that setting you can go the fast path. Additionally when that setting is set, you'd enforce the correct dates in committing and merging (read pulling) to carry it on to be true. > > If the use of generation number can somehow be limited narrowly, we > may be able to incrementally introduce it only for new commits, but > I haven't thought things through, so let me do so aloud here ;-) I think of it as "committer date == generation number", i.e. just a special form of noting the generation number with gaps in between. This loses the ability to know how many commits there are at maximum between 2 given "numbers" though, which I think is minor. > > Suppose we use it only for this purpose: > > * When we have two commits, C1 and C2, with generation numbers G1 >and G2, we can say "C1 cannot possibly be an ancestor of C2" if >G1 > G2. We cannot say anything else based on generation >numbers (or lack thereof). > > then I think we could just say "A newly created commit must record > generation number G that is larger than generation numbers of its > parent commits; ignore parents that lack generation number for the > purpose of this sentence". > > I am not sure if that limited use is all that useful, though. I did *not* propose to introduce the generation number, but rather meant: * we already have committer date * it works pretty well * only a tiny patch is required to tighten the heuristic to work even better (going forward) by avoiding accidents in the history that have a committer date earlier than their parents. * we postpone drastic changes (i.e. introduction of generation numbers or change of algorithms) for now. -- 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 7/9] merge-recursive: handle return values indicating errors
Johannes Schindelin writes:
> diff --git a/merge-recursive.c b/merge-recursive.c
> index 6ab7dfc..bb075e3 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -266,8 +266,10 @@ struct tree *write_tree_from_memory(struct merge_options
> *o)
> active_cache_tree = cache_tree();
>
> if (!cache_tree_fully_valid(active_cache_tree) &&
> - cache_tree_update(&the_index, 0) < 0)
> - die(_("error building trees"));
> + cache_tree_update(&the_index, 0) < 0) {
> + error(_("error building trees"));
> + return NULL;
> + }
This actually is a BUG(), isn't it? We have already verified that
the cache is merged, so cache_tree_update() ought to be able to come
up with the whole-tree hash.
> @@ -548,19 +550,17 @@ static int update_stages(const char *path, const struct
> diff_filespec *o,
>*/
> int clear = 1;
> int options = ADD_CACHE_OK_TO_ADD | ADD_CACHE_SKIP_DFCHECK;
> + int ret = 0;
> +
> if (clear)
> - if (remove_file_from_cache(path))
> - return -1;
> - if (o)
> - if (add_cacheinfo(o->mode, o->sha1, path, 1, 0, options))
> - return -1;
> - if (a)
> - if (add_cacheinfo(a->mode, a->sha1, path, 2, 0, options))
> - return -1;
> - if (b)
> - if (add_cacheinfo(b->mode, b->sha1, path, 3, 0, options))
> - return -1;
> - return 0;
> + ret = remove_file_from_cache(path);
> + if (!ret && o)
> + ret = add_cacheinfo(o->mode, o->sha1, path, 1, 0, options);
> + if (!ret && a)
> + ret = add_cacheinfo(a->mode, a->sha1, path, 2, 0, options);
> + if (!ret && b)
> + ret = add_cacheinfo(b->mode, b->sha1, path, 3, 0, options);
> + return ret;
> }
Aren't the preimage and the postimage doing the same thing? The
only two differences I spot are (1) it is clear in the original that
the returned value is -1 in the error case, even if the error
convention of remove_file_from_cache() and add_cacheinfo() were "0
is good, others are bad"; and (2) the control flow is easier to
follow in the original.
> @@ -736,7 +736,7 @@ static int make_room_for_path(struct merge_options *o,
> const char *path)
> return error(msg, path, _(": perhaps a D/F conflict?"));
> }
>
> -static void update_file_flags(struct merge_options *o,
> +static int update_file_flags(struct merge_options *o,
> const unsigned char *sha,
> unsigned mode,
> const char *path,
> @@ -777,8 +777,7 @@ static void update_file_flags(struct merge_options *o,
>
> if (make_room_for_path(o, path) < 0) {
> update_wd = 0;
> - free(buf);
> - goto update_index;
> + goto free_buf;
> }
> if (S_ISREG(mode) || (!has_symlinks && S_ISLNK(mode))) {
> int fd;
> @@ -801,20 +800,22 @@ static void update_file_flags(struct merge_options *o,
> } else
> die(_("do not know what to do with %06o %s '%s'"),
> mode, sha1_to_hex(sha), path);
> +free_buf:
> free(buf);
I somehow find the above change harder to follow than the original.
> }
> update_index:
> if (update_cache)
> add_cacheinfo(mode, sha, path, 0, update_wd,
> ADD_CACHE_OK_TO_ADD);
> + return 0;
> }
>
This one is in line with the stated goal of the patch.
> @@ -1028,21 +1030,23 @@ static void handle_change_delete(struct merge_options
> *o,
>* correct; since there is no true "middle point" between
>* them, simply reuse the base version for virtual merge base.
>*/
> - remove_file_from_cache(path);
> - update_file(o, 0, o_sha, o_mode, renamed ? renamed : path);
> + ret = remove_file_from_cache(path);
> + if (!ret)
> + ret = update_file(o, 0, o_sha, o_mode,
> + renamed ? renamed : path);
As you noted in the log message, this does change the behaviour. If
remove returns non-zero for whatever reason, we still did update()
in the original, but we no longer do. Does this have negative
effect to the overall codeflow?
Or, assuming that everybody returns -1 for errors, perhaps
ret = remove();
ret |= update();
may be a more faithful and safe conversion?
> @@ -1087,21 +1094,22 @@ static void conflict_rename_delete(struct
> merge_options *o,
> b_mode = dest->mode;
> }
>
> - handle_change_delete(o,
> + ret = handle_change_delete(o,
>o->call_depth ? orig->path : dest->path,
>orig->sha1, orig->mode,
>a_sha, a_mode,
Re: [PATCH 8/9] merge-recursive: switch to returning errors instead of dying
Johannes Schindelin writes:
> @@ -743,6 +741,8 @@ static int update_file_flags(struct merge_options *o,
> int update_cache,
> int update_wd)
> ...
> + ret = error_errno(_("do not know what to do with %06o
> %s '%s'"),
> + mode, sha1_to_hex(sha), path);
> free_buf:
OK, with a few more users of this label, it no longer looks so
strange to me to have this label here.
At least, match the indentation level to the existing one we see
below, though?
> free(buf);
> }
> update_index:
> - if (update_cache)
> + if (!ret && update_cache)
> add_cacheinfo(mode, sha, path, 0, update_wd,
> ADD_CACHE_OK_TO_ADD);
Unlike my reaction to "if (!ret) do this" in an earlier patch, I do
think this change is sensible. We wouldn't have reached here in the
original code if we saw errors, and some of the variable used to
call add_cacheinfo() at this point may be garbage when ret is
non-zero, i.e. we know we earlier saw an error.
> - return 0;
> + return ret;
> }
> ...
> if ((merge_status < 0) || !result_buf.ptr)
> - die(_("Failed to execute internal merge"));
> + ret = error(_("Failed to execute internal
> merge"));
>
> - if (write_sha1_file(result_buf.ptr, result_buf.size,
> + if (!ret && write_sha1_file(result_buf.ptr,
> result_buf.size,
> blob_type, result->sha))
Likewise.
> @@ -1861,7 +1867,7 @@ static int process_entry(struct merge_options *o,
>*/
> remove_file(o, 1, path, !a_mode);
> } else
> - die(_("Fatal merge failure, shouldn't happen."));
> + return error(_("Fatal merge failure, shouldn't happen."));
Isn't this BUG()?
--
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: [RFD] Place to document magic pathspecs like ":/" and pathspec handling
W dniu 2016-06-29 o 21:51, Junio C Hamano pisze: > On Wed, Jun 29, 2016 at 12:47 PM, Jakub Narębski wrote: >> I have noticed that the magic pathspec ":/" is described only in RelNotes >> for revision 1.7.6: >> |I think the reason might be that there was no good place to put that >> information in. Nowadays we have gitcli(7) manual page, but perhaps > $ git help glossary > > look for pathspec. Thanks. I haven't noticed that. But I think it is not the best place to keep this documentation. There are the following issues with it: * it is hard to find; I did not search _glossary_ for information about how Git handles pathspecs, I would search it if I didn't know what pathspec means. * it is longest entry in glossary, and the only one with nested list, but * it is also too short to describe how Git handles pathspecs in detail, and a bit cryptic; there are no examples (so grepping docs for '":/"' didn't found it). -- Jakub Narębski -- 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 9/9] am: make a direct call to merge_recursive
Johannes Schindelin writes:
> - cp.git_cmd = 1;
> + init_merge_options(&o);
> +
> + o.branch1 = "HEAD";
> + his_tree_name = xstrfmt("%.*s", linelen(state->msg), state->msg);
> + o.branch2 = his_tree_name;
>
> - argv_array_pushf(&cp.env_array, "GITHEAD_%s=%.*s",
> - sha1_to_hex(his_tree), linelen(state->msg),
> state->msg);
> if (state->quiet)
> - argv_array_push(&cp.env_array, "GIT_MERGE_VERBOSITY=0");
> + o.verbosity = 0;
>
> - argv_array_push(&cp.args, "merge-recursive");
> - argv_array_push(&cp.args, sha1_to_hex(orig_tree));
> - argv_array_push(&cp.args, "--");
> - argv_array_push(&cp.args, sha1_to_hex(our_tree));
> - argv_array_push(&cp.args, sha1_to_hex(his_tree));
> + status = merge_recursive_generic(&o, our_tree, his_tree, 1, bases,
> &result);
> + if (status < 0)
> + exit(128);
> + free(his_tree_name);
>
> - status = run_command(&cp) ? (-1) : 0;
> - discard_cache();
> - read_cache();
> return status;
> }
Is this a correct conversion?
We used to prepare the command line and called run_command() and
without dying returned status with the error status from the
merge-recursive that was spawned by run_command().
The new code does not report failure to the caller and instead dies.
--
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: [RFD] Place to document magic pathspecs like ":/" and pathspec handling
Jakub Narębski writes: > But I think it is not the best place to keep this documentation. All true. In case it was not obvious, I didn't mean to say "Here you find the information, shut up." It was "here is a pointer if you didn't find it, so that you can use it as a starting point to make a better documentation." An analogous entity in the world model of Git that appears everywhere and is not limited to a single command is the revision set calculus. Where do we describe it and how do we make it discoverable? Can the new way to describe pathspec and pathspec magic mimic the way it is done for the revisions? -- 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: topological index field for commit objects
On Wed, Jun 29, 2016 at 1:54 PM, Stefan Beller wrote:
> Chances are that the 10 years of history may be correct time wise as long
> as people don't introduce a bad date malevolently.
>
To answer my own speculation:
Even git.git violates the timing property, so there is no hope to find
many projects
to have "parents committer date < committer date"
$ git log --format="%H %ct %P" HEAD | {
> while read sha sha_date parents ; do
> for p in $parents ; do
> # echo "$id, $parents, $p"
> pd=$(git show -s --format="%ct" $p)
> if test $pd -gt $sha_date ; then
> echo $sha
> fi
> done
> done
> }
619a644d6daef56d70aeca85514e2d281eb483a5
776398709dee4050fc194fec45c5818ba9b01afe
ed19f367220a50e4e2a5c1a00b03c14eafcaba89
(out of curiosity these are:)
(2009-10-18 12:34:56, "checkout A...B" switches to the merge base
between A and B)
(2007-09-01 23:53:47, Keep last used delta base in the delta window)
(2006-03-03 23:29:56, Add a Documentation/git-tools.txt)
--
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: topological index field for commit objects
On Wed, Jun 29, 2016 at 02:37:17PM -0700, Stefan Beller wrote: > On Wed, Jun 29, 2016 at 1:54 PM, Stefan Beller wrote: > > Chances are that the 10 years of history may be correct time wise as long > > as people don't introduce a bad date malevolently. > > > > To answer my own speculation: > Even git.git violates the timing property, so there is no hope to find > many projects > to have "parents committer date < committer date" It's expected and reasonable for there to be some skew. People's clocks aren't all perfectly synced. The question is one of how _much_ skew you're willing to tolerate (and any algorithms obviously need to allow that much, too). In some places we allow 24 hours of skew between any two commits. In others we allow up to 5 skewed commits in a row. That may not be enough in general, though. E.g.: > 619a644d6daef56d70aeca85514e2d281eb483a5 This one is a 3-day skew. -Peff -- 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: topological index field for commit objects
W dniu 2016-06-29 o 22:56, Jeff King pisze: > On Wed, Jun 29, 2016 at 01:39:17PM -0700, Junio C Hamano wrote: > >>> Would it make sense to refuse creating commits that have a commit date >>> prior to its parents commit date (except when the user gives a >>> `--dammit-I-know-I-break-a-wildy-used-heuristic`)? >> >> I think that has also been discussed in the past. I do not think it >> would help very much in practice, as projects already have up to 10 >> years (and the ones migrated from CVS, even more) worth of commits >> they cannot rewrite that may record incorrect committer dates. > > Yep, it has been discussed and I agree it runs into a lot of corner > cases. > >> If the use of generation number can somehow be limited narrowly, we >> may be able to incrementally introduce it only for new commits, but >> I haven't thought things through, so let me do so aloud here ;-) > > I think the problem is that you really _do_ want generation numbers for > old commits. One of the most obvious cases is something like "tag > --contains HEAD", because it has to examine older tags. > > So your history looks something like: > > A -- B -- ... Z > \\ >v1.0 HEAD > > Without generation numbers (or some proxy), you have to walk the history > between B..Z to find the answer. With generation numbers, it is > immediately obvious. > > So this is the ideal case for generation numbers (the worst cases are > when the things you are looking for are in branchy, close history where > the generation numbers don't tell you much; but in such cases the > walking is usually not too bad). There are other approaches (special indices) that help reachability queries beside "generation number". > > So I think you really do want to be able to generate and store > generation numbers after the fact. That has an added bonus that you do > not have to worry about baking incorrect values into your objects; you > do the topological walk once, and you _know_ it is correct (at least as > correct as the parent links, but that is our source of truth). By the way, what should happen if you add a replacement (in the git-replace meaning) that creates a shortcut, therefore invalidating generation numbers, at least in strict sense - committerdate as generation number would be still good, I think? > I have patches that generate and store the numbers at pack time, similar > to the way we do the reachability bitmaps. They're not production ready, > but they could probably be made so without too much effort. You wouldn't > have ready-made generation numbers for commits since the last full > repack, but you can compute them incrementally based on what you do have > at a cost linear to the unpacked commits (this is the same for bitmaps). Do Git use EWAH / EWOK bitmaps for reachability analysis, or is it still limited to object counting? -- Jakub Narębski -- 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: topological index field for commit objects
On Wed, Jun 29, 2016 at 11:49:35PM +0200, Jakub Narębski wrote: > > So this is the ideal case for generation numbers (the worst cases are > > when the things you are looking for are in branchy, close history where > > the generation numbers don't tell you much; but in such cases the > > walking is usually not too bad). > > There are other approaches (special indices) that help reachability > queries beside "generation number". Yes, though generation numbers can help with more questions (e.g., "what is the merge base"). > By the way, what should happen if you add a replacement (in the git-replace > meaning) that creates a shortcut, therefore invalidating generation numbers, > at least in strict sense - committerdate as generation number would be still > good, I think? This is one of the open questions. My older patches turned them off when replacements and grafts are in effect. > > I have patches that generate and store the numbers at pack time, similar > > to the way we do the reachability bitmaps. They're not production ready, > > but they could probably be made so without too much effort. You wouldn't > > have ready-made generation numbers for commits since the last full > > repack, but you can compute them incrementally based on what you do have > > at a cost linear to the unpacked commits (this is the same for bitmaps). > > Do Git use EWAH / EWOK bitmaps for reachability analysis, or is it still > limited to object counting? At GitHub we are using them for --contains analysis, along with mass ahead/behind (e.g., as in https://github.com/gitster/git/branches). My plan is to send patches upstream, but they need some cleanup first. -Peff -- 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/5] sequencer: use skip_blank_lines() to find the commit subject
Johannes Schindelin writes:
> Just like we already taught the find_commit_subject() function (to make
> it consistent with the code in pretty.c), we now simply skip leading
> blank lines of the commit message.
>
> Signed-off-by: Johannes Schindelin
> ---
> sequencer.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 7d7add6..cdfac82 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -544,10 +544,8 @@ static int do_pick_commit(struct commit *commit, struct
> replay_opts *opts)
>* information followed by "\n\n".
>*/
> p = strstr(msg.message, "\n\n");
> - if (p) {
> - p += 2;
> - strbuf_addstr(&msgbuf, p);
> - }
> + if (p)
> + strbuf_addstr(&msgbuf, skip_blank_lines(p + 2));
>
> if (opts->record_origin) {
> if (!has_conforming_footer(&msgbuf, NULL, 0))
Here, msg.message was read in get_message() by calling
logmsg_reencode() that reads from get_commit_buffer(). The code
structure is exactly the same as the previous one.
Good.
--
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 1/5] commit -C: skip blank lines at the beginning of the message
Johannes Schindelin writes:
> Consistent with the pretty-printing machinery, we skip leading blank
> lines (if any) of existing commit messages.
>
> While Git itself only produces commit objects with a single empty line
> between commit header and commit message, it is legal to have more than
> one blank line (i.e. lines containing only white space, or no
> characters) at the beginning of the commit message, and the
> pretty-printing code already handles that.
>
> Signed-off-by: Johannes Schindelin
> ---
> builtin/commit.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 3f18942..1f6dbcd 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -715,7 +715,7 @@ static int prepare_to_commit(const char *index_file,
> const char *prefix,
> char *buffer;
> buffer = strstr(use_message_buffer, "\n\n");
> if (buffer)
> - strbuf_addstr(&sb, buffer + 2);
> + strbuf_addstr(&sb, skip_blank_lines(buffer + 2));
> hook_arg1 = "commit";
> hook_arg2 = use_message;
> } else if (fixup_message) {
use_message_buffer is the contents of the commit object read
elsewhere, and strstr() skips the header part, so this follows
exactly the same pattern as the one you fixed earlier. Good.
--
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: topological index field for commit objects
Jeff King writes: > Yes, though generation numbers can help with more questions (e.g., "what > is the merge base"). Hmm, how? I have two commits, with generation number 38 and 47, respectively. What generation number does the commit that is the merge base of these two commits? I know we can say that 38 cannot possibly be a descendant of 47, but can we say anything else that is useful? -- 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: topological index field for commit objects
On 29.06.2016 22:39, Junio C Hamano wrote: Stefan Beller writes: On Wed, Jun 29, 2016 at 11:59 AM, Junio C Hamano wrote: On Wed, Jun 29, 2016 at 11:31 AM, Marc Strapetz wrote: This is no RFE but rather recurring thoughts whenever I'm working with commit graphs: a topological index attribute for commit objects would be incredible useful. By "topological index" I mean a simple integer for which following condition holds true: Look for "generation numbers" in the list archive, perhaps? Thanks for the pointer to the interesting discussions. In http://www.spinics.net/lists/git/msg161363.html Linus wrote in a discussion with Jeff: Right now, we do *have* a "generation number". It's just that it's very easy to corrupt even by mistake. It's called "committer date". We could improve on it. Would it make sense to refuse creating commits that have a commit date prior to its parents commit date (except when the user gives a `--dammit-I-know-I-break-a-wildy-used-heuristic`)? I think that has also been discussed in the past. I do not think it would help very much in practice, as projects already have up to 10 years (and the ones migrated from CVS, even more) worth of commits they cannot rewrite that may record incorrect committer dates. You'd need something like "you can trust committer dates that are newer that this date" per project to switch between slow path and fast path, with an updated fsck that knows how to compute that number after you pulled from somebody who used that overriding option. If the use of generation number can somehow be limited narrowly, we may be able to incrementally introduce it only for new commits, but I haven't thought things through, so let me do so aloud here ;-) Suppose we use it only for this purpose: * When we have two commits, C1 and C2, with generation numbers G1 and G2, we can say "C1 cannot possibly be an ancestor of C2" if G1 > G2. We cannot say anything else based on generation numbers (or lack thereof). then I think we could just say "A newly created commit must record generation number G that is larger than generation numbers of its parent commits; ignore parents that lack generation number for the purpose of this sentence". From algorithm perspective, for already existing repositories you would still have to switch from an optimized generation number code to the current commit-time based code. That could things make even more complex and it's possibly expensive to determine whether a repository has full generation number support or not. On the other hand, for new repositories, you could immediately use generation number based algorithms. So it could be "A newly created commit must record generation number G that is larger than generation numbers of its parent commits if all parents commits have a generation number recorded; otherwise do not record a generation number". Something like "git filter-branch" might already be sufficient to convert repositories. Git versions released in 2019 may start issuing warnings if HEAD has no generation number assigned and Git versions released in 2025 may completely refuse to work with such repositories. In the interim period, a local cache as Jeff is proposing could serve as secondary source for generation numbers. This would allow to phase out current algorithms immediately. -Marc -- 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: topological index field for commit objects
On Wed, Jun 29, 2016 at 03:11:39PM -0700, Junio C Hamano wrote: > Jeff King writes: > > > Yes, though generation numbers can help with more questions (e.g., "what > > is the merge base"). > > Hmm, how? I have two commits, with generation number 38 and 47, > respectively. What generation number does the commit that is the > merge base of these two commits? > > I know we can say that 38 cannot possibly be a descendant of 47, but > can we say anything else that is useful? I don't think it can give you the answer immediately from those values, but in general generation numbers help you bound actual traversals and avoid walking down unproductive paths. So comparing 38 and 47 is not nearly as useful as walking from 47 down to 37 and saying "I know that I cannot possibly reach 38 by walking further". I haven't thought hard specifically about merge bases computation, so perhaps that is a case that isn't helped at all. It has been a while since I've looked into this, but I recall there were some computations that are hard to improve with bitmaps. I may also simply be mis-remembering; in my patches generations were tightly tied to having a packv4-style cache of commit properties that can be used rather than inflating the commit object itself. That cache helps any time you walk by speeding up the parse_commit() process. But it's a per-commit speedup. It doesn't change the algorithmic complexity of what you're doing. -Peff -- 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 5/5] Skip blank lines when matching ^{/foo}
Johannes Schindelin writes:
> When trying to match a pattern in the commit subject, simply skip leading
> blank lines in the commit message. This is consistent with the
> pretty-printing machinery: it silently ignores leading blank lines in the
> commit object's body.
>
> Signed-off-by: Johannes Schindelin
> ---
Hmm.
I just tried
$ git commit -s -m ' pull: fast-forward "pull --rebase=true"'
expecting that ":/^ pull:" would find it ;-)
The point of this patch is to make it not work, of course, which
may or may not be a good thing. I'd rather keep this separate from
the other fixes.
Also 4/5 is another different class of true bugfix, I would suspect,
so let's take it on a separate topic that can be merged down to
older maintainance tracks.
Thanks.
> sha1_name.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/sha1_name.c b/sha1_name.c
> index ca7ddd6..da354a5 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -912,7 +912,8 @@ static int get_sha1_oneline(const char *prefix, unsigned
> char *sha1,
> continue;
> buf = get_commit_buffer(commit, NULL);
> p = strstr(buf, "\n\n");
> - matches = negative ^ (p && !regexec(®ex, p + 2, 0, NULL, 0));
> + matches = negative ^ (p && !regexec(®ex,
> + skip_blank_lines(p + 2), 0, NULL, 0));
> unuse_commit_buffer(commit, buf);
>
> if (matches) {
--
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
What's cooking in git.git (Jun 2016, #10; Wed, 29)
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'. The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.
The first few batches for this cycle has been merged to 'master',
and new topics are trickling into 'next'.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--
[New Topics]
* cc/apply-am (2016-06-28) 41 commits
- apply: use error_errno() where possible
- builtin/am: use apply api in run_apply()
- apply: change error_routine when be_silent is set
- usage: add get_error_routine() and get_warn_routine()
- usage: add set_warn_routine()
- apply: don't print on stdout when be_silent is set
- apply: make 'be_silent' incompatible with 'apply_verbosely'
- apply: add 'be_silent' variable to 'struct apply_state'
- write_or_die: use warning() instead of fprintf(stderr, ...)
- environment: add set_index_file()
- apply: make some parsing functions static again
- apply: move libified code from builtin/apply.c to apply.{c,h}
- apply: rename and move opt constants to apply.h
- builtin/apply: rename option parsing functions
- builtin/apply: make create_one_file() return -1 on error
- builtin/apply: make try_create_file() return -1 on error
- builtin/apply: make write_out_results() return -1 on error
- builtin/apply: make write_out_one_result() return -1 on error
- builtin/apply: make create_file() return -1 on error
- builtin/apply: make add_index_file() return -1 on error
- builtin/apply: make add_conflicted_stages_file() return -1 on error
- builtin/apply: make remove_file() return -1 on error
- builtin/apply: make build_fake_ancestor() return -1 on error
- builtin/apply: change die_on_unsafe_path() to check_unsafe_path()
- builtin/apply: make gitdiff_*() return -1 on error
- builtin/apply: make gitdiff_*() return 1 at end of header
- builtin/apply: make parse_traditional_patch() return -1 on error
- builtin/apply: make apply_all_patches() return 128 or 1 on error
- builtin/apply: move check_apply_state() to apply.c
- builtin/apply: make check_apply_state() return -1 instead of die()ing
- apply: make init_apply_state() return -1 instead of exit()ing
- builtin/apply: move init_apply_state() to apply.c
- builtin/apply: make parse_ignorewhitespace_option() return -1 instead of
die()ing
- builtin/apply: make parse_whitespace_option() return -1 instead of die()ing
- builtin/apply: make parse_single_patch() return -1 on error
- builtin/apply: make parse_chunk() return a negative integer on error
- builtin/apply: make find_header() return -128 instead of die()ing
- builtin/apply: read_patch_file() return -1 instead of die()ing
- builtin/apply: make apply_patch() return -1 or -128 instead of die()ing
- apply: move 'struct apply_state' to apply.h
- apply: make some names more specific
"git am" has been taught to make an internal call to "git apply"'s
innards without spawning the latter as a separate process.
Needs review.
* dp/autoconf-curl-ssl (2016-06-28) 1 commit
- ./configure.ac: detect SSL in libcurl using curl-config
The ./configure script generated from configure.ac was taught how
to detect support of SSL by libcurl better.
Needs review.
* js/color-on-windows-comment (2016-06-28) 1 commit
(merged to 'next' on 2016-06-28 at 38a2ea1)
+ color.h: remove obsolete comment about limitations on Windows
For a long time, we carried an in-code comment that said our
colored output would work only when we use fprintf/fputs on
Windows, which no longer is the case for the past few years.
Will merge to 'master'.
* js/sign-empty-commit-fix (2016-06-29) 1 commit
- commit -S: avoid invalid pointer with empty message
"git commit --amend --allow-empty-message -S" for a commit without
any message body could have misidentified where the header of the
commit object ends.
Will merge to 'next'.
* js/t3404-grammo-fix (2016-06-29) 1 commit
- t3404: fix a grammo (commands are ran -> commands are run)
Will merge to 'next'.
* ls/p4-tmp-refs (2016-06-29) 1 commit
- git-p4: place temporary refs used for branch import under refs/git-p4-tmp
"git p4" used a location outside $GIT_DIR/refs/ to place its
temporary branches, which has been moved to refs/git-p4-tmp/.
Needs an ack from "git p4" experts.
* jc/pull-rebase-ff (2016-06-29) 1 commit
- pull: fast-forward "pull --rebase=true"
"git pull --rebase", when there is no new commits on our side since
we forked from the upstream, should be able to fast-forward without
invoking "git rebase", but it didn't.
Needs a real log message and a few tests.
* ps/rebase-i-auto-unstash-upon-abort (2016-06-29) 1 commit
- rebase -i: restore autostash on abort
"git rebase -i --autostash" did not restore t
Re: What's cooking in git.git (Jun 2016, #10; Wed, 29)
On 30/06/16 00:22, Junio C Hamano wrote:
[snip]
>
> * mh/ref-store (2016-06-20) 38 commits
> - refs: implement iteration over only per-worktree refs
> - refs: make lock generic
> - refs: add method to rename refs
> - refs: add methods to init refs db
> - refs: make delete_refs() virtual
> - refs: add method for initial ref transaction commit
> - refs: add methods for reflog
> - refs: add method iterator_begin
> - files_ref_iterator_begin(): take a ref_store argument
> - split_symref_update(): add a files_ref_store argument
> - lock_ref_sha1_basic(): add a files_ref_store argument
> - lock_ref_for_update(): add a files_ref_store argument
> - commit_ref_update(): add a files_ref_store argument
> - lock_raw_ref(): add a files_ref_store argument
> - repack_without_refs(): add a files_ref_store argument
> - refs: make peel_ref() virtual
> - refs: make create_symref() virtual
> - refs: make pack_refs() virtual
> - refs: make verify_refname_available() virtual
> - refs: make read_raw_ref() virtual
> - resolve_gitlink_ref(): rename path parameter to submodule
> - resolve_gitlink_ref(): avoid memory allocation in many cases
> - resolve_gitlink_ref(): implement using resolve_ref_recursively()
> - resolve_ref_recursively(): new function
> - read_raw_ref(): take a (struct ref_store *) argument
> - resolve_gitlink_packed_ref(): remove function
> - resolve_packed_ref(): rename function from resolve_missing_loose_ref()
> - refs: reorder definitions
> - refs: add a transaction_commit() method
> - {lock,commit,rollback}_packed_refs(): add files_ref_store arguments
> - resolve_missing_loose_ref(): add a files_ref_store argument
> - get_packed_ref(): add a files_ref_store argument
> - add_packed_ref(): add a files_ref_store argument
> - refs: create a base class "ref_store" for files_ref_store
> - refs: add a backend method structure
> - refs: rename struct ref_cache to files_ref_store
> - rename_ref_available(): add docstring
> - resolve_gitlink_ref(): eliminate temporary variable
> (this branch uses mh/ref-iterators and mh/split-under-lock; is tangled with
> mh/update-ref-errors.)
>
> The ref-store abstraction was introduced to the refs API so that we
> can plug in different backends to store references.
>
> Is everybody happy with this version?
> If so, will merge to 'next'.
A small fixup required for this one.
see http://thread.gmane.org/gmane.comp.version-control.git/298137
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
[PATCH 2/4] receive-pack: implement advertising and receiving push options
The pre/post receive hook may be interested in more information from the
user. This information can be transmitted when both client and server
support the "push-options" capability, which when used is a phase directly
after update commands ended by a flush pkt.
Similar to the atomic option, the server capability can be disabled via
the `receive.advertisePushOptions` config variable. While documenting
this, fix a nit in the `receive.advertiseAtomic` wording.
For now we only accept C strings with no new lines.
Signed-off-by: Stefan Beller
---
Documentation/config.txt | 7 +++-
Documentation/technical/pack-protocol.txt | 10 +++---
Documentation/technical/protocol-capabilities.txt | 8 +
builtin/receive-pack.c| 44 +++
4 files changed, 64 insertions(+), 5 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 626243f..3b199f9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2410,7 +2410,12 @@ rebase.instructionFormat
receive.advertiseAtomic::
By default, git-receive-pack will advertise the atomic push
- capability to its clients. If you don't want to this capability
+ capability to its clients. If you don't want this capability
+ to be advertised, set this variable to false.
+
+receive.advertisePushOptions::
+ By default, git-receive-pack will advertise the push options capability
+ to its clients. If you don't want this capability
to be advertised, set this variable to false.
receive.autogc::
diff --git a/Documentation/technical/pack-protocol.txt
b/Documentation/technical/pack-protocol.txt
index 8b36343..7a2ed30 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -454,7 +454,8 @@ The reference discovery phase is done nearly the same way
as it is in the
fetching protocol. Each reference obj-id and name on the server is sent
in packet-line format to the client, followed by a flush-pkt. The only
real difference is that the capability listing is different - the only
-possible values are 'report-status', 'delete-refs' and 'ofs-delta'.
+possible values are 'report-status', 'delete-refs', 'ofs-delta' and
+'push-options'.
Reference Update Request and Packfile Transfer
--
@@ -465,9 +466,10 @@ that it wants to update, it sends a line listing the
obj-id currently on
the server, the obj-id the client would like to update it to and the name
of the reference.
-This list is followed by a flush-pkt and then the packfile that should
-contain all the objects that the server will need to complete the new
-references.
+This list is followed by a flush-pkt. Then the push options are transmitted
+one per packet followed by another flush-pkt. After that the packfile that
+should contain all the objects that the server will need to complete the new
+references will be sent.
update-request= *shallow ( command-list | push-cert ) [packfile]
diff --git a/Documentation/technical/protocol-capabilities.txt
b/Documentation/technical/protocol-capabilities.txt
index eaab6b4..b71eda9 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -253,6 +253,14 @@ atomic pushes. If the pushing client requests this
capability, the server
will update the refs in one atomic transaction. Either all refs are
updated or none.
+push-options
+
+
+If the server sends the 'push-options' capability it is capable to accept
+push options after the update commands have been sent. If the pushing client
+requests this capability, the server will pass the options to the pre and post
+receive hooks that process this push request.
+
allow-tip-sha1-in-want
--
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 0da6852..68627ed 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -44,10 +44,12 @@ static struct strbuf fsck_msg_types = STRBUF_INIT;
static int receive_unpack_limit = -1;
static int transfer_unpack_limit = -1;
static int advertise_atomic_push = 1;
+static int advertise_push_options = 1;
static int unpack_limit = 100;
static int report_status;
static int use_sideband;
static int use_atomic;
+static int use_push_options;
static int quiet;
static int prefer_ofs_delta = 1;
static int auto_update_server_info;
@@ -193,6 +195,11 @@ static int receive_pack_config(const char *var, const char
*value, void *cb)
return 0;
}
+ if (strcmp(var, "receive.advertisepushoptions") == 0) {
+ advertise_push_options = git_config_bool(var, value);
+ return 0;
+ }
+
return git_default_config(var, value, cb);
}
@@ -207,6 +214,8 @@ static void show_ref(const char *path, const unsigned char
*sha1)
"report-status d
[PATCH 4/4] add a test for push options
The functions `mk_repo_pair` as well as `test_refs` are borrowed from
t5543-atomic-push, with additional hooks installed.
Signed-off-by: Stefan Beller
---
t/t5544-push-options.sh | 85 +
1 file changed, 85 insertions(+)
create mode 100755 t/t5544-push-options.sh
diff --git a/t/t5544-push-options.sh b/t/t5544-push-options.sh
new file mode 100755
index 000..49d5f80
--- /dev/null
+++ b/t/t5544-push-options.sh
@@ -0,0 +1,85 @@
+#!/bin/sh
+
+test_description='pushing to a repository using push options'
+
+. ./test-lib.sh
+
+mk_repo_pair () {
+ rm -rf workbench upstream &&
+ test_create_repo upstream &&
+ test_create_repo workbench &&
+ (
+ cd upstream &&
+ git config receive.denyCurrentBranch warn &&
+ mkdir -p .git/hooks &&
+ cat >.git/hooks/pre-receive <<-'EOF' &&
+ #!/bin/sh
+ cat $GIT_PUSH_OPTION_FILE >hooks/pre-receive.push_options
+ EOF
+ chmod u+x .git/hooks/pre-receive
+
+ cat >.git/hooks/post-receive <<-'EOF' &&
+ #!/bin/sh
+ cat $GIT_PUSH_OPTION_FILE >hooks/post-receive.push_options
+ EOF
+ chmod u+x .git/hooks/post-receive
+ ) &&
+ (
+ cd workbench &&
+ git remote add up ../upstream
+ )
+}
+
+# Compare the ref ($1) in upstream with a ref value from workbench ($2)
+# i.e. test_refs second HEAD@{2}
+test_refs () {
+ test $# = 2 &&
+ git -C upstream rev-parse --verify "$1" >expect &&
+ git -C workbench rev-parse --verify "$2" >actual &&
+ test_cmp expect actual
+}
+
+test_expect_success 'one push option works for a single branch' '
+ mk_repo_pair &&
+ (
+ cd workbench &&
+ test_commit one &&
+ git push --mirror up &&
+ test_commit two &&
+ git push --push-option=asdf up master
+ ) &&
+ test_refs master master &&
+ echo "asdf" >expect &&
+ test_cmp expect upstream/.git/hooks/pre-receive.push_options &&
+ test_cmp expect upstream/.git/hooks/post-receive.push_options
+'
+
+test_expect_success 'push option denied by remote' '
+ mk_repo_pair &&
+ git -C upstream config receive.advertisePushOptions false &&
+ (
+ cd workbench &&
+ test_commit one &&
+ git push --mirror up &&
+ test_commit two &&
+ test_must_fail git push --push-option=asdf up master
+ ) &&
+ test_refs master HEAD@{1}
+'
+
+test_expect_success 'two push options work' '
+ mk_repo_pair &&
+ (
+ cd workbench &&
+ test_commit one &&
+ git push --mirror up &&
+ test_commit two &&
+ git push --push-option=asdf --push-option="more structured
text" up master
+ ) &&
+ test_refs master master &&
+ printf "asdf\nmore structured text\n" >expect &&
+ test_cmp expect upstream/.git/hooks/pre-receive.push_options &&
+ test_cmp expect upstream/.git/hooks/post-receive.push_options
+'
+
+test_done
--
2.9.0.141.gdd65b60
--
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
[RFC PATCHv1 0/4] Push options in C Git
Allow a user to pass information along a push to the pre/post-receive hook
on the remote.
When using a remote that is more than just a plain Git host (e.g. Gerrit,
Git{hub/lab}, etc) this may become more obvious: The (server backend specific)
push options can instruct the server to:
* open a pull request
* send out emails asking for review
* (un)trigger continuous integration
* set priority for continuous integration (i.e. bots pushing may ask to be
treated with lower priority compared to humans)
* ...
Most of these actions can be done on the client side as well,
but in these remote-centric workflows it is easier to do that on the remote,
which is why we need to transport the information there.
More concrete examples:
* When you want a change in Gerrit to be submitted to refs/heads/master, you
push instead to a magic branch refs/for/master and Gerrit will create a change
for you (similar to a pull request). Instead we could imagine that you push
to a magical refs/heads/master with a push option "create-change".
* When pushing to Gerrit you can already attach some of this information by
adding a '%' followed by the parameter to the ref, i.e. when interacting with
Gerrit it is possible to do things like[1]:
git push origin
HEAD:refs/for/master%draft%topic=example%cc=jon@example.org
This is not appealing to our users as it looks like hacks upon hacks to make
it work. It would read better if it was spelled as:
git push origin HEAD:refs/for/master \
--push-option draft \
--push-option topic=example \
--push-option cc=jon@example.org
(with a short form that is even easier to type,
but this is is more intuitive already)
This is a patch series to Git core, which is developed at the same time
as a change is proposed to JGit by Dan Wang, see [2].
This code is also available at [3].
Thanks,
Stefan
[1] Not all Gerrit '%' options are documented, so here is a link to source code
instead :(
https://gerrit.googlesource.com/gerrit/+/refs/heads/master/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java#1141
[2] https://git.eclipse.org/r/#/c/74570/
[3] https://github.com/stefanbeller/git/tree/pushoptions
Stefan Beller (4):
push options: {pre,post}-receive hook learns about push options
receive-pack: implement advertising and receiving push options
push: accept push options
add a test for push options
Documentation/config.txt | 7 +-
Documentation/git-push.txt| 8 ++-
Documentation/githooks.txt| 4 ++
Documentation/technical/pack-protocol.txt | 10 +--
Documentation/technical/protocol-capabilities.txt | 8 +++
builtin/push.c| 16 -
builtin/receive-pack.c| 85 +++
send-pack.c | 29
send-pack.h | 3 +
t/t5544-push-options.sh | 85 +++
transport.c | 2 +
transport.h | 7 ++
12 files changed, 242 insertions(+), 22 deletions(-)
create mode 100755 t/t5544-push-options.sh
--
2.9.0.141.gdd65b60
--
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 1/4] push options: {pre,post}-receive hook learns about push options
The environment variable GIT_PUSH_OPTION_FILE is set to the push options
separated by new line.
The code is not executed as the push options are set to NULL, nor is the
new capability advertised.
The rationale for keeping the actual options inside a file instead of
putting them directly into an environment variable has multiple reasons:
1) After a discussion about environment variables and shells, we may not
want to put user data into an environment variable (see [1] for example).
2) If a user passes multiple push options, we need to pass them to the
hooks in a way, the hook can separate them. This could be done via
multiple environment variables (e.g. have GIT_PUSH_OPTIONS_COUNT and
GIT_PUSH_OPTIONS_{0,1,2,...} set), or put it all in one environment
variable and choose an appropriate separator. That is hard to parse
in both ways. For now we'll just put it in a file separated by new line,
such that the hook scripts can pickup the variables as
while read option ; do
process_push_option() $option
done < $GIT_PUSH_OPTION_FILE
3) environment variables are messed with in the run-command API depending
on the occurrence of a '=' to set or unset the variable. Having multiple
'=' is ok, such that we could have it set to a "key=value" pair.
4) When putting the options in a file, we need to take care of the race
condition of multiple clients pushing. That is actually rather easy.
5) We only inject new lines as separator into the file, so it is possible
to transmit binaries ("attach an image to a code review").
[1] 'Shellshock' https://lwn.net/Articles/614218/
Signed-off-by: Stefan Beller
---
Documentation/githooks.txt | 4
builtin/receive-pack.c | 41 -
2 files changed, 32 insertions(+), 13 deletions(-)
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index d82e912..dc80574 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -247,6 +247,8 @@ Both standard output and standard error output are
forwarded to
'git send-pack' on the other end, so you can simply `echo` messages
for the user.
+The push options are available in the variable GIT_PUSH_OPTION_FILE.
+
[[update]]
update
~~
@@ -322,6 +324,8 @@ a sample script `post-receive-email` provided in the
`contrib/hooks`
directory in Git distribution, which implements sending commit
emails.
+The push options are available in the variable GIT_PUSH_OPTION_FILE.
+
[[post-update]]
post-update
~~~
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 15c323a..0da6852 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -550,8 +550,16 @@ static void prepare_push_cert_sha1(struct child_process
*proc)
}
}
+struct receive_hook_feed_state {
+ struct command *cmd;
+ int skip_broken;
+ struct strbuf buf;
+ const char *push_options_file;
+};
+
typedef int (*feed_fn)(void *, const char **, size_t *);
-static int run_and_feed_hook(const char *hook_name, feed_fn feed, void
*feed_state)
+static int run_and_feed_hook(const char *hook_name, feed_fn feed,
+struct receive_hook_feed_state *feed_state)
{
struct child_process proc = CHILD_PROCESS_INIT;
struct async muxer;
@@ -567,6 +575,9 @@ static int run_and_feed_hook(const char *hook_name, feed_fn
feed, void *feed_sta
proc.argv = argv;
proc.in = -1;
proc.stdout_to_stderr = 1;
+ if (feed_state && feed_state->push_options_file)
+ argv_array_pushf(&proc.env_array, "GIT_PUSH_OPTION_FILE=%s",
+feed_state->push_options_file);
if (use_sideband) {
memset(&muxer, 0, sizeof(muxer));
@@ -606,12 +617,6 @@ static int run_and_feed_hook(const char *hook_name,
feed_fn feed, void *feed_sta
return finish_command(&proc);
}
-struct receive_hook_feed_state {
- struct command *cmd;
- int skip_broken;
- struct strbuf buf;
-};
-
static int feed_receive_hook(void *state_, const char **bufp, size_t *sizep)
{
struct receive_hook_feed_state *state = state_;
@@ -634,8 +639,10 @@ static int feed_receive_hook(void *state_, const char
**bufp, size_t *sizep)
return 0;
}
-static int run_receive_hook(struct command *commands, const char *hook_name,
- int skip_broken)
+static int run_receive_hook(struct command *commands,
+ const char *hook_name,
+ int skip_broken,
+ const char *push_options_file)
{
struct receive_hook_feed_state state;
int status;
@@ -646,6 +653,7 @@ static int run_receive_hook(struct command *commands, const
char *hook_name,
if (feed_receive_hook(&state, NULL, NULL))
return 0;
state.cmd = commands;
+ state.push_options_file = push_options_file;
status = run_and_feed_hook(hook_name, feed_r
[PATCH 3/4] push: accept push options
This implements everything that is required on the client side to make use
of push options from the porcelain push command.
Signed-off-by: Stefan Beller
---
Documentation/git-push.txt | 8 +++-
builtin/push.c | 16 +---
send-pack.c| 29 +
send-pack.h| 3 +++
transport.c| 2 ++
transport.h| 7 +++
6 files changed, 61 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 19f46b6..b0b1273 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -11,7 +11,7 @@ SYNOPSIS
[verse]
'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic] [-n |
--dry-run] [--receive-pack=]
[--repo=] [-f | --force] [-d | --delete] [--prune] [-v |
--verbose]
- [-u | --set-upstream]
+ [-u | --set-upstream] [--push-option=]
[--[no-]signed|--sign=(true|false|if-asked)]
[--force-with-lease[=[:]]]
[--no-verify] [ [...]]
@@ -156,6 +156,12 @@ already exists on the remote side.
Either all refs are updated, or on error, no refs are updated.
If the server does not support atomic pushes the push will fail.
+-L::
+--push-option::
+ Transmit the given string to the server, which passes them to
+ the pre-receive as well as the post-receive hook. Only C strings
+ containing no new lines are allowed.
+
--receive-pack=::
--exec=::
Path to the 'git-receive-pack' program on the remote
diff --git a/builtin/push.c b/builtin/push.c
index 4e9e4db..418f786 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -353,7 +353,8 @@ static int push_with_options(struct transport *transport,
int flags)
return 1;
}
-static int do_push(const char *repo, int flags)
+static int do_push(const char *repo, int flags,
+ const struct string_list *push_options)
{
int i, errs;
struct remote *remote = pushremote_get(repo);
@@ -376,6 +377,9 @@ static int do_push(const char *repo, int flags)
if (remote->mirror)
flags |= (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE);
+ if (push_options->nr)
+ flags |= TRANSPORT_PUSH_OPTIONS;
+
if ((flags & TRANSPORT_PUSH_ALL) && refspec) {
if (!strcmp(*refspec, "refs/tags/*"))
return error(_("--all and --tags are incompatible"));
@@ -406,13 +410,16 @@ static int do_push(const char *repo, int flags)
for (i = 0; i < url_nr; i++) {
struct transport *transport =
transport_get(remote, url[i]);
+ if (flags & TRANSPORT_PUSH_OPTIONS)
+ transport->push_options = push_options;
if (push_with_options(transport, flags))
errs++;
}
} else {
struct transport *transport =
transport_get(remote, NULL);
-
+ if (flags & TRANSPORT_PUSH_OPTIONS)
+ transport->push_options = push_options;
if (push_with_options(transport, flags))
errs++;
}
@@ -500,6 +507,8 @@ int cmd_push(int argc, const char **argv, const char
*prefix)
int push_cert = -1;
int rc;
const char *repo = NULL;/* default repository */
+ static struct string_list push_options = STRING_LIST_INIT_DUP;
+
struct option options[] = {
OPT__VERBOSITY(&verbosity),
OPT_STRING( 0 , "repo", &repo, N_("repository"),
N_("repository")),
@@ -533,6 +542,7 @@ int cmd_push(int argc, const char **argv, const char
*prefix)
0, "signed", &push_cert, "yes|no|if-asked", N_("GPG sign the
push"),
PARSE_OPT_OPTARG, option_parse_push_signed },
OPT_BIT(0, "atomic", &flags, N_("request atomic transaction on
remote side"), TRANSPORT_PUSH_ATOMIC),
+ OPT_STRING_LIST('L', "push-option", &push_options,
N_("server-specific"), N_("options to transmit")),
OPT_SET_INT('4', "ipv4", &family, N_("use IPv4 addresses only"),
TRANSPORT_FAMILY_IPV4),
OPT_SET_INT('6', "ipv6", &family, N_("use IPv6 addresses only"),
@@ -563,7 +573,7 @@ int cmd_push(int argc, const char **argv, const char
*prefix)
set_refspecs(argv + 1, argc - 1, repo);
}
- rc = do_push(repo, flags);
+ rc = do_push(repo, flags, &push_options);
if (rc == -1)
usage_with_options(push_usage, options);
else
diff --git a/send-pack.c b/send-pack.c
index 37ee04e..17c30a1 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -261,6 +261,7 @@ static int generate_push_cert(struct strbuf *req_buf,
const char *push_cert_nonce)
{
[PATCH] builtin/apply: include declaration of cmd_apply()
Signed-off-by: Ramsay Jones
---
Hi Christian,
If you need to re-roll your 'cc/apply-am' branch, could you please
squash this into the relevant patch. Commit 95a3b0ba ("apply: move
libified code from builtin/apply.c to apply.{c,h}", 22-04-2016)
removed this '#include "builtin.h"' line, which causes sparse to
issue a warning.
Thanks!
ATB,
Ramsay Jones
builtin/apply.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/apply.c b/builtin/apply.c
index 066cb29..9c66474 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1,4 +1,5 @@
#include "cache.h"
+#include "builtin.h"
#include "parse-options.h"
#include "lockfile.h"
#include "apply.h"
--
2.9.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 1/9] Report bugs consistently
Am 29.06.2016 um 13:36 schrieb Johannes Schindelin:
@@ -955,9 +955,8 @@ static struct merge_file_info merge_file_1(struct
merge_options *o,
if (!sha_eq(a->sha1, b->sha1))
result.clean = 0;
- } else {
- die(_("unsupported object type in the tree"));
- }
+ } else
+ die(_("BUG: unsupported object type in the tree"));
Would it perhaps make sense to remove the _() markup (here and a few
more instances in this patch)? It's simpler for developers to find the
code location when a "BUG:" is reported untranslated.
-- Hannes
--
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: git gui produces series of commits with exactly the same time
Hi everyone, I've found one case when this bug happens. 1. Create a commit 2. Amend it 3. Create several new commits All the new commits have the same "Author" timestamp until the git gui is restarted. Can anyone reproduce this? Best wishes, Andrei Faber On 29/06/2016 18:45, Johannes Schindelin wrote: Hi Andrei, On Wed, 29 Jun 2016, Andrei Faber wrote: I've noticed that git history contains series of commits with exactly the same time, despite the real commit time of these commits was different. All these commit were made using the git gui tool. I'm the only developer in this project. Is it possible that you played games with your GIT_AUTHOR_DATE environment variable? Ciao, Johannes -- 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 v4] Refactor recv_sideband()
On Wed, 29 Jun 2016 at 18:40:16, Junio C Hamano wrote: > Lukas, can you see what is in 'pu' after I push out today's > integration result in several hours and tell us if you like the > result of the SQUASH??? change? Looks good to me. Thank you both for working on this. Note that you should amend the last paragraph of the original commit message when you squash Nicos patch into mine. Oh, and one more detail: I wonder why we still use fwrite(), now that we know we can use xwrite() which guarantees atomicity. Is there a reason for that? Regards, Lukas -- 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: git tag --contains for cherry-picks
On Wed, Jun 29, 2016 at 12:48:33PM +0100, Laszlo Papp wrote: > Old releases are maintained with important bug fixes or even new features > in our case. It sometimes means that we need to cherry-pick commits across > branches, like from master to a specific release branch. > > Cherry-picking changes the hash of the commit, therefore, this may no > longer work for cherry-picks: > > git tag --contains > > I am thinking of having something like: > > git tag --contains-follow > > which would follow cherry-picks. I am not sure how easily and/or > efficiently this can be implemented, but my gut feeling is that in the vast > majority of the cases, the content check would bail out already at the > "subject line". Git generally considers commits "equivalent" based on the patch-id, whic his a sha1 of the diff (modulo some canonicalization). So you could ask right now for the patch-id of a particular commit: git show $commit | git patch-id >needle and then the patch-id of all tagged commits: git log -p --tags | git patch-id | sort >haystack Each line will have the patch-id followed by the commit id. You can then correlate them (join is part of GNU coreutils): join needle haystack | cut -d' ' -f2- >synonyms That gives you a list of synonym commits, which you can use to ask git which tags contain any of them: git tag $(for i in $(cat synonyms); do echo "--contains $i"; done) The big downside is that generating the haystack is expensive (it has to do a diff on every commit). But if this is something you do a lot, you can save the haystack and incrementally update it with new commits. Of course there are other ways of determining commit equivalence. You could find ones with duplicate commit messages, or duplicate subjects, or whatever. But if you have a cherry-picking workflow, I suspect the easiest thing may be to simply use "git cherry-pick -x", which will write the sha1 of the original commit into the cherry-picked commit message. You can then use that to correlate directly. So there's no specific "--contains-follow" as you want, but the tools are there to do it (and more flexibly and efficiently, depending on your needs). That doesn't necessarily mean it's not a good idea to make the simple case easier, though. -Peff -- 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: preview: What's cooking in git.git (Jun 2016, #10; Tue, 28)
On Wed, Jun 29, 2016 at 01:47:31PM +0200, Johannes Schindelin wrote: > On Tue, 28 Jun 2016, Junio C Hamano wrote: > > > * jk/ansi-color (2016-06-23) 7 commits > > (merged to 'next' on 2016-06-28 at 354989c) > > + color: support strike-through attribute > > + color: support "italic" attribute > > + color: allow "no-" for negating attributes > > + color: refactor parse_attr > > + add skip_prefix_mem helper > > + doc: refactor description of color format > > + color: fix max-size comment > > > > The output coloring scheme learned two new attributes, italic and > > strike, in addition to existing bold, reverse, etc. > > > > Will merge to 'master'. > > Please note that those "colors" do not work on Windows, at least as far as > I know, I only skimmed the code in set_attr(): > > https://github.com/git/git/blob/v2.9.0/compat/winansi.c#L175-L314 > > ... and it looks as if italic is plainly unsupported, and strike-through > is not handled. I suspect winansi doesn't handle 256-color or 24-bit color modes either, and those are also not supported on all terminals. All of the color output is subject to the user's terminal supporting it. It might be that we should make a more clear disclaimer in the documentation. -Peff -- 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 0/7] literal formatting in documentation
On Tue, Jun 28, 2016 at 01:40:08PM +0200, Matthieu Moy wrote: > This should address all comments from Peff. Thanks. The review is mind-numbing enough that I didn't do a complete read-through again, but just spot-checked a few places. This version looks good to me. -Peff -- 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] t/Makefile: add a rule to re-run previously-failed tests
On Wed, Jun 29, 2016 at 09:02:37AM +0200, Johannes Schindelin wrote: > It is the most convenient way to determine which tests failed after > running the entire test suite, in parallel, to look for left-over "trash > directory.t*" subdirectories in the t/ subdirectory. As Junio noted, this doesn't work with --root. I have sometimes used: grep 'failed [^0]' test-results/* for this purpose. > This patch automates the process of determinig which tests failed > previously and re-running them; It turned out to be quite convenient > when trying to squash bugs that crept in during rebases. I suspect your response will be "perl tools on Windows are too painful to use", but the "prove" tool which comes with perl can do this and more (e.g., running the failed tests first, and then following up with the others to double-check), and our test suite supports it quite well. $ grep -B1 PROVE config.mak # run tests in parallel, with slow ones first to keep pipelines full GIT_PROVE_OPTS = -j16 --state=slow,save $ cd t $ make prove ... reports some test failed ... $ prove --state=failed ... re-runs just the failed test ... -Peff -- 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] t/Makefile: add a rule to re-run previously-failed tests
While developing patch series, it is a good practice to run the test suite from time to time, just to make sure that obvious bugs are caught early. With complex patch series, it is common to run `make -j15 -k test`, i.e. run the tests in parallel and not stop at the first failing test but continue. This has the advantage of identifying possibly multiple problems without having to wait for the complete test suite to finish. It is particularly important to reduce the turn-around time thusly on Windows, where the test suite spends 45 minutes on the computer on which this patch was developed. It is the most convenient way to determine which tests failed after running the entire test suite, in parallel, to look for left-over "trash directory.t*" subdirectories in the t/ subdirectory. This patch automates the process of determinig which tests failed previously and re-running them; It turned out to be quite convenient when trying to squash bugs that crept in during rebases. Signed-off-by: Johannes Schindelin --- Published-As: https://github.com/dscho/git/releases/tag/failing-tests-v1 t/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/Makefile b/t/Makefile index 18e2b28..1459a7f 100644 --- a/t/Makefile +++ b/t/Makefile @@ -35,6 +35,8 @@ all: $(DEFAULT_TEST_TARGET) test: pre-clean $(TEST_LINT) $(MAKE) aggregate-results-and-cleanup +failed: $(patsubst trash,,$(patsubst directory.%,%.sh,$(wildcard trash\ directory.t[0-9]*))) + prove: pre-clean $(TEST_LINT) @echo "*** prove ***"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS) $(MAKE) clean-except-prove-cache -- 2.9.0.118.g0e1a633 base-commit: cf4c2cfe52be5bd973a4838f73a35d3959ce2f43 -- 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: git svn clone segmentation faul issue
Hi Yannis,
On Tue, 28 Jun 2016, Johannes Schindelin wrote:
> On Tue, 28 Jun 2016, Ioannis Kappas wrote:
>
> > Johannes Schindelin gmx.de> writes:
> >
> > > Would you mind giving them a whirl?
> >
> > The patch in "source code (zip)" seems to be missing the line in the prepare
> > () section of PKGBUILD to actually apply the fix:
> >
> > diff --git a/subversion/PKGBUILD b/subversion/PKGBUILD
> > --- a/subversion/PKGBUILD
> > +++ b/subversion/PKGBUILD
> >
> > @@ -101,6 +103,7 @@ prepare() {
> >patch -p1 -i ${srcdir}/17-fix-test-link.patch
> >patch -p1 -i ${srcdir}/18-fix-serf-config.patch
> >patch -p1 -i ${srcdir}/19-remove-contrib-from-configure.patch
> > + patch -p1 -i ${srcdir}/20-fix-stack-corruption-in-swig-wrappers.patch
> >patch -p1 -i ${srcdir}/subversion-1.9.1-msys2.patch
> >patch -p1 -i ${srcdir}/remove-checking-symlink.patch
> >patch -p1 -i ${srcdir}/90-use-copy-instead-symlink.patch
> >
> >
> > Would you be so kind to add the above and rebuild.
>
> Oh bummer. Sorry for that! I will fix it and rebuild tomorrow.
>
> > I have just tested it locally to work successfully, but I do not mind
> > retesting if you wish to provide another build :)
>
> Thanks, that would be much appreciated! I'll send a reply when I rebuilt
> the packages.
I just re-uploaded new packages, after rebuilding them and making sure
that the patch was actually applied.
May I ask you to re-test, just to make extra sure that the bug in question
is fixed?
Ciao,
Johannes
--
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 v2] git-p4: place temporary refs used for branch import under refs/git-p4-tmp
From: Lars Schneider
Git-P4 used to place temporary refs under "git-p4-tmp". Since 3da1f37
Git checks that all refs are placed under "refs". Instruct Git-P4 to
place temporary refs under "refs/git-p4-tmp". There are no backwards
compatibility considerations as these refs are transient.
Use "git show-ref --verify" to check the (non-)existience of the refs
instead of file checks assuming the file-based ref backend.
All refs under "refs" are shared across all worktrees. This is not
desired for temporary Git-P4 refs and will be adressed in a later patch.
Signed-off-by: Lars Schneider
---
Thank you Hannes for the sharp eye!
diff to v1:
* check non-existence of refs/git-p4-tmp instead of ref/git-p4-tmp
* use refs/git-p4-tmp instead of ref/git-p4-tmp in commit message
* check reference with "git show-ref --verify" to be future-proof (thanks
Junio!)
Cheers,
Lars
git-p4.py| 2 +-
t/t9801-git-p4-branch.sh | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index b6593cf..6b252df 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2274,7 +2274,7 @@ class P4Sync(Command, P4UserMap):
self.useClientSpec_from_options = False
self.clientSpecDirs = None
self.tempBranches = []
-self.tempBranchLocation = "git-p4-tmp"
+self.tempBranchLocation = "refs/git-p4-tmp"
self.largeFileSystem = None
if gitConfig('git-p4.largeFileSystem'):
diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
index 0aafd03..6a86d69 100755
--- a/t/t9801-git-p4-branch.sh
+++ b/t/t9801-git-p4-branch.sh
@@ -300,7 +300,7 @@ test_expect_success 'git p4 clone complex branches' '
test_path_is_file file2 &&
test_path_is_file file3 &&
! grep update file2 &&
- test_path_is_missing .git/git-p4-tmp
+ test_must_fail git show-ref --verify refs/git-p4-tmp
)
'
@@ -352,7 +352,7 @@ test_expect_success 'git p4 sync changes to two branches in
the same changelist'
test_path_is_file file2 &&
test_path_is_file file3 &&
! grep update file2 &&
- test_path_is_missing .git/git-p4-tmp
+ test_must_fail git show-ref --verify refs/git-p4-tmp
)
'
--
2.5.1
--
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: What's happening to the index
"Andy Falanga (afalanga)" writes: > The strange thing now is, after the script exits, I then call "git > fetch" in the recipe. I can see from the output of make that the remote > db is fetched. However, when I call "git show > origin/rpm:path/to/rpm_build_num" from the makefile I get the *previous* > number. Yet, as soon as the make process exits, I call "git show > origin/rpm:path/to/rpm_build_num" and it shows the correct number! My bet would be that you are running the commands in two different repositories. Try running pwd && git show origin/rpm:path/to/rpm_build_num In the Makefile and after make completes, and check that pwd returns the same thing. Also, to avoid getting the issue you previously had, try cd $(pwd) && pwd && git show origin/rpm:path/to/rpm_build_num too. > Is there some sort of strange file caching that happening > when make starts that, although the local db is updated, I don't get > what I'm after? Git can keep information either in RAM, hence not shared between git invocations (so running git within or outside the Makefile wouldn't matter), or on disk, but then inside or outside the Makefile doesn't matter either. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: git svn clone segmentation faul issue
Hi Eric, Eric Wong 80x24.org> writes: > > Ioannis.Kappas rbs.com wrote: > > Fortunately, a patch has already been submitted to subversion > > with (github) revision > > a074af86c8764404b28ce99d0bedcb668a321408 (at > > https://github.com/apache/subversion/commit/a074af86c8764404b28ce99d0bedcb66 8a321408 > > ) on the trunk to handle this and a couple of other similar > > cases. But by the looks of it has not been picked up yet in > > the latest subversion 1.9.4 release or the 1.9.x branch, > > perhaps because this patch was identified in sanity checks > > rather than coming out from a perceivable production issue? > > Thank you for documenting this. Curious, does this affect older > SVN versions or only 1.9.x? It does not appear so, at least not in the degree of failures that I am seeing it now, i.e. failing consistently. It only came to my attention after I upgraded to the latest git/perl/subversion release from a version that was more than a year's old. Given that the issue is with corruption in the perl interpreter stack, I suspect is due to one, or combination, of the following: - git in latest releases makes additional perl subversion calls, thus affecting the perl stack usage. - perl in latest releases affected the stack allocation profile. - subversion perl module in latest releases refactored/added new calls that has changed the perl stack usage. > > I don't know Perl internals well or SWIG at all; so reports > like these are very much appreciated. > 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
git gui produces series of commits with exactly the same time
Hi everyone, I've noticed that git history contains series of commits with exactly the same time, despite the real commit time of these commits was different. All these commit were made using the git gui tool. I'm the only developer in this project. Has anyone experienced the same problem? (I'm currently using Git 2.9.0 x64 under Windows.) -- Best wishes, Andrei Faber -- 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: git svn clone segmentation faul issue
Hi Johannes, Johannes Schindelin gmx.de> writes: > > Hi Yannis, > > [...] > > I just re-uploaded new packages, after rebuilding them and making sure > that the patch was actually applied. > > May I ask you to re-test, just to make extra sure that the bug in question > is fixed? I confirm the latest version, built from the 'source files (zip)' package, has resolved the issue. Thanks for the prompt response! Next task is to get the patch through to the next subversion official release. > > Ciao, > Johannes > -- 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 0/9] Use merge_recursive() directly in the builtin am
This is the long-awaited re-roll of the attempt to avoid spawning merge-recursive from the builtin am and use merge_recursive() directly instead. As indicated in the message of the final commit, the performance improvement is modest, if noticable. The *real* reason for the reroll is that I need a libified recursive merge to accelerate the interactive rebase by teaching the sequencer to do rebase -i's grunt work. In other words, this is one of those 13 (now 14) patch series leading up to a faster interactive rebase. It did take quite a while to go through the code "with a fine-toothed comb", and it did turn up a few surprises. For example, the recursive merge calls remove_file() a couple of times but actually does not always care about the return value, and rightfully so. When updating a working tree, for example, where a file was replaced by a directory, the code may create the directory first (implicitly deleting the file) and only then attempt to remove the file, failing because it is a directory already. One might argue that this logic is flawed, then, and I would agree. Right now my focus is on rebase -i and I want to avoid getting side-tracked by the recursive merge logic. So the clean-up will need to wait for another day (or week). We also need to be extra careful to retain backwards-compatibility. The test script t6022-merge-rename.sh, for example, verifies that `git pull` exits with status 128 in case of a fatal error. To that end, we need to make sure that fatal errors are handled by existing (builtin) users via exit(128) (or die(), which calls exit(128) at the end). New users (such as a builtin helper doing rebase -i's grunt work) may want to print some helpful advice what happened and how to get out of this mess before erroring out. In contrast to the original attempt at libifying merge_recursive() (as part of fixing a regression in the builtin am which wanted to print some advice, but could not, because the merge machinery die()d before it could), I no longer use the "gently" flag. Better to get it right to begin with: fatal errors are indicated by a negative return value. No dying without proper cause. In an earlier iteration of this patch series which was not sent to the mailing list, I used the special return vale -128 to indicate "real fatal errors". This turned out to be unnecessary: returning -1 always, to indicate that the operation could not complete successfully, is the appropriate way to handle all errors. As this patch series touches rather important code, I would really appreciate thorough reviews, with a particular focus on what regressions this patch series might introduce. Johannes Schindelin (8): Report bugs consistently merge-recursive: clarify code in was_tracked() Prepare the builtins for a libified merge_recursive() merge_recursive: abort properly upon errors merge-recursive: avoid returning a wholesale struct merge-recursive: allow write_tree_from_memory() to error out merge-recursive: handle return values indicating errors merge-recursive: switch to returning errors instead of dying Junio C Hamano (1): am: make a direct call to merge_recursive builtin/am.c | 27 ++-- builtin/checkout.c | 4 +- builtin/ls-files.c | 3 +- builtin/merge.c| 4 + builtin/update-index.c | 2 +- grep.c | 8 +- imap-send.c| 2 +- merge-recursive.c | 397 ++--- sequencer.c| 4 + sha1_file.c| 4 +- trailer.c | 2 +- transport.c| 2 +- wt-status.c| 4 +- 13 files changed, 279 insertions(+), 184 deletions(-) Published-As: https://github.com/dscho/git/releases/tag/am-3-merge-recursive-direct-v1 -- 2.9.0.268.gcabc8b0 base-commit: cf4c2cfe52be5bd973a4838f73a35d3959ce2f43 -- 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 1/9] Report bugs consistently
The vast majority of error messages in Git's source code which report a
bug use the convention to prefix the message with "BUG:".
As part of cleaning up merge-recursive to stop die()ing except in case of
detected bugs, let's just make the remainder of the bug reports consistent
with the de facto rule.
Signed-off-by: Johannes Schindelin
---
builtin/ls-files.c | 3 ++-
builtin/update-index.c | 2 +-
grep.c | 8
imap-send.c| 2 +-
merge-recursive.c | 13 ++---
sha1_file.c| 4 ++--
trailer.c | 2 +-
transport.c| 2 +-
wt-status.c| 4 ++--
9 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index f02e3d2..00ea91a 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -118,7 +118,8 @@ static void show_killed_files(struct dir_struct *dir)
*/
pos = cache_name_pos(ent->name, ent->len);
if (0 <= pos)
- die("bug in show-killed-files");
+ die("BUG: killed-file %.*s not found",
+ ent->len, ent->name);
pos = -pos - 1;
while (pos < active_nr &&
ce_stage(active_cache[pos]))
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 6cdfd5f..ba04b19 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1146,7 +1146,7 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
report(_("Untracked cache enabled for '%s'"),
get_git_work_tree());
break;
default:
- die("Bug: bad untracked_cache value: %d", untracked_cache);
+ die("BUG: bad untracked_cache value: %d", untracked_cache);
}
if (active_cache_changed) {
diff --git a/grep.c b/grep.c
index 1e15b62..f1ca0a0 100644
--- a/grep.c
+++ b/grep.c
@@ -643,10 +643,10 @@ static struct grep_expr *prep_header_patterns(struct
grep_opt *opt)
for (p = opt->header_list; p; p = p->next) {
if (p->token != GREP_PATTERN_HEAD)
- die("bug: a non-header pattern in grep header list.");
+ die("BUG: a non-header pattern in grep header list.");
if (p->field < GREP_HEADER_FIELD_MIN ||
GREP_HEADER_FIELD_MAX <= p->field)
- die("bug: unknown header field %d", p->field);
+ die("BUG: unknown header field %d", p->field);
compile_regexp(p, opt);
}
@@ -659,7 +659,7 @@ static struct grep_expr *prep_header_patterns(struct
grep_opt *opt)
h = compile_pattern_atom(&pp);
if (!h || pp != p->next)
- die("bug: malformed header expr");
+ die("BUG: malformed header expr");
if (!header_group[p->field]) {
header_group[p->field] = h;
continue;
@@ -1464,7 +1464,7 @@ static int grep_source_1(struct grep_opt *opt, struct
grep_source *gs, int colle
case GREP_BINARY_TEXT:
break;
default:
- die("bug: unknown binary handling mode");
+ die("BUG: unknown binary handling mode");
}
}
diff --git a/imap-send.c b/imap-send.c
index 938c691..cd39805 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -511,7 +511,7 @@ static int nfsnprintf(char *buf, int blen, const char *fmt,
...)
va_start(va, fmt);
if (blen <= 0 || (unsigned)(ret = vsnprintf(buf, blen, fmt, va)) >=
(unsigned)blen)
- die("Fatal: buffer too small. Please report a bug.");
+ die("BUG: buffer too small (%d < %d)", ret, blen);
va_end(va);
return ret;
}
diff --git a/merge-recursive.c b/merge-recursive.c
index 65cb5d6..98f4632 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -259,7 +259,7 @@ struct tree *write_tree_from_memory(struct merge_options *o)
fprintf(stderr, "BUG: %d %.*s\n", ce_stage(ce),
(int)ce_namelen(ce), ce->name);
}
- die("Bug in merge-recursive.c");
+ die("BUG: unmerged index entries in merge-recursive.c");
}
if (!active_cache_tree)
@@ -955,9 +955,8 @@ static struct merge_file_info merge_file_1(struct
merge_options *o,
if (!sha_eq(a->sha1, b->sha1))
result.clean = 0;
- } else {
- die(_("unsupported object type in the tree"));
- }
+ } else
+ die(_("BUG: unsupported object t
[PATCH 2/9] merge-recursive: clarify code in was_tracked()
It can be puzzling to see that was_tracked() tries to match an index
entry by name even if cache_name_pos() returned a negative value. Let's
clarify that cache_name_pos() implicitly looks for stage 0, while we are
also okay with finding other stages.
Signed-off-by: Johannes Schindelin
---
merge-recursive.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/merge-recursive.c b/merge-recursive.c
index 98f4632..bcb53f0 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -658,6 +658,7 @@ static int was_tracked(const char *path)
{
int pos = cache_name_pos(path, strlen(path));
+ /* cache_name_pos() looks for stage == 0, so pos may be < 0 */
if (pos < 0)
pos = -1 - pos;
while (pos < active_nr &&
--
2.9.0.268.gcabc8b0
--
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 7/9] merge-recursive: handle return values indicating errors
We are about to libify the recursive merge machinery, where we only
die() in case of a bug or memory contention. To that end, we must heed
negative return values as indicating errors.
This requires our functions to be careful to pass through error
conditions in call chains, and for quite a few functions this means
that they have to return values to begin with.
The next step will be to convert the places where we currently die() to
return negative values (read: -1) instead.
Note that we ignore errors reported by make_room_for_path(), consistent
with the previous behavior (update_file_flags() used the return value of
make_room_for_path() only to indicate an early return, but not a fatal
error): if the error is really a fatal error, we will notice later; If
not, it was not that serious a problem to begin with. (Witnesses in
favor of this reasoning are t4151-am-abort and t7610-mergetool, which
would start failing if we stopped on errors reported by
make_room_for_path()).
Signed-off-by: Johannes Schindelin
---
merge-recursive.c | 251 ++
1 file changed, 158 insertions(+), 93 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 6ab7dfc..bb075e3 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -266,8 +266,10 @@ struct tree *write_tree_from_memory(struct merge_options
*o)
active_cache_tree = cache_tree();
if (!cache_tree_fully_valid(active_cache_tree) &&
- cache_tree_update(&the_index, 0) < 0)
- die(_("error building trees"));
+ cache_tree_update(&the_index, 0) < 0) {
+ error(_("error building trees"));
+ return NULL;
+ }
result = lookup_tree(active_cache_tree->sha1);
@@ -548,19 +550,17 @@ static int update_stages(const char *path, const struct
diff_filespec *o,
*/
int clear = 1;
int options = ADD_CACHE_OK_TO_ADD | ADD_CACHE_SKIP_DFCHECK;
+ int ret = 0;
+
if (clear)
- if (remove_file_from_cache(path))
- return -1;
- if (o)
- if (add_cacheinfo(o->mode, o->sha1, path, 1, 0, options))
- return -1;
- if (a)
- if (add_cacheinfo(a->mode, a->sha1, path, 2, 0, options))
- return -1;
- if (b)
- if (add_cacheinfo(b->mode, b->sha1, path, 3, 0, options))
- return -1;
- return 0;
+ ret = remove_file_from_cache(path);
+ if (!ret && o)
+ ret = add_cacheinfo(o->mode, o->sha1, path, 1, 0, options);
+ if (!ret && a)
+ ret = add_cacheinfo(a->mode, a->sha1, path, 2, 0, options);
+ if (!ret && b)
+ ret = add_cacheinfo(b->mode, b->sha1, path, 3, 0, options);
+ return ret;
}
static void update_entry(struct stage_data *entry,
@@ -736,7 +736,7 @@ static int make_room_for_path(struct merge_options *o,
const char *path)
return error(msg, path, _(": perhaps a D/F conflict?"));
}
-static void update_file_flags(struct merge_options *o,
+static int update_file_flags(struct merge_options *o,
const unsigned char *sha,
unsigned mode,
const char *path,
@@ -777,8 +777,7 @@ static void update_file_flags(struct merge_options *o,
if (make_room_for_path(o, path) < 0) {
update_wd = 0;
- free(buf);
- goto update_index;
+ goto free_buf;
}
if (S_ISREG(mode) || (!has_symlinks && S_ISLNK(mode))) {
int fd;
@@ -801,20 +800,22 @@ static void update_file_flags(struct merge_options *o,
} else
die(_("do not know what to do with %06o %s '%s'"),
mode, sha1_to_hex(sha), path);
+free_buf:
free(buf);
}
update_index:
if (update_cache)
add_cacheinfo(mode, sha, path, 0, update_wd,
ADD_CACHE_OK_TO_ADD);
+ return 0;
}
-static void update_file(struct merge_options *o,
+static int update_file(struct merge_options *o,
int clean,
const unsigned char *sha,
unsigned mode,
const char *path)
{
- update_file_flags(o, sha, mode, path, o->call_depth || clean,
!o->call_depth);
+ return update_file_flags(o, sha, mode, path, o->call_depth || clean,
!o->call_depth);
}
/* Low level file merging, update and removal */
@@ -1010,7 +1011,7 @@ static int merge_file_one(struct merge_options *o,
return merge_file_1(o, &one, &a, &b, branch1, branch2, mfi);
}
-static void handle_change_delete(struct merge_options *o,
+static int handle_change_delete(struct merge_options *o,
const ch
[PATCH 4/9] merge_recursive: abort properly upon errors
There are a couple of places where return values indicating errors
are ignored. Let's teach them manners.
Signed-off-by: Johannes Schindelin
---
merge-recursive.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index bcb53f0..c4ece96 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1944,8 +1944,9 @@ int merge_recursive(struct merge_options *o,
saved_b2 = o->branch2;
o->branch1 = "Temporary merge branch 1";
o->branch2 = "Temporary merge branch 2";
- merge_recursive(o, merged_common_ancestors, iter->item,
- NULL, &merged_common_ancestors);
+ if (merge_recursive(o, merged_common_ancestors, iter->item,
+ NULL, &merged_common_ancestors) < 0)
+ return -1;
o->branch1 = saved_b1;
o->branch2 = saved_b2;
o->call_depth--;
@@ -1961,6 +1962,8 @@ int merge_recursive(struct merge_options *o,
o->ancestor = "merged common ancestors";
clean = merge_trees(o, h1->tree, h2->tree,
merged_common_ancestors->tree,
&mrtree);
+ if (clean < 0)
+ return clean;
if (o->call_depth) {
*result = make_virtual_commit(mrtree, "merged tree");
@@ -2017,6 +2020,9 @@ int merge_recursive_generic(struct merge_options *o,
hold_locked_index(lock, 1);
clean = merge_recursive(o, head_commit, next_commit, ca,
result);
+ if (clean < 0)
+ return clean;
+
if (active_cache_changed &&
write_locked_index(&the_index, lock, COMMIT_LOCK))
return error(_("Unable to write index."));
--
2.9.0.268.gcabc8b0
--
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 5/9] merge-recursive: avoid returning a wholesale struct
It is technically allowed, as per C89, for functions' return type to
be complete structs (i.e. *not* just pointers to structs), but it is
bad practice.
This is a very late attempt to contain the damage done by this developer
in 6d297f8 (Status update on merge-recursive in C, 2006-07-08) which
introduced such a return type.
It will also help the current effort to libify merge-recursive.c, as
it will allow us to return proper error codes later.
Signed-off-by: Johannes Schindelin
---
merge-recursive.c | 93 ++-
1 file changed, 50 insertions(+), 43 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index c4ece96..d56651c9 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -888,47 +888,47 @@ static int merge_3way(struct merge_options *o,
return merge_status;
}
-static struct merge_file_info merge_file_1(struct merge_options *o,
+static int merge_file_1(struct merge_options *o,
const struct diff_filespec *one,
const struct diff_filespec *a,
const struct diff_filespec *b,
const char *branch1,
- const char *branch2)
+ const char *branch2,
+ struct merge_file_info *result)
{
- struct merge_file_info result;
- result.merge = 0;
- result.clean = 1;
+ result->merge = 0;
+ result->clean = 1;
if ((S_IFMT & a->mode) != (S_IFMT & b->mode)) {
- result.clean = 0;
+ result->clean = 0;
if (S_ISREG(a->mode)) {
- result.mode = a->mode;
- hashcpy(result.sha, a->sha1);
+ result->mode = a->mode;
+ hashcpy(result->sha, a->sha1);
} else {
- result.mode = b->mode;
- hashcpy(result.sha, b->sha1);
+ result->mode = b->mode;
+ hashcpy(result->sha, b->sha1);
}
} else {
if (!sha_eq(a->sha1, one->sha1) && !sha_eq(b->sha1, one->sha1))
- result.merge = 1;
+ result->merge = 1;
/*
* Merge modes
*/
if (a->mode == b->mode || a->mode == one->mode)
- result.mode = b->mode;
+ result->mode = b->mode;
else {
- result.mode = a->mode;
+ result->mode = a->mode;
if (b->mode != one->mode) {
- result.clean = 0;
- result.merge = 1;
+ result->clean = 0;
+ result->merge = 1;
}
}
if (sha_eq(a->sha1, b->sha1) || sha_eq(a->sha1, one->sha1))
- hashcpy(result.sha, b->sha1);
+ hashcpy(result->sha, b->sha1);
else if (sha_eq(b->sha1, one->sha1))
- hashcpy(result.sha, a->sha1);
+ hashcpy(result->sha, a->sha1);
else if (S_ISREG(a->mode)) {
mmbuffer_t result_buf;
int merge_status;
@@ -940,62 +940,63 @@ static struct merge_file_info merge_file_1(struct
merge_options *o,
die(_("Failed to execute internal merge"));
if (write_sha1_file(result_buf.ptr, result_buf.size,
- blob_type, result.sha))
+ blob_type, result->sha))
die(_("Unable to add %s to database"),
a->path);
free(result_buf.ptr);
- result.clean = (merge_status == 0);
+ result->clean = (merge_status == 0);
} else if (S_ISGITLINK(a->mode)) {
- result.clean = merge_submodule(result.sha,
+ result->clean = merge_submodule(result->sha,
one->path, one->sha1,
a->sha1, b->sha1,
!o->call_depth);
} else if (S_ISLNK(a->mode)) {
- hashcpy(result.sha, a->sha1);
+ hashcpy(result->sha, a->sha1);
if (!sha_eq(a->sha1, b->sha1))
- result.clean = 0;
+ result->clean = 0;
} else
die(_("BUG: unsup
[PATCH 6/9] merge-recursive: allow write_tree_from_memory() to error out
It is possible that a tree cannot be written (think: disk full). We will want to give the caller a chance to clean up instead of letting the program die() in such a case. Signed-off-by: Johannes Schindelin --- merge-recursive.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index d56651c9..6ab7dfc 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1875,8 +1875,8 @@ int merge_trees(struct merge_options *o, else clean = 1; - if (o->call_depth) - *result = write_tree_from_memory(o); + if (o->call_depth && !(*result = write_tree_from_memory(o))) + return -1; return clean; } -- 2.9.0.268.gcabc8b0 -- 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 8/9] merge-recursive: switch to returning errors instead of dying
The recursive merge machinery is supposed to be a library function, i.e.
it should return an error when it fails. Originally the functions were
part of the builtin "merge-recursive", though, where it was simpler to
call die() and be done with error handling.
The existing callers were already prepared to detect negative return
values to indicate errors and to behave as previously: exit with code 128
(which is the same thing that die() does, after printing the message).
Signed-off-by: Johannes Schindelin
---
merge-recursive.c | 55 ++-
1 file changed, 30 insertions(+), 25 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index bb075e3..d5a593c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -710,12 +710,10 @@ static int make_room_for_path(struct merge_options *o,
const char *path)
/* Make sure leading directories are created */
status = safe_create_leading_directories_const(path);
if (status) {
- if (status == SCLD_EXISTS) {
+ if (status == SCLD_EXISTS)
/* something else exists */
- error(msg, path, _(": perhaps a D/F conflict?"));
- return -1;
- }
- die(msg, path, "");
+ return error(msg, path, _(": perhaps a D/F conflict?"));
+ return error(msg, path, "");
}
/*
@@ -743,6 +741,8 @@ static int update_file_flags(struct merge_options *o,
int update_cache,
int update_wd)
{
+ int ret = 0;
+
if (o->call_depth)
update_wd = 0;
@@ -763,9 +763,11 @@ static int update_file_flags(struct merge_options *o,
buf = read_sha1_file(sha, &type, &size);
if (!buf)
- die(_("cannot read object %s '%s'"), sha1_to_hex(sha),
path);
- if (type != OBJ_BLOB)
- die(_("blob expected for %s '%s'"), sha1_to_hex(sha),
path);
+ return error(_("cannot read object %s '%s'"),
sha1_to_hex(sha), path);
+ if (type != OBJ_BLOB) {
+ ret = error(_("blob expected for %s '%s'"),
sha1_to_hex(sha), path);
+ goto free_buf;
+ }
if (S_ISREG(mode)) {
struct strbuf strbuf = STRBUF_INIT;
if (convert_to_working_tree(path, buf, size, &strbuf)) {
@@ -786,8 +788,10 @@ static int update_file_flags(struct merge_options *o,
else
mode = 0666;
fd = open(path, O_WRONLY | O_TRUNC | O_CREAT, mode);
- if (fd < 0)
- die_errno(_("failed to open '%s'"), path);
+ if (fd < 0) {
+ ret = error_errno(_("failed to open '%s'"),
path);
+ goto free_buf;
+ }
write_in_full(fd, buf, size);
close(fd);
} else if (S_ISLNK(mode)) {
@@ -795,18 +799,18 @@ static int update_file_flags(struct merge_options *o,
safe_create_leading_directories_const(path);
unlink(path);
if (symlink(lnk, path))
- die_errno(_("failed to symlink '%s'"), path);
+ ret = error_errno(_("failed to symlink '%s'"),
path);
free(lnk);
} else
- die(_("do not know what to do with %06o %s '%s'"),
- mode, sha1_to_hex(sha), path);
+ ret = error_errno(_("do not know what to do with %06o
%s '%s'"),
+ mode, sha1_to_hex(sha), path);
free_buf:
free(buf);
}
update_index:
- if (update_cache)
+ if (!ret && update_cache)
add_cacheinfo(mode, sha, path, 0, update_wd,
ADD_CACHE_OK_TO_ADD);
- return 0;
+ return ret;
}
static int update_file(struct merge_options *o,
@@ -932,20 +936,22 @@ static int merge_file_1(struct merge_options *o,
hashcpy(result->sha, a->sha1);
else if (S_ISREG(a->mode)) {
mmbuffer_t result_buf;
- int merge_status;
+ int ret = 0, merge_status;
merge_status = merge_3way(o, &result_buf, one, a, b,
branch1, branch2);
if ((merge_status < 0) || !result_buf.ptr)
- die(_("Failed to execute internal merge"));
+ ret = error(_("Failed to execute internal
merge"));
- if (write_sha1_file(resul
[PATCH 3/9] Prepare the builtins for a libified merge_recursive()
A truly libified function does not die() just for fun. As such, the
recursive merge will convert all die() calls to return -1 instead in the
next commits, giving the caller a chance at least to print some helpful
message.
Let's prepare the builtins for this fatal error condition, even if we do
not really do more than imitating the previous die()'s exit(128): this is
what callers of e.g. `git merge` have come to expect.
Note that the callers of the sequencer (revert and cherry-pick) already
fail fast even for the return value -1; The only difference is that they
now get a chance to say " failed".
Signed-off-by: Johannes Schindelin
---
builtin/checkout.c | 4 +++-
builtin/merge.c| 4
sequencer.c| 4
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index c3486bd..14312f7 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -567,8 +567,10 @@ static int merge_working_tree(const struct checkout_opts
*opts,
o.ancestor = old->name;
o.branch1 = new->name;
o.branch2 = "local";
- merge_trees(&o, new->commit->tree, work,
+ ret = merge_trees(&o, new->commit->tree, work,
old->commit->tree, &result);
+ if (ret < 0)
+ exit(128);
ret = reset_tree(new->commit->tree, opts, 0,
writeout_error);
if (ret)
diff --git a/builtin/merge.c b/builtin/merge.c
index b555a1b..133b853 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -682,6 +682,8 @@ static int try_merge_strategy(const char *strategy, struct
commit_list *common,
hold_locked_index(&lock, 1);
clean = merge_recursive(&o, head,
remoteheads->item, reversed, &result);
+ if (clean < 0)
+ exit(128);
if (active_cache_changed &&
write_locked_index(&the_index, &lock, COMMIT_LOCK))
die (_("unable to write %s"), get_index_file());
@@ -1550,6 +1552,8 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
ret = try_merge_strategy(use_strategies[i]->name,
common, remoteheads,
head_commit, head_arg);
+ if (ret < 0)
+ exit(128);
if (!option_commit && !ret) {
merge_was_ok = 1;
/*
diff --git a/sequencer.c b/sequencer.c
index c6362d6..13b794a 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -293,6 +293,8 @@ static int do_recursive_merge(struct commit *base, struct
commit *next,
clean = merge_trees(&o,
head_tree,
next_tree, base_tree, &result);
+ if (clean < 0)
+ return clean;
if (active_cache_changed &&
write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
@@ -561,6 +563,8 @@ static int do_pick_commit(struct commit *commit, struct
replay_opts *opts)
if (!opts->strategy || !strcmp(opts->strategy, "recursive") ||
opts->action == REPLAY_REVERT) {
res = do_recursive_merge(base, next, base_label, next_label,
head, &msgbuf, opts);
+ if (res < 0)
+ return res;
write_message(&msgbuf, git_path_merge_msg());
} else {
struct commit_list *common = NULL;
--
2.9.0.268.gcabc8b0
--
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 9/9] am: make a direct call to merge_recursive
From: Junio C Hamano
Instead of spawning merge-recursive via run_command() in
run_fallback_merge_recursive(), make a direct call to the internal
merge_recursive_generic().
Here is a quick benchmark result, applying a patch for b4391657
(merge: drop 'git merge HEAD ' syntax, 2015-03-25)
that was still cooking in 'next' on 4b1fd356 (git-multimail: update
to release 1.2.0, 2015-10-11) which was the tip of 'master' at some
stage, on an x86-64 running Ubuntu:
real0m0.169s real0m0.163s
user0m0.108s user0m0.134s
sys 0m0.068s sys 0m0.033s
real0m0.175s real0m0.161s
user0m0.110s user0m0.120s
sys 0m0.066s sys 0m0.047s
real0m0.168s real0m0.162s
user0m0.124s user0m0.114s
sys 0m0.045s sys 0m0.051s
real0m0.167s real0m0.152s
user0m0.124s user0m0.122s
sys 0m0.045s sys 0m0.031s
real0m0.169s real0m0.164s
user0m0.131s user0m0.129s
sys 0m0.043s sys 0m0.041s
Left-hand side shows the original, right-hand side shows the result
of this optimization.
Timings on Windows:
original:
0.00user 0.01system 0:00.29elapsed
0.00user 0.00system 0:00.25elapsed
0.01user 0.00system 0:00.24elapsed
0.01user 0.00system 0:00.26elapsed
0.00user 0.01system 0:00.23elapsed
with optimization:
0.00user 0.01system 0:00.22elapsed
0.00user 0.00system 0:00.25elapsed
0.00user 0.01system 0:00.22elapsed
0.00user 0.00system 0:00.22elapsed
0.01user 0.00system 0:00.21elapsed
Signed-off-by: Junio C Hamano
Signed-off-by: Johannes Schindelin
---
It feels *slightly* wrong to submit your own patch to review,
however, please keep in mind that
1) I changed the patch (o.gently does not exist anymore, so I do
not set it), and
2) I added my own timings performed on Windows.
builtin/am.c | 27 ++-
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 3dfe70b..dd41154 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1587,25 +1587,26 @@ static int run_fallback_merge_recursive(const struct
am_state *state,
unsigned char *our_tree,
unsigned char *his_tree)
{
- struct child_process cp = CHILD_PROCESS_INIT;
+ const unsigned char *bases[1] = {orig_tree};
+ struct merge_options o;
+ struct commit *result;
+ char *his_tree_name;
int status;
- cp.git_cmd = 1;
+ init_merge_options(&o);
+
+ o.branch1 = "HEAD";
+ his_tree_name = xstrfmt("%.*s", linelen(state->msg), state->msg);
+ o.branch2 = his_tree_name;
- argv_array_pushf(&cp.env_array, "GITHEAD_%s=%.*s",
-sha1_to_hex(his_tree), linelen(state->msg),
state->msg);
if (state->quiet)
- argv_array_push(&cp.env_array, "GIT_MERGE_VERBOSITY=0");
+ o.verbosity = 0;
- argv_array_push(&cp.args, "merge-recursive");
- argv_array_push(&cp.args, sha1_to_hex(orig_tree));
- argv_array_push(&cp.args, "--");
- argv_array_push(&cp.args, sha1_to_hex(our_tree));
- argv_array_push(&cp.args, sha1_to_hex(his_tree));
+ status = merge_recursive_generic(&o, our_tree, his_tree, 1, bases,
&result);
+ if (status < 0)
+ exit(128);
+ free(his_tree_name);
- status = run_command(&cp) ? (-1) : 0;
- discard_cache();
- read_cache();
return status;
}
--
2.9.0.268.gcabc8b0
--
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: git svn clone segmentation faul issue
Hi Yannis, On Wed, 29 Jun 2016, Ioannis Kappas wrote: > Johannes Schindelin gmx.de> writes: > > > I just re-uploaded new packages, after rebuilding them and making sure > > that the patch was actually applied. > > > > May I ask you to re-test, just to make extra sure that the bug in > > question is fixed? > > I confirm the latest version, built from the 'source files (zip)' > package, has resolved the issue. Thank you so much! > Thanks for the prompt response! Next task is to get the patch through to > the next subversion official release. My more immediate concern is MSYS2 (the POSIX-emulating basis of Git for Windows), and I opened a Pull Request there: https://github.com/Alexpux/MSYS2-packages/pull/647 If this Pull Request is not picked up in time for the next Git for Windows version, I plan to use the packages that you tested. Thanks, Dscho -- 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: git gui produces series of commits with exactly the same time
Hi Andrei, On Wed, 29 Jun 2016, Andrei Faber wrote: > I've noticed that git history contains series of commits with exactly > the same time, despite the real commit time of these commits was > different. All these commit were made using the git gui tool. I'm the > only developer in this project. Is it possible that you played games with your GIT_AUTHOR_DATE environment variable? Ciao, Johannes -- 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: preview: What's cooking in git.git (Jun 2016, #10; Tue, 28)
Hi Junio, On Tue, 28 Jun 2016, Junio C Hamano wrote: > * jk/ansi-color (2016-06-23) 7 commits > (merged to 'next' on 2016-06-28 at 354989c) > + color: support strike-through attribute > + color: support "italic" attribute > + color: allow "no-" for negating attributes > + color: refactor parse_attr > + add skip_prefix_mem helper > + doc: refactor description of color format > + color: fix max-size comment > > The output coloring scheme learned two new attributes, italic and > strike, in addition to existing bold, reverse, etc. > > Will merge to 'master'. Please note that those "colors" do not work on Windows, at least as far as I know, I only skimmed the code in set_attr(): https://github.com/git/git/blob/v2.9.0/compat/winansi.c#L175-L314 ... and it looks as if italic is plainly unsupported, and strike-through is not handled. Ciao, Dscho -- 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 tag --contains for cherry-picks
Dear List, We have several release branches as well as a master branch where the active development happens. Old releases are maintained with important bug fixes or even new features in our case. It sometimes means that we need to cherry-pick commits across branches, like from master to a specific release branch. Cherry-picking changes the hash of the commit, therefore, this may no longer work for cherry-picks: git tag --contains I am thinking of having something like: git tag --contains-follow which would follow cherry-picks. I am not sure how easily and/or efficiently this can be implemented, but my gut feeling is that in the vast majority of the cases, the content check would bail out already at the "subject line". Again, just to recap: I would like to be able to list of releases (i.e. tags) in which a commit occurs even if it is cherry-picked because what matters for us in the end of the day is whether the feature or bugfix goes into a release. Best Regards, Laszlo Papp -- 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: git gui produces series of commits with exactly the same time
Hi Johannes, No. Another thing I've found is that these commits have different timestamp in the "Author" and "Committer" lines. The "Committer" lines have correct timestamps, and the "Author" timestamp is wrong. Best wishes, Andrei Faber On 29/06/2016 18:45, Johannes Schindelin wrote: Hi Andrei, On Wed, 29 Jun 2016, Andrei Faber wrote: I've noticed that git history contains series of commits with exactly the same time, despite the real commit time of these commits was different. All these commit were made using the git gui tool. I'm the only developer in this project. Is it possible that you played games with your GIT_AUTHOR_DATE environment variable? Ciao, Johannes -- 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] rebase -i: restore autostash on abort
Hi Patrick, On Wed, 29 Jun 2016, Patrick Steinhardt wrote: > When we abort an interactive rebase we do so by calling > `die_abort`, which cleans up after us by removing the rebase > state directory. If the user has requested to use the autostash > feature, though, the state directory may also contain a reference > to the autostash, which will now be deleted. > > Fix the issue by trying to re-apply the autostash in `die_abort`. > This will also handle the case where the autostash does not apply > cleanly anymore by recording it in a user-visible stash. I like it. Thanks for including the tests, they were very helpful to verify that my rebase--helper work is unaffected: the patch touches only those parts that are not moved into the rebase--helper. Thanks, Johannes -- 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 0/5] Use skip_blank_lines() for more commit messages
These patches are on top of js/find-commit-subject-ignore-leading-blanks
(whether you want to put them into the same topic branch before merging
or put them into their own add-on topic branch does not really matter to
me).
It occurred to me that there might be more code locations where finding
the commit object's body is hand-rolled, and could benefit from using
the skip_blank_lines() function to handle not-quite-standard but
still-allowed objects from being processed correctly.
I found a couple of instances, and made them consistent with the
pretty-printing machinery.
I made fine-grained commits to allow for dropping contentious patches
more easily.
There are four more instances which I did *not* change:
- fast-export: I designed this command to be as faithful in exporting
the commits as possible. As such, even non-standard commit objects
should be kept as-are.
- fmt-merge-msg: when showing signed commit messages, it is better to
show the commit messages verbatim rather than trimming them.
- when signing a commit, it is better to use the original commit message
than a munged one.
- notes' messages are left alone. I do not know whether there might be
an intentional use of leading blank lines, but I wanted to err on the
safe side.
I also considered treating tag messages the same way, but tags are much
more likely to be signed, and I did not want to risk munging the tag
messages just before signing them.
Johannes Schindelin (5):
commit -C: skip blank lines at the beginning of the message
sequencer: use skip_blank_lines() to find the commit subject
reset --hard: skip blank lines when reporting the commit subject
commit -S: avoid invalid pointer with empty message
Skip blank lines when matching ^{/foo}
builtin/commit.c | 2 +-
builtin/reset.c | 2 +-
commit.c | 6 +-
sequencer.c | 6 ++
sha1_name.c | 3 ++-
5 files changed, 11 insertions(+), 8 deletions(-)
Published-As:
https://github.com/dscho/git/releases/tag/more-blank-line-skipping-v1
--
2.9.0.270.g810e421
base-commit: 38a2ea1900489a76d9840002bf60bd7ff29f07cd
--
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 2/5] sequencer: use skip_blank_lines() to find the commit subject
Just like we already taught the find_commit_subject() function (to make
it consistent with the code in pretty.c), we now simply skip leading
blank lines of the commit message.
Signed-off-by: Johannes Schindelin
---
sequencer.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 7d7add6..cdfac82 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -544,10 +544,8 @@ static int do_pick_commit(struct commit *commit, struct
replay_opts *opts)
* information followed by "\n\n".
*/
p = strstr(msg.message, "\n\n");
- if (p) {
- p += 2;
- strbuf_addstr(&msgbuf, p);
- }
+ if (p)
+ strbuf_addstr(&msgbuf, skip_blank_lines(p + 2));
if (opts->record_origin) {
if (!has_conforming_footer(&msgbuf, NULL, 0))
--
2.9.0.270.g810e421
--
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 1/5] commit -C: skip blank lines at the beginning of the message
Consistent with the pretty-printing machinery, we skip leading blank
lines (if any) of existing commit messages.
While Git itself only produces commit objects with a single empty line
between commit header and commit message, it is legal to have more than
one blank line (i.e. lines containing only white space, or no
characters) at the beginning of the commit message, and the
pretty-printing code already handles that.
Signed-off-by: Johannes Schindelin
---
builtin/commit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 3f18942..1f6dbcd 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -715,7 +715,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
char *buffer;
buffer = strstr(use_message_buffer, "\n\n");
if (buffer)
- strbuf_addstr(&sb, buffer + 2);
+ strbuf_addstr(&sb, skip_blank_lines(buffer + 2));
hook_arg1 = "commit";
hook_arg2 = use_message;
} else if (fixup_message) {
--
2.9.0.270.g810e421
--
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 5/5] Skip blank lines when matching ^{/foo}
When trying to match a pattern in the commit subject, simply skip leading
blank lines in the commit message. This is consistent with the
pretty-printing machinery: it silently ignores leading blank lines in the
commit object's body.
Signed-off-by: Johannes Schindelin
---
sha1_name.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sha1_name.c b/sha1_name.c
index ca7ddd6..da354a5 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -912,7 +912,8 @@ static int get_sha1_oneline(const char *prefix, unsigned
char *sha1,
continue;
buf = get_commit_buffer(commit, NULL);
p = strstr(buf, "\n\n");
- matches = negative ^ (p && !regexec(®ex, p + 2, 0, NULL, 0));
+ matches = negative ^ (p && !regexec(®ex,
+ skip_blank_lines(p + 2), 0, NULL, 0));
unuse_commit_buffer(commit, buf);
if (matches) {
--
2.9.0.270.g810e421
--
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 4/5] commit -S: avoid invalid pointer with empty message
While it is not recommended, `git fsck` says that:
Not having a body is not a crime [...]
... which means that we cannot assume that the commit buffer contains an
empty line to separate header from body (essentially saying that there
is only a header).
So let's tread carefully here.
Signed-off-by: Johannes Schindelin
---
commit.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/commit.c b/commit.c
index 24d4715..0bb51a2 100644
--- a/commit.c
+++ b/commit.c
@@ -1090,11 +1090,15 @@ static const int gpg_sig_header_len =
sizeof(gpg_sig_header) - 1;
static int do_sign_commit(struct strbuf *buf, const char *keyid)
{
+ const char *eoh = strstr(buf->buf, "\n\n");
struct strbuf sig = STRBUF_INIT;
int inspos, copypos;
/* find the end of the header */
- inspos = strstr(buf->buf, "\n\n") - buf->buf + 1;
+ if (!eoh)
+ inspos = buf->len;
+ else
+ inspos = eoh - buf->buf + 1;
if (!keyid || !*keyid)
keyid = get_signing_key();
--
2.9.0.270.g810e421
--
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 3/5] reset --hard: skip blank lines when reporting the commit subject
When there are blank lines at the beginning of a commit message, the
pretty printing machinery already skips them when showing a commit
subject (or the complete commit message). We shall henceforth do the
same when reporting the commit subject after the user called
git reset --hard
Signed-off-by: Johannes Schindelin
---
builtin/reset.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index acd6278..5c6206b 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -103,7 +103,7 @@ static void print_new_head_line(struct commit *commit)
if (body) {
const char *eol;
size_t len;
- body += 2;
+ body = skip_blank_lines(body + 2);
eol = strchr(body, '\n');
len = eol ? eol - body : strlen(body);
printf(" %.*s\n", (int) len, body);
--
2.9.0.270.g810e421
--
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 0/2] Late edits to the rebase -i tests
I meant to send the first patch much earlier, but got side-tracked before I could add the --gpg-sign test. This is just another patch series in preparation for the rebase--helper (to be precise, this is the seventh out of fourteen patch series that have not yet been merged to master; Six are already in flight, so I will stop here until the queue empties a little.) Johannes Schindelin (2): t3404: fix another typo t3404: add a test for the --gpg-sign option t/t3404-rebase-interactive.sh | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) Published-As: https://github.com/dscho/git/releases/tag/rebase-i-tests-v1 -- 2.9.0.270.g810e421 base-commit: cf4c2cfe52be5bd973a4838f73a35d3959ce2f43 -- 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 2/2] t3404: add a test for the --gpg-sign option
To keep the time t3404 requires short (in this developer's Windows setup, this single test already takes a painful 8 minutes to pass), we avoid a full-blown GPG test and cop out by verifying the message displayed to the user upon an 'edit' command. Signed-off-by: Johannes Schindelin --- t/t3404-rebase-interactive.sh | 7 +++ 1 file changed, 7 insertions(+) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index c7ea8ba..4c96075 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1281,4 +1281,11 @@ test_expect_success 'editor saves as CR/LF' ' ) ' +EPIPHANY="'" +test_expect_success 'rebase -i --gpg-sign=' ' + set_fake_editor && + FAKE_LINES="edit 1" git rebase -i --gpg-sign=\" HEAD^ >out 2>err && + grep "$EPIPHANY-S\"$EPIPHANY" err +' + test_done -- 2.9.0.270.g810e421 -- 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 1/2] t3404: fix another typo
The past tense of "to run" is "run", not "ran". Signed-off-by: Johannes Schindelin --- t/t3404-rebase-interactive.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 66348f1..c7ea8ba 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -60,7 +60,7 @@ test_expect_success 'setup' ' test_commit P fileP ' -# "exec" commands are ran with the user shell by default, but this may +# "exec" commands are run with the user shell by default, but this may # be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work # to create a file. Unsetting SHELL avoids such non-portable behavior # in tests. It must be exported for it to take effect where needed. -- 2.9.0.270.g810e421 -- 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 v2 01/11] i18n: add--interactive: mark strings for translation
Mark simple strings (without interpolation) for translation.
Brackets around first parameter of ternary operator is necessary because
otherwise xgettext fails to extract strings marked for translation from
the rest of the file.
Signed-off-by: Vasco Almeida
---
git-add--interactive.perl | 68 +--
1 file changed, 36 insertions(+), 32 deletions(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 822f857..fb8e5de 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -4,6 +4,7 @@ use 5.008;
use strict;
use warnings;
use Git;
+use Git::I18N;
binmode(STDOUT, ":raw");
@@ -252,7 +253,7 @@ sub list_untracked {
}
my $status_fmt = '%12s %12s %s';
-my $status_head = sprintf($status_fmt, 'staged', 'unstaged', 'path');
+my $status_head = sprintf($status_fmt, __('staged'), __('unstaged'),
__('path'));
{
my $initial;
@@ -678,7 +679,7 @@ sub update_cmd {
my @mods = list_modified('file-only');
return if (!@mods);
- my @update = list_and_choose({ PROMPT => 'Update',
+ my @update = list_and_choose({ PROMPT => __('Update'),
HEADER => $status_head, },
@mods);
if (@update) {
@@ -690,7 +691,7 @@ sub update_cmd {
}
sub revert_cmd {
- my @update = list_and_choose({ PROMPT => 'Revert',
+ my @update = list_and_choose({ PROMPT => __('Revert'),
HEADER => $status_head, },
list_modified());
if (@update) {
@@ -724,13 +725,13 @@ sub revert_cmd {
}
sub add_untracked_cmd {
- my @add = list_and_choose({ PROMPT => 'Add untracked' },
+ my @add = list_and_choose({ PROMPT => __('Add untracked') },
list_untracked());
if (@add) {
system(qw(git update-index --add --), @add);
say_n_paths('added', @add);
} else {
- print "No untracked files.\n";
+ print __("No untracked files.\n");
}
print "\n";
}
@@ -1159,8 +1160,11 @@ sub edit_hunk_loop {
}
else {
prompt_yesno(
- 'Your edited hunk does not apply. Edit again '
- . '(saying "no" discards!) [y/n]? '
+ # TRANSLATORS: do not translate [y/n]
+ # The program will only accept that input
+ # at this point.
+ __('Your edited hunk does not apply. Edit again
'
+ . '(saying "no" discards!) [y/n]? ')
) or return undef;
}
}
@@ -1206,11 +1210,11 @@ sub apply_patch_for_checkout_commit {
run_git_apply 'apply '.$reverse, @_;
return 1;
} elsif (!$applies_index) {
- print colored $error_color, "The selected hunks do not apply to
the index!\n";
- if (prompt_yesno "Apply them to the worktree anyway? ") {
+ print colored $error_color, __("The selected hunks do not apply
to the index!\n");
+ if (prompt_yesno __("Apply them to the worktree anyway? ")) {
return run_git_apply 'apply '.$reverse, @_;
} else {
- print colored $error_color, "Nothing was applied.\n";
+ print colored $error_color, __("Nothing was
applied.\n");
return 0;
}
} else {
@@ -1230,9 +1234,9 @@ sub patch_update_cmd {
if (!@mods) {
if (@all_mods) {
- print STDERR "Only binary files changed.\n";
+ print STDERR __("Only binary files changed.\n");
} else {
- print STDERR "No changes.\n";
+ print STDERR __("No changes.\n");
}
return 0;
}
@@ -1390,12 +1394,12 @@ sub patch_update_file {
my $response = $1;
my $no = $ix > 10 ? $ix - 10 : 0;
while ($response eq '') {
- my $extra = "";
$no = display_hunks(\@hunk, $no);
if ($no < $num) {
- $extra = " ( to see more)";
+ print __("go to which hunk
( to see more)? ");
+ } else {
+ print __("go to which hunk? ");
}
- print "go to which hunk$extra? ";
[PATCH v2 04/11] i18n: add--interactive: mark plural strings
Mark plural strings for translation. Unfold each action case in one
entire sentence.
Pass new keyword for xgettext to extract.
Update test to include new subrotine Q__() for plural strings handling.
Signed-off-by: Vasco Almeida
---
Makefile | 3 ++-
git-add--interactive.perl | 24
perl/Git/I18N.pm | 4 +++-
t/t0202/test.pl | 11 ++-
4 files changed, 31 insertions(+), 11 deletions(-)
diff --git a/Makefile b/Makefile
index de5a030..eedf1fa 100644
--- a/Makefile
+++ b/Makefile
@@ -2061,7 +2061,8 @@ XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \
--keyword=_ --keyword=N_ --keyword="Q_:1,2"
XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \
--keyword=gettextln --keyword=eval_gettextln
-XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --keyword=__ --language=Perl
+XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --language=Perl \
+ --keyword=__ --keyword="Q__:1,2"
LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H)
LOCALIZED_SH = $(SCRIPT_SH) git-parse-remote.sh
LOCALIZED_PERL = $(SCRIPT_PERL)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 4e1e857..08badfa 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -666,12 +666,18 @@ sub status_cmd {
sub say_n_paths {
my $did = shift @_;
my $cnt = scalar @_;
- print "$did ";
- if (1 < $cnt) {
- print "$cnt paths\n";
- }
- else {
- print "one path\n";
+ if ($did eq 'added') {
+ printf(Q__("added one path\n", "added %d paths\n",
+ $cnt), $cnt);
+ } elsif ($did eq 'updated') {
+ printf(Q__("updated one path\n", "updated %d paths\n",
+ $cnt), $cnt);
+ } elsif ($did eq 'reversed') {
+ printf(Q__("reversed one path\n", "reversed %d paths\n",
+ $cnt), $cnt);
+ } else {
+ printf(Q__("touched one path\n", "touched %d paths\n",
+ $cnt), $cnt);
}
}
@@ -1508,8 +1514,10 @@ sub patch_update_file {
elsif ($other =~ /s/ && $line =~ /^s/) {
my @split = split_hunk($hunk[$ix]{TEXT},
$hunk[$ix]{DISPLAY});
if (1 < @split) {
- print colored $header_color, "Split
into ",
- scalar(@split), " hunks.\n";
+ print colored $header_color, sprintf(
+ Q__("Split into %d hunk.\n",
+ "Split into %d hunks.\n",
+ scalar(@split)),
scalar(@split));
}
splice (@hunk, $ix, 1, @split);
$num = scalar @hunk;
diff --git a/perl/Git/I18N.pm b/perl/Git/I18N.pm
index f889fd6..5a75dd5 100644
--- a/perl/Git/I18N.pm
+++ b/perl/Git/I18N.pm
@@ -13,7 +13,7 @@ BEGIN {
}
}
-our @EXPORT = qw(__);
+our @EXPORT = qw(__ Q__);
our @EXPORT_OK = @EXPORT;
sub __bootstrap_locale_messages {
@@ -44,6 +44,7 @@ BEGIN
eval {
__bootstrap_locale_messages();
*__ = \&Locale::Messages::gettext;
+ *Q__ = \&Locale::Messages::ngettext;
1;
} or do {
# Tell test.pl that we couldn't load the gettext library.
@@ -51,6 +52,7 @@ BEGIN
# Just a fall-through no-op
*__ = sub ($) { $_[0] };
+ *Q__ = sub ($$$) { $_[2] == 1 ? $_[0] : $_[1] };
};
}
diff --git a/t/t0202/test.pl b/t/t0202/test.pl
index 2c10cb4..98aa9a3 100755
--- a/t/t0202/test.pl
+++ b/t/t0202/test.pl
@@ -4,7 +4,7 @@ use lib (split(/:/, $ENV{GITPERLLIB}));
use strict;
use warnings;
use POSIX qw(:locale_h);
-use Test::More tests => 8;
+use Test::More tests => 11;
use Git::I18N;
my $has_gettext_library = $Git::I18N::__HAS_LIBRARY;
@@ -31,6 +31,7 @@ is_deeply(\@Git::I18N::EXPORT, \@Git::I18N::EXPORT_OK,
"sanity: Git::I18N export
# more gettext wrapper functions.
my %prototypes = (qw(
__ $
+ Q__ $$$
));
while (my ($sub, $proto) = each %prototypes) {
is(prototype(\&{"Git::I18N::$sub"}), $proto, "sanity: $sub has
a $proto prototype");
@@ -46,6 +47,14 @@ is_deeply(\@Git::I18N::EXPORT, \@Git::I18N::EXPORT_OK,
"sanity: Git::I18N export
my ($got, $expect) = (('TEST: A Perl test string') x 2);
is(__($got), $expect, "Passing a string through __() in the C locale
works");
+
+ my ($got_singular, $got_plural, $expect_singular, $expect_plural) =
+ (('TEST: 1 file', 'TEST: n files') x 2);
+
+ is(Q__($got_singular, $got_plural, 1), $expect_singular,
+ "Get sing
[PATCH v2 05/11] i18n: add--interactive: mark message for translation
Mark message assembled in place for translation, unfolding each use case
for each entry in the %patch_modes hash table.
Previously, this script relied on whether $patch_mode was set to run the
command patch_update_cmd() or show status and loop the main loop. Now,
it uses $cmd to indicate we must run patch_update_cmd() and $patch_mode
is used to tell which flavor of the %patch_modes are we on. This is
introduced in order to be able to mark and unfold the message prompt
knowing in which context we are.
The tracking of context was done previously by point %patch_mode_flavour
hash table to the correct entry of %patch_modes, focusing only on value
of %patch_modes. Now, we are also interested in the key ('staged',
'stash', 'checkout_head', ...).
Signed-off-by: Vasco Almeida
---
git-add--interactive.perl | 91 ++-
1 file changed, 83 insertions(+), 8 deletions(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 08badfa..5b89b97 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -91,6 +91,7 @@ sub colored {
}
# command line options
+my $cmd;
my $patch_mode;
my $patch_mode_revision;
@@ -171,7 +172,8 @@ my %patch_modes = (
},
);
-my %patch_mode_flavour = %{$patch_modes{stage}};
+$patch_mode = 'stage';
+my %patch_mode_flavour = %{$patch_modes{$patch_mode}};
sub run_cmd_pipe {
if ($^O eq 'MSWin32') {
@@ -1372,12 +1374,84 @@ sub patch_update_file {
for (@{$hunk[$ix]{DISPLAY}}) {
print;
}
- print colored $prompt_color, $patch_mode_flavour{VERB},
- ($hunk[$ix]{TYPE} eq 'mode' ? ' mode change' :
- $hunk[$ix]{TYPE} eq 'deletion' ? ' deletion' :
- ' this hunk'),
- $patch_mode_flavour{TARGET},
- " [y,n,q,a,d,/$other,?]? ";
+ if ($patch_mode eq 'stage') {
+ if ($hunk[$ix]{TYPE} eq 'mode') {
+ print colored $prompt_color,
+ sprintf(__("Stage mode change [y,n,q,a,d,/%s,?]?
"), $other);
+ } elsif ($hunk[$ix]{TYPE} eq 'deletion') {
+ print colored $prompt_color,
+ sprintf(__("Stage deletion [y,n,q,a,d,/%s,?]? "),
$other);
+ } else {
+ print colored $prompt_color,
+ sprintf(__("Stage this hunk [y,n,q,a,d,/%s,?]? "),
$other);
+ }
+ } elsif ($patch_mode eq 'stash') {
+ if ($hunk[$ix]{TYPE} eq 'mode') {
+ print colored $prompt_color,
+ sprintf(__("Stash mode change [y,n,q,a,d,/%s,?]?
"), $other);
+ } elsif ($hunk[$ix]{TYPE} eq 'deletion') {
+ print colored $prompt_color,
+ sprintf(__("Stash deletion [y,n,q,a,d,/%s,?]? "),
$other);
+ } else {
+ print colored $prompt_color,
+ sprintf(__("Stash this hunk [y,n,q,a,d,/%s,?]? "),
$other);
+ }
+ } elsif ($patch_mode eq 'reset_head') {
+ if ($hunk[$ix]{TYPE} eq 'mode') {
+ print colored $prompt_color,
+ sprintf(__("Unstage mode change [y,n,q,a,d,/%s,?]?
"), $other);
+ } elsif ($hunk[$ix]{TYPE} eq 'deletion') {
+ print colored $prompt_color,
+ sprintf(__("Unstage deletion [y,n,q,a,d,/%s,?]? "),
$other);
+ } else {
+ print colored $prompt_color,
+ sprintf(__("Unstage this hunk [y,n,q,a,d,/%s,?]?
"), $other);
+ }
+ } elsif ($patch_mode eq 'reset_nothead') {
+ if ($hunk[$ix]{TYPE} eq 'mode') {
+ print colored $prompt_color,
+ sprintf(__("Apply mode change to index
[y,n,q,a,d,/%s,?]? "), $other);
+ } elsif ($hunk[$ix]{TYPE} eq 'deletion') {
+ print colored $prompt_color,
+ sprintf(__("Apply deletion to index
[y,n,q,a,d,/%s,?]? "), $other);
+ } else {
+ print colored $prompt_color,
+ sprintf(__("Apply this hunk to index
[y,n,q,a,d,/%s,?]? "), $other);
+ }
+ } elsif ($patch_mode eq 'checkout_index') {
+ if ($hunk[$ix]{TYPE} eq 'mode') {
+ print colored $prompt_color,
+ sprintf(__("Discard mode change from worktree
[y,n,q,a,d,/%s,?]? "), $other);
+ } elsif ($hunk[$ix]{TYPE} eq 'deletion') {
+ pr
[PATCH v2 02/11] i18n: add--interactive: mark simple here documents for translation
Mark messages in here document without interpolation for translation.
Marking for translation by removing here documents this way, rather than
take advantage of "print __ << EOF" way, makes other instances of help
messages in clean.c match the first two in this file. Otherwise,
reusing here document would add a trailer newline to the message, making
them not match 100%, hence creating two entries in pot template for
translation rather than a single entry.
Signed-off-by: Vasco Almeida
---
git-add--interactive.perl | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index fb8e5de..e11a33d 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -636,25 +636,25 @@ sub list_and_choose {
}
sub singleton_prompt_help_cmd {
- print colored $help_color, <<\EOF ;
-Prompt help:
+ print colored $help_color, __(
+"Prompt help:
1 - select a numbered item
foo- select item based on unique prefix
- - (empty) select nothing
-EOF
+ - (empty) select nothing"),
+"\n";
}
sub prompt_help_cmd {
- print colored $help_color, <<\EOF ;
-Prompt help:
+ print colored $help_color, __(
+"Prompt help:
1 - select a single item
3-5- select a range of items
2-3,6-9- select multiple ranges
foo- select item based on unique prefix
-... - unselect specified items
* - choose all items
- - (empty) finish selecting
-EOF
+ - (empty) finish selecting"),
+"\n";
}
sub status_cmd {
@@ -1573,14 +1573,14 @@ sub quit_cmd {
}
sub help_cmd {
- print colored $help_color, <<\EOF ;
-status- show paths with changes
+ print colored $help_color, __(
+"status- show paths with changes
update- add working tree state to the staged set of changes
revert- revert staged set of changes back to the HEAD version
patch - pick hunks and update selectively
diff - view diff between HEAD and index
-add untracked - add contents of untracked files to the staged set of changes
-EOF
+add untracked - add contents of untracked files to the staged set of changes"),
+"\n";
}
sub process_args {
--
2.7.4
--
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 v2 06/11] i18n: add--interactive: i18n of help_patch_cmd
Mark help message of help_patch_cmd for translation. The message must
be unfolded to be free of variables so we can have high quality
translations.
Signed-off-by: Vasco Almeida
---
git-add--interactive.perl | 65 +++
1 file changed, 54 insertions(+), 11 deletions(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 5b89b97..acbfa4e 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1179,15 +1179,58 @@ sub edit_hunk_loop {
}
sub help_patch_cmd {
- my $verb = lc $patch_mode_flavour{VERB};
- my $target = $patch_mode_flavour{TARGET};
- print colored $help_color,
[PATCH v2 08/11] i18n: send-email: mark strings for translation
Mark strings often displayed to user for translation.
Signed-off-by: Vasco Almeida
---
git-send-email.perl | 53 +++--
1 file changed, 31 insertions(+), 22 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 6958785..2521832 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -28,6 +28,7 @@ use File::Temp qw/ tempdir tempfile /;
use File::Spec::Functions qw(catfile);
use Error qw(:try);
use Git;
+use Git::I18N;
Getopt::Long::Configure qw/ pass_through /;
@@ -795,12 +796,12 @@ foreach my $f (@files) {
}
if (!defined $auto_8bit_encoding && scalar %broken_encoding) {
- print "The following files are 8bit, but do not declare " .
- "a Content-Transfer-Encoding.\n";
+ print __("The following files are 8bit, but do not declare " .
+"a Content-Transfer-Encoding.\n");
foreach my $f (sort keys %broken_encoding) {
print "$f\n";
}
- $auto_8bit_encoding = ask("Which 8bit encoding should I declare
[UTF-8]? ",
+ $auto_8bit_encoding = ask(__("Which 8bit encoding should I declare
[UTF-8]? "),
valid_re => qr/.{4}/, confirm_only => 1,
default => "UTF-8");
}
@@ -827,7 +828,7 @@ if (defined $sender) {
# But it's a no-op to run sanitize_address on an already sanitized address.
$sender = sanitize_address($sender);
-my $to_whom = "To whom should the emails be sent (if anyone)?";
+my $to_whom = __("To whom should the emails be sent (if anyone)?");
my $prompting = 0;
if (!@initial_to && !defined $to_cmd) {
my $to = ask("$to_whom ",
@@ -857,7 +858,7 @@ sub expand_one_alias {
if ($thread && !defined $initial_reply_to && $prompting) {
$initial_reply_to = ask(
- "Message-ID to be used as In-Reply-To for the first email (if
any)? ",
+ __("Message-ID to be used as In-Reply-To for the first email
(if any)? "),
default => "",
valid_re => qr/\@.*\./, confirm_only => 1);
}
@@ -916,7 +917,10 @@ sub validate_address {
my $address = shift;
while (!extract_valid_address($address)) {
print STDERR "error: unable to extract a valid address from:
$address\n";
- $_ = ask("What to do with this address? ([q]uit|[d]rop|[e]dit):
",
+ # TRANSLATORS: Make sure to include [q] [d] [e] in your
+ # translation. The program will only accept English input
+ # at this point.
+ $_ = ask(__("What to do with this address?
([q]uit|[d]rop|[e]dit): "),
valid_re => qr/^(?:quit|q|drop|d|edit|e)/i,
default => 'q');
if (/^d/i) {
@@ -1291,17 +1295,22 @@ Message-Id: $message_id
if ($needs_confirm eq "inform") {
$confirm_unconfigured = 0; # squelch this message for
the rest of this run
$ask_default = "y"; # assume yes on EOF since user
hasn't explicitly asked for confirmation
- print "The Cc list above has been expanded by
additional\n";
- print "addresses found in the patch commit message.
By default\n";
- print "send-email prompts before sending whenever
this occurs.\n";
- print "This behavior is controlled by the
sendemail.confirm\n";
- print "configuration setting.\n";
- print "\n";
- print "For additional information, run 'git
send-email --help'.\n";
- print "To retain the current behavior, but squelch
this message,\n";
- print "run 'git config --global sendemail.confirm
auto'.\n\n";
+ print __(
+"The Cc list above has been expanded by additional
+addresses found in the patch commit message. By default
+send-email prompts before sending whenever this occurs.
+This behavior is controlled by the sendemail.confirm
+configuration setting.
+
+For additional information, run 'git send-email --help'.
+To retain the current behavior, but squelch this message,
+run 'git config --global sendemail.confirm auto'."),
+"\n\n";
}
- $_ = ask("Send this email? ([y]es|[n]o|[q]uit|[a]ll): ",
+ # TRANSLATORS: Make sure to include [y] [n] [q] [a] in your
+ # translation. The program will only accept English input
+ # at this point.
+ $_ = ask(__("Send this email? ([y]es|[n]o|[q]uit|[a]ll): "),
valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i,
default => $ask_default);
die "Send this email reply required" unless defined $_;
@@ -1403,7 +1412,7 @@ Message-Id: $message_id
if ($quiet) {
[PATCH v2 03/11] i18n: add--interactive: mark strings with interpolation for translation
Use of sprintf following die or error_msg is necessary for placeholder
substitution take place.
Signed-off-by: Vasco Almeida
---
git-add--interactive.perl | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index e11a33d..4e1e857 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -612,12 +612,12 @@ sub list_and_choose {
else {
$bottom = $top = find_unique($choice, @stuff);
if (!defined $bottom) {
- error_msg "Huh ($choice)?\n";
+ error_msg sprintf(__("Huh (%s)?\n"),
$choice);
next TOPLOOP;
}
}
if ($opts->{SINGLETON} && $bottom != $top) {
- error_msg "Huh ($choice)?\n";
+ error_msg sprintf(__("Huh (%s)?\n"), $choice);
next TOPLOOP;
}
for ($i = $bottom-1; $i <= $top-1; $i++) {
@@ -1048,7 +1048,7 @@ sub edit_hunk_manually {
my $hunkfile = $repo->repo_path . "/addp-hunk-edit.diff";
my $fh;
open $fh, '>', $hunkfile
- or die "failed to open hunk edit file for writing: " . $!;
+ or die sprintf(__("failed to open hunk edit file for writing:
%s"), $!);
print $fh "# Manual hunk edit mode -- see bottom for a quick guide\n";
print $fh @$oldtext;
my $participle = $patch_mode_flavour{PARTICIPLE};
@@ -1075,7 +1075,7 @@ EOF
}
open $fh, '<', $hunkfile
- or die "failed to open hunk edit file for reading: " . $!;
+ or die sprintf(__("failed to open hunk edit file for reading:
%s"), $!);
my @newtext = grep { !/^#/ } <$fh>;
close $fh;
unlink $hunkfile;
@@ -1225,7 +1225,7 @@ sub apply_patch_for_checkout_commit {
sub patch_update_cmd {
my @all_mods = list_modified($patch_mode_flavour{FILTER});
- error_msg "ignoring unmerged: $_->{VALUE}\n"
+ error_msg sprintf(__("ignoring unmerged: %s\n"), $_->{VALUE})
for grep { $_->{UNMERGED} } @all_mods;
@all_mods = grep { !$_->{UNMERGED} } @all_mods;
@@ -1407,11 +1407,13 @@ sub patch_update_file {
chomp $response;
}
if ($response !~ /^\s*\d+\s*$/) {
- error_msg "Invalid number:
'$response'\n";
+ error_msg sprintf(__("Invalid number:
'%s'\n"),
+$response);
} elsif (0 < $response && $response <= $num) {
$ix = $response - 1;
} else {
- error_msg "Sorry, only $num hunks
available.\n";
+ error_msg sprintf(__("Sorry, only %s
hunks available.\n"),
+$num);
}
next;
}
@@ -1449,7 +1451,7 @@ sub patch_update_file {
if ($@) {
my ($err,$exp) = ($@, $1);
$err =~ s/ at .*git-add--interactive
line \d+, line \d+.*$//;
- error_msg "Malformed search regexp
$exp: $err\n";
+ error_msg sprintf(__("Malformed search
regexp %s: %s\n"), $exp, $err);
next;
}
my $iy = $ix;
@@ -1612,18 +1614,18 @@ sub process_args {
$patch_mode = $1;
$arg = shift @ARGV or die __("missing --");
} else {
- die "unknown --patch mode: $1";
+ die sprintf(__("unknown --patch mode: %s"), $1);
}
} else {
$patch_mode = 'stage';
$arg = shift @ARGV or die __("missing --");
}
- die "invalid argument $arg, expecting --"
- unless $arg eq "--";
+ die sprintf(__("invalid argument %s, expecting --"),
+ $arg) unless $arg eq "--";
%patch_mode_flavour = %{$patch_modes{$patch_mode}};
}
elsif ($arg ne "--") {
- die "invalid argument $arg, expecting --";
+ die sprintf(__("invalid argument %s, expe
[PATCH v2 09/11] i18n: send-email: mark warnings and errors for translation
Mark warnings, errors and other messages for translation.
Signed-off-by: Vasco Almeida
---
git-send-email.perl | 36 ++--
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 2521832..e7f712e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -118,20 +118,20 @@ sub format_2822_time {
my $localmin = $localtm[1] + $localtm[2] * 60;
my $gmtmin = $gmttm[1] + $gmttm[2] * 60;
if ($localtm[0] != $gmttm[0]) {
- die "local zone differs from GMT by a non-minute interval\n";
+ die __("local zone differs from GMT by a non-minute
interval\n");
}
if ((($gmttm[6] + 1) % 7) == $localtm[6]) {
$localmin += 1440;
} elsif ((($gmttm[6] - 1) % 7) == $localtm[6]) {
$localmin -= 1440;
} elsif ($gmttm[6] != $localtm[6]) {
- die "local time offset greater than or equal to 24 hours\n";
+ die __("local time offset greater than or equal to 24 hours\n");
}
my $offset = $localmin - $gmtmin;
my $offhour = $offset / 60;
my $offmin = abs($offset % 60);
if (abs($offhour) >= 24) {
- die ("local time offset greater than or equal to 24 hours\n");
+ die __("local time offset greater than or equal to 24 hours\n");
}
return sprintf("%s, %2d %s %d %02d:%02d:%02d %s%02d%02d",
@@ -199,13 +199,13 @@ sub do_edit {
map {
system('sh', '-c', $editor.' "$@"', $editor, $_);
if (($? & 127) || ($? >> 8)) {
- die("the editor exited uncleanly, aborting
everything");
+ die(__("the editor exited uncleanly, aborting
everything"));
}
} @_;
} else {
system('sh', '-c', $editor.' "$@"', $editor, @_);
if (($? & 127) || ($? >> 8)) {
- die("the editor exited uncleanly, aborting everything");
+ die(__("the editor exited uncleanly, aborting
everything"));
}
}
}
@@ -299,7 +299,7 @@ my $help;
my $rc = GetOptions("h" => \$help,
"dump-aliases" => \$dump_aliases);
usage() unless $rc;
-die "--dump-aliases incompatible with other options\n"
+die __("--dump-aliases incompatible with other options\n")
if !$help and $dump_aliases and @ARGV;
$rc = GetOptions(
"sender|from=s" => \$sender,
@@ -362,7 +362,7 @@ unless ($rc) {
usage();
}
-die "Cannot run git format-patch from outside a repository\n"
+die __("Cannot run git format-patch from outside a repository\n")
if $format_patch and not $repo;
# Now, let's fill any that aren't set in with defaults:
@@ -617,7 +617,7 @@ while (defined(my $f = shift @ARGV)) {
}
if (@rev_list_opts) {
- die "Cannot run git format-patch from outside a repository\n"
+ die __("Cannot run git format-patch from outside a repository\n")
unless $repo;
push @files, $repo->command('format-patch', '-o', tempdir(CLEANUP =>
1), @rev_list_opts);
}
@@ -636,7 +636,7 @@ if (@files) {
print $_,"\n" for (@files);
}
} else {
- print STDERR "\nNo patch files specified!\n\n";
+ print STDERR __("\nNo patch files specified!\n\n");
usage();
}
@@ -728,7 +728,7 @@ EOT
$sender = $1;
next;
} elsif (/^(?:To|Cc|Bcc):/i) {
- print "To/Cc/Bcc fields are not interpreted yet, they
have been ignored\n";
+ print __("To/Cc/Bcc fields are not interpreted yet,
they have been ignored\n");
next;
}
print $c2 $_;
@@ -737,7 +737,7 @@ EOT
close $c2;
if ($summary_empty) {
- print "Summary email is empty, skipping it\n";
+ print __("Summary email is empty, skipping it\n");
$compose = -1;
}
} elsif ($annotate) {
@@ -1313,7 +1313,7 @@ Message-Id: $message_id
$_ = ask(__("Send this email? ([y]es|[n]o|[q]uit|[a]ll): "),
valid_re => qr/^(?:yes|y|no|n|quit|q|all|a)/i,
default => $ask_default);
- die "Send this email reply required" unless defined $_;
+ die __("Send this email reply required") unless defined $_;
if (/^n/i) {
return 0;
} elsif (/^q/i) {
@@ -1339,7 +1339,7 @@ Message-Id: $message_id
} else {
if (!defined $smtp_server) {
- die "The required SMTP server is not properly defined."
+ die __("The required SMTP server is not properly
defined.")
}
if ($smtp_encrypt
