* index-pack outside of repository?
From: Jeff King @ 2016-12-15 20:40 UTC (permalink / raw)
To: git
Running git on 'next', you can trigger a BUG:
$ cd /some/repo
$ git pack-objects --all --stdout </dev/null >/tmp/foo.pack
$ cd /not-a-git-repo
$ git index-pack --stdin </tmp/foo.pack
fatal: BUG: setup_git_env called without repository
This obviously comes from my b1ef400eec (setup_git_env: avoid blind
fall-back to ".git", 2016-10-20). What's going on is that index-pack
uses RUN_SETUP_GENTLY, but never actually handles the out-of-repo case.
When we use the internal git_dir to make "objects/pack/pack-xxx.pack",
it barfs.
In older versions of git will just blindly write into
".git/objects/pack", even though there's no repository there.
So I think complaining to the user is the right thing to do here. I
started to write a patch to have index-pack notice when it needs a repo
and doesn't have one, but the logic is actually a bit unclear. Do we
need to complain early _just_ when --stdin is specified, or does that
miss somes cases? Likewise, are there cases where --stdin can operate
without a repo? I couldn't think of any.
I'm actually wondering if the way it calls die() in 'next' is a pretty
reasonable way for things to work in general. It happens when we lazily
try to ask for the repository directory. So we don't have to replicate
logic to say "are we going to need a repo"; at the moment we need it, we
notice we don't have it and die. The only problem is that it says "BUG"
and not "this operation must be run in a git repository".
That strategy _might_ be a problem for some programs, which would want
to notice the issue early before doing work. But it seems like a
reasonable outcome for index-pack. Thoughts?
-Peff
^ permalink raw reply
* Is there a way to have local changes in a branch 'bake' while working in different branches?
From: Larry Minton @ 2016-12-15 20:14 UTC (permalink / raw)
To: git@vger.kernel.org
Lars Schneider recommended I ask you this question.
My question:
Let's say I have a code change that I want to 'bake' for a while locally, just to make sure some edge case doesn't pop up while I am working on other things. Is there any practical way of doing that?
I could constantly merge that 'bake me' branch into other branches as I work on them and then remove those changes from the branches before sending them out for code review, but sooner or later pretty much guaranteed to screw that up....
His response:
Good question. Your merging idea would work but I agree it might be cumbersome. In this situation I keep modified files in my tree. That would work for you too, but this would be inconvenient if you have many changed files. I wonder how the Git core guys manage this kind of situation.
Thanks,
Larry Minton
3ds Max Core team
LiveDesign Group
Media & Entertainment, Education Experiences, Impact (MEI)
^ permalink raw reply
* Re: [PATCH v2 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
From: Stefan Beller @ 2016-12-15 19:27 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Schindelin, git@vger.kernel.org, Junio C Hamano,
Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <20161215192016.qhbcyo7vb7petuwp@sigill.intra.peff.net>
On Thu, Dec 15, 2016 at 11:20 AM, Jeff King <peff@peff.net> wrote:
> On Thu, Dec 15, 2016 at 11:07:34AM -0800, Stefan Beller wrote:
>
>> On Thu, Dec 15, 2016 at 11:03 AM, Jeff King <peff@peff.net> wrote:
>> > wonder if it would be helpful to send that output to the list.
>>
>> Sure we can try.
>>
>> Another project I used to run through coverity (Gerrit), shows
>> similar characteristics w.r.t. false positives, so people complained
>> when I was force feeding them the niceties of static analysis.
>>
>> I'll just try to set it up and see how the mailing list reacts.
>> (Not sure if you can just add emails there or if the email has
>> to be verified or such.)
>
> I see you added it, but I don't see the confirmation email on the list.
> I wonder if it was HTML mail and vger ate it.
>
> -Peff
I think I'll setup a dummy gmail account to use for subscription
and then I'll configure that to forward all email from coverity to the list.
(the actual complaints about memleaks etc are plain text, so that
forwarding then should work)
^ permalink raw reply
* Re: [PATCH v2 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
From: Jeff King @ 2016-12-15 19:20 UTC (permalink / raw)
To: Stefan Beller
Cc: Johannes Schindelin, git@vger.kernel.org, Junio C Hamano,
Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <CAGZ79kZ3i-eoxMsVMsb+VBtEVQf2-Fovh_YM5NBN2pSOBHajBg@mail.gmail.com>
On Thu, Dec 15, 2016 at 11:07:34AM -0800, Stefan Beller wrote:
> On Thu, Dec 15, 2016 at 11:03 AM, Jeff King <peff@peff.net> wrote:
> > wonder if it would be helpful to send that output to the list.
>
> Sure we can try.
>
> Another project I used to run through coverity (Gerrit), shows
> similar characteristics w.r.t. false positives, so people complained
> when I was force feeding them the niceties of static analysis.
>
> I'll just try to set it up and see how the mailing list reacts.
> (Not sure if you can just add emails there or if the email has
> to be verified or such.)
I see you added it, but I don't see the confirmation email on the list.
I wonder if it was HTML mail and vger ate it.
-Peff
^ permalink raw reply
* Re: [PATCH v2 10/34] sequencer (rebase -i): allow continuing with staged changes
From: Junio C Hamano @ 2016-12-15 19:08 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <9978c32139c522c08d5a8f685011829cc830bfc0.1481642927.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> When an interactive rebase is interrupted, the user may stage changes
> before continuing, and we need to commit those changes in that case.
>
> Please note that the nested "if" added to the sequencer_continue() is
> not combined into a single "if" because it will be extended with an
> "else" clause in a later patch in this patch series.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> sequencer.c | 40 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/sequencer.c b/sequencer.c
> index 80469b6954..855d3ba503 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1829,6 +1829,42 @@ static int continue_single_pick(void)
> return run_command_v_opt(argv, RUN_GIT_CMD);
> }
>
> +static int commit_staged_changes(struct replay_opts *opts)
> +{
> + int amend = 0;
> +
> + if (has_unstaged_changes(1))
> + return error(_("cannot rebase: You have unstaged changes."));
The scripted one from 'master' seems to say
$path_to_the_file: needs update
You must edit all merge conflicts and then
mark them as resolved using git add
when editing an existing commit in this case. The updated message
looks more sensible for the situation, but I wonder if the control
should even reach at this point.
One bad thing about reviewing this series is that all the comments
are about codepaths that are not exercised, so they cannot be more
than "they look good". A comment "If the caller does X, this will
be better than the original" (or this will regress, for that matter)
cannot be validated for its relevance because we won't know the what
the caller does in the endgame while reviewing these earlier steps.
^ permalink raw reply
* Re: [PATCH v2 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
From: Stefan Beller @ 2016-12-15 19:07 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Schindelin, git@vger.kernel.org, Junio C Hamano,
Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <20161215190351.as76panrcz5rgibj@sigill.intra.peff.net>
On Thu, Dec 15, 2016 at 11:03 AM, Jeff King <peff@peff.net> wrote:
> wonder if it would be helpful to send that output to the list.
Sure we can try.
Another project I used to run through coverity (Gerrit), shows
similar characteristics w.r.t. false positives, so people complained
when I was force feeding them the niceties of static analysis.
I'll just try to set it up and see how the mailing list reacts.
(Not sure if you can just add emails there or if the email has
to be verified or such.)
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH v2 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
From: Jeff King @ 2016-12-15 19:03 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Stefan Beller, git, Junio C Hamano, Kevin Daudt,
Dennis Kaarsemaker
In-Reply-To: <ae521f75a105c6b9e54595d68bda3c5b62f313b6.1481642927.git.johannes.schindelin@gmx.de>
On Tue, Dec 13, 2016 at 04:30:01PM +0100, Johannes Schindelin wrote:
> + else {
> + unsigned char head[20];
> + struct commit *head_commit;
> + const char *head_message, *body;
> +
> + if (get_sha1("HEAD", head))
> + return error(_("need a HEAD to fixup"));
> + if (!(head_commit = lookup_commit_reference(head)))
> + return error(_("could not read HEAD"));
> + if (!(head_message = get_commit_buffer(head_commit, NULL)))
> + return error(_("could not read HEAD's commit message"));
This get_commit_buffer() may allocate a fresh buffer...
> + body = strstr(head_message, "\n\n");
> + if (!body)
> + body = "";
> + else
> + body = skip_blank_lines(body + 2);
> + if (write_message(body, strlen(body),
> + rebase_path_fixup_msg(), 0))
> + return error(_("cannot write '%s'"),
> + rebase_path_fixup_msg());
...and then this return leaks the result (the other code path hits
unuse_commit_buffer(), and is fine).
This leak was noticed by Coverity. It has a _ton_ of false positives
across the whole project, but it sends out a mail with new ones every
few days, which is usually short enough that I can process it in 30
seconds or so.
I _think_ that email just goes to me and Stefan right now. You can add
yourself at:
https://scan.coverity.com/projects/git?tab=project_settings
if you already have admin access to the project (which I think you
(Dscho) do). I wonder if it would be helpful to send that output to the
list.
-Peff
^ permalink raw reply
* Re: [PATCH v2 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
From: Jeff King @ 2016-12-15 18:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqqlgvhuj82.fsf@gitster.mtv.corp.google.com>
On Thu, Dec 15, 2016 at 10:42:53AM -0800, Junio C Hamano wrote:
> > + sprintf((char *)p, "%d", ++count);
>
> Do we know the area pointed at p (which is inside buf) long enough
> not to overflow? If the original were 9 and you incremented to get
> 10, you would need one extra byte.
Even if it is enough, I'd ask to please use xsnprintf(). In the off
chance that there's a programming error, we'd get a nice die("BUG")
instead of a buffer overflow (and it makes the code base easier to audit
for other overflows).
-Peff
^ permalink raw reply
* Re: [PATCH v2 09/34] sequencer (rebase -i): write an author-script file
From: Junio C Hamano @ 2016-12-15 18:50 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <09c2718e119f809093794410ae1a738c1cd122d1.1481642927.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> + strbuf_addstr(&buf, "GIT_AUTHOR_NAME='");
> + while (*message && *message != '\n' && *message != '\r')
> + if (skip_prefix(message, " <", &message))
> + break;
> + else if (*message != '\'')
> + strbuf_addch(&buf, *(message++));
> + else
> + strbuf_addf(&buf, "'\\\\%c'", *(message++));
> + strbuf_addstr(&buf, "'\nGIT_AUTHOR_EMAIL='");
> + while (*message && *message != '\n' && *message != '\r')
> + if (skip_prefix(message, "> ", &message))
> + break;
> + else if (*message != '\'')
> + strbuf_addch(&buf, *(message++));
> + else
> + strbuf_addf(&buf, "'\\\\%c'", *(message++));
Aren't these reading from an in-core commit object?
If so, it should use split_ident_line() for consistency with other
parts of the system to do this parsing. We should also already have
a helper for simple shell-quoting in quote.c and you would want to
use that instead of open coding like this.
^ permalink raw reply
* Re: [PATCH v2 08/34] sequencer (rebase -i): implement the short commands
From: Junio C Hamano @ 2016-12-15 18:43 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <42d43763075f3f2daa2cc06a8afc9bcfd8b20ac2.1481642927.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> For users' convenience, most rebase commands can be abbreviated, e.g.
> 'p' instead of 'pick' and 'x' instead of 'exec'. Let's teach the
> sequencer to handle those abbreviated commands just fine.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
Sensible.
^ permalink raw reply
* Re: [PATCH v2 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
From: Junio C Hamano @ 2016-12-15 18:42 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <ae521f75a105c6b9e54595d68bda3c5b62f313b6.1481642927.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> diff --git a/sequencer.c b/sequencer.c
> index f6e20b142a..271c21581d 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -45,6 +45,35 @@ static GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
> */
> static GIT_PATH_FUNC(rebase_path_done, "rebase-merge/done")
> /*
> + * The commit message that is planned to be used for any changes that
> + * need to be committed following a user interaction.
> + */
> +static GIT_PATH_FUNC(rebase_path_message, "rebase-merge/message")
> +/*
> + * The file into which is accumulated the suggested commit message for
> + * squash/fixup commands. When the first of a series of squash/fixups
The same comment as 03/34 applies here, regarding blank line to
separate logical unit.
> +static int update_squash_messages(enum todo_command command,
> + struct commit *commit, struct replay_opts *opts)
> +{
> + struct strbuf buf = STRBUF_INIT;
> + int count, res;
> + const char *message, *body;
> +
> + if (file_exists(rebase_path_squash_msg())) {
> + char *p, *p2;
> +
> + if (strbuf_read_file(&buf, rebase_path_squash_msg(), 2048) <= 0)
> + return error(_("could not read '%s'"),
> + rebase_path_squash_msg());
> +
> + if (buf.buf[0] != comment_line_char ||
> + !skip_prefix(buf.buf + 1, " This is a combination of ",
> + (const char **)&p))
> + return error(_("unexpected 1st line of squash message:"
> + "\n\n\t%.*s"),
> + (int)(strchrnul(buf.buf, '\n') - buf.buf),
> + buf.buf);
> + count = strtol(p, &p2, 10);
> +
> + if (count < 1 || *p2 != ' ')
> + return error(_("invalid 1st line of squash message:\n"
> + "\n\t%.*s"),
> + (int)(strchrnul(buf.buf, '\n') - buf.buf),
> + buf.buf);
> +
> + sprintf((char *)p, "%d", ++count);
Do we know the area pointed at p (which is inside buf) long enough
not to overflow? If the original were 9 and you incremented to get
10, you would need one extra byte.
> + if (!*p2)
> + *p2 = ' ';
> + else {
> + *(++p2) = 'c';
p2 points into buf; do we know this increment does not step beyond
its end? What is the meaning of a letter 'c' here (I do not see a
corresponding one in the scripted update_squash_messages)?
> + strbuf_insert(&buf, p2 - buf.buf, " ", 1);
> + }
> + }
> + else {
Style: "} else {" (I won't repeat this, as it will become too noisy).
> + unsigned char head[20];
> + struct commit *head_commit;
> + const char *head_message, *body;
> +
> + if (get_sha1("HEAD", head))
> + return error(_("need a HEAD to fixup"));
> + if (!(head_commit = lookup_commit_reference(head)))
> + return error(_("could not read HEAD"));
> + if (!(head_message = get_commit_buffer(head_commit, NULL)))
> + return error(_("could not read HEAD's commit message"));
> +
> + body = strstr(head_message, "\n\n");
> + if (!body)
> + body = "";
> + else
> + body = skip_blank_lines(body + 2);
I think I saw you used a helper function find_commit_subject() to do
the above in an earlier patch for "edit" in this series. Would it
make this part (and another one for "commit" we have after this
if/else) shorter?
> static int do_pick_commit(enum todo_command command, struct commit *commit,
> - struct replay_opts *opts)
> + struct replay_opts *opts, int final_fixup)
> {
> + int edit = opts->edit, cleanup_commit_message = 0;
> + const char *msg_file = edit ? NULL : git_path_merge_msg();
> unsigned char head[20];
> struct commit *base, *next, *parent;
> const char *base_label, *next_label;
> struct commit_message msg = { NULL, NULL, NULL, NULL };
> struct strbuf msgbuf = STRBUF_INIT;
> - int res, unborn = 0, allow;
> + int res, unborn = 0, amend = 0, allow;
>
> if (opts->no_commit) {
> /*
> @@ -749,7 +885,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
> else
> parent = commit->parents->item;
>
> - if (opts->allow_ff &&
> + if (opts->allow_ff && !is_fixup(command) &&
> ((parent && !hashcmp(parent->object.oid.hash, head)) ||
> (!parent && unborn)))
> return fast_forward_to(commit->object.oid.hash, head, unborn, opts);
> @@ -813,6 +949,28 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
> }
> }
>
> + if (is_fixup(command)) {
> + if (update_squash_messages(command, commit, opts))
> + return -1;
> + amend = 1;
> + if (!final_fixup)
> + msg_file = rebase_path_squash_msg();
> + else if (file_exists(rebase_path_fixup_msg())) {
> + cleanup_commit_message = 1;
> + msg_file = rebase_path_fixup_msg();
> + }
> + else {
> + const char *dest = git_path("SQUASH_MSG");
> + unlink(dest);
> + if (copy_file(dest, rebase_path_squash_msg(), 0666))
> + return error(_("could not rename '%s' to '%s'"),
> + rebase_path_squash_msg(), dest);
Perhaps an error from unlink(dest) before copy_file() should also
result in an error return? After all, that unlink() was added to
mimick the "cp" in the scripted one and copy_file() does not want
to overwrite an existing destination.
> + unlink(git_path("MERGE_MSG"));
Errors from this and other unlink() that emulates "rm -f" were
unchecked in the scripted original, so not checking for errors is
not a regression. I would check for an error if I were writing
this, however, because I know I would forget updating these after I
am done with the series.
> + msg_file = dest;
> + edit = 1;
> + }
> + }
> +
> if (!opts->strategy || !strcmp(opts->strategy, "recursive") || command == TODO_REVERT) {
> res = do_recursive_merge(base, next, base_label, next_label,
> head, &msgbuf, opts);
> @@ -868,8 +1026,13 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
> goto leave;
> }
> if (!opts->no_commit)
> - res = run_git_commit(opts->edit ? NULL : git_path_merge_msg(),
> - opts, allow, opts->edit, 0, 0);
> + res = run_git_commit(msg_file, opts, allow, edit, amend,
> + cleanup_commit_message);
The introduction of msg_file variable made this call (actually, the
logic to decide which file is affected) much nicer to understand.
> + if (!res && final_fixup) {
> + unlink(rebase_path_fixup_msg());
> + unlink(rebase_path_squash_msg());
> + }
>
> @@ -1475,6 +1660,21 @@ static int do_exec(const char *command_line)
> return status;
> }
>
> +static int is_final_fixup(struct todo_list *todo_list)
> +{
> + int i = todo_list->current;
> +
> + if (!is_fixup(todo_list->items[i].command))
> + return 0;
> +
> + while (++i < todo_list->nr)
> + if (is_fixup(todo_list->items[i].command))
> + return 0;
> + else if (todo_list->items[i].command < TODO_NOOP)
> + break;
What follows NOOP are comment and "drop" which is another comment in
disguise, so this one is excluding all the no-op commands in various
shapes, which makes sense but is clear only to a reader who bothered
to go back to "enum todo_command" and checked that fact. If a check
for "is it one of the no-op commands?" appears only here, a single
liner comment may be sufficient (but necessary) to help readers.
Otherwise a single-liner helper function (similar to is_fixup() you
have) with a descriptive name would be better than a single liner
comment.
^ permalink raw reply
* Re: git bug - merging JS / Node.js code with "git merge --squash"
From: Junio C Hamano @ 2016-12-15 18:10 UTC (permalink / raw)
To: Alexander Mills; +Cc: git
In-Reply-To: <CA+KyZp4HzsSOghKZnKgXEXy_gqw-9oTHveDHn1eRUY-_C6_jww@mail.gmail.com>
Alexander Mills <alexander.d.mills@gmail.com> writes:
> basically it is doing the merge with this line:
>
> git merge --squash -Xtheirs dev -m "squashing" &&
The order of command line parameters are "dashed options and their
arguments first, then refs (and then pathspecs if exists), so you
should make it a habit to move that "dev" at the end of the command
line. This does not have anything to do with the issue you are
asking for help, though.
I suspect the problem is not in --squash but with -Xtheirs. It is
"I give up correctly resolving conflicts and blindly take whatever
comes from their side, discarding what I did".
The "git merge" command stops and asks a human to help resolving a
conflict using their brain when it needs to because the command
knows it itself does not have any but the human user has a better
one ;-). -Xtheirs and -Xours disables this safety/sanity.
If you punt, it is expected that you would get garbage.
For example, your side may have introduced a new function (or
renamed an existing one) near where their side made a change that
resulted in conflicts. Naturally, your side also would have added
new callsites to that new function (you wouldn't be adding an unused
function). Imagine in such a scenario that you asked "discard what
I did in the conflicted section and blindly take whatever they did".
What happens? They may not have done anything near the places you
added calls to the new function and these sections of the code would
merge cleanly with or without -Xtheirs, but the actual new function
you added may be in the conflicted section that you asked to discard,
taking their changes.
^ permalink raw reply
* Re: [PATCH v2 3/6] update_unicode.sh: pin the uniset repo to a known good commit
From: Junio C Hamano @ 2016-12-15 17:50 UTC (permalink / raw)
To: Dennis Kaarsemaker; +Cc: Beat Bolli, git
In-Reply-To: <1481795248.4090.15.camel@kaarsemaker.net>
Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
> On Wed, 2016-12-14 at 00:31 +0100, Beat Bolli wrote:
>> + ( cd uniset && git checkout 4b186196dd )
>
> Micronit, but this is perhaps better written as
>
> git -C uniset checkout 4b186196dd
>
> to avoid the subshell and cd.
>
> D.
In the context of this script, I would say that is not even a
micronit. It is "you could do this if you wanted to".
^ permalink raw reply
* Re: [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit'
From: Jeff King @ 2016-12-15 17:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Chris Packham, GIT, Markus Hitter, Jacob Keller
In-Reply-To: <xmqqd1gtw0vi.fsf@gitster.mtv.corp.google.com>
On Thu, Dec 15, 2016 at 09:36:17AM -0800, Junio C Hamano wrote:
> > Did you want me to send a v4 to mark the strings for translation or
> > will you apply a fixup your end?
>
> I didn't follow the _() discussion (was there any?)
I think the discussion was just "we should do that".
> I do not think lack of _() is a show-stopper and my preference is to
> keep what I queued that does not have _(), and receive a separate
> follow-up patch that changes "msg" to _("msg") and does nothing
> else.
Here's a patch.
-- >8 --
Subject: merge: mark usage error strings for translation
The nearby error messages are already marked for
translation, but these new ones aren't.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/merge.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 668aaffb8..599d25c4c 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1164,7 +1164,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
const char *nargv[] = {"reset", "--merge", NULL};
if (orig_argc != 2)
- usage_msg_opt("--abort expects no arguments",
+ usage_msg_opt(_("--abort expects no arguments"),
builtin_merge_usage, builtin_merge_options);
if (!file_exists(git_path_merge_head()))
@@ -1180,7 +1180,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
const char *nargv[] = {"commit", NULL};
if (orig_argc != 2)
- usage_msg_opt("--continue expects no arguments",
+ usage_msg_opt(_("--continue expects no arguments"),
builtin_merge_usage, builtin_merge_options);
if (!file_exists(git_path_merge_head()))
--
2.11.0.348.g960a0b554
^ permalink raw reply related
* Re: [PATCH v2] fix pushing to //server/share/dir on Windows
From: Junio C Hamano @ 2016-12-15 17:49 UTC (permalink / raw)
To: Jeff King
Cc: Torsten Bögershausen, Johannes Sixt, Johannes Schindelin,
Git Mailing List
In-Reply-To: <20161215110155.it7ptkbju5etmnpn@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I don't have an opinion either way on what Windows would want, but note
> that the function already _does_ convert separators to slashes. With
> Johannes's original patch, you'd end up with a mix, like:
>
> \\server\share/dir1/file
>
> So this conversion is really just retaining the original behavior, and
> making it consistent throughout the path.
>
> Which isn't to say that the function as it currently exists isn't a
> little bit buggy. :)
>
> One of the points of normalizing, though, is that Git can then do
> textual comparisons between the output. So I think there's value in
> having a canonical internal representation, even if the OS could handle
> more exotic ones.
E.g. the log message of d53c2c6738 ("mingw: fix t9700's assumption
about directory separators", 2016-01-27) says the two kinds of
slashes are equivalent over there, but the patch text ends up doing
exactly that normalization.
5ca6b7bb47 ("config --show-origin: report paths with forward
slashes", 2016-03-23) is an example of us trying to normalize in
order to give consistent output to the users.
Having said all that, I do not have an opinion either way on what
Windows would want, either ;-)
^ permalink raw reply
* Re: [PATCHv3 1/3] merge: Add '--continue' option as a synonym for 'git commit'
From: Junio C Hamano @ 2016-12-15 17:36 UTC (permalink / raw)
To: Chris Packham; +Cc: GIT, Markus Hitter, Jeff King, Jacob Keller
In-Reply-To: <CAFOYHZD_mFMvggq4pedjGCz332i1-VcRKxu30iMzURfB3Mu8Vg@mail.gmail.com>
Chris Packham <judge.packham@gmail.com> writes:
> On Thu, Dec 15, 2016 at 7:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> The last one 3/3 is a nice touch that makes sure that we do not
>> forget what we discovered during the discussion. Very much
>> appreciated.
>>
>> Will queue. Thanks.
>
> Did you want me to send a v4 to mark the strings for translation or
> will you apply a fixup your end?
I didn't follow the _() discussion (was there any?)
I do not think lack of _() is a show-stopper and my preference is to
keep what I queued that does not have _(), and receive a separate
follow-up patch that changes "msg" to _("msg") and does nothing
else.
Thanks.
^ permalink raw reply
* Re: [PATCH v10 2/6] http: always warn if libcurl version is too old
From: Junio C Hamano @ 2016-12-15 17:29 UTC (permalink / raw)
To: Jeff King; +Cc: Brandon Williams, git, sbeller, bburky, jrnieder
In-Reply-To: <20161215002120.xlnlquuqqw25wngc@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Dec 14, 2016 at 02:39:51PM -0800, Brandon Williams wrote:
>
>> Always warn if libcurl version is too old because:
>>
>> 1. Even without a protocol whitelist, newer versions of curl have all
>> non-http protocols disabled by default.
>
> Technically, non-http and non-ftp. Maybe just "non-standard" would be
> more accurate.
>
> Not worth a re-roll, but if Junio hasn't applied yet, maybe worth fixing
> up while applying.
I just did a "rebase -i"; thanks always for your careful reading.
^ permalink raw reply
* [PATCH] git-p4: Fix multi-path changelist empty commits
From: George Vanburgh @ 2016-12-15 17:14 UTC (permalink / raw)
To: git
From: George Vanburgh <gvanburgh@bloomberg.net>
When importing from multiple perforce paths - we may attempt to import a changelist that contains files from two (or more) of these depot paths. Currently, this results in multiple git commits - one containing the changes, and the other(s) as empty commits. This behavior was introduced in commit 1f90a64 ("git-p4: reduce number of server queries for fetches", 2015-12-19).
Reproduction Steps:
1. Have a git repo cloned from a perforce repo using multiple depot paths (e.g. //depot/foo and //depot/bar).
2. Submit a single change to the perforce repo that makes changes in both //depot/foo and //depot/bar.
3. Run "git p4 sync" to sync the change from #2.
Change is synced as multiple commits, one for each depot path that was affected.
Using a set, instead of a list inside p4ChangesForPaths() ensures that each changelist is unique to the returned list, and therefore only a single commit is generated for each changelist.
Reported-by: James Farwell <jfarwell@vmware.com>
Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>
---
git-p4.py | 4 ++--
t/t9800-git-p4-basic.sh | 22 +++++++++++++++++++++-
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index fd5ca52..6307bc8 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -822,7 +822,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
die("cannot use --changes-block-size with non-numeric revisions")
block_size = None
- changes = []
+ changes = set()
# Retrieve changes a block at a time, to prevent running
# into a MaxResults/MaxScanRows error from the server.
@@ -841,7 +841,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
# Insert changes in chronological order
for line in reversed(p4_read_pipe_lines(cmd)):
- changes.append(int(line.split(" ")[1]))
+ changes.add(int(line.split(" ")[1]))
if not block_size:
break
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 0730f18..4d72e0b 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -131,6 +131,26 @@ test_expect_success 'clone two dirs, @all, conflicting files' '
)
'
+test_expect_success 'clone two dirs, each edited by submit, single git commit' '
+ (
+ cd "$cli" &&
+ echo sub1/f4 >sub1/f4 &&
+ p4 add sub1/f4 &&
+ echo sub2/f4 >sub2/f4 &&
+ p4 add sub2/f4 &&
+ p4 submit -d "sub1/f4 and sub2/f4"
+ ) &&
+ git p4 clone --dest="$git" //depot/sub1@all //depot/sub2@all &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git"
+ git ls-files >lines &&
+ test_line_count = 4 lines &&
+ git log --oneline p4/master >lines &&
+ test_line_count = 5 lines
+ )
+'
+
revision_ranges="2000/01/01,#head \
1,2080/01/01 \
2000/01/01,2080/01/01 \
@@ -147,7 +167,7 @@ test_expect_success 'clone using non-numeric revision ranges' '
(
cd "$git" &&
git ls-files >lines &&
- test_line_count = 6 lines
+ test_line_count = 8 lines
)
done
'
--
https://github.com/git/git/pull/311
^ permalink raw reply related
* Re: Additional git hooks
From: Jeff King @ 2016-12-15 14:19 UTC (permalink / raw)
To: Chiel ten Brinke; +Cc: git
In-Reply-To: <20161215141430.natk4mi7imixcoow@sigill.intra.peff.net>
On Thu, Dec 15, 2016 at 09:14:30AM -0500, Jeff King wrote:
> On Thu, Dec 15, 2016 at 02:57:18PM +0100, Chiel ten Brinke wrote:
>
> > Would patches introducing new git hooks, e.g. for post-fetch, be
> > eligible for acceptance?
>
> The general guidelines for adding hooks is laid out here:
>
> http://public-inbox.org/git/7vbq7ibxhh.fsf@gitster.siamese.dyndns.org/
One interesting follow-up, though (which seems sensible to me):
http://public-inbox.org/git/7vr5fraxbf.fsf@alter.siamese.dyndns.org/
-Peff
^ permalink raw reply
* [PATCH] README: replace gmane link with public-inbox
From: Jeff King @ 2016-12-15 14:17 UTC (permalink / raw)
To: Chiel ten Brinke; +Cc: git
In-Reply-To: <CAFw20syajXbjCQRcrqCv8pS9JwSge7-V4Hsg96n8SpYv2jJneQ@mail.gmail.com>
On Thu, Dec 15, 2016 at 02:57:18PM +0100, Chiel ten Brinke wrote:
> Btw, the link in the README
> http://news.gmane.org/gmane.comp.version-control.git/ is dead.
Yes, the status of gmane was up in the air for a while, but I think we
can give it up as dead now (at least for our purposes).
-- >8 --
Subject: README: replace gmane link with public-inbox
The general status and future of gmane is unclear at this
point, but certainly it does not seem to be carrying
gmane.comp.version-control.git at all anymore. Let's point
to public-inbox.org, which seems to be the favored archive
on the list these days (and which uses message-ids in its
URLs, making the links somewhat future-proof).
Reported-by: Chiel ten Brinke <ctenbrinke@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
README.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/README.md b/README.md
index bd8a918a9..c0cd5580e 100644
--- a/README.md
+++ b/README.md
@@ -33,7 +33,7 @@ requests, comments and patches to git@vger.kernel.org (read
[Documentation/SubmittingPatches][] for instructions on patch submission).
To subscribe to the list, send an email with just "subscribe git" in
the body to majordomo@vger.kernel.org. The mailing list archives are
-available at http://news.gmane.org/gmane.comp.version-control.git/,
+available at https://public-inbox.org/git,
http://marc.info/?l=git and other archival sites.
The maintainer frequently sends the "What's cooking" reports that
--
2.11.0.348.g960a0b554
^ permalink raw reply related
* Re: Additional git hooks
From: Jeff King @ 2016-12-15 14:14 UTC (permalink / raw)
To: Chiel ten Brinke; +Cc: git
In-Reply-To: <CAFw20syajXbjCQRcrqCv8pS9JwSge7-V4Hsg96n8SpYv2jJneQ@mail.gmail.com>
On Thu, Dec 15, 2016 at 02:57:18PM +0100, Chiel ten Brinke wrote:
> Would patches introducing new git hooks, e.g. for post-fetch, be
> eligible for acceptance?
The general guidelines for adding hooks is laid out here:
http://public-inbox.org/git/7vbq7ibxhh.fsf@gitster.siamese.dyndns.org/
-Peff
^ permalink raw reply
* Additional git hooks
From: Chiel ten Brinke @ 2016-12-15 13:57 UTC (permalink / raw)
To: git
Would patches introducing new git hooks, e.g. for post-fetch, be
eligible for acceptance?
If a more detailed explanation about a specific use case is required,
I'd be happy to provide it.
Btw, the link in the README
http://news.gmane.org/gmane.comp.version-control.git/ is dead.
Chiel
^ permalink raw reply
* [PATCH 4/6] Add DirDiffTool as additional option
From: Pierre Dumuid @ 2016-12-15 11:28 UTC (permalink / raw)
To: paulus, git; +Cc: Pierre Dumuid
In-Reply-To: <20161215112847.14719-1-pmdumuid@gmail.com>
Signed-off-by: Pierre Dumuid <pmdumuid@gmail.com>
---
gitk | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/gitk b/gitk
index a894f1d..5f27716 100755
--- a/gitk
+++ b/gitk
@@ -2661,6 +2661,9 @@ proc makewindow {} {
{mc "Diff this -> marked commit" command {diffvsmark 0}}
{mc "Diff marked commit -> this" command {diffvsmark 1}}
{mc "Revert this commit" command revert}
+
+ {mc "DirDiffTool this -> selected" command {externalDiffToolVsSel 0}}
+ {mc "DirDiffTool selected -> this" command {externalDiffToolVsSel 1}}
}
$rowctxmenu configure -tearoff 0
@@ -9254,6 +9257,20 @@ proc diffvssel {dirn} {
doseldiff $oldid $newid
}
+proc externalDiffToolVsSel {diffDirection} {
+ global rowmenuid selectedline
+
+ if {$selectedline eq {}} return
+ if {$diffDirection} {
+ set oldid [commitonrow $selectedline]
+ set newid $rowmenuid
+ } else {
+ set oldid $rowmenuid
+ set newid [commitonrow $selectedline]
+ }
+ [exec git difftool -d $oldid $newid]
+}
+
proc diffvsmark {dirn} {
global rowmenuid markedid
--
2.10.2
^ permalink raw reply related
* [PATCH 6/6] Rename 'remotes/' to 'r../' in heads
From: Pierre Dumuid @ 2016-12-15 11:28 UTC (permalink / raw)
To: paulus, git; +Cc: Pierre Dumuid
In-Reply-To: <20161215112847.14719-1-pmdumuid@gmail.com>
Signed-off-by: Pierre Dumuid <pmdumuid@gmail.com>
---
gitk | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/gitk b/gitk
index 0903d2d..6f50b06 100755
--- a/gitk
+++ b/gitk
@@ -6731,22 +6731,28 @@ proc drawtags {id x xt y1} {
set yb [expr {$yt + $linespc - 1}]
set xvals {}
set wvals {}
+ set newTags {}
+
set i -1
foreach tag $marks {
incr i
+ set newTag $tag
+ regsub {^remotes} $newTag "r.." newTag
+
if {$i >= $ntags && $i < $ntags + $nheads && $tag eq $mainhead} {
- set wid [font measure mainfontbold $tag]
+ set wid [font measure mainfontbold $newTag]
} else {
- set wid [font measure mainfont $tag]
+ set wid [font measure mainfont $newTag]
}
lappend xvals $xt
lappend wvals $wid
+ lappend newTags $newTag
set xt [expr {$xt + $wid + $extra}]
}
set t [$canv create line $x $y1 [lindex $xvals end] $y1 \
-width $lthickness -fill $reflinecolor -tags tag.$id]
$canv lower $t
- foreach tag $marks x $xvals wid $wvals {
+ foreach tag $marks x $xvals wid $wvals newTag $newTags {
set tag_quoted [string map {% %%} $tag]
set xl [expr {$x + $delta}]
set xr [expr {$x + $delta + $wid + $lthickness}]
@@ -6778,7 +6784,10 @@ proc drawtags {id x xt y1} {
$canv create polygon $x $yt $xr $yt $xr $yb $x $yb \
-width 1 -outline black -fill $col -tags tag.$id
if {[regexp {^(remotes/.*/|remotes/)} $tag match remoteprefix]} {
- set rwid [font measure mainfont $remoteprefix]
+ set newRemotePrefix $remoteprefix
+ regsub {^remotes} $newRemotePrefix "r.." newRemotePrefix
+
+ set rwid [font measure mainfont $newRemotePrefix]
set xi [expr {$x + 1}]
set yti [expr {$yt + 1}]
set xri [expr {$x + $rwid}]
@@ -6786,7 +6795,7 @@ proc drawtags {id x xt y1} {
-width 0 -fill $remotebgcolor -tags tag.$id
}
}
- set t [$canv create text $xl $y1 -anchor w -text $tag -fill $headfgcolor \
+ set t [$canv create text $xl $y1 -anchor w -text $newTag -fill $headfgcolor \
-font $font -tags [list tag.$id text]]
if {$ntags >= 0} {
$canv bind $t <1> $tagclick
--
2.10.2
^ permalink raw reply related
* [PATCH 5/6] gitk: Add a "Save file as" menu item
From: Pierre Dumuid @ 2016-12-15 11:28 UTC (permalink / raw)
To: paulus, git; +Cc: Pierre Dumuid, Andreas Amann
In-Reply-To: <20161215112847.14719-1-pmdumuid@gmail.com>
Previously, there was no easy way to save a particular file from the
currently selected revision.
This patch adds a menu item "Save file as" to the file list popup
menu, which opens a file selection dialog to determine the name under
which a file should be saved. The default filename is of the form
"[shortid] basename". If the current revision is the index, the
default pattern is of the form "[index] basename". This works for
both, the "Patch" and "Tree" view. The menu item is disabled for the
"local uncommitted changes" fake revision.
Signed-off-by: Andreas Amann <andreas.amann@web.de>
Signed-off-by: Pierre Dumuid <pmdumuid@gmail.com>
---
gitk | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/gitk b/gitk
index 5f27716..0903d2d 100755
--- a/gitk
+++ b/gitk
@@ -2693,6 +2693,7 @@ proc makewindow {} {
{mc "Highlight this too" command {flist_hl 0}}
{mc "Highlight this only" command {flist_hl 1}}
{mc "External diff" command {external_diff}}
+ {mc "Save file as" command {save_file_as}}
{mc "Blame parent commit" command {external_blame 1}}
{mc "Copy path" command {clipboard clear; clipboard append $flist_menu_file}}
}
@@ -3504,6 +3505,7 @@ proc sel_flist {w x y} {
proc pop_flist_menu {w X Y x y} {
global ctext cflist cmitmode flist_menu flist_menu_file
global treediffs diffids
+ global nullid
stopfinding
set l [lindex [split [$w index "@$x,$y"] "."] 0]
@@ -3521,6 +3523,12 @@ proc pop_flist_menu {w X Y x y} {
}
# Disable "External diff" item in tree mode
$flist_menu entryconf 2 -state $xdiffstate
+ set savefilestate "normal"
+ if {[lindex $diffids 0] eq $nullid} {
+ set savefilestate "disabled"
+ }
+ # Disable "Save file as" item "local uncommited changes" revision
+ $flist_menu entryconf 3 -state $savefilestate
tk_popup $flist_menu $X $Y
}
@@ -3632,6 +3640,34 @@ proc external_diff_get_one_file {diffid filename diffdir} {
"revision $diffid"]
}
+proc save_file_as {} {
+ global nullid nullid2
+ global flist_menu_file cmitmode
+ global diffids
+
+ set diffid [lindex $diffids 0]
+ if {$diffid == $nullid} {
+ return
+ } elseif {$diffid == $nullid2} {
+ set diffidtext [mc "index"]
+ set diffid ""
+ set whattext $diffidtext
+ } else {
+ set diffidtext [shortids $diffid]
+ set whattext "[mc "revision"] $diffidtext"
+ }
+ set diffid $diffid:
+ if {$cmitmode eq "tree"} {
+ set diffid $diffid./
+ }
+ set difffile "\[$diffidtext\] [file tail $flist_menu_file]"
+ set difffile [tk_getSaveFile -initialfile $difffile -title [mc "Save file as"] -parent .]
+ if {$difffile eq {}} {
+ return
+ }
+ save_file_from_commit $diffid$flist_menu_file $difffile $whattext
+}
+
proc external_diff {} {
global nullid nullid2
global flist_menu_file
--
2.10.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox