* Re: [PATCH v14 00/41] libify apply and use lib in am, part 2
From: Stefan Beller @ 2016-09-06 18:54 UTC (permalink / raw)
To: Christian Couder
Cc: git@vger.kernel.org, Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Eric Sunshine, Ramsay Jones, Johannes Sixt,
René Scharfe, Stefan Naewe, Christian Couder
In-Reply-To: <20160904201833.21676-1-chriscool@tuxfamily.org>
On Sun, Sep 4, 2016 at 1:17 PM, Christian Couder
<christian.couder@gmail.com> wrote:
> Goal
> ~~~~
>
> This is a patch series about libifying `git apply` functionality, and
> using this libified functionality in `git am`, so that no 'git apply'
> process is spawn anymore. This makes `git am` significantly faster, so
> `git rebase`, when it uses the am backend, is also significantly
> faster.
>
I reviewed this v14 and all patches look good to me.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH] rebase -i: improve advice on bad instruction lines
From: Ralf Thielow @ 2016-09-06 18:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Ralf Thielow
In-Reply-To: <20160906180838.865-1-ralf.thielow@gmail.com>
2016-09-06 20:08 GMT+02:00 Ralf Thielow <ralf.thielow@gmail.com>:
> - warn "$(gettext "You can fix this with 'git rebase --edit-todo'.")"
> + warn "$(gettext "You can fix this with 'git rebase --edit-todo' and then run 'git rebase --continue'.")"
> die "$(gettext "Or you can abort the rebase with 'git rebase --abort'.")"
Please don't apply as is. There are some test failures due to the text change.
I'll send an updated version.
^ permalink raw reply
* Re: [PATCH 1/3] Demonstrate a problem: our pickaxe code assumes NUL-terminated buffers
From: Jeff King @ 2016-09-06 18:43 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano
In-Reply-To: <ca678535c64570add58cfff95709c3c67384139d.1473090278.git.johannes.schindelin@gmx.de>
On Mon, Sep 05, 2016 at 05:45:02PM +0200, Johannes Schindelin wrote:
> Typically, on Linux the test passes. On Windows, it fails virtually
> every time due to an access violation (that's a segmentation fault for
> you Unix-y people out there). And Windows would be correct: the
> regexec() call wants to operate on a regular, NUL-terminated string,
> there is no NUL in the mmap()ed memory range, and it is undefined
> whether the next byte is even legal to access.
>
> When run with --valgrind it demonstrates quite clearly the breakage, of
> course.
>
> So we simply mark it with `test_expect_success` for now.
I'd prefer if this were marked as expect_failure. It fails reliably for
me on Linux, even without --valgrind. But even if that were not so,
there is no reason to hurt bisectability of somebody running with
"--valgrind" (not when it costs so little to mark it correctly).
-Peff
^ permalink raw reply
* Re: [PATCH 2/3] diff_populate_filespec: NUL-terminate buffers
From: Jeff King @ 2016-09-06 18:41 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano
In-Reply-To: <alpine.DEB.2.20.1609061613270.129229@virtualbox>
On Tue, Sep 06, 2016 at 06:02:59PM +0200, Johannes Schindelin wrote:
> It will still be quite tricky, because we have to touch a function that is
> rather at the bottom of the food chain: diff_populate_filespec() is called
> from fill_textconv(), which in turn is called from pickaxe_match(), and
> only pickaxe_match() knows whether we want to call regexec() or not (it
> depends on its regexp parameter).
>
> Adding a flag to diff_populate_filespec() sounds really reasonable until
> you see how many call sites fill_textconv() has.
I was thinking of something quite gross, like a global "switch to using
slower-but-safer NUL termination" flag (but I agree with Junio's point
elsewhere that we do not even know if it is "slower").
> > I thought that operated on the diff content itself, which would always
> > be in a heap buffer (which should be NUL terminated, but if it isn't,
> > that would be a separate fix from this).
>
> That is true.
>
> Except when preimage or postimage does not exist. In which case we call
>
> regexec(regexp, two->ptr, 1, ®match, 0);
>
> or the same with one->ptr. Note the notable absence of two->size.
Thanks, I forgot about that case.
> > [1] We do make the assumption elsewhere that git objects are
> > NUL-terminated, but that is enforced by the object-reading code
> > (with the exception of streamed blobs, but those are obviously dealt
> > with separately anyway).
>
> I know. I am the reason you introduced that, because I added code to
> fsck.c that assumes that tag/commit messages are NUL-terminated.
Sort of. I think it has been part of the design since e871b64
(unpack_sha1_file: zero-pad the unpacked object., 2005-05-25), though I
do recall that we missed a code path that did its allocation differently
(in index-pack, IIRC).
Anyway, that is neither here nor there for the diff code, which as you
noticed may operate on things besides git objects.
> So now for the better idea.
>
> While I was researching the code for this reply, I hit upon one thing that
> I never knew existed, introduced in f96e567 (grep: use REG_STARTEND for
> all matching if available, 2010-05-22). Apparently, NetBSD introduced an
> extension to regexec() where you can specify buffer boundaries using
> REG_STARTEND. Which is pretty much what we need.
Yes, and compat/regex support this, too. My question is whether it is
portable. I see:
> diff --git a/diff.c b/diff.c
> index 534c12e..2c5a360 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -951,7 +951,13 @@ static int find_word_boundaries(mmfile_t *buffer,
> regex_t *word_regex,
> {
> if (word_regex && *begin < buffer->size) {
> regmatch_t match[1];
> - if (!regexec(word_regex, buffer->ptr + *begin, 1, match,
> 0)) {
> + int f = 0;
> +#ifdef REG_STARTEND
> + match[0].rm_so = 0;
> + match[0].rm_eo = *end - *begin;
> + f = REG_STARTEND;
> +#endif
> + if (!regexec(word_regex, buffer->ptr + *begin, 1, match,
> f)) {
What happens to those poor souls on systems without REG_STARTEND? Do
they get to keep segfaulting?
I think the solution is to push them into setting NO_REGEX. So looking
at this versus a "regexecn", it seems:
- this lets people keep using their native regexec if it supports
STARTEND
- this is a bit more clunky to use at the callsites (though we could
_create_ a portable regexecn wrapper that uses this technique on top
of the native regex library)
But I much prefer this approach to copying the data just to add a NUL.
-Peff
^ permalink raw reply
* Re: If a branch moves a submodule, "merge --ff[-only]" succeeds while "merge --no-ff" fails with conflicts
From: Dakota Hawkins @ 2016-09-06 18:40 UTC (permalink / raw)
To: Git Mailing List
In-Reply-To: <CAG0BQX=wvpkJ=PQWV-NbmhuPV8yzvd_KYKzJmsfWq9xStZ2bnQ@mail.gmail.com>
Is there any additional information I could provide that would be helpful?
Dakota
On Fri, Sep 2, 2016 at 3:22 PM, Dakota Hawkins <dakotahawkins@gmail.com> wrote:
> Below is a simple reproduction of the issue.
>
> The _real_ problem is that this is how our pull request merges work,
> they're not allowed to do fast-forward merges. To work around this we
> are having to split this up into two pull requests/merges: One that
> copies the submodules to the new location and includes any fixes
> required to support the move, and a second that removes the old
> locations.
>
> ## Setup steps
> git clone https://github.com/dakotahawkins/submodule-move-merge-bug-main-repo.git
> cd submodule-move-merge-bug-main-repo
> ## How it was initially constructed
> # git submodule add ../submodule-move-merge-bug-submodule-repo.git
> ./submodule-location-1
> # git commit -m "Added submodule in its initial location"
> # git push
> # git checkout -b move-submodule
> # git mv ./submodule-location-1 ./submodule-location-2
> # git commit -m "Moved submodule"
> # git push --set-upstream origin move-submodule
> git branch move-submodule origin/move-submodule
>
> ## Test fast-forward merge, this will work
> git checkout -b merge-ff-test master # warning: unable to rmdir
> submodule-location-2: Directory not empty
> rm -rf ./submodule-location-2
> git merge --ff-only move-submodule
>
> ## Test no-fast-forward merge, this will fail with conflicts:
> git checkout -b merge-no-ff-test master
> git merge --no-ff move-submodule
> # Auto-merging submodule-location-2
> # Adding as submodule-location-2~move-submodule instead
> # Automatic merge failed; fix conflicts and then commit the result.
> git status
> # On branch merge-no-ff-test
> # You have unmerged paths.
> # (fix conflicts and run "git commit")
> # (use "git merge --abort" to abort the merge)
> #
> # Changes to be committed:
> #
> # modified: .gitmodules
> # deleted: submodule-location-1
> #
> # Unmerged paths:
> # (use "git add <file>..." to mark resolution)
> #
> # added by us: submodule-location-2
> #
> # fatal: Not a git repository: 'submodule-location-1/.git'
> # Submodule changes to be committed:
> #
> # * submodule-location-1 07fec24...0000000:
^ permalink raw reply
* Re: [PATCH 0/3] Fix a segfault caused by regexec() being called on mmap()ed data
From: Jeff King @ 2016-09-06 18:29 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.2.20.1609061521410.129229@virtualbox>
On Tue, Sep 06, 2016 at 04:06:32PM +0200, Johannes Schindelin wrote:
> > I think re_search() the correct replacement function but it's been a
> > while since I've looked into it.
>
> The segfault I investigated happened in a call to strlen(). I see many
> calls to strlen() in compat/regex/... The one that triggers the segfault
> is in regexec(), compat/regex/regexec.c:241.
Yes, that is the important one, I think. The others are for patterns,
error msgs, etc. Of course strlen() is not the only function that cares
about NUL delimiters (and there might even be a "while (*p)" somewhere
in the code).
I always assumed the _point_ of re_search taking a ptr/len pair was
exactly to handle this case. The documentation[1] says:
`string` is the string you want to match; it can contain newline and
null characters. `size` is the length of that string.
Which seems pretty definitive to me (that's for re_match(), but
re_search() is defined in the docs in terms of re_match()).
[1] http://www.delorie.com/gnu/docs/regex/regex_47.html
> As to re_search(): I have not been able to reason about its callees in a
> reasonable amount of time. I agree that they *should* not run over the
> buffer, but I cannot easily verify it.
Between the documentation above, and the fact that your new test passes
when we switch to it (see below), I feel pretty good about it.
> The bigger problem is that re_search() is defined in the __USE_GNU section
> of regex.h, and I do not think it is appropriate to universally #define
> said constant before #include'ing regex.h. So it would appear that major
> surgery would be required if we wanted to use regular expressions on
> strings that are not NUL-terminated.
We can contain this to the existing compat/regexec/regexec.c, and just
provide a wrapper that is similar to regexec but takes a ptr/len pair.
Like:
diff --git a/compat/regex/regex.h b/compat/regex/regex.h
index 61c9683..b2dd0b7 100644
--- a/compat/regex/regex.h
+++ b/compat/regex/regex.h
@@ -569,6 +569,11 @@ extern int regexec (const regex_t *__restrict __preg,
regmatch_t __pmatch[__restrict_arr],
int __eflags);
+extern int regexecn (const regex_t *__restrict __preg,
+ const char *__restrict __cstring, size_t __length,
+ size_t __nmatch, regmatch_t __pmatch[__restrict_arr],
+ int __eflags);
+
extern size_t regerror (int __errcode, const regex_t *__restrict __preg,
char *__restrict __errbuf, size_t __errbuf_size);
diff --git a/compat/regex/regexec.c b/compat/regex/regexec.c
index eb5e1d4..8afe26b 100644
--- a/compat/regex/regexec.c
+++ b/compat/regex/regexec.c
@@ -217,15 +217,16 @@ static reg_errcode_t extend_buffers (re_match_context_t *mctx)
We return 0 if we find a match and REG_NOMATCH if not. */
int
-regexec (
+regexecn (
const regex_t *__restrict preg,
const char *__restrict string,
+ size_t length,
size_t nmatch,
regmatch_t pmatch[],
int eflags)
{
reg_errcode_t err;
- int start, length;
+ int start;
if (eflags & ~(REG_NOTBOL | REG_NOTEOL | REG_STARTEND))
return REG_BADPAT;
@@ -238,7 +239,7 @@ regexec (
else
{
start = 0;
- length = strlen (string);
+ /* length already passed in */
}
__libc_lock_lock (dfa->lock);
@@ -252,6 +253,17 @@ regexec (
return err != REG_NOERROR;
}
+int
+regexec (
+ const regex_t *__restrict preg,
+ const char *__restrict string,
+ size_t nmatch,
+ regmatch_t pmatch[],
+ int eflags)
+{
+ return regexecn(preg, string, strlen(string), nmatch, pmatch, eflags);
+}
+
#ifdef _LIBC
# include <shlib-compat.h>
versioned_symbol (libc, __regexec, regexec, GLIBC_2_3_4);
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 55067ca..fdd08dd 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -50,9 +50,9 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,
xdemitconf_t xecfg;
if (!one)
- return !regexec(regexp, two->ptr, 1, ®match, 0);
+ return !regexecn(regexp, two->ptr, two->size, 1, ®match, 0);
if (!two)
- return !regexec(regexp, one->ptr, 1, ®match, 0);
+ return !regexecn(regexp, one->ptr, one->size, 1, ®match, 0);
/*
* We have both sides; need to run textual diff and see if
^ permalink raw reply related
* Re: [PATCH v14 30/41] Move libified code from builtin/apply.c to apply.{c,h}
From: Stefan Beller @ 2016-09-06 18:29 UTC (permalink / raw)
To: Christian Couder
Cc: git@vger.kernel.org, Junio C Hamano, Jeff King,
Ævar Arnfjörð Bjarmason, Karsten Blees,
Nguyen Thai Ngoc Duy, Eric Sunshine, Ramsay Jones, Johannes Sixt,
René Scharfe, Stefan Naewe, Christian Couder
In-Reply-To: <20160904201833.21676-31-chriscool@tuxfamily.org>
On Sun, Sep 4, 2016 at 1:18 PM, Christian Couder
<christian.couder@gmail.com> wrote:
> As most of the apply code in builtin/apply.c has been libified by a number of
> previous commits, it can now be moved to apply.{c,h}, so that more code can
> use it.
>
> Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> apply.c | 4731 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> apply.h | 19 +
> builtin/apply.c | 4733 +------------------------------------------------------
> 3 files changed, 4751 insertions(+), 4732 deletions(-)
So I wanted to review this patch, so I wrote a patch to review this patch. ;)
https://public-inbox.org/git/82367750-adea-6dee-198a-e39ac7a84b31@gmail.com/T/#t
> + if (!state->apply_in_reverse &&
> + state->ws_error_action != nowarn_ws_error)
> + check_whitespace(state, line, len, patch->ws_rule);
> + added++;
> + newlines--;
> + trailing = 0;
> + break;
> +
> + /*
> + * We allow "\ No newline at end of file". Depending
> + * on locale settings when the patch was produced we
> + * don't know what this line looks like. The only
> + * thing we do know is that it begins with "\ ".
The previous three lines are white space broken AFAICT. The seem to be
white space broken in the original location as well, so no need for a reroll
just for this. But in case you do a reroll, you may want to fix these
on the fly?
> +
> +int apply_option_parse_exclude(const struct option *opt,
> + const char *arg, int unset)
> +
> +int apply_option_parse_include(const struct option *opt,
> + const char *arg, int unset)
> +int apply_option_parse_p(const struct option *opt,
> + const char *arg,
> + int unset)
These three functions seem slightly different, not just moved.
Oh you removed the static key word!
Apart from the one minor nit and the removal of the static keyword,
the rest is just moved code.
Thanks,
Stefan
^ permalink raw reply
* [PATCH] rebase -i: improve advice on bad instruction lines
From: Ralf Thielow @ 2016-09-06 18:08 UTC (permalink / raw)
To: git; +Cc: gitster, Ralf Thielow
If we found bad instruction lines in the instruction sheet
of interactive rebase, we give the user advice on how to
fix it. However, we don't tell the user what to do afterwards.
Give the user advice to run 'git rebase --continue' after
the fix.
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
---
git-rebase--interactive.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b1ba21c..029594e 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1041,7 +1041,7 @@ The possible behaviours are: ignore, warn, error.")"
# placed before the commit of the next action
checkout_onto
- warn "$(gettext "You can fix this with 'git rebase --edit-todo'.")"
+ warn "$(gettext "You can fix this with 'git rebase --edit-todo' and then run 'git rebase --continue'.")"
die "$(gettext "Or you can abort the rebase with 'git rebase --abort'.")"
fi
}
--
2.10.0.304.gf2ff484
^ permalink raw reply related
* Bug? ran into a "fatal" using interactive rebase
From: Ralf Thielow @ 2016-09-06 18:08 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Junio C Hamano
Hi,
today I accidentally triggered a "fatal" using interactive rebase.
If you edit the instruction sheet after 'rebase -i' and add an unknown
command, Git stops because it doesn't know the command.
That's fine, however, now we are in a state where 'git status' fails with
interactive rebase in progress; onto 311f279
fatal: Could not open file .git/rebase-merge/done for reading: No such
file or directory
After finishing the interactive rebase, things are fixed. Looks like a
special case that isn't handled well, yet.
My Git version is the current 'next' branch.
Ralf
^ permalink raw reply
* Git Miniconference at Plumbers
From: Jon Loeliger @ 2016-09-06 17:42 UTC (permalink / raw)
To: git
Folks,
I have recently been enlisted by folks at the Linux Foundation to
help run a Miniconference on Git at the Plumbers Conference [*]
this fall.
We currently have both Junio Hamano and Josh Triplett signed
up to do talks. Hopefully, though not confirmed yet, Junio
will give us a brief "State of the Git Union" to set the stage
for a few more presentations and some discussion about the
Future of Git. Josh has volunteered to talk about a patch
series manager he's been developing.
Rumor also suggests that the Man In The Bowtie might make
an appearance as well. With luck, Mr. Bottomley might
offer a speaking role as well! We'll see.
I would like to solicit one or two more solid talks from
the community, either the Linux Kernel community or the
Git community proper. If you are so interested, please
send me some mail.
Thanks,
jdl
[*] -- http://www.linuxplumbersconf.org/2016/
^ permalink raw reply
* Re: [PATCH] xdiff: remove unneeded declarations
From: Stefan Beller @ 2016-09-06 17:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Stefan Beller
In-Reply-To: <20160903031648.14465-1-sbeller@google.com>
On Fri, Sep 2, 2016 at 8:16 PM, Stefan Beller <sbeller@google.com> wrote:
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> xdiff/xemit.c | 9 ---------
> 1 file changed, 9 deletions(-)
Despite the moved coloring patch moving into a different direction, I
think this is still
an improvement to the code.
Thanks,
Stefan
>
> diff --git a/xdiff/xemit.c b/xdiff/xemit.c
> index 49aa16f..b52b4b9 100644
> --- a/xdiff/xemit.c
> +++ b/xdiff/xemit.c
> @@ -22,15 +22,6 @@
>
> #include "xinclude.h"
>
> -
> -
> -
> -static long xdl_get_rec(xdfile_t *xdf, long ri, char const **rec);
> -static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *ecb);
> -
> -
> -
> -
> static long xdl_get_rec(xdfile_t *xdf, long ri, char const **rec) {
>
> *rec = xdf->recs[ri]->ptr;
> --
> 2.10.0.rc2.22.g25cb54d.dirty
>
^ permalink raw reply
* Re: [PATCHv4] diff.c: emit moved lines with a different color
From: Jakub Narębski @ 2016-09-06 17:51 UTC (permalink / raw)
To: Stefan Beller
Cc: Stefan Beller, git@vger.kernel.org, Junio C Hamano, Jacob Keller
In-Reply-To: <CAGZ79kZPzEYV=gdhzXQetXoe4+1zdh67eyL-gGh9EOCSbRwzWw@mail.gmail.com>
W dniu 06.09.2016 o 19:03, Stefan Beller pisze:
> On Tue, Sep 6, 2016 at 7:05 AM, Jakub Narębski <jnareb@gmail.com> wrote:
>> If not for `color.moved`, I would have thought that instead of adding
>> new command line option `--color-moved` (and the fact that it is on
>> by default), we could simply reuse duplication of code movement
>> detection as a signal of stronger detection, namely "-M -M" (and also
>> "-C -C" to handle copy detection) that git-blame uses...
>
> Can you please elaborate on how you'd use that as a user?
>
> The -M and -C options only operate on the file level, e.g.
> these options are very good at things introduced via:
>
> git mv A B
> $EDIT B # only a little.
>
> So these options make no sense when operating only on one
> file or on many files that stay the same and only change very little.
>
> The goal of my patch here is to improve cases like 11979b98
> (2005-11-18, http.c: reorder to avoid compilation failure.)
>
> In that case we just move code around, not necessarily across file
> boundaries.
>
> So that seems orthogonal to the -M/-C option as it operates on another
> level. (file vs line)
The idea for an alternative way of turning on color marking of moved
lines was to follow an example of "git blame", where _doubling_
of a command means more extensive move / copy detection (accompanied
by new values for `diff.renames`).
From git-blame(1) manpage:
-C|<num>|
In addition to -M, detect lines moved or copied from other files
that were modified in the same commit. [...]. When this option
is given twice, the command additionally looks for copies from
other files in the commit that creates the file. When this option
is given three times, the command additionally looks for copies
from other files in any commit.
Color marking of moved lines may be considered enhancing of exiting
whole-file movement and whole-file copy detection.
But it is not a good UI if the feature is to be turned on by default.
Your proposal of adding `--color-moved` and `color.moved` is better.
> In another email you asked whether this new approach works in the
> word-by-word diff, which it unfortunately doesn't yet, but I would think
> that it is the same problem (line vs word granularity).
I don't know how it is done internally, but I think word diff is done
by using words (as defined by `diff.<driver>.wordRegex`) in place
of lines...
Best,
--
Jakub Narębski
^ permalink raw reply
* How to simulate a real checkout to test a new smudge filter?
From: john smith @ 2016-09-06 17:47 UTC (permalink / raw)
To: git
I am looking for a way to force smudge filter to run by simulating a
real life checkout. Let's say I just created a new branch and did not
modify any files but want to test my new smudge filter. According to
some answers such as
https://stackoverflow.com/questions/22909620/git-smudge-clean-filter-between-branches
and
https://stackoverflow.com/questions/21652242/git-re-checkout-files-after-creating-smudge-filter
it should be possible by running:
git checkout HEAD --
but in doesn't work with git 2.9.0. Method suggested in accepted
answer here
https://stackoverflow.com/questions/17223527/how-do-i-force-git-to-checkout-the-master-branch-and-remove-carriage-returns-aft
works but I don't like because it seems fragile. Is there a safe way
to do what I want to do in Git still today?
--
<wempwer@gmail.com>
^ permalink raw reply
* Re: [PATCHv4] diff.c: emit moved lines with a different color
From: Stefan Beller @ 2016-09-06 17:03 UTC (permalink / raw)
To: Jakub Narębski
Cc: Stefan Beller, git@vger.kernel.org, Junio C Hamano, Jacob Keller
In-Reply-To: <15618224-a9f3-bbe7-3556-8fd8aab2a2a4@gmail.com>
On Tue, Sep 6, 2016 at 7:05 AM, Jakub Narębski <jnareb@gmail.com> wrote:
> W dniu 06.09.2016 o 09:01, Stefan Beller pisze:
>
>> ---
>>
>> * moved new data structures into struct diff_options
>> * color.moved=bool as well as --[no-]color-moved to {dis,en}able the new feature
>> * color.diff.movedfrom and color.diff.movedto to control the colors
>> * added a test
> [...]
>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index 0bcb679..5daf77a 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -974,14 +974,22 @@ This does not affect linkgit:git-format-patch[1] or the
>> 'git-diff-{asterisk}' plumbing commands. Can be overridden on the
>> command line with the `--color[=<when>]` option.
>>
>> +color.moved::
>> + A boolean value, whether a diff should color moved lines
>> + differently. The moved lines are searched for in the diff only.
>> + Duplicated lines from somewhere in the project that are not
>> + part of the diff are not colored as moved.
>> + Defaults to true.
>
> [...]
>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
>> index 705a873..13b6a2a 100644
>> --- a/Documentation/diff-options.txt
>> +++ b/Documentation/diff-options.txt
>> @@ -234,6 +234,13 @@ ifdef::git-diff[]
>> endif::git-diff[]
>> It is the same as `--color=never`.
>>
>> +--[no-]color-moved::
>> + Show moved blocks in a different color.
>> +ifdef::git-diff[]
>> + It can be changed by the `diff.ui` and `color.diff`
>> + configuration settings.
>> +endif::git-diff[]
>
> If not for `color.moved`, I would have thought that instead of adding
> new command line option `--color-moved` (and the fact that it is on
> by default), we could simply reuse duplication of code movement
> detection as a signal of stronger detection, namely "-M -M" (and also
> "-C -C" to handle copy detection) that git-blame uses...
Can you please elaborate on how you'd use that as a user?
The -M and -C options only operate on the file level, e.g.
these options are very good at things introduced via:
git mv A B
$EDIT B # only a little.
So these options make no sense when operating only on one
file or on many files that stay the same and only change very little.
The goal of my patch here is to improve cases like 11979b98
(2005-11-18, http.c: reorder to avoid compilation failure.)
In that case we just move code around, not necessarily across file
boundaries.
So that seems orthogonal to the -M/-C option as it operates on another
level. (file vs line)
In another email you asked whether this new approach works in the
word-by-word diff, which it unfortunately doesn't yet, but I would think
that it is the same problem (line vs word granularity)
So what I am asking here is, how would you imagine a better user interface
for what I am trying to do, or do you think I should adapt my goal?
Thanks,
Stefan
>
> --
> Jakub Narębski
>
^ permalink raw reply
* Re: [WIP PATCH v2] diff.c: emit moved lines with a different color
From: Stefan Beller @ 2016-09-06 16:47 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jacob Keller, Stefan Beller, Git mailing list,
Jakub Narębski
In-Reply-To: <xmqqoa41p59i.fsf@gitster.mtv.corp.google.com>
On Tue, Sep 6, 2016 at 5:44 AM, Junio C Hamano <gitster@pobox.com> wrote:
> By the way, not running xdiff twice would also remove another worry
> I have about correctness, in that the approach depends on xdiff
> machinery to produce byte-for-byte identical result given the same
> pair of input.
As we use different parameters to the xdiff machinery (e.g. context = 1 line)
the output is not byte-for-byte identical.
> The output may currently be reproducible, but that
> is an unnecessary and an expensive thing to rely on.
My original design was to not store the lines in the hashmap but
only pointers to them, such that the additional memory pressure was
assumed less than storing the whole output of the xdiff machinery.
That point is moot though in the current implementation, so it
would be better indeed if we run the xdiff machinery once and store
all its output and then operate on that, even from a memory perspective.
>
> You may be able to save tons of memory if you do not store the line
> contents duplicated. The first pass callback can tell the line
> numbers in preimage and postimage [*1*], so your record for a
> removed line could be a pair <struct diff_filespec *, long lineno>
> with whatever hash value you need to throw it into the hash bucket.
Yeah I guess I'll go that way in the next patch then.
>
> I know we use a hash function and a line comparison function that
> are aware of -b/-w comparison in xdiff machinery, but I didn't see
> you use them in your hashtable. Can you match moved lines when
> operating under various whitespace-change-ignoring modes?
Not yet.
Thanks,
Stefan
>
> Thanks.
>
>
> [Footnote]
>
> *1* You can learn all sort of things from emit_callback structure;
> if you need to pass more data from the caller of xdi_diff_outf()
> to the callback, you can even add new fields to it.
>
^ permalink raw reply
* Re: [PATCH] gpg-interface: reflect stderr to stderr
From: Johannes Schindelin @ 2016-09-06 16:43 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <alpine.DEB.2.20.1609061839370.129229@virtualbox>
Hi Michael,
okay, final mail on this issue today:
On Tue, 6 Sep 2016, Johannes Schindelin wrote:
> Your original issue seemed to be that the gpg command could succeed, but
> still no signature be seen. There *must* be a way to test whether the
> called program added a signature, simply by testing whether *any*
> characters were written.
>
> And if characters were written that were not actually a GPG signature,
> maybe the enterprisey user who configured the gpg command to be her magic
> script actually meant something else than a GPG signature to be added?
I actually just saw that this is *precisely* what the code does already:
if (ret || signature->len == bottom)
return error(_("gpg failed to sign the data"));
Why is this not good enough?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] gpg-interface: reflect stderr to stderr
From: Johannes Schindelin @ 2016-09-06 16:42 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <alpine.DEB.2.20.1609061827290.129229@virtualbox>
Hi Michael,
On Tue, 6 Sep 2016, Johannes Schindelin wrote:
> On Tue, 6 Sep 2016, Michael J Gruber wrote:
>
> > A full blown approach would use --status-fd=4 or such rather than hijacking stderr.
> > This would require an extension of pipe_command() etc. to handle yet another fd.
For the record: I do not know whether that would work, either. So unless
we are fairly certain that it *would* work, I'd rather not spend the time.
Your original issue seemed to be that the gpg command could succeed, but
still no signature be seen. There *must* be a way to test whether the
called program added a signature, simply by testing whether *any*
characters were written.
And if characters were written that were not actually a GPG signature,
maybe the enterprisey user who configured the gpg command to be her magic
script actually meant something else than a GPG signature to be added?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Unbreak interactive GPG prompt upon signing
From: Johannes Schindelin @ 2016-09-06 16:39 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <ced7502d-0095-bd90-19e3-c14d0e4d4f07@drmicha.warpmail.net>
Hi Michael,
On Tue, 6 Sep 2016, Michael J Gruber wrote:
> Johannes Schindelin venit, vidit, dixit 06.09.2016 10:01:
> > With the recent update in efee955 (gpg-interface: check gpg signature
> > creation status, 2016-06-17), we ask GPG to send all status updates to
> > stderr, and then catch the stderr in an strbuf.
> >
> > But GPG might fail, and send error messages to stderr. And we simply
> > do not show them to the user.
> >
> > Even worse: this swallows any interactive prompt for a passphrase. And
> > detaches stderr from the tty so that the passphrase cannot be read.
> >
> > So while the first problem could be fixed (by printing the captured
> > stderr upon error), the second problem cannot be easily fixed, and
> > presents a major regression.
>
> My Git has that commit and does ask me for the passphrase on the tty.
> Also, I do get error messages:
>
> git tag -u pebcak -s testt -m m
> error: gpg failed to sign the data
> error: unable to sign the tag
That is not GPG's error message. It just leaves users puzzled, is what it
does.
> which we could (maybe should) amend by gpg's stderr.
Right. But then we still do not solve the problem. The problem being that
some platforms cannot use getpass(prompt): it simply does not exist.
On Windows, we do not even have a /dev/tty (technically, GPG, being an
MSYS2 program, knows about /dev/tty, but we spawn it from a non-MSYS2
program, so there is a disconnect).
> > So let's just revert commit efee9553a4f97b2ecd8f49be19606dd4cf7d9c28.
>
> That "just" reintroduces the problem that the orignal patch solves.
Right. Which is: when some user misconfigures gpg, causing Git to run
something different that simply succeeds, there is no signature.
This is a minor issue, as it requires a user to configure gpg, and do a
bad job at it.
Not being able to input the passphrase on Windows is a major issue, as the
user has done nothing wrong.
> The passphrase/tty issue must be Windows specific - or the non-issue
> Linux-specific, if you prefer.
Sure. Let's talk about semantics. Oh wait, maybe we should work on
resolving the issue instead.
> > This fixes https://github.com/git-for-windows/git/issues/871
To reiterate: this is the problem I need to see solved.
Ciao,
Dscho
^ permalink raw reply
* Re: Windows Git will not start external diff at all
From: Johannes Schindelin @ 2016-09-06 16:34 UTC (permalink / raw)
To: Jaakko Pääkkönen; +Cc: git
In-Reply-To: <CADr93XDA3CdgGBqBJTdmCHg_ZzGhsBf5hwwORJaCNB-V7o+APg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 711 bytes --]
Hi Jaakko,
On Tue, 6 Sep 2016, Jaakko Pääkkönen wrote:
> I am using beyond compare, but it does not really matter which one
> because even I create a dummy script as a external diff program, it
> will not get called ever. Only internal diff is started.
Any chance you can come up with an MCVE [*1*] using a dummy script?
>
> Re-installing git will not remove the problem. I am using the latest git
>
> Any hints how I can debug git difftool?
Sure. First hint: set GIT_TRACE=1. Next hint: difftool is a shell script,
so you can modify it, say, by inserting a `set -x` which will trace the
commands that are called.
Ciao,
Johannes
Footnote *1*: http://stackoverflow.com/help/mcve
^ permalink raw reply
* Re: [PATCH] gpg-interface: reflect stderr to stderr
From: Johannes Schindelin @ 2016-09-06 16:30 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <18a7e2984121d988137c135ec560fee56506981b.1473167263.git.git@drmicha.warpmail.net>
Hi Michael,
On Tue, 6 Sep 2016, Michael J Gruber wrote:
> efee955 ("gpg-interface: check gpg signature creation status",
> 2016-06-17) used stderr to capture gpg's status output, which is the
> only reliable way for status checks. As a side effect, stderr was not
> shown to the user any more.
>
> In case of a gpg error, reflect the whole captured buffer to stderr.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
> A full blown approach would use --status-fd=4 or such rather than hijacking stderr.
> This would require an extension of pipe_command() etc. to handle yet another fd.
As I indicated in my patch, this is not enough on Windows. In fact, my
first version of a patch tried to do exactly what you presented here, and
all it did was make the error message a bit more verbose:
-- snip --
error: gpg failed to sign the data:
[GNUPG:] USERID_HINT <key> Johannes Schindelin <johannes.schindelin@gmx.de>
[GNUPG:] NEED_PASSPHRASE <key> <key2> 1 0
gpg: cannot open tty `no tty': No such file or directory
error: unable to sign the tag
-- snap --
This is not a fix for the issue reported on the Git for Windows, but only
half a fix.
Ciao,
Dscho
^ permalink raw reply
* Re: Your email
From: Johannes Schindelin @ 2016-09-06 16:27 UTC (permalink / raw)
To: Idan Shimoni; +Cc: git
In-Reply-To: <CAFfNYUmFPBvYLk4c4N-rAH-huDi0QsEsXA-Z9gN5pMiuwejVhQ@mail.gmail.com>
Hi Idan,
please only write public mails when discussing Open Source.
On Tue, 6 Sep 2016, Idan Shimoni wrote:
> "This" top-posting is auto generated when hitting on the magic button
> called "Reply", and it is the generic way that the industry works. How
> do you expect me to know that this is considered "very very rude on this
> emailing list" if it is not mention in any place on your page.
The top-posting is actually not auto-generated at all. It is your decision
to write on top of the quoted text, or not.
> When I sent my email in the first time I did not sent it to Stefan, I
> found this email (git@vger.kernel.org) at one of the pages as a
> "support" for Git for Windows.
You probably also saw the note that this is an Open Source project, where
that support is handled by volunteers. You did not pay anything, after
all.
> When I sent my email I did not think that the support is also for
> different operating systems, as I found it on a "Git for Windows"
> page, and as for the version from which I upgraded or from what I
> upgraded to it is not really matter as you now understand now because
> Git for Windows removed the Git-Cheetah.
>
> On the other end in my terms and my colleagues here, Stefan last email
> and yours are considered very very rude, if you WANT to help people
> then help them, do not attack them as you did.
You know, replies like these I could do without. I do try to help. I even
explain to you why your replies are met with such hostility, and how you
could avoid that in the future. And what do I get in return?
> And lastly as for the ticket at
> https://github.com/git-for-windows/git/issues/875 I was not sure if it
> is the repository for this, as later on I understood that it is
> related to a different product.
>
> So, there you go.
>
> I am very very very sorry for disturbing and annoying both of you and
> will not address you or your colleagues in the future any more.
>
>
> * Sure not use any of your products as well
That is your choice, of course. You will have to live with the choices
Stefan and I make on our end, too.
Ciao,
Johannes
^ permalink raw reply
* [PATCH v2 6/6] git-gui: Update Japanese information
From: Satoshi Yasushima @ 2016-09-06 16:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Pat Thoyts,
Satoshi Yasushima
In-Reply-To: <1473177741-9576-1-git-send-email-s.yasushima@gmail.com>
Signed-off-by: Satoshi Yasushima <s.yasushima@gmail.com>
---
po/ja.po | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/po/ja.po b/po/ja.po
index deaf8e3..208651c 100644
--- a/po/ja.po
+++ b/po/ja.po
@@ -1,15 +1,17 @@
# Translation of git-gui to Japanese
# Copyright (C) 2007 Shawn Pearce
# This file is distributed under the same license as the git-gui package.
+#
# しらいし ななこ <nanako3@bluebottle.com>, 2007.
+# Satoshi Yasushima <s.yasushima@gmail.com>, 2016.
#
msgid ""
msgstr ""
"Project-Id-Version: git-gui\n"
"Report-Msgid-Bugs-To: \n"
"POT-Creation-Date: 2016-05-27 17:52+0900\n"
-"PO-Revision-Date: 2010-02-02 19:03+0900\n"
-"Last-Translator: しらいし ななこ <nanako3@lavabit.com>\n"
+"PO-Revision-Date: 2016-06-22 12:50+0900\n"
+"Last-Translator: Satoshi Yasushima <s.yasushima@gmail.com>\n"
"Language-Team: Japanese\n"
"Language: ja\n"
"MIME-Version: 1.0\n"
--
2.8.2.windows.1
^ permalink raw reply related
* [PATCH v2 4/6] git-gui: Add Japanese language code
From: Satoshi Yasushima @ 2016-09-06 16:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Pat Thoyts,
Satoshi Yasushima
In-Reply-To: <1473177741-9576-1-git-send-email-s.yasushima@gmail.com>
Signed-off-by: Satoshi Yasushima <s.yasushima@gmail.com>
---
po/ja.po | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/po/ja.po b/po/ja.po
index b140e8b..23974cc 100644
--- a/po/ja.po
+++ b/po/ja.po
@@ -11,7 +11,7 @@ msgstr ""
"PO-Revision-Date: 2010-02-02 19:03+0900\n"
"Last-Translator: しらいし ななこ <nanako3@lavabit.com>\n"
"Language-Team: Japanese\n"
-"Language: \n"
+"Language: ja\n"
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=UTF-8\n"
"Content-Transfer-Encoding: 8bit\n"
--
2.8.2.windows.1
^ permalink raw reply related
* [PATCH v2 5/6] git-gui: Update Japanese translation
From: Satoshi Yasushima @ 2016-09-06 16:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Pat Thoyts,
Satoshi Yasushima
In-Reply-To: <1473177741-9576-1-git-send-email-s.yasushima@gmail.com>
Signed-off-by: Satoshi Yasushima <s.yasushima@gmail.com>
---
po/ja.po | 77 +++++++++++++++++++++++++++++-----------------------------------
1 file changed, 35 insertions(+), 42 deletions(-)
diff --git a/po/ja.po b/po/ja.po
index 23974cc..deaf8e3 100644
--- a/po/ja.po
+++ b/po/ja.po
@@ -102,6 +102,8 @@ msgstr "準備完了"
msgid ""
"Display limit (gui.maxfilesdisplayed = %s) reached, not showing all %s files."
msgstr ""
+"表示可能な限界 (gui.maxfilesdisplayed = %s) に達しため、全体で%s個のファイル"
+"を表示できません"
#: git-gui.sh:2101
msgid "Unmodified"
@@ -128,23 +130,20 @@ msgid "File type changed, not staged"
msgstr "ファイル型変更、コミット未予定"
#: git-gui.sh:2109 git-gui.sh:2110
-#, fuzzy
msgid "File type changed, old type staged for commit"
-msgstr "ファイル型変更、コミット未予定"
+msgstr "ファイル型変更、旧型コミット予定済"
#: git-gui.sh:2111
msgid "File type changed, staged"
msgstr "ファイル型変更、コミット予定済"
#: git-gui.sh:2112
-#, fuzzy
msgid "File type change staged, modification not staged"
-msgstr "ファイル型変更、コミット未予定"
+msgstr "ファイル型変更コミット予定済、変更コミット未予定"
#: git-gui.sh:2113
-#, fuzzy
msgid "File type change staged, file missing"
-msgstr "ファイル型変更、コミット予定済"
+msgstr "ファイル型変更コミット予定済、ファイル無し"
#: git-gui.sh:2115
msgid "Untracked, not staged"
@@ -408,10 +407,9 @@ msgstr "SSH キーを表示"
#: git-gui.sh:3014 git-gui.sh:3146
msgid "Usage"
-msgstr ""
+msgstr "使い方"
#: git-gui.sh:3095 lib/blame.tcl:573
-#, fuzzy
msgid "Error"
msgstr "エラー"
@@ -1112,9 +1110,8 @@ msgid "Find Text..."
msgstr "テキストを検索"
#: lib/blame.tcl:288
-#, fuzzy
msgid "Goto Line..."
-msgstr "複製…"
+msgstr "指定行に移動…"
#: lib/blame.tcl:297
msgid "Do Full Copy Detection"
@@ -1310,7 +1307,7 @@ msgstr "共有(最高速・非推奨・バックアップ無し)"
#: lib/choose_repository.tcl:545
msgid "Recursively clone submodules too"
-msgstr ""
+msgstr "サブモジュールも再帰的に複製する"
#: lib/choose_repository.tcl:579 lib/choose_repository.tcl:626
#: lib/choose_repository.tcl:772 lib/choose_repository.tcl:842
@@ -1435,12 +1432,11 @@ msgstr "ファイル"
#: lib/choose_repository.tcl:981
msgid "Cannot clone submodules."
-msgstr ""
+msgstr "サブモジュールが複製できません。"
#: lib/choose_repository.tcl:990
-#, fuzzy
msgid "Cloning submodules"
-msgstr "%s から複製しています"
+msgstr "サブモジュールを複製しています"
#: lib/choose_repository.tcl:1015
msgid "Initial file checkout failed."
@@ -1515,11 +1511,11 @@ msgstr "前"
#: lib/search.tcl:52
msgid "RegExp"
-msgstr ""
+msgstr "正規表現"
#: lib/search.tcl:54
msgid "Case"
-msgstr ""
+msgstr "大文字小文字を区別"
#: lib/status_bar.tcl:87
#, tcl-format
@@ -1635,9 +1631,9 @@ msgid "Running %s requires a selected file."
msgstr "ファイルを選択してから %s を起動してください。"
#: lib/tools.tcl:91
-#, fuzzy, tcl-format
+#, tcl-format
msgid "Are you sure you want to run %1$s on file \"%2$s\"?"
-msgstr "本当に %s を起動しますか?"
+msgstr "本当にファイル \"%2$s\"で %1$s を起動しますか?"
#: lib/tools.tcl:95
#, tcl-format
@@ -1817,16 +1813,15 @@ msgstr "トラッキングブランチを合わせる"
#: lib/option.tcl:151
msgid "Use Textconv For Diffs and Blames"
-msgstr ""
+msgstr "diff と注釈に textconv を使う"
#: lib/option.tcl:152
msgid "Blame Copy Only On Changed Files"
msgstr "変更されたファイルのみコピー検知を行なう"
#: lib/option.tcl:153
-#, fuzzy
msgid "Maximum Length of Recent Repositories List"
-msgstr "最近使ったリポジトリ"
+msgstr "最近使ったリポジトリ一覧の上限"
#: lib/option.tcl:154
msgid "Minimum Letters To Blame Copy On"
@@ -1842,7 +1837,7 @@ msgstr "diff の文脈行数"
#: lib/option.tcl:157
msgid "Additional Diff Parameters"
-msgstr ""
+msgstr "diff の追加引数"
#: lib/option.tcl:158
msgid "Commit Message Text Width"
@@ -1858,19 +1853,19 @@ msgstr "ファイル内容のデフォールトエンコーディング"
#: lib/option.tcl:161
msgid "Warn before committing to a detached head"
-msgstr ""
+msgstr "分離 HEAD のコミット前に警告する"
#: lib/option.tcl:162
msgid "Staging of untracked files"
-msgstr ""
+msgstr "管理外のファイルをコミット予定する"
#: lib/option.tcl:163
msgid "Show untracked files"
-msgstr ""
+msgstr "管理外のファイルを表示する"
#: lib/option.tcl:164
msgid "Tab spacing"
-msgstr ""
+msgstr "タブ幅"
#: lib/option.tcl:210
msgid "Change"
@@ -1979,22 +1974,19 @@ msgstr "%s から削除されたトラッキング・ブランチを刈ってい
#: lib/transport.tcl:25
msgid "fetch all remotes"
-msgstr ""
+msgstr "すべてのリモートを取得"
#: lib/transport.tcl:26
-#, fuzzy
msgid "Fetching new changes from all remotes"
-msgstr "%s から新しい変更をフェッチしています"
+msgstr "すべてのリモートから新しい変更をフェッチしています"
#: lib/transport.tcl:40
-#, fuzzy
msgid "remote prune all remotes"
-msgstr "リモート刈込 %s"
+msgstr "リモート刈込 すべてのリモート"
#: lib/transport.tcl:41
-#, fuzzy
msgid "Pruning tracking branches deleted from all remotes"
-msgstr "%s から削除されたトラッキング・ブランチを刈っています"
+msgstr "すべてのリモートから削除されたトラッキング・ブランチを刈っています"
#: lib/transport.tcl:54 lib/transport.tcl:92 lib/transport.tcl:110
#: lib/remote_add.tcl:162
@@ -2247,7 +2239,7 @@ msgstr "コミットに %s を加えています"
#: lib/index.tcl:380
#, tcl-format
msgid "Stage %d untracked files?"
-msgstr ""
+msgstr "管理外の %d ファイルをコミット予定としますか？"
#: lib/index.tcl:428
#, tcl-format
@@ -2452,6 +2444,13 @@ msgid ""
" \n"
" Do you really want to proceed with your Commit?"
msgstr ""
+"分離 HEAD での変更をコミットしようとしています。"
+"これは潜在的に危険な行為で、理由は別のブランチへの切り替えで"
+"変更が消失し、reflog からの事後復旧も困難となるためです。"
+"おそらくこのコミットはキャンセルし新しく作成したブランチで"
+"行うべきです。\n"
+"\n"
+" 本当にコミットを続行しますか？"
#: lib/commit.tcl:290
msgid "Calling commit-msg hook..."
@@ -2593,11 +2592,11 @@ msgstr "%2$s にある %1$s をセットアップします"
#: lib/line.tcl:17
msgid "Goto Line:"
-msgstr ""
+msgstr "行番号"
#: lib/line.tcl:23
msgid "Go"
-msgstr ""
+msgstr "移動"
#: lib/branch_create.tcl:23
msgid "Create Branch"
@@ -2681,9 +2680,3 @@ msgstr "スペルチェッカーが予想外の EOF を返しました"
#: lib/spellcheck.tcl:392
msgid "Spell Checker Failed"
msgstr "スペルチェック失敗"
-
-#~ msgid "Displaying only %s of %s files."
-#~ msgstr "全体で%s個の内の%sファイルだけ表示しています"
-
-#~ msgid "Case-Sensitive"
-#~ msgstr "大文字小文字を区別"
--
2.8.2.windows.1
^ permalink raw reply related
* Re: [PATCH 3/3] diff_grep: add assertions verifying that the buffers are NUL-terminated
From: Johannes Schindelin @ 2016-09-06 16:04 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20160906070836.7brjtijxq7nukjkq@sigill.intra.peff.net>
Hi Peff,
On Tue, 6 Sep 2016, Jeff King wrote:
> On Mon, Sep 05, 2016 at 05:45:09PM +0200, Johannes Schindelin wrote:
>
> > Before calling regexec() on the file contents, we better be certain that
> > the strings fulfill the contract of C strings assumed by said function.
>
> If you have a buffer that is exactly "size" bytes and you are worried
> about regexec reading off the end, then...
>
> > diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
> > index 55067ca..88820b6 100644
> > --- a/diffcore-pickaxe.c
> > +++ b/diffcore-pickaxe.c
> > @@ -49,6 +49,8 @@ static int diff_grep(mmfile_t *one, mmfile_t *two,
> > xpparam_t xpp;
> > xdemitconf_t xecfg;
> >
> > + assert(!one || one->ptr[one->size] == '\0');
> > + assert(!two || two->ptr[two->size] == '\0');
> > if (!one)
> > return !regexec(regexp, two->ptr, 1, ®match, 0);
>
> ...don't your asserts also read off the end?
Yes, they would read off the end, *unless* a NUL was somehow appended to
the buffers.
> So you might still segfault, though you do catch a case where we have N
> bytes of junk before the end of the page (and you have a 255/256 chance
> of catching it).
Right. The assertion may fail, or a segfault happen. In both cases,
assumptions are violated and we need to fix the code.
Ciao,
Dscho
^ 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