* [PATCH v2 06/14] i18n: config: mark error message for translation
From: Vasco Almeida @ 2016-09-12 11:29 UTC (permalink / raw)
To: git
Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA, Junio C Hamano
In-Reply-To: <1473259758-11836-1-git-send-email-vascomalmeida@sapo.pt>
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
builtin/config.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 6cbf733..05843a0 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -622,8 +622,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
value = normalize_value(argv[0], argv[1]);
ret = git_config_set_in_file_gently(given_config_source.file, argv[0], value);
if (ret == CONFIG_NOTHING_SET)
- error("cannot overwrite multiple values with a single value\n"
- " Use a regexp, --add or --replace-all to change %s.", argv[0]);
+ error(_("cannot overwrite multiple values with a single value\n"
+ " Use a regexp, --add or --replace-all to change %s."), argv[0]);
return ret;
}
else if (actions == ACTION_SET_ALL) {
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] Do not record unstaged deleted file upon recursive merge if file was moved outside of working tree with enabled sparse-checkout.
From: Mikhail Filippov @ 2016-09-12 11:07 UTC (permalink / raw)
To: git
In-Reply-To: <xmqqeg4q6mw4.fsf@gitster.mtv.corp.google.com>
>> +test_expect_success 'move file/sparse-checkout/merge should not delete moved file' '
>> + git rm -rf . &&
>> + git clean -fdqx &&
>> + rm -rf .git &&
>> + git init &&
>
> Yuck. This is inherited from existing tests but I think they need
> to be cleaned up. It is not your fault, and it is not in the scope
> of this change.
It's part need for cleanup repository from the previous case.
^ permalink raw reply
* Re: Diff impromements: keep block whole
From: Kevin Daudt @ 2016-09-12 10:38 UTC (permalink / raw)
To: KES; +Cc: git
In-Reply-To: <1190311473672986@web29o.yandex.ru>
On Mon, Sep 12, 2016 at 12:36:26PM +0300, KES wrote:
> Does it be more meaning full which change was done when diff will be displayed as:
> @@ -1278,6 +1311,16 @@ sub process {
> if( ref $result eq 'HASH' ) {
> $code = $result->{ code };
> @args = DB::eval( $result->{ expr } );
>
> redo PROCESS;
> }
> + elsif( ref $result eq 'ARRAY' ) {
> + $code = shift @$result;
> + @args = ();
> + for my $expr ( @$result ) {
> + push @args, [ DB::eval( $expr ) ];
> + }
> +
> + redo PROCESS;
> + }
>
>
> instead of:
>
> @@ -1278,6 +1311,16 @@ sub process {
> if( ref $result eq 'HASH' ) {
> $code = $result->{ code };
> @args = DB::eval( $result->{ expr } );
> +
> + redo PROCESS;
> + }
> + elsif( ref $result eq 'ARRAY' ) {
> + $code = shift @$result;
> + @args = ();
> + for my $expr ( @$result ) {
> + push @args, [ DB::eval( $expr ) ];
> + }
> +
> redo PROCESS;
> }
>
Yes, it would definately be more meaningfull, but for an algorithm it's
not that clear cut. Michael Hagerty is currently working on a patch
series[1] to give git some heuristics to get better diffs, but is hard
to get right.
You can already enable this by adding --compaction-heuristic to the diff
command, but this can also give odd results in other cases, so it's not
enabled by default.
Kevin.
[1]:https://public-inbox.org/git/cover.1473068229.git.mhagger@alum.mit.edu/
^ permalink raw reply
* Diff impromements: keep block whole
From: KES @ 2016-09-12 9:36 UTC (permalink / raw)
To: git
Does it be more meaning full which change was done when diff will be displayed as:
@@ -1278,6 +1311,16 @@ sub process {
if( ref $result eq 'HASH' ) {
$code = $result->{ code };
@args = DB::eval( $result->{ expr } );
redo PROCESS;
}
+ elsif( ref $result eq 'ARRAY' ) {
+ $code = shift @$result;
+ @args = ();
+ for my $expr ( @$result ) {
+ push @args, [ DB::eval( $expr ) ];
+ }
+
+ redo PROCESS;
+ }
instead of:
@@ -1278,6 +1311,16 @@ sub process {
if( ref $result eq 'HASH' ) {
$code = $result->{ code };
@args = DB::eval( $result->{ expr } );
+
+ redo PROCESS;
+ }
+ elsif( ref $result eq 'ARRAY' ) {
+ $code = shift @$result;
+ @args = ();
+ for my $expr ( @$result ) {
+ push @args, [ DB::eval( $expr ) ];
+ }
+
redo PROCESS;
}
^ permalink raw reply
* Re: [PATCH] Do not record unstaged deleted file upon recursive merge if file was moved outside of working tree with enabled sparse-checkout.
From: Mikhail Filippov @ 2016-09-12 8:38 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <xmqqeg4q6mw4.fsf@gitster.mtv.corp.google.com>
It's a very unexpected behaviour when a user sees a deleted file after a merge with enabled sparse-checkout. Moreover, when the user resolves merge conflicts and commits the changes with the command "git commit -am xxx", a repository can be broken because all the moved files will be deleted. Finally, it's really hard to find a user who deleted these files because "git log file" doesn't show any merge commits by default. I'm not sure that my fix is correct but I checked all tests and I didn't find a better way to prevent files deleting.
12.09.2016, 02:24, "Junio C Hamano" <gitster@pobox.com>:
> Mikhail Filippov <mikhail@filippov.me> writes:
>
>> ---
>
> You'd need a lot more explanation on why this is needed
> (i.e. without it what behaviour you would get, and why that
> behaviour is wrong).
>
>> merge-recursive.c | 9 +++++---
>> t/t6042-merge-rename-corner-cases.sh | 42 ++++++++++++++++++++++++++++++++++++
>> 2 files changed, 48 insertions(+), 3 deletions(-)
>>
>> diff --git a/merge-recursive.c b/merge-recursive.c
>> index e349126..25dc701 100644
>> --- a/merge-recursive.c
>> +++ b/merge-recursive.c
>> @@ -1724,9 +1724,12 @@ static int merge_content(struct merge_options *o,
>> */
>> path_renamed_outside_HEAD = !path2 || !strcmp(path, path2);
>> if (!path_renamed_outside_HEAD) {
>> - add_cacheinfo(o, mfi.mode, &mfi.oid, path,
>> - 0, (!o->call_depth), 0);
>> - return mfi.clean;
>> + struct stat st;
>> + if (lstat(path, &st) == 0) {
>> + add_cacheinfo(o, mfi.mode, &mfi.oid, path,
>> + 0, (!o->call_depth), 0);
>> + return mfi.clean;
>> + }
>
> I cannot guess what you are trying to achieve without explanation in
> the proposed log message, but I can say that this unconditional
> checking of a working tree file cannot be correct (there may or may
> not be other things that are wrong with this change, which cannot be
> judged without more information).
>
> Imagine your o->call_depth is not zero, i.e. we are making a virtual
> common ancestor with this merge, in which case any of the three
> trees involved may have nothing to do with the current working tree
> files.
>
>> +test_expect_success 'move file/sparse-checkout/merge should not delete moved file' '
>> + git rm -rf . &&
>> + git clean -fdqx &&
>> + rm -rf .git &&
>> + git init &&
>
> Yuck. This is inherited from existing tests but I think they need
> to be cleaned up. It is not your fault, and it is not in the scope
> of this change.
>
>> + git status >output &&
>> + cp output /tmp/a &&
>
> Huh?
>
>> + test_i18ngrep "nothing to commit" output
>> +'
>> +
>> test_done
--
Mikhail Filippov
Software Developer
JetBrains
http://jetbrains.com
“The Drive To Develop"
^ permalink raw reply
* Re: [PATCH v2 21/25] sequencer: refactor write_message()
From: Johannes Sixt @ 2016-09-12 8:35 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano, Jakub Narębski
In-Reply-To: <da2293aee439da2274e30304e2d1f097b9644e64.1473590966.git.johannes.schindelin@gmx.de>
Am 11.09.2016 um 12:55 schrieb Johannes Schindelin:
> -static int write_message(struct strbuf *msgbuf, const char *filename)
> +static int write_with_lock_file(const char *filename,
> + const void *buf, size_t len, int append_eol)
> {
> static struct lock_file msg_file;
>
> int msg_fd = hold_lock_file_for_update(&msg_file, filename, 0);
> if (msg_fd < 0)
> return error_errno(_("Could not lock '%s'"), filename);
> - if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
> - return error_errno(_("Could not write to %s"), filename);
> - strbuf_release(msgbuf);
> + if (write_in_full(msg_fd, buf, len) < 0)
> + return error_errno(_("Could not write to '%s'"), filename);
> + if (append_eol && write(msg_fd, "\n", 1) < 0)
> + return error_errno(_("Could not write eol to '%s"), filename);
> if (commit_lock_file(&msg_file) < 0)
> return error(_("Error wrapping up %s."), filename);
>
> return 0;
> }
The two error paths in the added lines should both
rollback_lock_file(&msg_file);
, I think. But I do notice that this is not exactly new, so...
-- Hannes
^ permalink raw reply
* Re: [PATCH v2 20/25] sequencer: left-trim lines read from the script
From: Johannes Schindelin @ 2016-09-12 8:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <xmqqsht657ls.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Sun, 11 Sep 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > Interactive rebase's scripts may be indented; we need to handle this
> > case, too, now that we prepare the sequencer to process interactive
> > rebases.
>
> Hmph, have we ever given the sequencer instructions indented to the
> user to edit?
No.
We also never provide the user with an indented todo script in the
sequencer. Yet we still parse it. Because Postel's Law.
> I do not offhand see why we want to be lenient here,
> especially only to the left.
Postel's Law.
We do not care about the right: that's only comments (onelines, for the
users' pleasure).
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] git-gui: respect commit.gpgsign again
From: Johannes Schindelin @ 2016-09-12 8:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Pat Thoyts
In-Reply-To: <xmqqr38q6r5b.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Sun, 11 Sep 2016, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Thanks. There are a couple more git-gui patches waiting for quite a long
> > time. So you prefer them still as patches to git-gui.git?
>
> I prefer not to have to worry about them myself ;-) That means that
> even if Pat steps down, the next maintainer for git-gui project
> would not be me, so I wouldn't be making a unilateral decision to
> re-root git-gui.git project one-level down.
Makes sense.
> > Also, I just noticed poor wording. Would you mind fixing it up by
> >
> > s/committing/& with GPG signature/
>
> Ouch, I didn't notice. It's not in 'next' yet, so let me see if I
> can futz with the history.
Thank you!
Dscho
^ permalink raw reply
* Re: Bug: git-add .* errors out
From: Pranit Bauva @ 2016-09-12 5:59 UTC (permalink / raw)
To: Thomas Gummerer; +Cc: Git List
In-Reply-To: <20160911193906.GC8254@hank>
Hey Thomas,
On Mon, Sep 12, 2016 at 1:09 AM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> Hi,
>
> On 09/12, Pranit Bauva wrote:
>> Hey everyone,
>>
>> One of my friend was trying to add files using the command `git add
>> .*` and got an error that "fatal: ..: '..' is outside repository"
>> which did seem a little obvious to me. But then I tried to reproduce
>> it in my machine with `git add ".*"` and it didn't error out. I am
>> currently using git 2.9.3 on Ubuntu 15.04 while he is using git 1.9.1
>> on Ubuntu 16.04. What might have gone wrong?
>
> The difference seems to be that you quoted the .*, which leaves the .*
> in place for gits internal pathspec machinery, which then only
> considers paths inside of the repository.
>
> The non quoted version your friend used meanwhile is expanded by the
> shell itself, which seems to be expanding it to ., the current
> directory, and .., the parent directory. This behaviour also depends
> on the shell used, for me .* in bash includes the current as well as
> the parent directory, while .* in zsh doesn't include either of these.
This seems to be a reasonable reason. I had guessed the behaviour of
the shell but didn't really know what was happening with the quoted
version. And I guess it cannot really be fixed because it depends on
the shell. I tried it out on bash though forgot to mention it. Thanks
for the help!
Regards,
Pranit Bauva
^ permalink raw reply
* Re: git commit -p with file arguments
From: Jacob Keller @ 2016-09-12 4:56 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jakub Narębski, Christian Neukirchen, Git mailing list
In-Reply-To: <xmqqa8fd6fs7.fsf@gitster.mtv.corp.google.com>
On Sun, Sep 11, 2016 at 6:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> You are excused ;-)
>
> In ancient days, "git commit <pathspec>" was to add the contents
> from working tree files that match <pathspec> to what is already in
> the index and create a commit from that state. This ran against the
> intuition of many users who knew older systems (e.g. cvs) and we had
> to migrate it to the current behaviour by breaking backward
> compatibility.
I see.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH] doc: mention `git -c` in git-config(1)
From: David Glasser @ 2016-09-12 2:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqeg4p6fxb.fsf@gitster.mtv.corp.google.com>
On Sun, Sep 11, 2016 at 6:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
> The patch that was commented on in that exchange should be part of
> v2.10.0 already.
My mistake: I was accidentally searching for the paragraph I added in
config.txt instead of git-config.txt. Thanks and sorry for wasting
your time!
--dave
--
glasser@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/
^ permalink raw reply
* Re: git commit -p with file arguments
From: Junio C Hamano @ 2016-09-12 1:57 UTC (permalink / raw)
To: Jacob Keller; +Cc: Jakub Narębski, Christian Neukirchen, Git mailing list
In-Reply-To: <CA+P7+xqdWbERVQenZJTLyirBy2VFrKV2-uBOxB1CwcWs+gy43A@mail.gmail.com>
Jacob Keller <jacob.keller@gmail.com> writes:
> Yes, I'm actually confused by "git commit <files>" *not* usinng what's
> in the index already, so I think that isn't intuitive as is.
You are excused ;-)
In ancient days, "git commit <pathspec>" was to add the contents
from working tree files that match <pathspec> to what is already in
the index and create a commit from that state. This ran against the
intuition of many users who knew older systems (e.g. cvs) and we had
to migrate it to the current behaviour by breaking backward
compatibility.
^ permalink raw reply
* Re: [PATCH] doc: mention `git -c` in git-config(1)
From: Junio C Hamano @ 2016-09-12 1:54 UTC (permalink / raw)
To: David Glasser; +Cc: git
In-Reply-To: <CAN7QDoJOzVvCzM80v=jP7RTB0od4exFLMZGkvPx9Uz6Lkm16Tg@mail.gmail.com>
David Glasser <glasser@davidglasser.net> writes:
> On Tue, Aug 23, 2016 at 11:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> David Glasser <glasser@davidglasser.net> writes:
>>
>> That might be something we want to fix up further in later patches;
>> the change we see in this patch is good regardless.
>
>
> Perhaps I am looking at the wrong branch, but I'm not sure that this
> got merged? Is there something I should do to move this along?
Are you asking about "might be something we want to fix up further",
which I do not think anybody did (and you certainly didn't)?
The patch that was commented on in that exchange should be part of
v2.10.0 already.
$ git blame -s -L264,269 v2.10.0 Documentation/git-config.txt
7da9800f 264) values of a key from all files will be used.
7da9800f 265)
ae1f7094 266) You may override individual configuration parameters when running any git
ae1f7094 267) command by using the `-c` option. See linkgit:git[1] for details.
ae1f7094 268)
17014090 269) All writing options will per default write to the repository specific
$ git show ae1f7094
commit ae1f7094f7a68fcff3d07358d83f5f483f0c300c
Author: David Glasser <glasser@davidglasser.net>
Date: Tue Aug 23 10:33:21 2016 -0700
doc: mention `git -c` in git-config(1)
Signed-off-by: David Glasser <glasser@davidglasser.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 6843114..636b3eb 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -263,6 +263,9 @@ The files are read in the order given above, with last value found taking
precedence over values read earlier. When multiple values are taken then all
values of a key from all files will be used.
+You may override individual configuration parameters when running any git
+command by using the `-c` option. See linkgit:git[1] for details.
+
All writing options will per default write to the repository specific
configuration file. Note that this also affects options like '--replace-all'
and '--unset'. *'git config' will only ever change one file at a time*.
^ permalink raw reply related
* Re: [RFC/PATCH] ls-files: adding support for submodules
From: Junio C Hamano @ 2016-09-12 1:47 UTC (permalink / raw)
To: Stefan Beller; +Cc: Brandon Williams, git@vger.kernel.org
In-Reply-To: <xmqqmvje6qbe.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> So a "ls-files" that is done internally in the end-user facing "git
> grep --recurse-submodules" needs to be run _without_ recursing
> itself at least once to learn "lib/" is a submodule. A flat "here
> are everything we have" does not sound like a good building block.
... unless you are _only_ interested in grepping (or in general
working outside Git repository) in the files in the working tree,
i.e. "git grep" without <tree-ish> nor "--cached".
A lot of the time you are interested in the current state of files,
not even in "the state recorded in the tip of the history" but in
"the state as I have in my working tree, the state as my compiler
sees it".
I am a bit torn. Clearly this is an important special case, but it
would make the codepath for object database case and working tree
case even further apart between "git grep [--cached | <tree-ish>]"
and the "find in the working tree" codepath, which does not sound
friendly to the codebase.
^ permalink raw reply
* Re: [RFC/PATCH] ls-files: adding support for submodules
From: Jeff King @ 2016-09-12 0:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, Brandon Williams, git@vger.kernel.org
In-Reply-To: <20160912005106.fj7adhmtfklv4vir@sigill.intra.peff.net>
On Sun, Sep 11, 2016 at 08:51:06PM -0400, Jeff King wrote:
> On Sun, Sep 11, 2016 at 03:10:13PM -0700, Junio C Hamano wrote:
>
> > So a "ls-files" that is done internally in the end-user facing "git
> > grep --recurse-submodules" needs to be run _without_ recursing
> > itself at least once to learn "lib/" is a submodule. A flat "here
> > are everything we have" does not sound like a good building block.
>
> I do not use submodules myself, but I could imagine that you may have
> scripts outside of git that do not care about the submodule divisions at
> all, and would be happy with the flat block. E.g., our "make tags"
> target uses "git ls-files" so find all of the source files. I could
> imagine projects with submodules that would want to do so recursively (I
> could also imagine projects that do _not_ want to do so; it would depend
> on your workflow and how tightly bound the submodules are). Another
> plausible example would be something grep-like that has features
> git-grep does not (I don't use "ack", but perhaps "git ls-files
> --recurse-submodules -z | xargs --null ack ..." is something people
> would want to do).
None of that negates your point, btw, which is that this does not seem
like a great building block for "git grep --recurse-submodules". Just
that it seems plausible to me that people could find recursive
"ls-files" useful on its own.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] ls-files: adding support for submodules
From: Jeff King @ 2016-09-12 0:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, Brandon Williams, git@vger.kernel.org
In-Reply-To: <xmqqmvje6qbe.fsf@gitster.mtv.corp.google.com>
On Sun, Sep 11, 2016 at 03:10:13PM -0700, Junio C Hamano wrote:
> So a "ls-files" that is done internally in the end-user facing "git
> grep --recurse-submodules" needs to be run _without_ recursing
> itself at least once to learn "lib/" is a submodule. A flat "here
> are everything we have" does not sound like a good building block.
I do not use submodules myself, but I could imagine that you may have
scripts outside of git that do not care about the submodule divisions at
all, and would be happy with the flat block. E.g., our "make tags"
target uses "git ls-files" so find all of the source files. I could
imagine projects with submodules that would want to do so recursively (I
could also imagine projects that do _not_ want to do so; it would depend
on your workflow and how tightly bound the submodules are). Another
plausible example would be something grep-like that has features
git-grep does not (I don't use "ack", but perhaps "git ls-files
--recurse-submodules -z | xargs --null ack ..." is something people
would want to do).
-Peff
^ permalink raw reply
* Re: Git Miniconference at Plumbers
From: Jeff King @ 2016-09-12 0:42 UTC (permalink / raw)
To: Jon Loeliger; +Cc: git
In-Reply-To: <E1bhKNo-0005m2-5z@mylo.jdl.com>
On Tue, Sep 06, 2016 at 12:42:04PM -0500, Jon Loeliger wrote:
> I have recently been enlisted by folks at the Linux Foundation to
> help run a Miniconference on Git at the Plumbers Conference [*]
> this fall.
I see the conference runs for 4 days; I assume the Git portion will just
be one day. Do you know yet which day?
-Peff
^ permalink raw reply
* Re: [PATCH] doc: mention `git -c` in git-config(1)
From: David Glasser @ 2016-09-12 0:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqd1kzwegi.fsf@gitster.mtv.corp.google.com>
On Tue, Aug 23, 2016 at 11:02 AM, Junio C Hamano <gitster@pobox.com> wrote:
> David Glasser <glasser@davidglasser.net> writes:
>
> That might be something we want to fix up further in later patches;
> the change we see in this patch is good regardless.
Perhaps I am looking at the wrong branch, but I'm not sure that this
got merged? Is there something I should do to move this along?
--dave
--
glasser@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/
^ permalink raw reply
* Re: [PATCH v2 20/25] sequencer: left-trim lines read from the script
From: Junio C Hamano @ 2016-09-11 23:39 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <09e05e7ac23164625f2076ff06d2b034101878da.1473590966.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> Interactive rebase's scripts may be indented; we need to handle this
> case, too, now that we prepare the sequencer to process interactive
> rebases.
Hmph, have we ever given the sequencer instructions indented to the
user to edit? I do not offhand see why we want to be lenient here,
especially only to the left.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> sequencer.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/sequencer.c b/sequencer.c
> index 7953a05..5e5d113 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -875,6 +875,9 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
> char *end_of_object_name;
> int i, saved, status, padding;
>
> + /* left-trim */
> + bol += strspn(bol, " \t");
> +
> for (i = 0; i < ARRAY_SIZE(todo_command_strings); i++)
> if (skip_prefix(bol, todo_command_strings[i], &bol)) {
> item->command = i;
^ permalink raw reply
* Re: [PATCH v2 21/25] sequencer: refactor write_message()
From: Junio C Hamano @ 2016-09-11 23:38 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <da2293aee439da2274e30304e2d1f097b9644e64.1473590966.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> The write_message() function safely writes an strbuf to a file.
> Sometimes it is inconvenient to require an strbuf, though: the text to
> be written may not be stored in a strbuf, or the strbuf should not be
> released after writing.
>
> Let's refactor "safely writing string to a file" into
> write_with_lock_file(), and make write_message() use it. The new
> function makes it easy to create new convenience function
> write_file_gently(); as some of the upcoming callers of this new
> function would want to append a newline character, add a flag for it in
> write_file_gently(), and thus in write_with_lock_file().
Makes sense. As an abstraction, "give me strbuf for my sole use,
and I'll trash its contents when I am done" feels like a horrible
helper interface; perhaps that was overly aggressively refactored by
noticing that then-current callers all released the strbuf, but
still feels wrong.
And this makes the underlying helper a lot more useful.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> sequencer.c | 23 +++++++++++++++++++----
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 5e5d113..aa949d4 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -242,22 +242,37 @@ static void print_advice(int show_hint, struct replay_opts *opts)
> }
> }
>
> -static int write_message(struct strbuf *msgbuf, const char *filename)
> +static int write_with_lock_file(const char *filename,
> + const void *buf, size_t len, int append_eol)
> {
> static struct lock_file msg_file;
>
> int msg_fd = hold_lock_file_for_update(&msg_file, filename, 0);
> if (msg_fd < 0)
> return error_errno(_("Could not lock '%s'"), filename);
> - if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
> - return error_errno(_("Could not write to %s"), filename);
> - strbuf_release(msgbuf);
> + if (write_in_full(msg_fd, buf, len) < 0)
> + return error_errno(_("Could not write to '%s'"), filename);
> + if (append_eol && write(msg_fd, "\n", 1) < 0)
> + return error_errno(_("Could not write eol to '%s"), filename);
> if (commit_lock_file(&msg_file) < 0)
> return error(_("Error wrapping up %s."), filename);
>
> return 0;
> }
>
> +static int write_message(struct strbuf *msgbuf, const char *filename)
> +{
> + int res = write_with_lock_file(filename, msgbuf->buf, msgbuf->len, 0);
> + strbuf_release(msgbuf);
> + return res;
> +}
> +
> +static int write_file_gently(const char *filename,
> + const char *text, int append_eol)
> +{
> + return write_with_lock_file(filename, text, strlen(text), append_eol);
> +}
> +
> /*
> * Reads a file that was presumably written by a shell script, i.e.
> * with an end-of-line marker that needs to be stripped.
^ permalink raw reply
* Re: [PATCH v2 22/25] sequencer: remove overzealous assumption in rebase -i mode
From: Junio C Hamano @ 2016-09-11 23:35 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <d0066b1a8dc6d8d6d45161203cd5c08ef8f2216f.1473590966.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> While at it, error out early if the "instruction sheet" (i.e. the todo
> script) could not be parsed.
Sounds good.
^ permalink raw reply
* Re: [PATCH v2 24/25] sequencer: quote filenames in error messages
From: Junio C Hamano @ 2016-09-11 23:33 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <db1816339ba9e259e929a45d644c33160f57c5bb.1473590966.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> This makes the code consistent by fixing quite a couple of error messages.
Looks OK. While at it, we may want another one to downcase the
first word, perhaps?
These may not be messages added by your series and can be left
outside this series, but I have to point out that
if (commit_lock_file(&msg_file) < 0)
return error(_("Error wrapping up '%s'."), filename);
results in "error: Error wrapping up", which sounds quite funny.
"failed to finalize" or something would flow a bit better, I'd say.
> diff --git a/sequencer.c b/sequencer.c
> index 1e7f29e..465e018 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -255,7 +255,7 @@ static int write_with_lock_file(const char *filename,
> if (append_eol && write(msg_fd, "\n", 1) < 0)
> return error_errno(_("Could not write eol to '%s"), filename);
> if (commit_lock_file(&msg_file) < 0)
> - return error(_("Error wrapping up %s."), filename);
> + return error(_("Error wrapping up '%s'."), filename);
>
> return 0;
> }
^ permalink raw reply
* Re: [PATCH v2 25/25] sequencer: remove bogus hint for translators
From: Junio C Hamano @ 2016-09-11 23:30 UTC (permalink / raw)
To: Johannes Schindelin, Ævar Arnfjörð Bjarmason
Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <8314699c144c258ca45094beef5953b612eb7f15.1473590966.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> When translating error messages, we need to be careful *not* to translate
> the todo commands such as "pick", "reword", etc because they are commands,
> and Git would not understand translated versions of those commands.
>
> Therefore, translating the commands in the error messages would simply be
> misleading.
This comes from b9c993e0 ("i18n: git-revert literal "me" messages",
2011-02-22) where it said
Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
i18n: git-revert literal "me" messages
Translate messages that use the `me' variable. These are all error
messages referencing the command name, so the name shouldn't be
translated.
Reported-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This looks like a positive attempt to remind translators that their
translation must flow with these literal command names that will not
get translated in place, not a bogus hint at all, at least to me.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> sequencer.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 465e018..cdff0f1 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -697,8 +697,6 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
> return fast_forward_to(commit->object.oid.hash, head, unborn, opts);
>
> if (parent && parse_commit(parent) < 0)
> - /* TRANSLATORS: The first %s will be "revert" or
> - "cherry-pick", the second %s a SHA1 */
> return error(_("%s: cannot parse parent commit %s"),
> command_to_string(command),
> oid_to_hex(&parent->object.oid));
^ permalink raw reply
* Re: [PATCH] Do not record unstaged deleted file upon recursive merge if file was moved outside of working tree with enabled sparse-checkout.
From: Junio C Hamano @ 2016-09-11 23:24 UTC (permalink / raw)
To: Mikhail Filippov; +Cc: git
In-Reply-To: <1473625505-7641-1-git-send-email-mikhail@filippov.me>
Mikhail Filippov <mikhail@filippov.me> writes:
> ---
You'd need a lot more explanation on why this is needed
(i.e. without it what behaviour you would get, and why that
behaviour is wrong).
> merge-recursive.c | 9 +++++---
> t/t6042-merge-rename-corner-cases.sh | 42 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 48 insertions(+), 3 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index e349126..25dc701 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1724,9 +1724,12 @@ static int merge_content(struct merge_options *o,
> */
> path_renamed_outside_HEAD = !path2 || !strcmp(path, path2);
> if (!path_renamed_outside_HEAD) {
> - add_cacheinfo(o, mfi.mode, &mfi.oid, path,
> - 0, (!o->call_depth), 0);
> - return mfi.clean;
> + struct stat st;
> + if (lstat(path, &st) == 0) {
> + add_cacheinfo(o, mfi.mode, &mfi.oid, path,
> + 0, (!o->call_depth), 0);
> + return mfi.clean;
> + }
I cannot guess what you are trying to achieve without explanation in
the proposed log message, but I can say that this unconditional
checking of a working tree file cannot be correct (there may or may
not be other things that are wrong with this change, which cannot be
judged without more information).
Imagine your o->call_depth is not zero, i.e. we are making a virtual
common ancestor with this merge, in which case any of the three
trees involved may have nothing to do with the current working tree
files.
> +test_expect_success 'move file/sparse-checkout/merge should not delete moved file' '
> + git rm -rf . &&
> + git clean -fdqx &&
> + rm -rf .git &&
> + git init &&
Yuck. This is inherited from existing tests but I think they need
to be cleaned up. It is not your fault, and it is not in the scope
of this change.
> + git status >output &&
> + cp output /tmp/a &&
Huh?
> + test_i18ngrep "nothing to commit" output
> +'
> +
> test_done
^ permalink raw reply
* Re: [PATCH v2 2/4] update-index: use the same structure for chmod as add
From: Junio C Hamano @ 2016-09-11 22:28 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Johannes Schindelin, Jeff King, Jan Keromnes,
Ingo Brückl, Edward Thomson
In-Reply-To: <20160911103028.5492-3-t.gummerer@gmail.com>
Thomas Gummerer <t.gummerer@gmail.com> writes:
> @@ -955,10 +941,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
> PARSE_OPT_NOARG | /* disallow --cacheinfo=<mode> form */
> PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
> (parse_opt_cb *) cacheinfo_callback},
> - {OPTION_CALLBACK, 0, "chmod", &set_executable_bit, N_("(+/-)x"),
> - N_("override the executable bit of the listed files"),
> - PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
> - chmod_callback},
> + OPT_STRING( 0, "chmod", &chmod_arg, N_("(+/-)x"),
> + N_("override the executable bit of the listed files")),
> {OPTION_SET_INT, 0, "assume-unchanged", &mark_valid_only, NULL,
> N_("mark files as \"not changing\""),
> PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, MARK_FLAG},
> @@ -1018,6 +1002,15 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
> if (argc == 2 && !strcmp(argv[1], "-h"))
> usage_with_options(update_index_usage, options);
>
> + if (!chmod_arg)
> + force_mode = 0;
> + else if (!strcmp(chmod_arg, "-x"))
> + force_mode = 0666;
> + else if (!strcmp(chmod_arg, "+x"))
> + force_mode = 0777;
> + else
> + die(_("option 'chmod' expects \"+x\" or \"-x\""));
> +
I am afraid that this changes the behaviour drastically.
"git update-index" is an oddball command that takes options and then
processes them immediately, exactly because it was designed to take
git update-index --chmod=-x A --chmod=+x B --add C
and say things like "A and B are not in the index and you are
attempting to add them before giving me --add option".
git update-index --add --chmod=-x A --chmod=+x B C
is expected to add A as non-executable, and B and C as executable.
Many exotic parse-options callback mechanisms used in this command
were invented exactly to support its quirky way of not doing "get a
list of options and use the last one". And this patch breaks it for
only one option without changing the others.
If we were willing to take such a big backward compatiblity hit in
the upcoming release (which I personally won't be affected, but old
scripts by others need to be audited and adjusted, which I won't
volunteer to do myself), we should make such a change consistently,
e.g. "git update-index A --add --remove B" should no longer error
out when it sees A and it is not yet in the index because "--add"
hasn't been given yet, or A is in the index but is missing from the
working tree because "--remove" hasn't been given yet. Then it may
be more justifiable if "update-index --chmod=-x A --chmod=+x B"
added A as an executable. With the current form of this patch, it
is not.
Can we do this "fix" without this change?
^ permalink raw reply
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