* [PATCH 4/6] fsck: tighten error-checks of "git fsck <head>"
From: Jeff King @ 2017-01-16 21:33 UTC (permalink / raw)
To: git; +Cc: Michael Haggerty, Johannes Schindelin
In-Reply-To: <20170116212231.ojoqzlajpszifaf3@sigill.intra.peff.net>
Instead of checking reachability from the refs, you can ask
fsck to check from a particular set of heads. However, the
error checking here is quite lax. In particular:
1. It claims lookup_object() will report an error, which
is not true. It only does a hash lookup, and the user
has no clue that their argument was skipped.
2. When either the name or sha1 cannot be resolved, we
continue to exit with a successful error code, even
though we didn't check what the user asked us to.
This patch fixes both of these cases.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/fsck.c | 7 +++++--
t/t1450-fsck.sh | 5 +++++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index f527d8a02..c7d0590e5 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -755,9 +755,11 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
if (!get_sha1(arg, sha1)) {
struct object *obj = lookup_object(sha1);
- /* Error is printed by lookup_object(). */
- if (!obj)
+ if (!obj) {
+ error("%s: object missing", sha1_to_hex(sha1));
+ errors_found |= ERROR_OBJECT;
continue;
+ }
obj->used = 1;
if (name_objects)
@@ -768,6 +770,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
continue;
}
error("invalid parameter: expected sha1, got '%s'", arg);
+ errors_found |= ERROR_OBJECT;
}
/*
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index c1b2dda33..2f3b05276 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -605,4 +605,9 @@ test_expect_success 'fsck notices dangling objects' '
)
'
+test_expect_success 'fsck $name notices bogus $name' '
+ test_must_fail git fsck bogus &&
+ test_must_fail git fsck $_z40
+'
+
test_done
--
2.11.0.642.gd6f8cda6c
^ permalink raw reply related
* Re: [PATCH 1/2] configure.ac: fix old iconv check
From: Jeff King @ 2017-01-16 21:41 UTC (permalink / raw)
To: Bernd Kuhls; +Cc: git
In-Reply-To: <20170116195638.3713-1-bernd.kuhls@writeme.com>
On Mon, Jan 16, 2017 at 08:56:37PM +0100, Bernd Kuhls wrote:
> According to
> https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Running-the-Compiler.html
> the parameter syntax of AC_COMPILE_IFELSE is
>
> (input, [action-if-true], [action-if-false])
>
> Displaying "no" when the test was positive and enabling support for old
> iconv implementations by OLD_ICONV=UnfortunatelyYes when the test fails
> it obviously wrong. This patch switches the actions to fix the problem.
Hrm. But the test code is:
# Define OLD_ICONV if your library has an old iconv(), where the second
# (input buffer pointer) parameter is declared with type (const char **).
AC_DEFUN([OLDICONVTEST_SRC], [
AC_LANG_PROGRAM([[
#include <iconv.h>
extern size_t iconv(iconv_t cd,
char **inbuf, size_t *inbytesleft,
char **outbuf, size_t *outbytesleft);
]], [])])
Which will compile correctly for the _new_ code, but not the old. So
when the test is positive, then "no", we do not need to set the
OLD_ICONV flag.
The logic would probably be clearer if the test were reversed. It would
mean that other compilation problems (e.g., you do not have iconv.h at
all) would fail to see OLD_ICONV, but that seems reasonable.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2017, #02; Sun, 15)
From: Jeff King @ 2017-01-16 21:44 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Johannes Schindelin, Junio C Hamano, git
In-Reply-To: <677a335f-889c-2924-b7bd-93c2b6663175@kdbg.org>
On Mon, Jan 16, 2017 at 09:33:07PM +0100, Johannes Sixt wrote:
> However, Jeff's patch is intended to catch exactly these cases (not for the
> cases where this happens accidentally, but when they happen with malicious
> intent).
>
> We are talking about user-provided data that is reproduced by die() or
> error(). I daresay that we do not have a single case where it is intended
> that this data is intentionally multi-lined, like a commit message. It can
> only be an accident or malicious when it spans across lines.
>
> I know we allow CR and LF in file names, but in all cases where such a name
> appears in an error message, it is *not important* that the data is
> reproduced exactly. On the contrary, it is usually more helpful to know that
> something strange is going on. The question marks are a strong indication to
> the user for this.
Yes, exactly. Thanks for explaining this better than I obviously was
doing. :)
> > If you absolutely insist, I will spend time to find a plausible example
> > and use that in the regression test.
>
> I don't want to see you on an endeavor with dubious results. I'd prefer to
> wait until the first case of "incorrectly munged data" is reported because,
> as I said, I have a gut feeling that there is none.
Agreed.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2017, #02; Sun, 15)
From: Jeff King @ 2017-01-16 22:00 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Johannes Sixt, Junio C Hamano, git
In-Reply-To: <alpine.DEB.2.20.1701161746200.3469@virtualbox>
On Mon, Jan 16, 2017 at 06:06:35PM +0100, Johannes Schindelin wrote:
> > And I think that would apply to any input parameter we show via
> > error(), etc, if it is connected to a newline (ideally we would
> > show newlines as "?", too, but we cannot tell the difference
> > between ones from parameters, and ones that are part of the error
> > message).
>
> I think it is doing users a really great disservice to munge up CR or LF
> into question marks. I *guarantee* you that it confuses users. And not
> because they are dumb, but because the code violates the Law of Least
> Surprise.
I'm not sure if you realize that with stock git, the example from your
test looks like this (at least in my terminal):
$ git.v2.11.0 rev-parse --abbrev-ref "$(printf 'CR/LF\r\n')" >/dev/null
': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
The "\r" causes us to overwrite the rest of the message, including the
actual filename. With my patch it's:
$ git rev-parse --abbrev-ref "$(printf 'CR/LF\r\n')" >/dev/null
fatal: ambiguous argument 'CR/LF?': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
I am certainly sympathetic to the idea that the "?" is ugly and
potentially confusing. But I think it's at least a step forward for this
particular example.
I'll snip liberally from the rest of your response, because I think what
JSixt wrote covers it.
> > > While at it, let's lose the unnecessary curly braces.
> >
> > Please don't. Obviously C treats the "if/else" as a single unit, but
> > IMHO it's less error-prone to include the braces any time there are
> > multiple visual lines. E.g., something like:
> >
> > while (foo)
> > if (bar)
> > one();
> > else
> > two();
> > three();
> >
> > is much easier to spot as wrong when you would require braces either
> > way (and not relevant here, but I'd say that even an inner block with a
> > comment deserves braces for the same reason).
>
> There is no documentation about the preferred coding style.
Documentation/CodingGuidelines says:
- We avoid using braces unnecessarily. I.e.
if (bla) {
x = 1;
}
is frowned upon. A gray area is when the statement extends
over a few lines, and/or you have a lengthy comment atop of
it. Also, like in the Linux kernel, if there is a long list
of "else if" statements, it can make sense to add braces to
single line blocks.
I think this is pretty clearly the "gray area" mentioned there. Which
yes, does not say "definitely do it this way", but I hope makes it clear
that you're supposed to use judgement about readability.
-Peff
^ permalink raw reply
* [PATCH] CodingGuidelines: clarify multi-line brace style
From: Jeff King @ 2017-01-16 22:08 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Johannes Sixt, Junio C Hamano, git
In-Reply-To: <20170116220014.bwi5xi2br56lyqsw@sigill.intra.peff.net>
On Mon, Jan 16, 2017 at 05:00:14PM -0500, Jeff King wrote:
> > > Please don't. Obviously C treats the "if/else" as a single unit, but
> > > IMHO it's less error-prone to include the braces any time there are
> > > multiple visual lines. E.g., something like:
> > >
> > > while (foo)
> > > if (bar)
> > > one();
> > > else
> > > two();
> > > three();
> > >
> > > is much easier to spot as wrong when you would require braces either
> > > way (and not relevant here, but I'd say that even an inner block with a
> > > comment deserves braces for the same reason).
> >
> > There is no documentation about the preferred coding style.
>
> Documentation/CodingGuidelines says:
>
> - We avoid using braces unnecessarily. I.e.
>
> if (bla) {
> x = 1;
> }
>
> is frowned upon. A gray area is when the statement extends
> over a few lines, and/or you have a lengthy comment atop of
> it. Also, like in the Linux kernel, if there is a long list
> of "else if" statements, it can make sense to add braces to
> single line blocks.
>
> I think this is pretty clearly the "gray area" mentioned there. Which
> yes, does not say "definitely do it this way", but I hope makes it clear
> that you're supposed to use judgement about readability.
So here's a patch.
I know we've usually tried to keep this file to guidelines and not
rules, but clearly it has not been clear-cut enough in this instance.
-- >8 --
Subject: [PATCH] CodingGuidelines: clarify multi-line brace style
There are some "gray areas" around when to omit braces from
a conditional or loop body. Since that seems to have
resulted in some arguments, let's be a little more clear
about our preferred style.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/CodingGuidelines | 37 ++++++++++++++++++++++++++++++++-----
1 file changed, 32 insertions(+), 5 deletions(-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 4cd95da6b..0e336e99d 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -206,11 +206,38 @@ For C programs:
x = 1;
}
- is frowned upon. A gray area is when the statement extends
- over a few lines, and/or you have a lengthy comment atop of
- it. Also, like in the Linux kernel, if there is a long list
- of "else if" statements, it can make sense to add braces to
- single line blocks.
+ is frowned upon. But there are a few exceptions:
+
+ - When the statement extends over a few lines (e.g., a while loop
+ with an embedded conditional, or a comment). E.g.:
+
+ while (foo) {
+ if (x)
+ one();
+ else
+ two();
+ }
+
+ if (foo) {
+ /*
+ * This one requires some explanation,
+ * so we're better off with braces to make
+ * it obvious that the indentation is correct.
+ */
+ doit();
+ }
+
+ - When there are multiple arms to a conditional, it can make
+ sense to add braces to single line blocks for consistency.
+ E.g.:
+
+ if (foo) {
+ doit();
+ } else {
+ one();
+ two();
+ three();
+ }
- We try to avoid assignments in the condition of an "if" statement.
--
2.11.0.642.gd6f8cda6c
^ permalink raw reply related
* Re: [RFC] stash: support filename argument
From: Thomas Gummerer @ 2017-01-16 22:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, kes-kes
In-Reply-To: <xmqqvatfc0rt.fsf@gitster.mtv.corp.google.com>
On 01/15, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > While working on a repository, it's often helpful to stash the changes
> > of a single or multiple files, and leave others alone. Unfortunately
> > git currently offers no such option. git stash -p can be used to work
> > around this, but it's often impractical when there are a lot of changes
> > over multiple files.
> >
> > Add a --file option to git stash save, which allows for stashing a
> > single file. Specifying the --file argument multiple times allows
> > stashing more than one file at a time.
> >
> > Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> > ---
> >
> > Marked as RFC and without documentation updates to first get a feeling
> > for the user interface, and whether people are interested in this
> > change.
> >
> > Ideally I wanted the the user interface to look like something like:
> > git stash save -- [<filename1,...>], but unfortunately that's already
> > taken up by the stash message. So to preserve backward compatibility
> > I used the new --file argument.
>
> I haven't spent enough time to think if it even makes sense to
> "stash" partially, leaving the working tree still dirty. My initial
> reaction was "then stashing away the dirty WIP state to get a spiffy
> clean working environment becomes impossible", and I still need time
> to recover from that ;-) So as to the desirablity of this "feature",
> I have no strong opinion for or against yet.
As others mentioned, this would be similar to stash -p. My main
usecase is stashing away a config file or changes in one test file
while I forget to test something without these changes. I definitely
used git stash -p on more than one occasion to simulate this behaviour
before.
> But if we were to do this, then we should bite the bullet and
> declare that "stash save <message>" was a mistake. It should have
> been "stash save -m <message>" and we should transition to that (one
> easy way out would be to find another verb that is not 'save').
>
> Then we can do "git stash save [-m <msg>] [--] [pathspec...]" which
> follows the usual command line convention.
I like this interface much better than the one in my patch. I'm not
sure about the verb we could use for the transition, maybe 'push'. If
anyone can come up with a different suggestion I'd be happy to hear
it, as I'm not very good at naming things.
--
Thomas
^ permalink raw reply
* [BUG/RFC] Git Gui: GIT_DIR leaks to spawned Git Bash
From: Max Kirillov @ 2017-01-16 22:40 UTC (permalink / raw)
To: Pat Thoyts; +Cc: git, Junio C Hamano, Johannes Schindelin
Hello.
Apparently various GIT_* environment variables (most
interesting is GIT_DIR but AFAIR there were more) leak to
shell session launched from Git Gui's "Git Bash" menu item.
Which can cause unexpected results if user navigates in that
shell to some other repository (in can also include starting
another terminal window etc. to make user forget original
context) and tries to do something there.
I think is would be appropriate to clean up the environment
while starting any program from git gui. It may have
downside that the environment could be set intentionally,
and now is's removed, but (1) there seems to be no way to
distinguish intentionally vs automatically set GIT_DIR, and
(2) the spawned program may be intended to be used
independly even if the environment was set intentionally for
git gui.
But I would like to note that I find it wrong for the "git"
dispatcher to set up any enrironment to the spawned
command-specific executable. Because it mixes the concerns
of that variables - are they user-settable variables to
tweak the git's behavior, or internal ones to transport the
discovered paths and other info to scripts which cannot do
the standard discovery? Not clear. There has been bugs
related to that already, for example [1].
[1] https://public-inbox.org/git/1409784120-2228-1-git-send-email-max@max630.net/
--
Max
^ permalink raw reply
* [RFH - Tcl/Tk] use of procedure before declaration?
From: Philip Oakley @ 2017-01-16 22:44 UTC (permalink / raw)
To: Git List; +Cc: Pat Thoyts
I'm looking into a user git-gui problem
(https://github.com/git-for-windows/git/issues/1014) that I'd seen in the
past - I'd started some patches back in Dec 2015
http://public-inbox.org/git/1450310287-4936-1-git-send-email-philipoakley@iee.org/
I'm trying to make sure I have covered the corner cases correctly, and I'm
not sure if the current code actually works as advertised.
In
https://github.com/git/git/blob/master/git-gui/lib/choose_repository.tcl#L242
the procedure `_unset_recentrepo` is called, however the procedure isn't
declared until line 248. My reading of the various Tcl tutorials suggest
(but not explictly) that this isn't the right way.
Should 3c6a287 ("git-gui: Keep repo_config(gui.recentrepos) and .gitconfig
in sync", 2010-01-23) have declared `proc _unset_recentrepo {p}` before
`proc _get_recentrepos {}` ?
--
Philip
^ permalink raw reply
* HEAD's reflog entry for a renamed branch
From: Kyle Meyer @ 2017-01-16 23:17 UTC (permalink / raw)
To: git
Hello,
I noticed that, after renaming the current branch, the corresponding
message in .git/logs/HEAD is empty.
For example, running
$ mkdir test-repo
$ cd test-repo
$ git init
$ echo abc >file.txt
$ git add file.txt
$ git commit -m"Add file.txt"
$ git branch -m master new-master
resulted in the following reflogs:
$ cat .git/logs/refs/heads/new-master
00000... 68730... Kyle Meyer <kyle@kyleam.com> 1484607020 -0500 commit (initial): Add file.txt
68730... 68730... Kyle Meyer <kyle@kyleam.com> 1484607020 -0500 Branch: renamed refs/heads/master to refs/heads/new-master
$ cat .git/logs/HEAD
00000... 68730... Kyle Meyer <kyle@kyleam.com> 1484607020 -0500 commit (initial): Add file.txt
68730... 00000... Kyle Meyer <kyle@kyleam.com> 1484607020 -0500
I expected the second line of .git/logs/HEAD to mirror the second line
of .git/logs/refs/heads/new-master. Are the empty message and null sha1
in HEAD's entry intentional?
--
Kyle
^ permalink raw reply
* Re: [PATCH 0/6] fsck --connectivity-check misses some corruption
From: Jeff King @ 2017-01-16 23:26 UTC (permalink / raw)
To: git; +Cc: Michael Haggerty, Johannes Schindelin
In-Reply-To: <20170116212231.ojoqzlajpszifaf3@sigill.intra.peff.net>
On Mon, Jan 16, 2017 at 04:22:31PM -0500, Jeff King wrote:
> I came across a repository today that was missing an object, and for
> which "git fsck" reported the error but "git fsck --connectivity-check"
> did not. It turns out that the shortcut taken by --connectivity-check
> violates some assumptions made by the rest of fsck (namely, that every
> object in the repo has a "struct object" loaded).
>
> And fsck being a generally neglected tool, I couldn't help but find
> several more bugs on the way. :)
>
> [1/6]: t1450: clean up sub-objects in duplicate-entry test
> [2/6]: fsck: report trees as dangling
> [3/6]: fsck: prepare dummy objects for --connectivity-check
> [4/6]: fsck: tighten error-checks of "git fsck <head>"
> [5/6]: fsck: do not fallback "git fsck <bogus>" to "git fsck"
> [6/6]: fsck: check HAS_OBJ more consistently
>
> builtin/fsck.c | 131 ++++++++++++++++++++++++++++++++++++++++++++------------
> t/t1450-fsck.sh | 70 ++++++++++++++++++++++++++++--
> 2 files changed, 171 insertions(+), 30 deletions(-)
Oh, one thing I forgot to mention: I tried test-merging this with my
other recent topic in the area, jk/loose-object-fsck. There are a few
conflicts in the test script, but nothing hard to resolve (just both of
them adding tests at the end, but both are careful to clean up after
themselves).
-Peff
^ permalink raw reply
* Re: [RFC] stash: support filename argument
From: Jeff King @ 2017-01-16 23:49 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Junio C Hamano, Thomas Gummerer, git, kes-kes
In-Reply-To: <c0d46a97-b1c0-d9c9-e475-28e0368ac61f@gmx.net>
On Mon, Jan 16, 2017 at 01:41:02AM +0100, Stephan Beyer wrote:
> However, going further, the next feature to be requested could be "git
> stash --patch" ;D This leads me to think that the mentioned simulation
> of partial stashes might be something everyone might come up with who
> has basic understanding of git features and "git stash", and might be
> the more flexible and probably better solution to the problem.
Don't we have "git stash -p" already?[1]
I use it all the time, and it's very handy for picking apart changes. I
have often wanted "git stash -p <paths>" to work, though.
-Peff
[1] I had to double check that it was not something I hacked together
locally and forgot about. :) It's from dda1f2a5c (Implement 'git
stash save --patch', 2009-08-13). I also worked up a "git stash
apply -p", but I remember it was buggy, and I haven't looked at it
in years. It's at:
https://github.com/peff/git/commit/2c4ed43987b20cfb1605fbd7648d81e454c45c71
if anybody is curious. I don't even recall what the problems were at
this point.
^ permalink raw reply
* Re: [PATCH] request-pull: drop old USAGE stuff
From: Jeff King @ 2017-01-16 23:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Wolfram Sang, git
In-Reply-To: <xmqqr343c0pl.fsf@gitster.mtv.corp.google.com>
On Sun, Jan 15, 2017 at 04:23:02PM -0800, Junio C Hamano wrote:
> Wolfram Sang <wsa@the-dreams.de> writes:
>
> > request-pull uses OPTIONS_SPEC, so no need for (meanwhile incomplete)
> > USAGE and LONG_USAGE anymore.
> >
> > Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> > ---
>
> Makes sense. These are not used anywhere after we switched to use
> parse-options.
It does seem a shame that parse-options does not show this explanatory
text. But I guess nobody really cares that much, and you can always use
"--help" to get even more details.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2017, #02; Sun, 15)
From: Jacob Keller @ 2017-01-17 1:33 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Schindelin, Johannes Sixt, Junio C Hamano,
Git mailing list
In-Reply-To: <20170116220014.bwi5xi2br56lyqsw@sigill.intra.peff.net>
On Mon, Jan 16, 2017 at 2:00 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Jan 16, 2017 at 06:06:35PM +0100, Johannes Schindelin wrote:
>
>> > And I think that would apply to any input parameter we show via
>> > error(), etc, if it is connected to a newline (ideally we would
>> > show newlines as "?", too, but we cannot tell the difference
>> > between ones from parameters, and ones that are part of the error
>> > message).
>>
>> I think it is doing users a really great disservice to munge up CR or LF
>> into question marks. I *guarantee* you that it confuses users. And not
>> because they are dumb, but because the code violates the Law of Least
>> Surprise.
>
> I'm not sure if you realize that with stock git, the example from your
> test looks like this (at least in my terminal):
>
> $ git.v2.11.0 rev-parse --abbrev-ref "$(printf 'CR/LF\r\n')" >/dev/null
> ': unknown revision or path not in the working tree.
> Use '--' to separate paths from revisions, like this:
> 'git <command> [<revision>...] -- [<file>...]'
>
> The "\r" causes us to overwrite the rest of the message, including the
> actual filename. With my patch it's:
>
> $ git rev-parse --abbrev-ref "$(printf 'CR/LF\r\n')" >/dev/null
> fatal: ambiguous argument 'CR/LF?': unknown revision or path not in the working tree.
> Use '--' to separate paths from revisions, like this:
> 'git <command> [<revision>...] -- [<file>...]'
>
> I am certainly sympathetic to the idea that the "?" is ugly and
> potentially confusing. But I think it's at least a step forward for this
> particular example.
>
Would it be possible to convert the CR to a literal \r printing? Since
it's pretty common to show these characters as slash-escaped? (Or is
that too much of a Unix world thing?) I know I'd find \r less
confusing than '?'
Thanks,
Jake
^ permalink raw reply
* "git fetch -p" incorrectly deletes branches
From: Reimar Döffinger @ 2017-01-17 6:04 UTC (permalink / raw)
To: git
Hello!
The command:
git fetch -p -v origin master:refs/heads/test
Deletes refs/heads/test every second time when run repeatedly:
$ git fetch -p -v origin master:refs/heads/test
From https://github.com/git/git
* [new branch] master -> test
= [up to date] master -> origin/master
$ git fetch -p -v origin master:refs/heads/test
From https://github.com/git/git
- [deleted] (none) -> test
= [up to date] master -> test
= [up to date] master -> origin/master
(the command is the result of cutting down the test case,
so yes it is rather silly, but the issue appears in much
less obviously silly cases and causes a lot of confusion)
This is specific to the case of specifying the origin branch
without "full path" AND the right side with.
Wild-guess on cause: "master" is auto-expanded into both
"refs/tags/master" and "refs/heads/master" and instead of
fetching either/merging the result, both are fetched.
Combined with a time-of-check/time-of-use style race condition
on the code that checks if a ref is "up to date" on top of it,
it would result in this behaviour.
Also note that this behaviour appears also when fetch.prune=yes
is set in the config (instead of -p on the command-line),
which makes it much less obvious and there is no option to turn
of prune just for that command to work-around this.
I hope someone has the time to make sure nobody else has to
debug this ever again ;-)
Regards,
Reimar Döffinger
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2017, #02; Sun, 15)
From: Jeff King @ 2017-01-17 7:52 UTC (permalink / raw)
To: Jacob Keller
Cc: Johannes Schindelin, Johannes Sixt, Junio C Hamano,
Git mailing list
In-Reply-To: <CA+P7+xqi8cXK8ZEdvy3U9jJ9wZwkGLYNR0j_xvvCJwq12B4G8g@mail.gmail.com>
On Mon, Jan 16, 2017 at 05:33:44PM -0800, Jacob Keller wrote:
> > I am certainly sympathetic to the idea that the "?" is ugly and
> > potentially confusing. But I think it's at least a step forward for this
> > particular example.
>
> Would it be possible to convert the CR to a literal \r printing? Since
> it's pretty common to show these characters as slash-escaped? (Or is
> that too much of a Unix world thing?) I know I'd find \r less
> confusing than '?'
I discussed this in the original commit message.
Yes, it's possible, but it's a little tricky. We want to fprintf() the
whole thing as a unit (to increase the chances of it being done in an
atomic write()). We don't want to use a dynamic buffer, since we might
be called from a signal handler, or when malloc has failed, etc.
So I tried to stick to something that could reliably done in place. We
could probably get by with 2 static buffers and copy from one to the
other (if stack space is an issue, the current one is 4K, which is
probably excessively large anyway).
Mostly I just assumed that it was highly unlikely anybody would see this
escaping in the first place. So I tried to go with the simplest
solution.
-Peff
^ permalink raw reply
* Re: [BUG/RFC] Git Gui: GIT_DIR leaks to spawned Git Bash
From: Johannes Schindelin @ 2017-01-17 10:52 UTC (permalink / raw)
To: Max Kirillov; +Cc: Pat Thoyts, git, Junio C Hamano
In-Reply-To: <20170116224022.GA8539@jessie.local>
Hi Max,
On Tue, 17 Jan 2017, Max Kirillov wrote:
> Apparently various GIT_* environment variables (most
> interesting is GIT_DIR but AFAIR there were more) leak to
> shell session launched from Git Gui's "Git Bash" menu item.
Given that you call it from Git GUI, i.e. for a *specific* worktree, I do
not think that this bug is *all* that critical. After all, you won't even
notice unless you use the very same Git Bash to navigate to a *different*
worktree.
Having said that, if you have the time and energy to come up with a patch,
I will definitely try my best to help you get it integrated.
And having said *that*, I have to admit that I was unable to reproduce:
GIT_DIR was set neither when starting Git GUI from the Start Menu (and
then navigating to a previously-opened worktree because I am too lazy to
navigate manually), nor when starting Git GUI from a Git CMD in a worktree
via `git gui`. In both cases, the environment contained only the following
variables whose name starts with "GIT_": $GIT_ASKPASS, $GIT_ASK_YESNO and
$GIT_EXEC_PATH.
I tested with Git for Windows v2.11.0(3), the latest offical version.
Thanks,
Johannes
^ permalink raw reply
* Re: [RFH - Tcl/Tk] use of procedure before declaration?
From: Johannes Schindelin @ 2017-01-17 11:29 UTC (permalink / raw)
To: Philip Oakley; +Cc: Git List, Pat Thoyts
In-Reply-To: <F9099DB3F0374D898776BD2621BF36FA@PhilipOakley>
Hi Philip,
On Mon, 16 Jan 2017, Philip Oakley wrote:
> In
> https://github.com/git/git/blob/master/git-gui/lib/choose_repository.tcl#L242
> the procedure `_unset_recentrepo` is called, however the procedure isn't
> declared until line 248. My reading of the various Tcl tutorials suggest
> (but not explictly) that this isn't the right way.
Indeed, calling a procedure before it is declared sounds incorrect.
Since documentation can be treacherous, let's just test it. With a `tclsh`
whose `$tcl_version` variable claims that this is version 8.6, this
script:
```tcl
hello Philip
proc hello {arg} {
puts "Hi, $arg"
}
```
... yields the error message:
invalid command name "hello"
while executing
"hello Philip"
... while this script:
```tcl
proc hello {arg} {
puts "Hi, $arg"
}
hello Philip
```
... prints the expected "Hi, Philip".
Having said that, in the code to which you linked, the procedure is not
actually called before it is declared, as the call is inside another
procedure.
Indeed, the entire file declares one object-oriented class, so no code
gets executed in that file:
https://github.com/git/git/blob/d7dffce1c/git-gui/lib/choose_repository.tcl#L4
(I guess proper indentation would make it easier to understand that this
file is defining a class, not executing anything yet).
And it is perfectly legitimate to use not-yet-declared procedures in other
procedures, otherwise recursion would not work.
> Should 3c6a287 ("git-gui: Keep repo_config(gui.recentrepos) and .gitconfig
> in sync", 2010-01-23) have declared `proc _unset_recentrepo {p}` before
> `proc _get_recentrepos {}` ?
Given the findings above, I believe that the patch is actually correct.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v5 3/7] tag: add format specifier to gpg_verify_tag
From: Jeff King @ 2017-01-17 15:24 UTC (permalink / raw)
To: santiago; +Cc: git, gitster, sunshine, walters, Lukas Puehringer
In-Reply-To: <20170115184705.10376-4-santiago@nyu.edu>
On Sun, Jan 15, 2017 at 01:47:01PM -0500, santiago@nyu.edu wrote:
> From: Lukas Puehringer <luk.puehringer@gmail.com>
>
> Calling functions for gpg_verify_tag() may desire to print relevant
> information about the header for further verification. Add an optional
> format argument to print any desired information after GPG verification.
Hrm. Maybe I am missing something, but what does:
verify_and_format_tag(sha1, name, fmt, flags);
get you over:
gpg_verify_tag(sha1, name, flags);
pretty_print_ref(name, sha1, fmt);
? The latter seems much more flexible, and I do not see how the
verification step impacts the printing at all (or vice versa).
I could understand it more if there were patches later in the series
that somehow used the format and verification results together. But I
didn't see that.
-Peff
^ permalink raw reply
* Re: [PATCH v5 3/7] tag: add format specifier to gpg_verify_tag
From: Jeff King @ 2017-01-17 15:30 UTC (permalink / raw)
To: santiago; +Cc: git, gitster, sunshine, walters, Lukas Puehringer
In-Reply-To: <20170117152455.k6zkeclsyawzpl2n@sigill.intra.peff.net>
On Tue, Jan 17, 2017 at 10:24:55AM -0500, Jeff King wrote:
> On Sun, Jan 15, 2017 at 01:47:01PM -0500, santiago@nyu.edu wrote:
>
> > From: Lukas Puehringer <luk.puehringer@gmail.com>
> >
> > Calling functions for gpg_verify_tag() may desire to print relevant
> > information about the header for further verification. Add an optional
> > format argument to print any desired information after GPG verification.
>
> Hrm. Maybe I am missing something, but what does:
>
> verify_and_format_tag(sha1, name, fmt, flags);
>
> get you over:
>
> gpg_verify_tag(sha1, name, flags);
> pretty_print_ref(name, sha1, fmt);
>
> ? The latter seems much more flexible, and I do not see how the
> verification step impacts the printing at all (or vice versa).
>
> I could understand it more if there were patches later in the series
> that somehow used the format and verification results together. But I
> didn't see that.
Having read through the rest of the series, it looks like you'd
sometimes have to do:
int ret;
ret = gpg_verify_tag(sha1, name, flags);
pretty_print_ref(name, sha1, fmt);
if (ret)
... do something ...
and this function lets you do it in a single line.
Still, I think I'd rather see it done as a wrapper than modifying
gpg_verify_tag().
-Peff
^ permalink raw reply
* fatal: bad revision 'git rm -r --ignore-unmatch -- folder'
From: jean-christophe manciot @ 2017-01-17 15:30 UTC (permalink / raw)
To: git
Hi there,
I'm trying to purge a complete folder and its files from the
repository history with:
git-Games# git filter-branch 'git rm -r --ignore-unmatch --
Ubuntu/16.04/' --tag-name-filter cat -- --all HEAD
fatal: bad revision 'git rm -r --ignore-unmatch -- Ubuntu/16.04/'
git does not find the folder although it's there:
git-Games# ll Ubuntu/16.04/
total 150316
drwxr-x--- 2 actionmystique actionmystique 4096 Nov 19 13:40 ./
drwxr-x--- 4 actionmystique actionmystique 4096 Oct 30 14:02 ../
-rwxr-x--- 1 actionmystique actionmystique 2190394 May 9 2016
residualvm_0.3.0~git-1_amd64.deb*
...
-rw-r--r-- 1 actionmystique actionmystique 67382492 Oct 13 09:15
scummvm-dbgsym_1.9.0_amd64.deb
What is going on?
--
Jean-Christophe
^ permalink raw reply
* Re: [PATCH v5 5/7] builtin/tag: add --format argument for tag -v
From: Jeff King @ 2017-01-17 15:34 UTC (permalink / raw)
To: santiago; +Cc: git, gitster, sunshine, walters, Lukas Puehringer
In-Reply-To: <20170115184705.10376-6-santiago@nyu.edu>
On Sun, Jan 15, 2017 at 01:47:03PM -0500, santiago@nyu.edu wrote:
> -static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
> +static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
> + void *cb_data)
> {
> const char **p;
> char ref[PATH_MAX];
> int had_error = 0;
> unsigned char sha1[20];
>
> +
> for (p = argv; *p; p++) {
> if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p)
> >= sizeof(ref)) {
Funny extra line?
> {
> - return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
> + int flags;
> + char *fmt_pretty = cb_data;
> + flags = GPG_VERIFY_VERBOSE;
> +
> + if (fmt_pretty)
> + flags = GPG_VERIFY_QUIET;
> +
> + return verify_and_format_tag(sha1, ref, fmt_pretty, flags);
It seems funny that VERBOSE and QUIET are bit-flags. What happens when
you ask for GPG_VERIFY_VERBOSE|GPG_VERIFY_QUIET?
I suppose this is actually not a problem in _this_ patch, but in the
very first one that adds the QUIET flag. I'm not sure if the problem is
that the options should be more orthogonal, or that they are just badly
named to appear as opposites when they aren't.
-Peff
^ permalink raw reply
* Re: [PATCH v5 7/7] t/t7004-tag: Add --format specifier tests
From: Jeff King @ 2017-01-17 15:35 UTC (permalink / raw)
To: santiago; +Cc: git, gitster, sunshine, walters
In-Reply-To: <20170115184705.10376-8-santiago@nyu.edu>
On Sun, Jan 15, 2017 at 01:47:05PM -0500, santiago@nyu.edu wrote:
> From: Santiago Torres <santiago@nyu.edu>
>
> tag -v now supports --format specifiers to inspect the contents of a tag
> upon verification. Add two tests to ensure this behavior is respected in
> future changes.
>
> Signed-off-by: Santiago Torres <santiago@nyu.edu>
> ---
> t/t7004-tag.sh | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 07869b0c0..b2b81f203 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -874,6 +874,22 @@ test_expect_success GPG 'verifying a forged tag should fail' '
> test_must_fail git tag -v forged-tag
> '
>
> +test_expect_success 'verifying a proper tag with --format pass and format accordingly' '
> + cat >expect <<-\EOF
Trailing whitespace after "EOF" here (and below). Otherwise the tests
look reasonable to me.
-Peff
^ permalink raw reply
* Re: [PATCH v5 0/7] Add --format to tag verification
From: Jeff King @ 2017-01-17 15:36 UTC (permalink / raw)
To: santiago; +Cc: git, gitster, sunshine, walters
In-Reply-To: <20170115184705.10376-1-santiago@nyu.edu>
On Sun, Jan 15, 2017 at 01:46:58PM -0500, santiago@nyu.edu wrote:
> From: Santiago Torres <santiago@nyu.edu>
>
> This is the fifth iteration of [1][2][3][4], and as a result of the
> discussion in [5]. The main goal of this patch series is to bring
> --format to git tag verification so that upper-layer tools can inspect
> the content of a tag and make decisions based on it.
Thanks for picking this back up. I didn't see any bugs, but I had a few
interface nits, which I sent inline.
-Peff
^ permalink raw reply
* Re: fatal: bad revision 'git rm -r --ignore-unmatch -- folder'
From: Jeff King @ 2017-01-17 15:38 UTC (permalink / raw)
To: jean-christophe manciot; +Cc: git
In-Reply-To: <CAKcFC3aqjLNUNKPDZ08rO_SBkY84uvHBerCxnSchAe8rH0dnMg@mail.gmail.com>
On Tue, Jan 17, 2017 at 04:30:48PM +0100, jean-christophe manciot wrote:
> I'm trying to purge a complete folder and its files from the
> repository history with:
>
> git-Games# git filter-branch 'git rm -r --ignore-unmatch --
> Ubuntu/16.04/' --tag-name-filter cat -- --all HEAD
> fatal: bad revision 'git rm -r --ignore-unmatch -- Ubuntu/16.04/'
Did you forget "--tree-filter" or "--index-filter" before the "git rm"
parameter? Without an option it will be interpreted as a refname, which
it obviously isn't.
-Peff
^ permalink raw reply
* [PATCH v5 3/3] Retire the scripted difftool
From: Johannes Schindelin @ 2017-01-17 15:55 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, David Aguilar, Dennis Kaarsemaker, Paul Sbarra
In-Reply-To: <cover.1484668473.git.johannes.schindelin@gmx.de>
It served its purpose, but now we have a builtin difftool. Time for the
Perl script to enjoy Florida.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.gitignore | 1 -
Makefile | 1 -
builtin/difftool.c | 41 ----------
.../examples/git-difftool.perl | 0
git.c | 7 +-
t/t7800-difftool.sh | 94 +++++++++++-----------
6 files changed, 47 insertions(+), 97 deletions(-)
rename git-legacy-difftool.perl => contrib/examples/git-difftool.perl (100%)
diff --git a/.gitignore b/.gitignore
index 5555ae025b..6722f78f9a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -76,7 +76,6 @@
/git-init-db
/git-interpret-trailers
/git-instaweb
-/git-legacy-difftool
/git-log
/git-ls-files
/git-ls-remote
diff --git a/Makefile b/Makefile
index 8cf5bef034..e9aa6ae57c 100644
--- a/Makefile
+++ b/Makefile
@@ -522,7 +522,6 @@ SCRIPT_LIB += git-sh-setup
SCRIPT_LIB += git-sh-i18n
SCRIPT_PERL += git-add--interactive.perl
-SCRIPT_PERL += git-legacy-difftool.perl
SCRIPT_PERL += git-archimport.perl
SCRIPT_PERL += git-cvsexportcommit.perl
SCRIPT_PERL += git-cvsimport.perl
diff --git a/builtin/difftool.c b/builtin/difftool.c
index 2115e548a5..42ad9e804a 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -616,30 +616,6 @@ static int run_file_diff(int prompt, const char *prefix,
exit(ret);
}
-/*
- * NEEDSWORK: this function can go once the legacy-difftool Perl script is
- * retired.
- *
- * We intentionally avoid reading the config directly here, to avoid messing up
- * the GIT_* environment variables when we need to fall back to exec()ing the
- * Perl script.
- */
-static int use_builtin_difftool(void) {
- struct child_process cp = CHILD_PROCESS_INIT;
- struct strbuf out = STRBUF_INIT;
- int ret;
-
- argv_array_pushl(&cp.args,
- "config", "--bool", "difftool.usebuiltin", NULL);
- cp.git_cmd = 1;
- if (capture_command(&cp, &out, 6))
- return 0;
- strbuf_trim(&out);
- ret = !strcmp("true", out.buf);
- strbuf_release(&out);
- return ret;
-}
-
int cmd_difftool(int argc, const char **argv, const char *prefix)
{
int use_gui_tool = 0, dir_diff = 0, prompt = -1, symlinks = 0,
@@ -671,23 +647,6 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)
OPT_END()
};
- /*
- * NEEDSWORK: Once the builtin difftool has been tested enough
- * and git-legacy-difftool.perl is retired to contrib/, this preamble
- * can be removed.
- */
- if (!use_builtin_difftool()) {
- const char *path = mkpath("%s/git-legacy-difftool",
- git_exec_path());
-
- if (sane_execvp(path, (char **)argv) < 0)
- die_errno("could not exec %s", path);
-
- return 0;
- }
- prefix = setup_git_directory();
- trace_repo_setup(prefix);
- setup_work_tree();
/* NEEDSWORK: once we no longer spawn anything, remove this */
setenv(GIT_DIR_ENVIRONMENT, absolute_path(get_git_dir()), 1);
setenv(GIT_WORK_TREE_ENVIRONMENT, absolute_path(get_git_work_tree()), 1);
diff --git a/git-legacy-difftool.perl b/contrib/examples/git-difftool.perl
similarity index 100%
rename from git-legacy-difftool.perl
rename to contrib/examples/git-difftool.perl
diff --git a/git.c b/git.c
index c58181e5ef..bd4d668a21 100644
--- a/git.c
+++ b/git.c
@@ -424,12 +424,7 @@ static struct cmd_struct commands[] = {
{ "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE },
{ "diff-index", cmd_diff_index, RUN_SETUP },
{ "diff-tree", cmd_diff_tree, RUN_SETUP },
- /*
- * NEEDSWORK: Once the redirection to git-legacy-difftool.perl in
- * builtin/difftool.c has been removed, this entry should be changed to
- * RUN_SETUP | NEED_WORK_TREE
- */
- { "difftool", cmd_difftool },
+ { "difftool", cmd_difftool, RUN_SETUP | NEED_WORK_TREE },
{ "fast-export", cmd_fast_export, RUN_SETUP },
{ "fetch", cmd_fetch, RUN_SETUP },
{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index e94910c563..aa0ef02597 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -23,10 +23,8 @@ prompt_given ()
test "$prompt" = "Launch 'test-tool' [Y/n]? branch"
}
-# NEEDSWORK: lose all the PERL prereqs once legacy-difftool is retired.
-
# Create a file on master and change it on branch
-test_expect_success PERL 'setup' '
+test_expect_success 'setup' '
echo master >file &&
git add file &&
git commit -m "added file" &&
@@ -38,7 +36,7 @@ test_expect_success PERL 'setup' '
'
# Configure a custom difftool.<tool>.cmd and use it
-test_expect_success PERL 'custom commands' '
+test_expect_success 'custom commands' '
difftool_test_setup &&
test_config difftool.test-tool.cmd "cat \"\$REMOTE\"" &&
echo master >expect &&
@@ -51,21 +49,21 @@ test_expect_success PERL 'custom commands' '
test_cmp expect actual
'
-test_expect_success PERL 'custom tool commands override built-ins' '
+test_expect_success 'custom tool commands override built-ins' '
test_config difftool.vimdiff.cmd "cat \"\$REMOTE\"" &&
echo master >expect &&
git difftool --tool vimdiff --no-prompt branch >actual &&
test_cmp expect actual
'
-test_expect_success PERL 'difftool ignores bad --tool values' '
+test_expect_success 'difftool ignores bad --tool values' '
: >expect &&
test_must_fail \
git difftool --no-prompt --tool=bad-tool branch >actual &&
test_cmp expect actual
'
-test_expect_success PERL 'difftool forwards arguments to diff' '
+test_expect_success 'difftool forwards arguments to diff' '
difftool_test_setup &&
>for-diff &&
git add for-diff &&
@@ -78,40 +76,40 @@ test_expect_success PERL 'difftool forwards arguments to diff' '
rm for-diff
'
-test_expect_success PERL 'difftool ignores exit code' '
+test_expect_success 'difftool ignores exit code' '
test_config difftool.error.cmd false &&
git difftool -y -t error branch
'
-test_expect_success PERL 'difftool forwards exit code with --trust-exit-code' '
+test_expect_success 'difftool forwards exit code with --trust-exit-code' '
test_config difftool.error.cmd false &&
test_must_fail git difftool -y --trust-exit-code -t error branch
'
-test_expect_success PERL 'difftool forwards exit code with --trust-exit-code for built-ins' '
+test_expect_success 'difftool forwards exit code with --trust-exit-code for built-ins' '
test_config difftool.vimdiff.path false &&
test_must_fail git difftool -y --trust-exit-code -t vimdiff branch
'
-test_expect_success PERL 'difftool honors difftool.trustExitCode = true' '
+test_expect_success 'difftool honors difftool.trustExitCode = true' '
test_config difftool.error.cmd false &&
test_config difftool.trustExitCode true &&
test_must_fail git difftool -y -t error branch
'
-test_expect_success PERL 'difftool honors difftool.trustExitCode = false' '
+test_expect_success 'difftool honors difftool.trustExitCode = false' '
test_config difftool.error.cmd false &&
test_config difftool.trustExitCode false &&
git difftool -y -t error branch
'
-test_expect_success PERL 'difftool ignores exit code with --no-trust-exit-code' '
+test_expect_success 'difftool ignores exit code with --no-trust-exit-code' '
test_config difftool.error.cmd false &&
test_config difftool.trustExitCode true &&
git difftool -y --no-trust-exit-code -t error branch
'
-test_expect_success PERL 'difftool stops on error with --trust-exit-code' '
+test_expect_success 'difftool stops on error with --trust-exit-code' '
test_when_finished "rm -f for-diff .git/fail-right-file" &&
test_when_finished "git reset -- for-diff" &&
write_script .git/fail-right-file <<-\EOF &&
@@ -126,13 +124,13 @@ test_expect_success PERL 'difftool stops on error with --trust-exit-code' '
test_cmp expect actual
'
-test_expect_success PERL 'difftool honors exit status if command not found' '
+test_expect_success 'difftool honors exit status if command not found' '
test_config difftool.nonexistent.cmd i-dont-exist &&
test_config difftool.trustExitCode false &&
test_must_fail git difftool -y -t nonexistent branch
'
-test_expect_success PERL 'difftool honors --gui' '
+test_expect_success 'difftool honors --gui' '
difftool_test_setup &&
test_config merge.tool bogus-tool &&
test_config diff.tool bogus-tool &&
@@ -143,7 +141,7 @@ test_expect_success PERL 'difftool honors --gui' '
test_cmp expect actual
'
-test_expect_success PERL 'difftool --gui last setting wins' '
+test_expect_success 'difftool --gui last setting wins' '
difftool_test_setup &&
: >expect &&
git difftool --no-prompt --gui --no-gui >actual &&
@@ -157,7 +155,7 @@ test_expect_success PERL 'difftool --gui last setting wins' '
test_cmp expect actual
'
-test_expect_success PERL 'difftool --gui works without configured diff.guitool' '
+test_expect_success 'difftool --gui works without configured diff.guitool' '
difftool_test_setup &&
echo branch >expect &&
git difftool --no-prompt --gui branch >actual &&
@@ -165,7 +163,7 @@ test_expect_success PERL 'difftool --gui works without configured diff.guitool'
'
# Specify the diff tool using $GIT_DIFF_TOOL
-test_expect_success PERL 'GIT_DIFF_TOOL variable' '
+test_expect_success 'GIT_DIFF_TOOL variable' '
difftool_test_setup &&
git config --unset diff.tool &&
echo branch >expect &&
@@ -175,7 +173,7 @@ test_expect_success PERL 'GIT_DIFF_TOOL variable' '
# Test the $GIT_*_TOOL variables and ensure
# that $GIT_DIFF_TOOL always wins unless --tool is specified
-test_expect_success PERL 'GIT_DIFF_TOOL overrides' '
+test_expect_success 'GIT_DIFF_TOOL overrides' '
difftool_test_setup &&
test_config diff.tool bogus-tool &&
test_config merge.tool bogus-tool &&
@@ -193,7 +191,7 @@ test_expect_success PERL 'GIT_DIFF_TOOL overrides' '
# Test that we don't have to pass --no-prompt to difftool
# when $GIT_DIFFTOOL_NO_PROMPT is true
-test_expect_success PERL 'GIT_DIFFTOOL_NO_PROMPT variable' '
+test_expect_success 'GIT_DIFFTOOL_NO_PROMPT variable' '
difftool_test_setup &&
echo branch >expect &&
GIT_DIFFTOOL_NO_PROMPT=true git difftool branch >actual &&
@@ -202,7 +200,7 @@ test_expect_success PERL 'GIT_DIFFTOOL_NO_PROMPT variable' '
# git-difftool supports the difftool.prompt variable.
# Test that GIT_DIFFTOOL_PROMPT can override difftool.prompt = false
-test_expect_success PERL 'GIT_DIFFTOOL_PROMPT variable' '
+test_expect_success 'GIT_DIFFTOOL_PROMPT variable' '
difftool_test_setup &&
test_config difftool.prompt false &&
echo >input &&
@@ -212,7 +210,7 @@ test_expect_success PERL 'GIT_DIFFTOOL_PROMPT variable' '
'
# Test that we don't have to pass --no-prompt when difftool.prompt is false
-test_expect_success PERL 'difftool.prompt config variable is false' '
+test_expect_success 'difftool.prompt config variable is false' '
difftool_test_setup &&
test_config difftool.prompt false &&
echo branch >expect &&
@@ -221,7 +219,7 @@ test_expect_success PERL 'difftool.prompt config variable is false' '
'
# Test that we don't have to pass --no-prompt when mergetool.prompt is false
-test_expect_success PERL 'difftool merge.prompt = false' '
+test_expect_success 'difftool merge.prompt = false' '
difftool_test_setup &&
test_might_fail git config --unset difftool.prompt &&
test_config mergetool.prompt false &&
@@ -231,7 +229,7 @@ test_expect_success PERL 'difftool merge.prompt = false' '
'
# Test that the -y flag can override difftool.prompt = true
-test_expect_success PERL 'difftool.prompt can overridden with -y' '
+test_expect_success 'difftool.prompt can overridden with -y' '
difftool_test_setup &&
test_config difftool.prompt true &&
echo branch >expect &&
@@ -240,7 +238,7 @@ test_expect_success PERL 'difftool.prompt can overridden with -y' '
'
# Test that the --prompt flag can override difftool.prompt = false
-test_expect_success PERL 'difftool.prompt can overridden with --prompt' '
+test_expect_success 'difftool.prompt can overridden with --prompt' '
difftool_test_setup &&
test_config difftool.prompt false &&
echo >input &&
@@ -250,7 +248,7 @@ test_expect_success PERL 'difftool.prompt can overridden with --prompt' '
'
# Test that the last flag passed on the command-line wins
-test_expect_success PERL 'difftool last flag wins' '
+test_expect_success 'difftool last flag wins' '
difftool_test_setup &&
echo branch >expect &&
git difftool --prompt --no-prompt branch >actual &&
@@ -263,7 +261,7 @@ test_expect_success PERL 'difftool last flag wins' '
# git-difftool falls back to git-mergetool config variables
# so test that behavior here
-test_expect_success PERL 'difftool + mergetool config variables' '
+test_expect_success 'difftool + mergetool config variables' '
test_config merge.tool test-tool &&
test_config mergetool.test-tool.cmd "cat \$LOCAL" &&
echo branch >expect &&
@@ -277,49 +275,49 @@ test_expect_success PERL 'difftool + mergetool config variables' '
test_cmp expect actual
'
-test_expect_success PERL 'difftool.<tool>.path' '
+test_expect_success 'difftool.<tool>.path' '
test_config difftool.tkdiff.path echo &&
git difftool --tool=tkdiff --no-prompt branch >output &&
lines=$(grep file output | wc -l) &&
test "$lines" -eq 1
'
-test_expect_success PERL 'difftool --extcmd=cat' '
+test_expect_success 'difftool --extcmd=cat' '
echo branch >expect &&
echo master >>expect &&
git difftool --no-prompt --extcmd=cat branch >actual &&
test_cmp expect actual
'
-test_expect_success PERL 'difftool --extcmd cat' '
+test_expect_success 'difftool --extcmd cat' '
echo branch >expect &&
echo master >>expect &&
git difftool --no-prompt --extcmd=cat branch >actual &&
test_cmp expect actual
'
-test_expect_success PERL 'difftool -x cat' '
+test_expect_success 'difftool -x cat' '
echo branch >expect &&
echo master >>expect &&
git difftool --no-prompt -x cat branch >actual &&
test_cmp expect actual
'
-test_expect_success PERL 'difftool --extcmd echo arg1' '
+test_expect_success 'difftool --extcmd echo arg1' '
echo file >expect &&
git difftool --no-prompt \
--extcmd sh\ -c\ \"echo\ \$1\" branch >actual &&
test_cmp expect actual
'
-test_expect_success PERL 'difftool --extcmd cat arg1' '
+test_expect_success 'difftool --extcmd cat arg1' '
echo master >expect &&
git difftool --no-prompt \
--extcmd sh\ -c\ \"cat\ \$1\" branch >actual &&
test_cmp expect actual
'
-test_expect_success PERL 'difftool --extcmd cat arg2' '
+test_expect_success 'difftool --extcmd cat arg2' '
echo branch >expect &&
git difftool --no-prompt \
--extcmd sh\ -c\ \"cat\ \$2\" branch >actual &&
@@ -327,7 +325,7 @@ test_expect_success PERL 'difftool --extcmd cat arg2' '
'
# Create a second file on master and a different version on branch
-test_expect_success PERL 'setup with 2 files different' '
+test_expect_success 'setup with 2 files different' '
echo m2 >file2 &&
git add file2 &&
git commit -m "added file2" &&
@@ -339,7 +337,7 @@ test_expect_success PERL 'setup with 2 files different' '
git checkout master
'
-test_expect_success PERL 'say no to the first file' '
+test_expect_success 'say no to the first file' '
(echo n && echo) >input &&
git difftool -x cat branch <input >output &&
grep m2 output &&
@@ -348,7 +346,7 @@ test_expect_success PERL 'say no to the first file' '
! grep branch output
'
-test_expect_success PERL 'say no to the second file' '
+test_expect_success 'say no to the second file' '
(echo && echo n) >input &&
git difftool -x cat branch <input >output &&
grep master output &&
@@ -357,7 +355,7 @@ test_expect_success PERL 'say no to the second file' '
! grep br2 output
'
-test_expect_success PERL 'ending prompt input with EOF' '
+test_expect_success 'ending prompt input with EOF' '
git difftool -x cat branch </dev/null >output &&
! grep master output &&
! grep branch output &&
@@ -365,12 +363,12 @@ test_expect_success PERL 'ending prompt input with EOF' '
! grep br2 output
'
-test_expect_success PERL 'difftool --tool-help' '
+test_expect_success 'difftool --tool-help' '
git difftool --tool-help >output &&
grep tool output
'
-test_expect_success PERL 'setup change in subdirectory' '
+test_expect_success 'setup change in subdirectory' '
git checkout master &&
mkdir sub &&
echo master >sub/sub &&
@@ -384,11 +382,11 @@ test_expect_success PERL 'setup change in subdirectory' '
'
run_dir_diff_test () {
- test_expect_success PERL "$1 --no-symlinks" "
+ test_expect_success "$1 --no-symlinks" "
symlinks=--no-symlinks &&
$2
"
- test_expect_success PERL,SYMLINKS "$1 --symlinks" "
+ test_expect_success SYMLINKS "$1 --symlinks" "
symlinks=--symlinks &&
$2
"
@@ -510,7 +508,7 @@ do
done >actual
EOF
-test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstaged changes' '
+test_expect_success SYMLINKS 'difftool --dir-diff --symlink without unstaged changes' '
cat >expect <<-EOF &&
file
$PWD/file
@@ -547,7 +545,7 @@ write_script modify-file <<\EOF
echo "new content" >file
EOF
-test_expect_success PERL 'difftool --no-symlinks does not overwrite working tree file ' '
+test_expect_success 'difftool --no-symlinks does not overwrite working tree file ' '
echo "orig content" >file &&
git difftool --dir-diff --no-symlinks --extcmd "$PWD/modify-file" branch &&
echo "new content" >expect &&
@@ -560,7 +558,7 @@ echo "tmp content" >"$2/file" &&
echo "$2" >tmpdir
EOF
-test_expect_success PERL 'difftool --no-symlinks detects conflict ' '
+test_expect_success 'difftool --no-symlinks detects conflict ' '
(
TMPDIR=$TRASH_DIRECTORY &&
export TMPDIR &&
@@ -573,7 +571,7 @@ test_expect_success PERL 'difftool --no-symlinks detects conflict ' '
)
'
-test_expect_success PERL 'difftool properly honors gitlink and core.worktree' '
+test_expect_success 'difftool properly honors gitlink and core.worktree' '
git submodule add ./. submod/ule &&
test_config -C submod/ule diff.tool checktrees &&
test_config -C submod/ule difftool.checktrees.cmd '\''
@@ -587,7 +585,7 @@ test_expect_success PERL 'difftool properly honors gitlink and core.worktree' '
)
'
-test_expect_success PERL,SYMLINKS 'difftool --dir-diff symlinked directories' '
+test_expect_success SYMLINKS 'difftool --dir-diff symlinked directories' '
git init dirlinks &&
(
cd dirlinks &&
--
2.11.0.windows.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox