* Re: [PATCH 0/2] add format specifiers to display trailers
From: Keller, Jacob E @ 2016-11-29 18:43 UTC (permalink / raw)
To: gitster@pobox.com, jacob.keller@gmail.com
Cc: git@vger.kernel.org, jonathantanmy@google.com
In-Reply-To: <xmqqwpfwkar2.fsf@gitster.mtv.corp.google.com>
On Mon, 2016-11-21 at 09:23 -0800, Junio C Hamano wrote:
> Jacob Keller <jacob.keller@gmail.com> writes:
>
> > > We have %s and %b so that we can reconstruct the whole thing by
> > > using both. It is unclear how %bT fits in this picture. I
> > > wonder
> > > if we also need another placeholder that expands to the body of
> > > the
> > > message without the trailer---otherwise the whole set would
> > > become
> > > incoherent, no?
> >
> > I'm not entirely sure what to do here. I just wanted a way to
> > easily
> > format "just the trailers" of a message. We could add something
> > that
> > formats just the non-trailers, that's not too difficult. Not really
> > sure what I'd call it though.
>
> I was wondering if %(log:<name of a part>) was a better way to go.
>
> %(log:title) and %(log:body) would be equivalents of traditional %s
> and %b, and %(log:body) in turn would be a shorter way to write
> %(log:description)%+(log:trailer), i.e. show the message body, and
> if there is a trailer block, add it after adding a blank line.
>
> Or something like that?
That would work for me.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH v1 1/1] convert: git cherry-pick -Xrenormalize did not work
From: Junio C Hamano @ 2016-11-29 18:42 UTC (permalink / raw)
To: tboegi; +Cc: git, eevee.reply
In-Reply-To: <20161129163023.23403-1-tboegi@web.de>
tboegi@web.de writes:
> From: Torsten Bögershausen <tboegi@web.de>
>
> Working with a repo that used to be all CRLF. At some point it
> was changed to all LF, with `text=auto` in .gitattributes.
> Trying to cherry-pick a commit from before the switchover fails:
>
> $ git cherry-pick -Xrenormalize <commit>
> fatal: CRLF would be replaced by LF in [path]
OK. That's a very clear description of the symptom that can be
observed from the surface.
> Whenever crlf_action is CRLF_TEXT_XXX and not CRLF_AUTO_XXX,
> SAFE_CRLF_RENORMALIZE must be turned into CRLF_SAFE_FALSE.
Aside from needing s/CRLF_SAFE/SAFE_CRLF/, this however lacks
"Otherwise, because of X and Y, Z ends up doing W" to explain
the "must be" part. Care to explain it a bit more?
Thanks.
> Reported-by: Eevee (Lexy Munroe) <eevee@veekun.com>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
>
> Thanks for reporting.
> Here is a less invasive patch.
> Please let me know, if the patch is OK for you
> (email address, does it work..)
>
> convert.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/convert.c b/convert.c
> index be91358..526ec1d 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -286,7 +286,9 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
> checksafe = SAFE_CRLF_FALSE;
> else if (has_cr_in_index(path))
> convert_crlf_into_lf = 0;
> - }
> + } else if (checksafe == SAFE_CRLF_RENORMALIZE)
> + checksafe = SAFE_CRLF_FALSE;
> +
> if (checksafe && len) {
> struct text_stat new_stats;
> memcpy(&new_stats, &stats, sizeof(new_stats));
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2016, #06; Mon, 28)
From: Jeff King @ 2016-11-29 18:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqfumagmso.fsf@gitster.mtv.corp.google.com>
On Tue, Nov 29, 2016 at 10:31:51AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I'm actually considering scrapping the approach you've queued above, and
> > just teaching verify_path() to reject any index entry starting with
> > ".git" that is a symlink.
>
> Hmph, that's a thought.
I was resistant to it at first because we'll have to deal with all of
the headaches of matching case-folding, but if we just match ".git*" and
not ".gitmodules", ".gitattributes", etc, it actually gets easier. I
think we can basically build off of the existing is_hfs_dotgit() and
is_ntfs_dotgit() functions.
I haven't written the code yet, though, so there may be complications.
-Peff
^ permalink raw reply
* Re: [PATCH] RelNotes: typo fix in 2.11.0 notes
From: Junio C Hamano @ 2016-11-29 18:35 UTC (permalink / raw)
To: Tobias Klauser; +Cc: git
In-Reply-To: <20161129095725.13280-1-tklauser@distanz.ch>
Tobias Klauser <tklauser@distanz.ch> writes:
> s/paht/path/ in the "Backwards compatibility notes" section of the
> 2.11.0 release notes.
>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> ---
This looks somewhat familiar. Perhaps
https://public-inbox.org/git/1477668782.1869.4.camel@seestieto.com/
> Documentation/RelNotes/2.11.0.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/RelNotes/2.11.0.txt b/Documentation/RelNotes/2.11.0.txt
> index b7b7dd361ef0..4c8a9be60f52 100644
> --- a/Documentation/RelNotes/2.11.0.txt
> +++ b/Documentation/RelNotes/2.11.0.txt
> @@ -5,7 +5,7 @@ Backward compatibility notes.
>
> * An empty string used as a pathspec element has always meant
> 'everything matches', but it is too easy to write a script that
> - finds a path to remove in $path and run 'git rm "$paht"' by
> + finds a path to remove in $path and run 'git rm "$path"' by
> mistake (when the user meant to give "$path"), which ends up
> removing everything. This release starts warning about the
> use of an empty string that is used for 'everything matches' and
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2016, #06; Mon, 28)
From: Junio C Hamano @ 2016-11-29 18:31 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20161129065912.xa7itc3os425mr3r@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I'm actually considering scrapping the approach you've queued above, and
> just teaching verify_path() to reject any index entry starting with
> ".git" that is a symlink.
Hmph, that's a thought.
^ permalink raw reply
* Re: gitk crashes on RHEL
From: Stefan Beller @ 2016-11-29 17:41 UTC (permalink / raw)
To: Alessandro Renieri, Paul Mackerras, eric.frederich, Stefan Naewe,
rappazzo
Cc: git@vger.kernel.org
In-Reply-To: <CAKXGFGMGqnGJSEBx8=FXfG3pGEcpFGjLNUH23VTo4LEo75kTKg@mail.gmail.com>
On Tue, Nov 29, 2016 at 1:51 AM, Alessandro Renieri <a.renieri@gmail.com> wrote:
> On Redhat Enterprise gitk returns the following error when launched:
>
> Error in startup script: unknown color name "lime"
> (processing "-fill" option)
> invoked from within
> "$progresscanv create rect -1 0 0 $h -fill lime"
> (procedure "makewindow" line 201)
> invoked from within
> "makewindow"
> (file "/..../bin/git-exe/bin/gitk" line 12434)
>
> The fix is to change lime with {lime green}
>
> Regards
+cc Paul Mackeras, and people involved in the last bug report
See discussion at
https://public-inbox.org/git/CAAoZyYNnWk-yE9TG_Fpxxs-oRN-yEsm_YFs+Ej7muQ+5YCW43w@mail.gmail.com/#t
^ permalink raw reply
* Re: [PATCH 31/35] pathspec: allow querying for attributes
From: Stefan Beller @ 2016-11-29 17:37 UTC (permalink / raw)
To: Brandon Williams; +Cc: Junio C Hamano, Duy Nguyen, git@vger.kernel.org
In-Reply-To: <20161128221123.GC150448@google.com>
On Mon, Nov 28, 2016 at 2:11 PM, Brandon Williams <bmwill@google.com> wrote:
> On 11/10, Stefan Beller wrote:
>> @@ -500,6 +586,18 @@ void copy_pathspec(struct pathspec *dst, const struct pathspec *src)
>>
>> void clear_pathspec(struct pathspec *pathspec)
>> {
>> + int i, j;
>> + for (i = 0; i < pathspec->nr; i++) {
>> + if (!pathspec->items[i].attr_match_nr)
>> + continue;
>> + for (j = 0; j < pathspec->items[j].attr_match_nr; j++)
>> + free(pathspec->items[i].attr_match[j].value);
>> + free(pathspec->items[i].attr_match);
>> + if (pathspec->items[i].attr_check)
>> + git_attr_check_clear(pathspec->items[i].attr_check);
>> + free(pathspec->items[i].attr_check);
>> + }
>> +
>> free(pathspec->items);
>> pathspec->items = NULL;
>
> You may also want to add logic like this to the 'copy_pathspec' function
> so that when a pathspec struct is copied, the destination also has
> ownership of its own attribute items.
>
Thanks for the review comments, I'll plan on resending this series after the
submodule checkout series (which is nowhere near done, but the foundation
is set with sb/submodule-intern-gitdir as a preparation.)
After discussion with Jonathan Nieder, I think it may be possible to make
this new pathspec magic work without the need for thread safety. This is
archived by constructing all relevant attr_check stacks before the preload_index
functionality (which is threaded) and then use the preconstructed attr_checks
to get attr_results just like in this series.
That approach would come with the benefit of not needing mutexes
at all inside the attr code.
^ permalink raw reply
* [PATCH] difftool.c: mark a file-local symbol with static
From: Ramsay Jones @ 2016-11-29 17:36 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, GIT Mailing-list
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
Hi Johannes,
If you need to re-roll your 'js/difftool-builtin' branch, could
you please squash this into the relevant patch.
Thanks!
Also, due to a problem in my config.mak file on Linux (a commented
out line that had a line continuation '\', grrrrr!), gcc issued a
warning, thus:
builtin/difftool.c: In function ‘run_dir_diff’:
builtin/difftool.c:568:13: warning: zero-length gnu_printf format string [-Wformat-zero-length]
warning("");
^
I am not sure why -Wno-format-zero-length is set in DEVELOPER_CFLAGS,
but do you really need to space the output with an an 'empty'
"warning:" line? (Just curious).
ATB,
Ramsay Jones
builtin/difftool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/difftool.c b/builtin/difftool.c
index 3480920..830369c 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -170,7 +170,7 @@ struct path_entry {
char path[FLEX_ARRAY];
};
-int path_entry_cmp(struct path_entry *a, struct path_entry *b, void *key)
+static int path_entry_cmp(struct path_entry *a, struct path_entry *b, void *key)
{
return strcmp(a->path, key ? key : b->path);
}
--
2.9.0
^ permalink raw reply related
* [PATCH v1 1/1] convert: git cherry-pick -Xrenormalize did not work
From: tboegi @ 2016-11-29 16:30 UTC (permalink / raw)
To: git, eevee.reply; +Cc: Torsten Bögershausen
In-Reply-To: <6a7e155-f399-c9f8-c69e-8164e0735dfb@veekun.com>
From: Torsten Bögershausen <tboegi@web.de>
Working with a repo that used to be all CRLF. At some point it
was changed to all LF, with `text=auto` in .gitattributes.
Trying to cherry-pick a commit from before the switchover fails:
$ git cherry-pick -Xrenormalize <commit>
fatal: CRLF would be replaced by LF in [path]
Whenever crlf_action is CRLF_TEXT_XXX and not CRLF_AUTO_XXX,
SAFE_CRLF_RENORMALIZE must be turned into CRLF_SAFE_FALSE.
Reported-by: Eevee (Lexy Munroe) <eevee@veekun.com>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
Thanks for reporting.
Here is a less invasive patch.
Please let me know, if the patch is OK for you
(email address, does it work..)
convert.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/convert.c b/convert.c
index be91358..526ec1d 100644
--- a/convert.c
+++ b/convert.c
@@ -286,7 +286,9 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
checksafe = SAFE_CRLF_FALSE;
else if (has_cr_in_index(path))
convert_crlf_into_lf = 0;
- }
+ } else if (checksafe == SAFE_CRLF_RENORMALIZE)
+ checksafe = SAFE_CRLF_FALSE;
+
if (checksafe && len) {
struct text_stat new_stats;
memcpy(&new_stats, &stats, sizeof(new_stats));
--
2.10.0
^ permalink raw reply related
* Re: [PATCH v2 00/11] git worktree (re)move
From: Duy Nguyen @ 2016-11-29 13:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Git Mailing List, Johannes Sixt
In-Reply-To: <CACsJy8DQDPzZGJXLpTVHVFUeupPpp5e=b9z4m7xceJWrxPfF3Q@mail.gmail.com>
On Tue, Nov 29, 2016 at 07:08:16PM +0700, Duy Nguyen wrote:
> On Tue, Nov 29, 2016 at 3:20 AM, Junio C Hamano <gitster@pobox.com> wrote:
> > Junio C Hamano <gitster@pobox.com> writes:
> >
> >> Does this round address the issue raised in
> >>
> >> http://public-inbox.org/git/alpine.DEB.2.20.1611161041040.3746@virtualbox
> >>
> >> by Dscho?
>
> It does not (and is sort of expected), quoting from the commit message
>
> copy.c: convert copy_file() to copy_dir_recursively()
>
> This finally enables busybox's copy_file() code under a new name
> (because "copy_file" is already taken in Git code base). Because this
> comes from busybox, POSIXy (or even Linuxy) behavior is expected. More
> changes may be needed for Windows support.
>
> I could "#ifdef WINDOWS return -ENOSYS" for now, which would make it
> build. "git worktree move" won't work on Windows of course...
Another way, as pointed out by j6t, is go with "move within filesystem
only", at least at the first step. Which is probably a good idea
anyway so we can concentrate on git-specific stuff before going to
minor and complicated copy/move details.
If you drop all the "copy.c: " patches and squash this to "worktree
move: new command", and if Windows rename() can move directories, then
git should build and new tests pass.
-- 8< --
diff --git a/builtin/worktree.c b/builtin/worktree.c
index f114965..d8d0127 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -569,9 +569,9 @@ static int move_worktree(int ac, const char **av, const char *prefix)
wt->path, dst.buf);
/* second try.. */
- if (copy_dir_recursively(wt->path, dst.buf))
- die(_("failed to copy '%s' to '%s'"),
- wt->path, dst.buf);
+ if (copy_dir_recursively(dst.buf, wt->path))
+ die_errno(_("failed to copy '%s' to '%s'"),
+ wt->path, dst.buf);
else {
struct strbuf sb = STRBUF_INIT;
diff --git a/cache.h b/cache.h
index a50a61a..2d4edf6 100644
--- a/cache.h
+++ b/cache.h
@@ -1857,6 +1857,7 @@ extern void fprintf_or_die(FILE *, const char *fmt, ...);
extern int copy_fd(int ifd, int ofd);
extern int copy_file(const char *dst, const char *src, int mode);
extern int copy_file_with_time(const char *dst, const char *src, int mode);
+extern int copy_dir_recursively(const char *dst, const char *src);
extern void write_or_die(int fd, const void *buf, size_t count);
extern void fsync_or_die(int fd, const char *);
diff --git a/copy.c b/copy.c
index 4de6a11..b232aec 100644
--- a/copy.c
+++ b/copy.c
@@ -65,3 +65,9 @@ int copy_file_with_time(const char *dst, const char *src, int mode)
return copy_times(dst, src);
return status;
}
+
+int copy_dir_recursively(const char *dst, const char *src)
+{
+ errno = ENOSYS;
+ return -1;
+}
-- 8< --
^ permalink raw reply related
* [GIT PULL] l10n updates for 2.11.0 round 3 with ru and ca translations
From: Jiang Xin @ 2016-11-29 13:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Dimitriy Ryazantcev, Alex Henrie, Git List
Hi Junio,
Final l10n updates for this release cycle, please pull.
The following changes since commit 6366c34b895613482fa32f1abe1c3ca043905ad2:
l10n: de.po: translate 210 new messages (2016-11-28 18:49:25 +0100)
are available in the git repository at:
git://github.com/git-l10n/git-po tags/l10n-2.11.0-rnd3.1
for you to fetch changes up to 082ed8f8706bdb0645ceb13f8ba3cb8ccfb58d5d:
Merge branch 'russian-l10n' of https://github.com/DJm00n/git-po-ru
(2016-11-29 21:19:43 +0800)
----------------------------------------------------------------
l10n-2.11.0-rnd3.1: update ru and ca translations
----------------------------------------------------------------
Alex Henrie (1):
l10n: ca.po: update translation
Dimitriy Ryazantcev (1):
l10n: ru.po: update Russian translation
Jiang Xin (1):
Merge branch 'russian-l10n' of https://github.com/DJm00n/git-po-ru
po/ca.po | 8907 ++++++++++++++++++++++++++++++++++----------------------------
po/ru.po | 8340 ++++++++++++++++++++++++++++++++--------------------------
2 files changed, 9473 insertions(+), 7774 deletions(-)
--
Jiang Xin
^ permalink raw reply
* Re: [PATCH] allow git-p4 to create shelved changelists
From: Luke Diamand @ 2016-11-29 12:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Vinicius Kursancew, Lars Schneider, Git Users
In-Reply-To: <xmqqeg1vjug2.fsf@gitster.mtv.corp.google.com>
On 28 November 2016 at 19:06, Junio C Hamano <gitster@pobox.com> wrote:
> Vinicius Kursancew <viniciusalexandre@gmail.com> writes:
>
>> This patch adds a "--shelve" option to the submit subcommand, it will
>> save the changes to a perforce shelve instead of commiting them.
Looks good to me, thanks!
Works a treat.
Ack.
>>
>> Vinicius Kursancew (1):
>> git-p4: allow submit to create shelved changelists.
>>
>> Documentation/git-p4.txt | 5 +++++
>> git-p4.py | 36 ++++++++++++++++++++++--------------
>> t/t9807-git-p4-submit.sh | 31 +++++++++++++++++++++++++++++++
>> 3 files changed, 58 insertions(+), 14 deletions(-)
>
> Thanks, but I am a wrong person to review this change, so I'll
> summon two people who appear in "git shortlog --since=18.months"
> output to help review it.
>
>
^ permalink raw reply
* Re: [PATCH v2 00/11] git worktree (re)move
From: Duy Nguyen @ 2016-11-29 12:17 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, Johannes Schindelin, Git Mailing List
In-Reply-To: <73836804-a581-85a4-b0c0-2aa46a7f9b8d@kdbg.org>
On Tue, Nov 29, 2016 at 4:25 AM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 28.11.2016 um 21:20 schrieb Junio C Hamano:
>>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Does this round address the issue raised in
>>>
>>>
>>> http://public-inbox.org/git/alpine.DEB.2.20.1611161041040.3746@virtualbox
>>>
>>> by Dscho?
>>>
>>> Even if you are not tracking a fifo, for example, your working tree
>>> may have one created in t/trash* directory during testing, and
>>> letting platform "cp -r" taking care of it (if that is possible---I
>>> didn't look at the code that calls busybox copy to see if you are
>>> doing something exotic or just blindly copying everything in the
>>> directory) may turn out to be a more portable way to do this, and I
>>> suspect that the cost of copying one whole-tree would dominate the
>>> run_command() overhead.
>>
>>
>> Please do not take the above as me saying "you must spawn the
>> platform cp -r".
>
>
> copy_dir_recursively is used in 'worktree move' when the move is across file
> systems. My stance on it is to punt in this case. *I* would not trust Git,
> or any other program that is not *specifically* made to copy a whole
> directory structure, to get all cases right when a simple rename() is not
> sufficent.
This is why I did not write new copy code. The code was from busybox,
probably battle tested for many years now. Of course bugs can slip in
when I integrated it to git.
> And, uh, oh, it does a remove_dir_recursively() after it has
> finshed copying. No, Git is not a tool to move directories, thank you very
> much!
I guess you won't like my (unsent) patch for moving .git dir either
;-) which does make me nervous. The thing is, these operations require
some more modification in .git. We can't ask the user to "move this
directory yourself, then come back to me and I will fix up the rest in
.git". First step "only support moving within the same filesystem"
works for me. But I don't know how rename() works on Windows...
--
Duy
^ permalink raw reply
* Re: [PATCH v2 00/11] git worktree (re)move
From: Duy Nguyen @ 2016-11-29 12:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <xmqqshqbicga.fsf@gitster.mtv.corp.google.com>
On Tue, Nov 29, 2016 at 3:20 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Does this round address the issue raised in
>>
>> http://public-inbox.org/git/alpine.DEB.2.20.1611161041040.3746@virtualbox
>>
>> by Dscho?
It does not (and is sort of expected), quoting from the commit message
copy.c: convert copy_file() to copy_dir_recursively()
This finally enables busybox's copy_file() code under a new name
(because "copy_file" is already taken in Git code base). Because this
comes from busybox, POSIXy (or even Linuxy) behavior is expected. More
changes may be needed for Windows support.
I could "#ifdef WINDOWS return -ENOSYS" for now, which would make it
build. "git worktree move" won't work on Windows of course...
>> Even if you are not tracking a fifo, for example, your working tree
>> may have one created in t/trash* directory during testing, and
>> letting platform "cp -r" taking care of it (if that is possible---I
>> didn't look at the code that calls busybox copy to see if you are
>> doing something exotic or just blindly copying everything in the
>> directory) may turn out to be a more portable way to do this, and I
>> suspect that the cost of copying one whole-tree would dominate the
>> run_command() overhead.
>
> Please do not take the above as me saying "you must spawn the
> platform cp -r".
For the the record this was my first move but it will make it much
harder to handle errors, and there's no native "cp" on Windows.
--
Duy
^ permalink raw reply
* git-gui paste not working
From: Peter Flood @ 2016-11-29 10:13 UTC (permalink / raw)
To: git
I've just upgraded to macOS 10.12.1 and now I can't paste into git-gui
(the commit message box).
It seems to work if the text is copied from within git-gui but not from
outside. I've tried using the latest dmg from
https://git-scm.com/download/mac (2.10.1) but it doesn't fix the problem.
^ permalink raw reply
* [PATCH] RelNotes: typo fix in 2.11.0 notes
From: Tobias Klauser @ 2016-11-29 9:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqinrdlr3o.fsf@gitster.mtv.corp.google.com>
s/paht/path/ in the "Backwards compatibility notes" section of the
2.11.0 release notes.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
Documentation/RelNotes/2.11.0.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/RelNotes/2.11.0.txt b/Documentation/RelNotes/2.11.0.txt
index b7b7dd361ef0..4c8a9be60f52 100644
--- a/Documentation/RelNotes/2.11.0.txt
+++ b/Documentation/RelNotes/2.11.0.txt
@@ -5,7 +5,7 @@ Backward compatibility notes.
* An empty string used as a pathspec element has always meant
'everything matches', but it is too easy to write a script that
- finds a path to remove in $path and run 'git rm "$paht"' by
+ finds a path to remove in $path and run 'git rm "$path"' by
mistake (when the user meant to give "$path"), which ends up
removing everything. This release starts warning about the
use of an empty string that is used for 'everything matches' and
--
2.11.0.rc3.5.g7cdf2ab.dirty
^ permalink raw reply related
* gitk crashes on RHEL
From: Alessandro Renieri @ 2016-11-29 9:51 UTC (permalink / raw)
To: git
On Redhat Enterprise gitk returns the following error when launched:
Error in startup script: unknown color name "lime"
(processing "-fill" option)
invoked from within
"$progresscanv create rect -1 0 0 $h -fill lime"
(procedure "makewindow" line 201)
invoked from within
"makewindow"
(file "/..../bin/git-exe/bin/gitk" line 12434)
The fix is to change lime with {lime green}
Regards
^ permalink raw reply
* [PATCH 1/2] mergetool: honor mergetool.$tool.trustExitCode for built-in tools
From: David Aguilar @ 2016-11-29 9:38 UTC (permalink / raw)
To: Git ML; +Cc: Junio C Hamano, Dun Peal
Built-in merge tools contain a hard-coded assumption about
whether or not a tool's exit code can be trusted to determine
the success or failure of a merge. Tools whose exit codes are
not trusted contain calls to check_unchanged() in their
merge_cmd() functions.
A problem with this is that the trustExitCode configuration is
not honored for built-in tools.
Teach built-in tools to honor the trustExitCode configuration.
Extend run_merge_cmd() so that it is responsible for calling
check_unchanged() when a tool's exit code cannot be trusted.
Remove check_unchanged() calls from scriptlets since they are no
longer responsible for calling it.
When no configuration is present, exit_code_trustable() is
checked to see whether the exit code should be trusted.
The default implementation returns false.
Tools whose exit codes can be trusted override
exit_code_trustable() to true.
Reported-by: Dun Peal <dunpealer@gmail.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
git-mergetool--lib.sh | 56 ++++++++++++++++++++++++++++++++++++++----------
mergetools/araxis | 2 --
mergetools/bc | 2 --
mergetools/codecompare | 2 --
mergetools/deltawalker | 6 +++++-
mergetools/diffmerge | 4 ++++
mergetools/diffuse | 2 --
mergetools/ecmerge | 2 --
mergetools/emerge | 4 ++++
mergetools/examdiff | 2 --
mergetools/kdiff3 | 4 ++++
mergetools/kompare | 4 ++++
mergetools/meld | 3 +--
mergetools/opendiff | 2 --
mergetools/p4merge | 2 --
mergetools/tkdiff | 4 ++++
mergetools/tortoisemerge | 2 --
mergetools/vimdiff | 2 --
mergetools/winmerge | 2 --
mergetools/xxdiff | 2 --
20 files changed, 71 insertions(+), 38 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 9abd00be2..9a8b97a2a 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -125,16 +125,7 @@ setup_user_tool () {
}
merge_cmd () {
- trust_exit_code=$(git config --bool \
- "mergetool.$1.trustExitCode" || echo false)
- if test "$trust_exit_code" = "false"
- then
- touch "$BACKUP"
- ( eval $merge_tool_cmd )
- check_unchanged
- else
- ( eval $merge_tool_cmd )
- fi
+ ( eval $merge_tool_cmd )
}
}
@@ -162,6 +153,28 @@ setup_tool () {
echo "$1"
}
+ # Most tools' exit codes cannot be trusted, so By default we ignore
+ # their exit code and check the merged file's modification time in
+ # check_unchanged() to determine whether or not the merge was
+ # successful. The return value from run_merge_cmd, by default, is
+ # determined by check_unchanged().
+ #
+ # When a tool's exit code can be trusted then the return value from
+ # run_merge_cmd is simply the tool's exit code, and check_unchanged()
+ # is not called.
+ #
+ # The return value of exit_code_trustable() tells us whether or not we
+ # can trust the tool's exit code.
+ #
+ # User-defined and built-in tools default to false.
+ # Built-in tools advertise that their exit code is trustable by
+ # redefining exit_code_trustable() to true.
+
+ exit_code_trustable () {
+ false
+ }
+
+
if ! test -f "$MERGE_TOOLS_DIR/$tool"
then
setup_user_tool
@@ -197,6 +210,19 @@ get_merge_tool_cmd () {
fi
}
+trust_exit_code () {
+ if git config --bool "mergetool.$1.trustExitCode"
+ then
+ :; # OK
+ elif exit_code_trustable
+ then
+ echo true
+ else
+ echo false
+ fi
+}
+
+
# Entry point for running tools
run_merge_tool () {
# If GIT_PREFIX is empty then we cannot use it in tools
@@ -225,7 +251,15 @@ run_diff_cmd () {
# Run a either a configured or built-in merge tool
run_merge_cmd () {
- merge_cmd "$1"
+ mergetool_trust_exit_code=$(trust_exit_code "$1")
+ if test "$mergetool_trust_exit_code" = "true"
+ then
+ merge_cmd "$1"
+ else
+ touch "$BACKUP"
+ merge_cmd "$1"
+ check_unchanged
+ fi
}
list_merge_tool_candidates () {
diff --git a/mergetools/araxis b/mergetools/araxis
index 64f97c5e9..e2407b65b 100644
--- a/mergetools/araxis
+++ b/mergetools/araxis
@@ -3,7 +3,6 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if $base_present
then
"$merge_tool_path" -wait -merge -3 -a1 \
@@ -12,7 +11,6 @@ merge_cmd () {
"$merge_tool_path" -wait -2 \
"$LOCAL" "$REMOTE" "$MERGED" >/dev/null 2>&1
fi
- check_unchanged
}
translate_merge_tool_path() {
diff --git a/mergetools/bc b/mergetools/bc
index b6319d206..3a69e60fa 100644
--- a/mergetools/bc
+++ b/mergetools/bc
@@ -3,7 +3,6 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if $base_present
then
"$merge_tool_path" "$LOCAL" "$REMOTE" "$BASE" \
@@ -12,7 +11,6 @@ merge_cmd () {
"$merge_tool_path" "$LOCAL" "$REMOTE" \
-mergeoutput="$MERGED"
fi
- check_unchanged
}
translate_merge_tool_path() {
diff --git a/mergetools/codecompare b/mergetools/codecompare
index 3f0486bc8..9f60e8da6 100644
--- a/mergetools/codecompare
+++ b/mergetools/codecompare
@@ -3,7 +3,6 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if $base_present
then
"$merge_tool_path" -MF="$LOCAL" -TF="$REMOTE" -BF="$BASE" \
@@ -12,7 +11,6 @@ merge_cmd () {
"$merge_tool_path" -MF="$LOCAL" -TF="$REMOTE" \
-RF="$MERGED"
fi
- check_unchanged
}
translate_merge_tool_path() {
diff --git a/mergetools/deltawalker b/mergetools/deltawalker
index b3c71b623..ee6f374bc 100644
--- a/mergetools/deltawalker
+++ b/mergetools/deltawalker
@@ -16,6 +16,10 @@ merge_cmd () {
fi >/dev/null 2>&1
}
-translate_merge_tool_path() {
+translate_merge_tool_path () {
echo DeltaWalker
}
+
+exit_code_trustable () {
+ true
+}
diff --git a/mergetools/diffmerge b/mergetools/diffmerge
index f138cb4e7..9b6355b98 100644
--- a/mergetools/diffmerge
+++ b/mergetools/diffmerge
@@ -12,3 +12,7 @@ merge_cmd () {
--result="$MERGED" "$LOCAL" "$REMOTE"
fi
}
+
+exit_code_trustable () {
+ true
+}
diff --git a/mergetools/diffuse b/mergetools/diffuse
index 02e0843f4..5a3ae8b56 100644
--- a/mergetools/diffuse
+++ b/mergetools/diffuse
@@ -3,7 +3,6 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if $base_present
then
"$merge_tool_path" \
@@ -13,5 +12,4 @@ merge_cmd () {
"$merge_tool_path" \
"$LOCAL" "$MERGED" "$REMOTE" | cat
fi
- check_unchanged
}
diff --git a/mergetools/ecmerge b/mergetools/ecmerge
index 13c2e439d..6c5101c4f 100644
--- a/mergetools/ecmerge
+++ b/mergetools/ecmerge
@@ -3,7 +3,6 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if $base_present
then
"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
@@ -12,5 +11,4 @@ merge_cmd () {
"$merge_tool_path" "$LOCAL" "$REMOTE" \
--default --mode=merge2 --to="$MERGED"
fi
- check_unchanged
}
diff --git a/mergetools/emerge b/mergetools/emerge
index 7b895fdb1..d1ce513ff 100644
--- a/mergetools/emerge
+++ b/mergetools/emerge
@@ -20,3 +20,7 @@ merge_cmd () {
translate_merge_tool_path() {
echo emacs
}
+
+exit_code_trustable () {
+ true
+}
diff --git a/mergetools/examdiff b/mergetools/examdiff
index 7b524d408..e72b06fc4 100644
--- a/mergetools/examdiff
+++ b/mergetools/examdiff
@@ -3,14 +3,12 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if $base_present
then
"$merge_tool_path" -merge "$LOCAL" "$BASE" "$REMOTE" -o:"$MERGED" -nh
else
"$merge_tool_path" -merge "$LOCAL" "$REMOTE" -o:"$MERGED" -nh
fi
- check_unchanged
}
translate_merge_tool_path() {
diff --git a/mergetools/kdiff3 b/mergetools/kdiff3
index 793d1293b..0264ed5b2 100644
--- a/mergetools/kdiff3
+++ b/mergetools/kdiff3
@@ -21,3 +21,7 @@ merge_cmd () {
>/dev/null 2>&1
fi
}
+
+exit_code_trustable () {
+ true
+}
diff --git a/mergetools/kompare b/mergetools/kompare
index 433686c12..e8c0bfa67 100644
--- a/mergetools/kompare
+++ b/mergetools/kompare
@@ -5,3 +5,7 @@ can_merge () {
diff_cmd () {
"$merge_tool_path" "$LOCAL" "$REMOTE"
}
+
+exit_code_trustable () {
+ true
+}
diff --git a/mergetools/meld b/mergetools/meld
index 83ebdfb4c..bc178e888 100644
--- a/mergetools/meld
+++ b/mergetools/meld
@@ -7,7 +7,7 @@ merge_cmd () {
then
check_meld_for_output_version
fi
- touch "$BACKUP"
+
if test "$meld_has_output_option" = true
then
"$merge_tool_path" --output "$MERGED" \
@@ -15,7 +15,6 @@ merge_cmd () {
else
"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
fi
- check_unchanged
}
# Check whether we should use 'meld --output <file>'
diff --git a/mergetools/opendiff b/mergetools/opendiff
index 0942b2a73..b608dd6de 100644
--- a/mergetools/opendiff
+++ b/mergetools/opendiff
@@ -3,7 +3,6 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if $base_present
then
"$merge_tool_path" "$LOCAL" "$REMOTE" \
@@ -12,5 +11,4 @@ merge_cmd () {
"$merge_tool_path" "$LOCAL" "$REMOTE" \
-merge "$MERGED" | cat
fi
- check_unchanged
}
diff --git a/mergetools/p4merge b/mergetools/p4merge
index 5a608abf9..7a5b291dd 100644
--- a/mergetools/p4merge
+++ b/mergetools/p4merge
@@ -20,14 +20,12 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if ! $base_present
then
cp -- "$LOCAL" "$BASE"
create_virtual_base "$BASE" "$REMOTE"
fi
"$merge_tool_path" "$BASE" "$REMOTE" "$LOCAL" "$MERGED"
- check_unchanged
}
create_empty_file () {
diff --git a/mergetools/tkdiff b/mergetools/tkdiff
index 618c438e8..eee5cb57e 100644
--- a/mergetools/tkdiff
+++ b/mergetools/tkdiff
@@ -10,3 +10,7 @@ merge_cmd () {
"$merge_tool_path" -o "$MERGED" "$LOCAL" "$REMOTE"
fi
}
+
+exit_code_trustable () {
+ true
+}
diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
index 3b89f1c82..d7ab666a5 100644
--- a/mergetools/tortoisemerge
+++ b/mergetools/tortoisemerge
@@ -5,7 +5,6 @@ can_diff () {
merge_cmd () {
if $base_present
then
- touch "$BACKUP"
basename="$(basename "$merge_tool_path" .exe)"
if test "$basename" = "tortoisegitmerge"
then
@@ -17,7 +16,6 @@ merge_cmd () {
-base:"$BASE" -mine:"$LOCAL" \
-theirs:"$REMOTE" -merged:"$MERGED"
fi
- check_unchanged
else
echo "$merge_tool_path cannot be used without a base" 1>&2
return 1
diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index 74ea6d547..a841ffdb4 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -4,7 +4,6 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
case "$1" in
gvimdiff|vimdiff)
if $base_present
@@ -31,7 +30,6 @@ merge_cmd () {
fi
;;
esac
- check_unchanged
}
translate_merge_tool_path() {
diff --git a/mergetools/winmerge b/mergetools/winmerge
index f3819d316..74d03259f 100644
--- a/mergetools/winmerge
+++ b/mergetools/winmerge
@@ -6,10 +6,8 @@ diff_cmd () {
merge_cmd () {
# mergetool.winmerge.trustExitCode is implicitly false.
# touch $BACKUP so that we can check_unchanged.
- touch "$BACKUP"
"$merge_tool_path" -u -e -dl Local -dr Remote \
"$LOCAL" "$REMOTE" "$MERGED"
- check_unchanged
}
translate_merge_tool_path() {
diff --git a/mergetools/xxdiff b/mergetools/xxdiff
index 05b443394..e284811ff 100644
--- a/mergetools/xxdiff
+++ b/mergetools/xxdiff
@@ -6,7 +6,6 @@ diff_cmd () {
}
merge_cmd () {
- touch "$BACKUP"
if $base_present
then
"$merge_tool_path" -X --show-merged-pane \
@@ -21,5 +20,4 @@ merge_cmd () {
-R 'Accel.SearchForward: "Ctrl-G"' \
--merged-file "$MERGED" "$LOCAL" "$REMOTE"
fi
- check_unchanged
}
--
2.11.0.rc3.6.g2e567fd
^ permalink raw reply related
* [PATCH 2/2] mergetools/vimdiff: trust Vim's exit code
From: David Aguilar @ 2016-11-29 9:38 UTC (permalink / raw)
To: Git ML
Cc: Junio C Hamano, Dun Peal, Charles Bailey, Dickson Wong,
Michael J Gruber, Dan McGee, Markus Heidelberg, SZEDER Gábor,
James Bowes
Allow vimdiff users to signal that they do not want to use the
result of a merge by exiting with ":cquit", which tells Vim to
exit with an error code.
This is better than the current behavior because it allows users
to directly flag that the merge is bad, using a standard Vim
feature, rather than relying on a timestamp heuristic that is
unforgiving to users that save in-progress merge files.
The original behavior can be restored by configuring
mergetool.vimdiff.trustExitCode to false.
Reported-by: Dun Peal <dunpealer@gmail.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
I've included anyone that has ever touched the vimdiff feature on the Cc:
list since I'm assuming that you use vimdiff.
This change is a slight change in default behavior when using
mergetool with vimdiff, but I think it's a better default overall.
mergetools/vimdiff | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index a841ffdb4..10d86f3e1 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -42,3 +42,7 @@ translate_merge_tool_path() {
;;
esac
}
+
+exit_code_trustable () {
+ true
+}
--
2.11.0.rc3.6.g2e567fd
^ permalink raw reply related
* Re: [PATCH] diff: handle --no-abbrev outside of repository
From: Jeff King @ 2016-11-29 7:06 UTC (permalink / raw)
To: Jack Bates; +Cc: git, Jack Bates
In-Reply-To: <20161128182508.10570-1-jack@nottheoilrig.com>
On Mon, Nov 28, 2016 at 11:25:08AM -0700, Jack Bates wrote:
> diff --git a/diff.c b/diff.c
> index ec87283..0447eff 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3106,7 +3106,8 @@ static const char *diff_abbrev_oid(const struct object_id *oid, int abbrev)
> abbrev = FALLBACK_DEFAULT_ABBREV;
> if (abbrev > GIT_SHA1_HEXSZ)
> die("BUG: oid abbreviation out of range: %d", abbrev);
> - hex[abbrev] = '\0';
> + if (abbrev)
> + hex[abbrev] = '\0';
> return hex;
> }
This hunk made me wonder if there is a regression in v2.11, as this
fallback code is new in that version. But I don't think so.
This new code doesn't handle abbrev==0 the same as find_unique_abbrev()
does, and that's clearly a bug. But I couldn't find any way to trigger
it with the existing code.
The obvious way is with --no-abbrev, but that doesn't work without yet
without your patch.
A less obvious way is --abbrev=0, but that gets munged internally to
MINIMUM_ABBREV.
The most obscure is "git -c core.abbrev=0", but that barfs completely on
a too-small abbreviation (without even giving a good error message,
which is something we might want to fix).
So I think there is no regression, and we only have to worry about it as
part of the feature you are adding here (it might be worth calling out
the bug fix specifically in the commit message, though, or even putting
it in its own patch).
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2016, #06; Mon, 28)
From: Jeff King @ 2016-11-29 6:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqk2bngn03.fsf@gitster.mtv.corp.google.com>
On Mon, Nov 28, 2016 at 04:15:08PM -0800, Junio C Hamano wrote:
> * jk/nofollow-attr-ignore (2016-11-02) 5 commits
> - exclude: do not respect symlinks for in-tree .gitignore
> - attr: do not respect symlinks for in-tree .gitattributes
> - exclude: convert "check_index" into a flags field
> - attr: convert "macro_ok" into a flags field
> - add open_nofollow() helper
>
> As we do not follow symbolic links when reading control files like
> .gitignore and .gitattributes from the index, match the behaviour
> and not follow symbolic links when reading them from the working
> tree. This also tightens security a bit by not leaking contents of
> an unrelated file in the error messages when it is pointed at by
> one of these files that is a symbolic link.
>
> Perhaps we want to cover .gitmodules too with the same mechanism?
Yes, sorry I haven't pushed that forward. I started on covering
.gitmodules, too, but it's much more complicated than the other two,
because we sometimes read them via "git config -f". So we have to
somehow teach git-config to start using O_NOFOLLOW in those cases.
I'm actually considering scrapping the approach you've queued above, and
just teaching verify_path() to reject any index entry starting with
".git" that is a symlink.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2016, #06; Mon, 28)
From: Jeff King @ 2016-11-29 6:51 UTC (permalink / raw)
To: Brandon Williams; +Cc: Junio C Hamano, git
In-Reply-To: <20161129063759.6mgmpqx3kbyuqjwi@sigill.intra.peff.net>
On Tue, Nov 29, 2016 at 01:37:59AM -0500, Jeff King wrote:
> 2. Grep threads doing more complicated stuff that needs to take a
> lock. You might try building with -fsanitize=thread to see if it
> turns up anything.
I tried this and it didn't find anything useful. It complains about
multiple threads calling want_color() at the same time, which you can
silence with something like:
diff --git a/builtin/grep.c b/builtin/grep.c
index 2c727ef49..d48846f40 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -207,6 +207,12 @@ static void start_threads(struct grep_opt *opt)
{
int i;
+ /*
+ * trigger want_color() for its side effect of caching the result;
+ * otherwise the threads will fight over setting the cache
+ */
+ want_color(GIT_COLOR_AUTO);
+
pthread_mutex_init(&grep_mutex, NULL);
pthread_mutex_init(&grep_read_mutex, NULL);
pthread_mutex_init(&grep_attr_mutex, NULL);
But the problem persists even with that patch, so it is something else.
It may still be a threading problem; -fsanitize=thread isn't perfect. I
also couldn't get the stress-test to fail when compiled with it. But
that may simply mean that the timing of the resulting binary is changed
enough not to trigger the issue.
-Peff
^ permalink raw reply related
* Re: What's cooking in git.git (Nov 2016, #06; Mon, 28)
From: Jeff King @ 2016-11-29 6:37 UTC (permalink / raw)
To: Brandon Williams; +Cc: Junio C Hamano, git
In-Reply-To: <20161129010538.GA121643@google.com>
On Mon, Nov 28, 2016 at 05:05:38PM -0800, Brandon Williams wrote:
> On 11/28, Junio C Hamano wrote:
> > * bw/grep-recurse-submodules (2016-11-22) 6 commits
> > - grep: search history of moved submodules
> > - grep: enable recurse-submodules to work on <tree> objects
> > - grep: optionally recurse into submodules
> > - grep: add submodules as a grep source type
> > - submodules: load gitmodules file from commit sha1
> > - submodules: add helper functions to determine presence of submodules
> >
> > "git grep" learns to optionally recurse into submodules
> >
> > Has anybody else seen t7814 being flakey with this series?
>
> Which tests in particular are you seeing issues with? I can't see any
> issues running it locally.
It looks like tests 14 and 15 are racy. The whole script usually passes,
but if I run it under my stress script[1], it fails within 5-10 seconds
on one of those two.
The failures always look like (this one is from test 15, but the one in
test 14 is similar):
--- expect 2016-11-29 06:26:37.874664486 +0000
+++ actual 2016-11-29 06:26:37.878664486 +0000
@@ -1,2 +1 @@
-file:foobar
sub-moved/file:foobar
I haven't dug into it, but I don't see anything obviously racy-looking
in the test, so presumably it's in the code. Without looking, but
knowing the nature of the code, I'd guess the probable avenues are:
1. A read() or write() that gets split under load (just because
there's processes piping to other processes here).
2. Grep threads doing more complicated stuff that needs to take a
lock. You might try building with -fsanitize=thread to see if it
turns up anything.
-Peff
[1] https://github.com/peff/git/blob/meta/stress
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2016, #05; Wed, 23)
From: Jonathan Tan @ 2016-11-29 4:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Stefan Beller
In-Reply-To: <xmqqpolfgndq.fsf@gitster.mtv.corp.google.com>
On Mon, Nov 28, 2016 at 4:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jonathan Tan <jonathantanmy@google.com> writes:
>
>> On 11/23/2016 03:21 PM, Junio C Hamano wrote:
>>> * jt/use-trailer-api-in-commands (2016-11-02) 6 commits
>>> - sequencer: use trailer's trailer layout
>>> - trailer: have function to describe trailer layout
>>> - trailer: avoid unnecessary splitting on lines
>>> - commit: make ignore_non_trailer take buf/len
>>> - SQUASH???
>>> - trailer: be stricter in parsing separators
>>>
>>> Commands that operate on a log message and add lines to the trailer
>>> blocks, such as "format-patch -s", "cherry-pick (-x|-s)", and
>>> "commit -s", have been taught to use the logic of and share the
>>> code with "git interpret-trailer".
>>>
>>> What's the doneness of this topic?
>>
>> Stefan Beller mentioned [1] that this seemed OK to him from a cursory
>> read. Do I need to look for another reviewer (or a more thorough
>> review)?
>
> I gave it a cursory review when it was queued, too, so another
> cursory read does not help very much ;) If I recall correctly, I
> got an impression that it was reasonably well done.
>
> I haven't had a chance to look at the series again to see if the
> SQUASH is just the simple matter of squashing it into the one
> previous, which is the main reason why I haven't decided if it is
> ready to be in 'next'.
>
> Thanks.
Sorry, I'm not sure what you mean by this. The SQUASH is an update in
documentation (for a C function) (reproduced below [1]) which can be
squashed cleanly (just to confirm, I tried it using "git rebase -i").
It can also be rebased cleanly onto master or next (at least, as of
now).
If you are asking about whether I think the contents of your SQUASH
commit is reasonable, I think that it is.
[1]
$ git show 7e7587d
commit 7e7587d563ddb540875075e5a84359a8a96a5188
Author: Junio C Hamano <gitster@pobox.com>
Date: Wed Nov 2 19:28:53 2016 -0700
SQUASH???
diff --git a/trailer.c b/trailer.c
index dc525e3..eefe91d 100644
--- a/trailer.c
+++ b/trailer.c
@@ -565,7 +565,9 @@ static int token_matches_item(const char *tok,
struct arg_item *item, int tok_le
/*
* If the given line is of the form
* "<token><optional whitespace><separator>..." or "<separator>...", return the
- * location of the separator. Otherwise, return -1.
+ * location of the separator. Otherwise, return -1. The optional whitespace
+ * is allowed there primarily to allow things like "Bug #43" where <token> is
+ * "Bug" and <separator> is "#".
*
* The separator-starts-line case (in which this function returns 0) is
* distinguished from the non-well-formed-line case (in which this function
^ permalink raw reply related
* Re: What's cooking in git.git (Nov 2016, #06; Mon, 28)
From: Brandon Williams @ 2016-11-29 1:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqk2bngn03.fsf@gitster.mtv.corp.google.com>
On 11/28, Junio C Hamano wrote:
> * bw/grep-recurse-submodules (2016-11-22) 6 commits
> - grep: search history of moved submodules
> - grep: enable recurse-submodules to work on <tree> objects
> - grep: optionally recurse into submodules
> - grep: add submodules as a grep source type
> - submodules: load gitmodules file from commit sha1
> - submodules: add helper functions to determine presence of submodules
>
> "git grep" learns to optionally recurse into submodules
>
> Has anybody else seen t7814 being flakey with this series?
Which tests in particular are you seeing issues with? I can't see any
issues running it locally.
--
Brandon Williams
^ 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