* Re: builtin difftool parsing issue
From: Stefan Beller @ 2017-01-03 18:47 UTC (permalink / raw)
To: Paul Sbarra, Jacob Keller
Cc: Johannes Schindelin, David Aguilar, Dennis Kaarsemaker,
git@vger.kernel.org, Junio C Hamano
In-Reply-To: <CAGf+dSjM9nuroeSM9mkQmO3ho4XcZhLo1CR76q-jbeQ-WNGG+Q@mail.gmail.com>
On Mon, Jan 2, 2017 at 11:05 AM, Paul Sbarra <sbarra.paul@gmail.com> wrote:
>> I would have expected `git difftool --submodule=diff ...` to work... What
>> are the problems?
>
> The docs for difftool state...
> "git difftool is a frontend to git diff and accepts the same options
> and arguments."
I think such a sentence in the man page is dangerous, as nobody
was caught this issue until now. There have been numerous authors
and reviewers that touched "diff --submodule=<format>, but as there
is no back-reference, hinting that the patch is only half done, and the
difftool also needs to implement such an option.
We should reword the man page either as
"git difftool is a frontend to git diff and accepts the most(?) options
and arguments."
or even be explicit and list the arguments instead. There we could also
describe differences if any (e.g. the formats available might be different
for --submodule=<format>)
^ permalink raw reply
* [PATCHv2] submodule.c: use GIT_DIR_ENVIRONMENT consistently
From: Stefan Beller @ 2017-01-03 18:30 UTC (permalink / raw)
To: gitster, l.s.r; +Cc: git, Stefan Beller
In-Reply-To: <6dd0a31e-d877-5311-37ef-313ed9ab9716@web.de>
In C code we have the luxury of having constants for all the important
things that are hard coded. This is the only place in C, that hard codes
the git directory environment variable, so fix it.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
v2:
argv_array_pushf and realigned.
v1:
Signed-off-by-the-format-patch-config ;)
This is the only occurrence for "GIT_DIR=" in C, but what about ".git"
git grep "\.git\"" *.c finds some places, which we may want to convert
to DEFAULT_GIT_DIR_ENVIRONMENT?
(mainly things that are newer if I can judge the places correctly
lots of submodules, worktrees and the no data in ".git" bug AFAICT)
Thanks,
Stefan
submodule.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/submodule.c b/submodule.c
index ece17315d6..973b9f3f96 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1333,5 +1333,6 @@ void prepare_submodule_repo_env(struct argv_array *out)
if (strcmp(*var, CONFIG_DATA_ENVIRONMENT))
argv_array_push(out, *var);
}
- argv_array_push(out, "GIT_DIR=.git");
+ argv_array_pushf(out, "%s=%s", GIT_DIR_ENVIRONMENT,
+ DEFAULT_GIT_DIR_ENVIRONMENT);
}
--
2.11.0.259.ga95e92af08.dirty
^ permalink raw reply related
* Re: [PATCH] pathspec: give better message for submodule related pathspec error
From: Stefan Beller @ 2017-01-03 18:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Brandon Williams, Jeff King
In-Reply-To: <xmqqh95j60uh.fsf@gitster.mtv.corp.google.com>
On Sat, Dec 31, 2016 at 5:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> Every once in a while someone complains to the mailing list to have
>> run into this weird assertion[1].
>>
>> The usual response from the mailing list is link to old discussions[2],
>> and acknowledging the problem stating it is known.
>>
>> For now just improve the user visible error message.
>
> Thans. judging from the date: header I take this is meant as v3 that
> supersedes v2 done on Wednesday.
Yes, that is correct. Sorry for being sloppy not numbering the
patches correctly.
>
> It is not clear in the above that what this thing is. Given that we
> are de-asserting it, is the early part of the new code diagnosing an
> end-user error (i.e. you gave me a pathspec but that extends into a
> submodule which is a no-no)? The "was a submodule issue" comment
> added is "this is an end-user mistake, there is nothing to fix in
> the code"?
This is not a fix in the code, but purely improving an error message.
So far anytime someone run into this assert, it was related to submodules.
I do not know the pathspec code well enough to claim this condition
can be produced via submodules *only*, though.
So I proposed a more defensive patch, which diagnoses if it is the
"no-no, pathspec extends into a submodule" first and then throws
a generic error afterwards in case it is not the submodule issue.
> I take that the new "BUG" thing tells the Git developers that no
> sane codepath should throw an pathspec_item that satisfies the
> condition of the if() statement for non-submodules?
If we want to keep the semantics of the assert around, then we
have to have a blank statement if the submodule error message
is not triggered.
I assume if we print this BUG, then there is an actual bug.
>
>> diff --git a/pathspec.c b/pathspec.c
>> index 22ca74a126..b446d79615 100644
>> --- a/pathspec.c
>> +++ b/pathspec.c
>> @@ -313,8 +313,23 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
>> }
>>
>> /* sanity checks, pathspec matchers assume these are sane */
>> - assert(item->nowildcard_len <= item->len &&
>> - item->prefix <= item->len);
>> + if (item->nowildcard_len > item->len ||
>> + item->prefix > item->len) {
>> + /* Historically this always was a submodule issue */
>> + for (i = 0; i < active_nr; i++) {
>> + struct cache_entry *ce = active_cache[i];
>> + int ce_len = ce_namelen(ce);
>> + int len = ce_len < item->len ? ce_len : item->len;
>> + if (!S_ISGITLINK(ce->ce_mode))
>> + continue;
>
> Computation of ce_len and len are better done after this check, no?
Yes, though I trusted the modern-day-compilers to get it right. Will
fix in a reroll.
>> +test_expect_success 'setup a submodule' '
>> + test_commit 1 &&
>> + git submodule add ./ sub &&
>
> Is this adding our own project as its submodule?
Yes it is.
>
> It MIGHT be a handy hack when writing a test, but let's stop doing
> that insanity.
I agree that this is not a good idea.
> No sane project does that in real life, doesn't it?
If such a project was cloned with submodules, it would recurse endlessly. :)
> Create a subdirectory, make it a repository, have a commit there and
> bind that as our own submodule. That would be a more normal way to
> start your own superproject and its submodule pair if they originate
> together at the same place.
I wonder if we want to have a helper function in test-lib.sh to be used
for that. This use case (have a repository and a submodule) happens in
a lot of tests, so we could make life easier by providing a function
in the library so it is even easier than this HACK.
> Better yet create a separate repository, have a commit there, and
> then pull it in with "git submodule add && git submodule init" into
> our repository. That would be the normal way to borrow somebody
> else's project as a part of your own superproject.
The library function could do that, yes.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH v3 08/16] pathspec: always show mnemonic and name in unsupported_magic
From: Brandon Williams @ 2017-01-03 18:15 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <CACsJy8Cja1um2oFDWufyk_7xaZbf9+=kyuWFx==Vb0nHiMqiwA@mail.gmail.com>
On 01/03, Duy Nguyen wrote:
> On Wed, Dec 14, 2016 at 6:14 AM, Brandon Williams <bmwill@google.com> wrote:
> > @@ -340,8 +336,9 @@ static void NORETURN unsupported_magic(const char *pattern,
> > continue;
> > if (sb.len)
> > strbuf_addch(&sb, ' ');
> > - if (short_magic & m->bit)
> > - strbuf_addf(&sb, "'%c'", m->mnemonic);
> > +
> > + if (m->mnemonic)
> > + strbuf_addf(&sb, "'(%c)%s'", m->mnemonic, m->name);
> > else
> > strbuf_addf(&sb, "'%s'", m->name);
> > }
>
> The die() call is out of diff context, but it'll print
>
> pathspec magic not supported by this command: (!)top
>
> which looks too much like :(<name>)<mnemonic> pathspec syntax too me
> and threw me off a bit. And it's a bit cryptic, isn't it? Since this
> is meant for human, maybe we can just write
>
> pathspec magic not supported by this command: top (mnemonic: '!')
I was trying to keep it short and sweet, turns out that ends up being
more difficult to understand. I like your suggestion, it definitely
makes things much clearer.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v2 2/2] t9813: avoid using pipes
From: Stefan Beller @ 2017-01-03 17:58 UTC (permalink / raw)
To: Pranit Bauva; +Cc: git@vger.kernel.org, luke, Johannes Sixt
In-Reply-To: <20170102184536.10488-2-pranit.bauva@gmail.com>
On Mon, Jan 2, 2017 at 10:45 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> The exit code of the upstream in a pipe is ignored thus we should avoid
> using it.
for commands under test, i.e. git things. Other parts can be piped if that makes
the test easier. Though I guess that can be guessed by the reader as well,
as you only convert git commands on upstream pipes.
> By writing out the output of the git command to a file, we can
> test the exit codes of both the commands.
>
> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Thanks for taking ownership of this issue as well. :)
> ---
> t/t9813-git-p4-preserve-users.sh | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh
> index 798bf2b67..9d7550ff3 100755
> --- a/t/t9813-git-p4-preserve-users.sh
> +++ b/t/t9813-git-p4-preserve-users.sh
> @@ -118,12 +118,12 @@ test_expect_success 'not preserving user with mixed authorship' '
> make_change_by_user usernamefile3 Derek derek@example.com &&
> P4EDITOR=cat P4USER=alice P4PASSWD=secret &&
> export P4EDITOR P4USER P4PASSWD &&
> - git p4 commit |\
> - grep "git author derek@example.com does not match" &&
> + git p4 commit >actual 2>&1 &&
Why do we need to pipe 2>&1 here?
Originally the piping only fed the stdout to grep, so this patch changes the
test? Maybe
2>actual.err &&
test_must_be_empty actual.err
instead?
Thanks,
Stefan
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2016, #08; Tue, 27)
From: Brandon Williams @ 2017-01-03 17:58 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <CACsJy8BxsEtU0q4VBpRpELTnubmL624n35Hw3HPhBVx4=6b5DQ@mail.gmail.com>
On 01/03, Duy Nguyen wrote:
> On Thu, Dec 29, 2016 at 5:06 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> > On Thu, Dec 29, 2016 at 1:18 AM, Brandon Williams <bmwill@google.com> wrote:
> >> On 12/27, Junio C Hamano wrote:
> >>> * bw/pathspec-cleanup (2016-12-14) 16 commits
> >>> - pathspec: rename prefix_pathspec to init_pathspec_item
> >>> - pathspec: small readability changes
> >>> - pathspec: create strip submodule slash helpers
> >>> - pathspec: create parse_element_magic helper
> >>> - pathspec: create parse_long_magic function
> >>> - pathspec: create parse_short_magic function
> >>> - pathspec: factor global magic into its own function
> >>> - pathspec: simpler logic to prefix original pathspec elements
> >>> - pathspec: always show mnemonic and name in unsupported_magic
> >>> - pathspec: remove unused variable from unsupported_magic
> >>> - pathspec: copy and free owned memory
> >>> - pathspec: remove the deprecated get_pathspec function
> >>> - ls-tree: convert show_recursive to use the pathspec struct interface
> >>> - dir: convert fill_directory to use the pathspec struct interface
> >>> - dir: remove struct path_simplify
> >>> - mv: remove use of deprecated 'get_pathspec()'
> >>>
> >>> Code clean-up in the pathspec API.
> >>>
> >>> Waiting for the (hopefully) final round of review before 'next'.
> >>
> >> What more needs to be reviewed for this series?
> >
> > I wanted to have a look at it but I successfully managed to put if off
> > so far. Will do soonish.
>
> I have just sent a couple of minor comments. The rest looks good!
Thanks! I'll go take a look.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH] don't use test_must_fail with grep
From: Stefan Beller @ 2017-01-03 17:52 UTC (permalink / raw)
To: Pranit Bauva; +Cc: git@vger.kernel.org
In-Reply-To: <20161231114412.23439-1-pranit.bauva@gmail.com>
On Sat, Dec 31, 2016 at 3:44 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> test_must_fail should only be used for testing git commands. To test the
> failure of other commands use `!`.
>
> Reported-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
Thanks for writing up such a patch!
I had put it on my todo list, but you
were faster on actually going through.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH] archive-zip: load userdiff config
From: René Scharfe @ 2017-01-03 17:24 UTC (permalink / raw)
To: Jeff King, git
In-Reply-To: <20170102222509.ho7motscnffrtnfh@sigill.intra.peff.net>
Am 02.01.2017 um 23:25 schrieb Jeff King:
> Since 4aff646d17 (archive-zip: mark text files in archives,
> 2015-03-05), the zip archiver will look at the userdiff
> driver to decide whether a file is text or binary. This
> usually doesn't need to look any further than the attributes
> themselves (e.g., "-diff", etc). But if the user defines a
> custom driver like "diff=foo", we need to look at
> "diff.foo.binary" in the config. Prior to this patch, we
> didn't actually load it.
Ah, didn't think of that, obviously.
Would it make sense for userdiff_find_by_path() to die if
userdiff_config() hasn't been called, yet?
> I also happened to notice that zipfiles are created using the local
> timezone (because they have no notion of the timezone, so we have to
> pick _something_). That's probably the least-terrible option, but it was
> certainly surprising to me when I tried to bit-for-bit reproduce a
> zipfile from GitHub on my local machine.
That reminds me of an old request to allow users better control over the
meta-data written into archives. Being able to specify a time zone
offset could be a start.
> archive-zip.c | 7 +++++++
> t/t5003-archive-zip.sh | 22 ++++++++++++++++++----
> 2 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/archive-zip.c b/archive-zip.c
> index 9db47357b0..b429a8d974 100644
> --- a/archive-zip.c
> +++ b/archive-zip.c
> @@ -554,11 +554,18 @@ static void dos_time(time_t *time, int *dos_date, int *dos_time)
> *dos_time = t->tm_sec / 2 + t->tm_min * 32 + t->tm_hour * 2048;
> }
>
> +static int archive_zip_config(const char *var, const char *value, void *data)
> +{
> + return userdiff_config(var, value);
> +}
> +
> static int write_zip_archive(const struct archiver *ar,
> struct archiver_args *args)
> {
> int err;
>
> + git_config(archive_zip_config, NULL);
> +
I briefly thought about moving this call to archive.c with the rest of
the config-related stuff, but I agree it's better kept here.
> dos_time(&args->time, &zip_date, &zip_time);
>
> zip_dir = xmalloc(ZIP_DIRECTORY_MIN_SIZE);
> diff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh
> index 14744b2a4b..55c7870997 100755
> --- a/t/t5003-archive-zip.sh
> +++ b/t/t5003-archive-zip.sh
> @@ -64,6 +64,12 @@ check_zip() {
> test_cmp_bin $original/nodiff.crlf $extracted/nodiff.crlf &&
> test_cmp_bin $original/nodiff.lf $extracted/nodiff.lf
> "
> +
> + test_expect_success UNZIP " validate that custom diff is unchanged " "
> + test_cmp_bin $original/custom.cr $extracted/custom.cr &&
> + test_cmp_bin $original/custom.crlf $extracted/custom.crlf &&
> + test_cmp_bin $original/custom.lf $extracted/custom.lf
> + "
> }
>
> test_expect_success \
> @@ -78,6 +84,9 @@ test_expect_success \
> printf "text\r" >a/nodiff.cr &&
> printf "text\r\n" >a/nodiff.crlf &&
> printf "text\n" >a/nodiff.lf &&
> + printf "text\r" >a/custom.cr &&
> + printf "text\r\n" >a/custom.crlf &&
> + printf "text\n" >a/custom.lf &&
> printf "\0\r" >a/binary.cr &&
> printf "\0\r\n" >a/binary.crlf &&
> printf "\0\n" >a/binary.lf &&
> @@ -112,15 +121,20 @@ test_expect_success 'add files to repository' '
> test_expect_success 'setup export-subst and diff attributes' '
> echo "a/nodiff.* -diff" >>.git/info/attributes &&
> echo "a/diff.* diff" >>.git/info/attributes &&
> + echo "a/custom.* diff=custom" >>.git/info/attributes &&
> + git config diff.custom.binary true &&
> echo "substfile?" export-subst >>.git/info/attributes &&
> git log --max-count=1 "--pretty=format:A${SUBSTFORMAT}O" HEAD \
> >a/substfile1
> '
>
> -test_expect_success \
> - 'create bare clone' \
> - 'git clone --bare . bare.git &&
> - cp .git/info/attributes bare.git/info/attributes'
> +test_expect_success 'create bare clone' '
> + git clone --bare . bare.git &&
> + cp .git/info/attributes bare.git/info/attributes &&
> + # Recreate our changes to .git/config rather than just copying it, as
> + # we do not want to clobber core.bare or other settings.
> + git -C bare.git config diff.custom.binary true
> +'
>
> test_expect_success \
> 'remove ignored file' \
>
Looks good, thanks!
René
^ permalink raw reply
* [PATCH 2/2] stash: --[no-]include-untracked option for create
From: Marc Strapetz @ 2017-01-03 15:53 UTC (permalink / raw)
To: git; +Cc: Marc Strapetz
In-Reply-To: <20170103155356.11213-1-marc.strapetz@syntevo.com>
Expose internal option to include untracked files
for the stash 'create' subcommand.
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
---
Documentation/git-stash.txt | 2 +-
git-stash.sh | 14 ++++++++++++--
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 2e9cef06e..cc7944e59 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -16,7 +16,7 @@ SYNOPSIS
'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [<message>]]
'git stash' clear
-'git stash' create [<message>]
+'git stash' create [-u|--[no-]include-untracked] [<message>]
'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
DESCRIPTION
diff --git a/git-stash.sh b/git-stash.sh
index c6b9db694..16f5fe93e 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -10,7 +10,7 @@ USAGE="list [<options>]
or: $dashless [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [<message>]]
or: $dashless clear
- or: $dashless create [<message>]
+ or: $dashless create [-u|--[no-]include-untracked] [<message>]
or: $dashless store [-m|--message <message>] [-q|--quiet] <commit>"
SUBDIRECTORY_OK=Yes
@@ -629,7 +629,17 @@ clear)
;;
create)
shift
- create_stash "$*" && echo "$w_commit"
+ case "$1" in
+ -u|--include-untracked)
+ untracked=untracked
+ shift
+ ;;
+ --no-include-untracked)
+ untracked=
+ shift
+ ;;
+ esac
+ create_stash "$*" "$untracked" && echo "$w_commit"
;;
store)
shift
--
2.11.0
^ permalink raw reply related
* [PATCH 1/2] stash: fix USAGE text
From: Marc Strapetz @ 2017-01-03 15:53 UTC (permalink / raw)
To: git; +Cc: Marc Strapetz
Add missing usage description for stash subcommands
'create' and 'store'.
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
---
git-stash.sh | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/git-stash.sh b/git-stash.sh
index 10c284d1a..c6b9db694 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -9,7 +9,9 @@ USAGE="list [<options>]
or: $dashless branch <branchname> [<stash>]
or: $dashless [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [<message>]]
- or: $dashless clear"
+ or: $dashless clear
+ or: $dashless create [<message>]
+ or: $dashless store [-m|--message <message>] [-q|--quiet] <commit>"
SUBDIRECTORY_OK=Yes
OPTIONS_SPEC=
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v3 06/21] t1700: add tests for core.splitIndex
From: Junio C Hamano @ 2017-01-03 12:58 UTC (permalink / raw)
To: Christian Couder
Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð Bjarmason,
Christian Couder
In-Reply-To: <CAP8UFD1TwVvsvuffyHuzse_9afbNvSEJtyQyWzn6Rc4KwJNwHQ@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> Ok, I will add a patch to update the style of the existing tests at
> the beginning of the series and then use the same new style in the
> tests I add in later patches.
That's not what I meant---I was expecting and was willing to accept
a corrected patch that leaves existing old-fashioned ones as they
are while making sure that added ones are modern, to reduce the cost
of finishing this series, leaving the style fixes of existing ones
for future clean-up task that can be done by anybody after the dust
from this series settles.
A preparatory clean-up patch before the series like you plan is fine
(eh, rather, "extra nice"), so... thanks.
^ permalink raw reply
* Re: [RFC PATCH 0/5] Localise error headers
From: Duy Nguyen @ 2017-01-03 12:26 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Git Mailing List
In-Reply-To: <cover.1483354746.git.git@drmicha.warpmail.net>
On Mon, Jan 2, 2017 at 6:14 PM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> Currently, the headers "error: ", "warning: " etc. - generated by die(),
> warning() etc. - are not localized, but we feed many localized error messages
> into these functions so that we produce error messages with mixed localisation.
>
> This series introduces variants of die() etc. that use localised variants of
> the headers, i.e. _("error: ") etc., and are to be fed localized messages. So,
> instead of die(_("not workee")), which would produce a mixed localisation (such
> as "error: geht ned"), one should use die_(_("not workee")) (resulting in
> "Fehler: geht ned").
Another option, not as thorough, but less effort, is changing
die/err/warn default routines to the "porcelain" versions where we do
_("fatal:") internally _with_ die(), not die_(). We can set this for
porcelain commands that we know can be fully i18n-ized. Then maybe
die_() will fill in the gap if there's still need for it.
--
Duy
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2016, #08; Tue, 27)
From: Duy Nguyen @ 2017-01-03 12:20 UTC (permalink / raw)
To: Brandon Williams; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <CACsJy8CmKBpWa=yi44vGUe56CmeT-Swh_M_XxMeA+xkLLUhQ3Q@mail.gmail.com>
On Thu, Dec 29, 2016 at 5:06 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Thu, Dec 29, 2016 at 1:18 AM, Brandon Williams <bmwill@google.com> wrote:
>> On 12/27, Junio C Hamano wrote:
>>> * bw/pathspec-cleanup (2016-12-14) 16 commits
>>> - pathspec: rename prefix_pathspec to init_pathspec_item
>>> - pathspec: small readability changes
>>> - pathspec: create strip submodule slash helpers
>>> - pathspec: create parse_element_magic helper
>>> - pathspec: create parse_long_magic function
>>> - pathspec: create parse_short_magic function
>>> - pathspec: factor global magic into its own function
>>> - pathspec: simpler logic to prefix original pathspec elements
>>> - pathspec: always show mnemonic and name in unsupported_magic
>>> - pathspec: remove unused variable from unsupported_magic
>>> - pathspec: copy and free owned memory
>>> - pathspec: remove the deprecated get_pathspec function
>>> - ls-tree: convert show_recursive to use the pathspec struct interface
>>> - dir: convert fill_directory to use the pathspec struct interface
>>> - dir: remove struct path_simplify
>>> - mv: remove use of deprecated 'get_pathspec()'
>>>
>>> Code clean-up in the pathspec API.
>>>
>>> Waiting for the (hopefully) final round of review before 'next'.
>>
>> What more needs to be reviewed for this series?
>
> I wanted to have a look at it but I successfully managed to put if off
> so far. Will do soonish.
I have just sent a couple of minor comments. The rest looks good!
--
Duy
^ permalink raw reply
* Re: [PATCH v3 08/16] pathspec: always show mnemonic and name in unsupported_magic
From: Duy Nguyen @ 2017-01-03 12:14 UTC (permalink / raw)
To: Brandon Williams; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <1481670870-66754-9-git-send-email-bmwill@google.com>
On Wed, Dec 14, 2016 at 6:14 AM, Brandon Williams <bmwill@google.com> wrote:
> @@ -340,8 +336,9 @@ static void NORETURN unsupported_magic(const char *pattern,
> continue;
> if (sb.len)
> strbuf_addch(&sb, ' ');
> - if (short_magic & m->bit)
> - strbuf_addf(&sb, "'%c'", m->mnemonic);
> +
> + if (m->mnemonic)
> + strbuf_addf(&sb, "'(%c)%s'", m->mnemonic, m->name);
> else
> strbuf_addf(&sb, "'%s'", m->name);
> }
The die() call is out of diff context, but it'll print
pathspec magic not supported by this command: (!)top
which looks too much like :(<name>)<mnemonic> pathspec syntax too me
and threw me off a bit. And it's a bit cryptic, isn't it? Since this
is meant for human, maybe we can just write
pathspec magic not supported by this command: top (mnemonic: '!')
--
Duy
^ permalink raw reply
* Re: [PATCH v3 06/16] pathspec: copy and free owned memory
From: Duy Nguyen @ 2017-01-03 12:06 UTC (permalink / raw)
To: Brandon Williams; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <1481670870-66754-7-git-send-email-bmwill@google.com>
On Wed, Dec 14, 2016 at 6:14 AM, Brandon Williams <bmwill@google.com> wrote:
> void clear_pathspec(struct pathspec *pathspec)
> {
> + int i;
> +
> + for (i = 0; i < pathspec->nr; i++) {
> + free(pathspec->items[i].match);
> + free(pathspec->items[i].original);
> + }
> free(pathspec->items);
> pathspec->items = NULL;
We should set pathspec->nr to zero so that calling this function again
won't cause any harm.
> }
--
Duy
^ permalink raw reply
* Re: [PATCH v3 02/16] dir: remove struct path_simplify
From: Duy Nguyen @ 2017-01-03 12:02 UTC (permalink / raw)
To: Brandon Williams; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <1481670870-66754-3-git-send-email-bmwill@google.com>
On Wed, Dec 14, 2016 at 6:14 AM, Brandon Williams <bmwill@google.com> wrote:
> @@ -2010,14 +1987,11 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d
> return root;
> }
>
> -int read_directory(struct dir_struct *dir, const char *path, int len, const struct pathspec *pathspec)
> +int read_directory(struct dir_struct *dir, const char *path,
> + int len, const struct pathspec *pathspec)
> {
> - struct path_simplify *simplify;
> struct untracked_cache_dir *untracked;
>
> - /*
> - * Check out create_simplify()
> - */
> if (pathspec)
> GUARD_PATHSPEC(pathspec,
> PATHSPEC_FROMTOP |
This GUARD_PATHSPEC macro should be moved into simplify_away() and
exclude_pathspec_matches(), so that next time somebody adds a new
pathspec magic, they can basically grep GUARD_PATHSPEC and determine
if these code can support their favorite magic or not.
--
Duy
^ permalink raw reply
* Re: [PATCH] diff: add interhunk context config option
From: Pranit Bauva @ 2017-01-03 6:19 UTC (permalink / raw)
To: Vegard Nossum; +Cc: Junio C Hamano, Git List, René Scharfe
In-Reply-To: <20170102233114.20778-1-vegard.nossum@oracle.com>
Hey Vegard,
On Tue, Jan 3, 2017 at 5:01 AM, Vegard Nossum <vegard.nossum@oracle.com> wrote:
> The --inter-hunk-context= option was added in commit 6d0e674a5754
> ("diff: add option to show context between close hunks"). This patch
> allows configuring a default for this option.
>
> Cc: René Scharfe <l.s.r@web.de>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
> Documentation/diff-options.txt | 2 ++
> diff.c | 8 ++++++++
> 2 files changed, 10 insertions(+)
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index e6215c372..163b65444 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -511,6 +511,8 @@ endif::git-format-patch[]
> --inter-hunk-context=<lines>::
> Show the context between diff hunks, up to the specified number
> of lines, thereby fusing hunks that are close to each other.
> + Defaults to `diff.interhunkcontext` or 0 if the config option
> + is unset.
Seems good. But I think you have missed adding the documentation of
this to Documentation/diff-config.txt . Generally whatever config
variable one adds, is added to Documentation/config.txt but in this
case, there exists a separate Documentation/diff-config.txt for all
"diff" related things which is included in Documentation/config.txt .
Also, you need to link the link both the parts of this documentation.
Please refer to commit id aaab84203 so as to know what I am talking
about.
> -W::
> --function-context::
> diff --git a/diff.c b/diff.c
> index 84dba60c4..40b4e6afe 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -33,6 +33,7 @@ static int diff_rename_limit_default = 400;
> static int diff_suppress_blank_empty;
> static int diff_use_color_default = -1;
> static int diff_context_default = 3;
> +static int diff_interhunk_context_default = 0;
> static const char *diff_word_regex_cfg;
> static const char *external_diff_cmd_cfg;
> static const char *diff_order_file_cfg;
> @@ -248,6 +249,12 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
> return -1;
> return 0;
> }
> + if (!strcmp(var, "diff.interhunkcontext")) {
> + diff_interhunk_context_default = git_config_int(var, value);
> + if (diff_interhunk_context_default < 0)
> + return -1;
> + return 0;
> + }
> if (!strcmp(var, "diff.renames")) {
> diff_detect_rename_default = git_config_rename(var, value);
> return 0;
> @@ -3371,6 +3378,7 @@ void diff_setup(struct diff_options *options)
> options->rename_limit = -1;
> options->dirstat_permille = diff_dirstat_permille_default;
> options->context = diff_context_default;
> + options->interhunkcontext = diff_interhunk_context_default;
> options->ws_error_highlight = ws_error_highlight_default;
> DIFF_OPT_SET(options, RENAME_EMPTY);
On a first look, it seems that we can overwrite the default config
values by using a different command line argument which is good.
Also, tests are missing. It seems that t/t4032 might be a good place
to add those tests.
Rest all is quite good! :)
Regards,
Pranit Bauva
^ permalink raw reply
* Re: [PATCH 13/17] refs: convert each_reflog_ent_fn to struct object_id
From: Jeff King @ 2017-01-03 1:32 UTC (permalink / raw)
To: Michael Haggerty; +Cc: brian m. carlson, git discussion list
In-Reply-To: <d5fc2830-37c2-9274-77b7-97ecc5f9b763@alum.mit.edu>
On Tue, Jan 03, 2017 at 12:30:40AM +0100, Michael Haggerty wrote:
> > I think
> > my next series is going to include a small sscanf-style parser to parse
> > these. Right now, using constants here is going to leave it extremely
> > difficult to read. Something like the following for the OIDs:
> >
> > strbuf_git_scanf(sb, "%h %h ", &ooid, &noid);
> >
> > and then following up parsing the remainder.
>
> Maybe something with an interface like skip_prefix wouldn't be too
> obnoxious:
>
> const char *p = sb.buf;
> if (oid_prefix(p, &ooid, &p) &&
> *p++ == ' ' &&
> oid_prefix(p, &noid, &p) && ...
Yeah, I've used C code before that had a very similar interface for
parsing, and when used consistently it's pretty pleasant. Something
like:
if (parse_oid(p, &oid, &p) &&
skip_whitespace(p, &p) &&
parse_refname(p, &refname, &p))
etc is nicer than some of the magic numbers we end up using in the
various parsers (I don't think anybody needs to mass-convert, I just
mean that something like parse_oid() seems like a step in a good
direction).
-Peff
^ permalink raw reply
* [PATCH] diff: add interhunk context config option
From: Vegard Nossum @ 2017-01-02 23:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Vegard Nossum, René Scharfe
The --inter-hunk-context= option was added in commit 6d0e674a5754
("diff: add option to show context between close hunks"). This patch
allows configuring a default for this option.
Cc: René Scharfe <l.s.r@web.de>
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
Documentation/diff-options.txt | 2 ++
diff.c | 8 ++++++++
2 files changed, 10 insertions(+)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index e6215c372..163b65444 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -511,6 +511,8 @@ endif::git-format-patch[]
--inter-hunk-context=<lines>::
Show the context between diff hunks, up to the specified number
of lines, thereby fusing hunks that are close to each other.
+ Defaults to `diff.interhunkcontext` or 0 if the config option
+ is unset.
-W::
--function-context::
diff --git a/diff.c b/diff.c
index 84dba60c4..40b4e6afe 100644
--- a/diff.c
+++ b/diff.c
@@ -33,6 +33,7 @@ static int diff_rename_limit_default = 400;
static int diff_suppress_blank_empty;
static int diff_use_color_default = -1;
static int diff_context_default = 3;
+static int diff_interhunk_context_default = 0;
static const char *diff_word_regex_cfg;
static const char *external_diff_cmd_cfg;
static const char *diff_order_file_cfg;
@@ -248,6 +249,12 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
return -1;
return 0;
}
+ if (!strcmp(var, "diff.interhunkcontext")) {
+ diff_interhunk_context_default = git_config_int(var, value);
+ if (diff_interhunk_context_default < 0)
+ return -1;
+ return 0;
+ }
if (!strcmp(var, "diff.renames")) {
diff_detect_rename_default = git_config_rename(var, value);
return 0;
@@ -3371,6 +3378,7 @@ void diff_setup(struct diff_options *options)
options->rename_limit = -1;
options->dirstat_permille = diff_dirstat_permille_default;
options->context = diff_context_default;
+ options->interhunkcontext = diff_interhunk_context_default;
options->ws_error_highlight = ws_error_highlight_default;
DIFF_OPT_SET(options, RENAME_EMPTY);
--
2.11.0.1.gaa10c3f
^ permalink raw reply related
* Re: [PATCH 13/17] refs: convert each_reflog_ent_fn to struct object_id
From: Michael Haggerty @ 2017-01-02 23:30 UTC (permalink / raw)
To: brian m. carlson; +Cc: git discussion list, Jeff King
In-Reply-To: <20170102191256.fjqsns3rgjyehzgp@genre.crustytoothpaste.net>
On 01/02/2017 08:12 PM, brian m. carlson wrote:
> On Mon, Jan 02, 2017 at 04:07:16PM +0100, Michael Haggerty wrote:
>> On 01/01/2017 08:18 PM, brian m. carlson wrote:
>>> /* old SP new SP name <email> SP time TAB msg LF */
>>> if (sb->len < 83 || sb->buf[sb->len - 1] != '\n' ||
>>> - get_sha1_hex(sb->buf, osha1) || sb->buf[40] != ' ' ||
>>> - get_sha1_hex(sb->buf + 41, nsha1) || sb->buf[81] != ' ' ||
>>> + get_oid_hex(sb->buf, &ooid) || sb->buf[40] != ' ' ||
>>> + get_oid_hex(sb->buf + 41, &noid) || sb->buf[81] != ' ' ||
>>
>> Some magic numbers above could be converted to use constants.
>
> Yes, I saw that. I opted to leave it as it is for the moment.
Totally understandable.
> I think
> my next series is going to include a small sscanf-style parser to parse
> these. Right now, using constants here is going to leave it extremely
> difficult to read. Something like the following for the OIDs:
>
> strbuf_git_scanf(sb, "%h %h ", &ooid, &noid);
>
> and then following up parsing the remainder.
Maybe something with an interface like skip_prefix wouldn't be too
obnoxious:
const char *p = sb.buf;
if (oid_prefix(p, &ooid, &p) &&
*p++ == ' ' &&
oid_prefix(p, &noid, &p) && ...
> [...]
Michael
^ permalink raw reply
* [PATCH] archive-zip: load userdiff config
From: Jeff King @ 2017-01-02 22:25 UTC (permalink / raw)
To: git; +Cc: René Scharfe
Since 4aff646d17 (archive-zip: mark text files in archives,
2015-03-05), the zip archiver will look at the userdiff
driver to decide whether a file is text or binary. This
usually doesn't need to look any further than the attributes
themselves (e.g., "-diff", etc). But if the user defines a
custom driver like "diff=foo", we need to look at
"diff.foo.binary" in the config. Prior to this patch, we
didn't actually load it.
Signed-off-by: Jeff King <peff@peff.net>
---
I'd be surprised if anybody actually triggered this in practice. I don't
think any of the custom-driver fields except "binary" matter, and using
direct attributes is almost always easier than setting up a custom
driver. Though you could also do trickery with:
git -c diff.default.binary=true archive ...
if you wanted to be really clever.
I ran across this while investigating a case where somebody's zipfile
was all marked as binary (it turned out not to be related; the issue was
just that their Git was pre-4aff646d17).
I also happened to notice that zipfiles are created using the local
timezone (because they have no notion of the timezone, so we have to
pick _something_). That's probably the least-terrible option, but it was
certainly surprising to me when I tried to bit-for-bit reproduce a
zipfile from GitHub on my local machine.
archive-zip.c | 7 +++++++
t/t5003-archive-zip.sh | 22 ++++++++++++++++++----
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/archive-zip.c b/archive-zip.c
index 9db47357b0..b429a8d974 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -554,11 +554,18 @@ static void dos_time(time_t *time, int *dos_date, int *dos_time)
*dos_time = t->tm_sec / 2 + t->tm_min * 32 + t->tm_hour * 2048;
}
+static int archive_zip_config(const char *var, const char *value, void *data)
+{
+ return userdiff_config(var, value);
+}
+
static int write_zip_archive(const struct archiver *ar,
struct archiver_args *args)
{
int err;
+ git_config(archive_zip_config, NULL);
+
dos_time(&args->time, &zip_date, &zip_time);
zip_dir = xmalloc(ZIP_DIRECTORY_MIN_SIZE);
diff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh
index 14744b2a4b..55c7870997 100755
--- a/t/t5003-archive-zip.sh
+++ b/t/t5003-archive-zip.sh
@@ -64,6 +64,12 @@ check_zip() {
test_cmp_bin $original/nodiff.crlf $extracted/nodiff.crlf &&
test_cmp_bin $original/nodiff.lf $extracted/nodiff.lf
"
+
+ test_expect_success UNZIP " validate that custom diff is unchanged " "
+ test_cmp_bin $original/custom.cr $extracted/custom.cr &&
+ test_cmp_bin $original/custom.crlf $extracted/custom.crlf &&
+ test_cmp_bin $original/custom.lf $extracted/custom.lf
+ "
}
test_expect_success \
@@ -78,6 +84,9 @@ test_expect_success \
printf "text\r" >a/nodiff.cr &&
printf "text\r\n" >a/nodiff.crlf &&
printf "text\n" >a/nodiff.lf &&
+ printf "text\r" >a/custom.cr &&
+ printf "text\r\n" >a/custom.crlf &&
+ printf "text\n" >a/custom.lf &&
printf "\0\r" >a/binary.cr &&
printf "\0\r\n" >a/binary.crlf &&
printf "\0\n" >a/binary.lf &&
@@ -112,15 +121,20 @@ test_expect_success 'add files to repository' '
test_expect_success 'setup export-subst and diff attributes' '
echo "a/nodiff.* -diff" >>.git/info/attributes &&
echo "a/diff.* diff" >>.git/info/attributes &&
+ echo "a/custom.* diff=custom" >>.git/info/attributes &&
+ git config diff.custom.binary true &&
echo "substfile?" export-subst >>.git/info/attributes &&
git log --max-count=1 "--pretty=format:A${SUBSTFORMAT}O" HEAD \
>a/substfile1
'
-test_expect_success \
- 'create bare clone' \
- 'git clone --bare . bare.git &&
- cp .git/info/attributes bare.git/info/attributes'
+test_expect_success 'create bare clone' '
+ git clone --bare . bare.git &&
+ cp .git/info/attributes bare.git/info/attributes &&
+ # Recreate our changes to .git/config rather than just copying it, as
+ # we do not want to clobber core.bare or other settings.
+ git -C bare.git config diff.custom.binary true
+'
test_expect_success \
'remove ignored file' \
--
2.11.0.519.g31435224cf
^ permalink raw reply related
* Re: builtin difftool parsing issue
From: Paul Sbarra @ 2017-01-02 19:05 UTC (permalink / raw)
To: Johannes Schindelin
Cc: David Aguilar, Dennis Kaarsemaker, git, Junio C Hamano
In-Reply-To: <alpine.DEB.2.20.1701021712010.3469@virtualbox>
> I would have expected `git difftool --submodule=diff ...` to work... What
> are the problems?
The docs for difftool state...
"git difftool is a frontend to git diff and accepts the same options
and arguments."
which could lead a user to expect passing --submodule=diff to have a
similar behavior for difftool. It would be especially useful when
combined with --dir-diff.
Unfortunately, due to the way the left/right directories are built up,
difftool needs to handle this option itself. Currently a file
representing the submodule directory is created that contains the
hash.
if (S_ISGITLINK(lmode) || S_ISGITLINK(rmode)) {
strbuf_reset(&buf);
strbuf_addf(&buf, "Subproject commit %s", oid_to_hex(&loid));
add_left_or_right(&submodules, src_path, buf.buf, 0);
strbuf_reset(&buf);
strbuf_addf(&buf, "Subproject commit %s", oid_to_hex(&roid));
if (!oidcmp(&loid, &roid))
strbuf_addstr(&buf, "-dirty");
add_left_or_right(&submodules, dst_path, buf.buf, 1);
continue;
}
To achieve the desired behavior a diff command would need to be run
within the submodule. A further complication is whether submodules
should be processed recursively. I'm not sure whether or not diff
handles them recursively. I believe the logic to parse and build up
the files would need to be factored out such that it could be called
for the super-project as well as each submodule change.
This is all out of scope for your effort as the existing (perl-based)
difftool doesn't do this either. However, it's a feature that would
provide a significant simplification to the workflow used at the
office to review changes.
^ permalink raw reply
* Re: [PATCH v3 00/23] Delete directories left empty after ref deletion
From: Jacob Keller @ 2017-01-02 18:54 UTC (permalink / raw)
To: Michael Haggerty
Cc: Jeff King, Philip Oakley, Junio C Hamano, Git mailing list,
David Turner
In-Reply-To: <ab92dc60-c623-0f6f-868b-b74b1d6dbd2e@alum.mit.edu>
On Mon, Jan 2, 2017 at 10:14 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> On 01/02/2017 05:19 AM, Jeff King wrote:
>> On Sun, Jan 01, 2017 at 12:36:11PM -0800, Jacob Keller wrote:
>>
>>> But how likely is it to end up with differing binaries running on the
>>> exact same repository concurrently? Basically, I am trying to see
>>> whether or not we could accidentally end up causing problems by trying
>>> to race with other git processes that haven't yet been made safe
>>> against race? Is the worst case only that some git operation would
>>> fail and you would have to retry?
>>
>> Yes, I think that is the worst case.
>>
>> A more likely scenario might be something like a server accepting pushes
>> or other ref updates from both JGit and regular git (or maybe libgit2
>> and regular git).
>>
>> IMHO it's not really worth worrying about too much. Certain esoteric
>> setups might have a slightly higher chance of a pretty obscure race
>> condition happening on a very busy repository. I hate to say "eh, ship
>> it, we'll see if anybody complains". But I'd be surprised to get a
>> single report about this.
>
> I agree. I think these races would mostly affect busy hosting sites and
> result in failed updates rather than corruption. And the races can
> already occur whenever the user runs `git pack-refs`. By contrast, the
> failure to delete empty directories is more likely to affect normal users.
>
> That being said, if Junio wants to merge all but the last two patches in
> one release, then merge the last two a release or two later, I have no
> objections.
>
> Michael
>
I only wanted to make sure that the failure mode would not result in
corruption. I believe that both you and Peff have alleviated my fears.
Thanks,
Jake
^ permalink raw reply
* [PATCH v2 2/2] t9813: avoid using pipes
From: Pranit Bauva @ 2017-01-02 18:45 UTC (permalink / raw)
To: git; +Cc: sbeller, luke, j6t, Pranit Bauva
In-Reply-To: <20161231114412.23439-1-pranit.bauva@gmail.com>
The exit code of the upstream in a pipe is ignored thus we should avoid
using it. By writing out the output of the git command to a file, we can
test the exit codes of both the commands.
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
t/t9813-git-p4-preserve-users.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh
index 798bf2b67..9d7550ff3 100755
--- a/t/t9813-git-p4-preserve-users.sh
+++ b/t/t9813-git-p4-preserve-users.sh
@@ -118,12 +118,12 @@ test_expect_success 'not preserving user with mixed authorship' '
make_change_by_user usernamefile3 Derek derek@example.com &&
P4EDITOR=cat P4USER=alice P4PASSWD=secret &&
export P4EDITOR P4USER P4PASSWD &&
- git p4 commit |\
- grep "git author derek@example.com does not match" &&
+ git p4 commit >actual 2>&1 &&
+ grep "git author derek@example.com does not match" actual &&
make_change_by_user usernamefile3 Charlie charlie@example.com &&
- git p4 commit |\
- grep "git author charlie@example.com does not match" &&
+ git p4 commit >actual 2>&1 &&
+ grep "git author charlie@example.com does not match" actual &&
make_change_by_user usernamefile3 alice alice@example.com &&
git p4 commit >actual 2>&1 &&
--
2.11.0
^ permalink raw reply related
* [PATCH v2 1/2] don't use test_must_fail with grep
From: Pranit Bauva @ 2017-01-02 18:45 UTC (permalink / raw)
To: git; +Cc: sbeller, luke, j6t, Pranit Bauva
In-Reply-To: <20161231114412.23439-1-pranit.bauva@gmail.com>
test_must_fail should only be used for testing git commands. To test the
failure of other commands use `!`.
Reported-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
t/t3510-cherry-pick-sequence.sh | 6 +++---
t/t5504-fetch-receive-strict.sh | 2 +-
t/t5516-fetch-push.sh | 2 +-
t/t5601-clone.sh | 2 +-
t/t6030-bisect-porcelain.sh | 2 +-
t/t7610-mergetool.sh | 2 +-
t/t9001-send-email.sh | 2 +-
t/t9117-git-svn-init-clone.sh | 12 ++++++------
t/t9813-git-p4-preserve-users.sh | 8 ++++----
t/t9814-git-p4-rename.sh | 6 +++---
10 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 372307c21..0acf4b146 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -385,7 +385,7 @@ test_expect_success '--continue respects opts' '
git cat-file commit HEAD~1 >picked_msg &&
git cat-file commit HEAD~2 >unrelatedpick_msg &&
git cat-file commit HEAD~3 >initial_msg &&
- test_must_fail grep "cherry picked from" initial_msg &&
+ ! grep "cherry picked from" initial_msg &&
grep "cherry picked from" unrelatedpick_msg &&
grep "cherry picked from" picked_msg &&
grep "cherry picked from" anotherpick_msg
@@ -426,9 +426,9 @@ test_expect_failure '--signoff is automatically propagated to resolved conflict'
git cat-file commit HEAD~1 >picked_msg &&
git cat-file commit HEAD~2 >unrelatedpick_msg &&
git cat-file commit HEAD~3 >initial_msg &&
- test_must_fail grep "Signed-off-by:" initial_msg &&
+ ! grep "Signed-off-by:" initial_msg &&
grep "Signed-off-by:" unrelatedpick_msg &&
- test_must_fail grep "Signed-off-by:" picked_msg &&
+ ! grep "Signed-off-by:" picked_msg &&
grep "Signed-off-by:" anotherpick_msg
'
diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
index 9b19cff72..49d3621a9 100755
--- a/t/t5504-fetch-receive-strict.sh
+++ b/t/t5504-fetch-receive-strict.sh
@@ -152,7 +152,7 @@ test_expect_success 'push with receive.fsck.missingEmail=warn' '
git --git-dir=dst/.git config --add \
receive.fsck.badDate warn &&
git push --porcelain dst bogus >act 2>&1 &&
- test_must_fail grep "missingEmail" act
+ ! grep "missingEmail" act
'
test_expect_success \
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 26b2cafc4..0fc5a7c59 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1004,7 +1004,7 @@ test_expect_success 'push --porcelain' '
test_expect_success 'push --porcelain bad url' '
mk_empty testrepo &&
test_must_fail git push >.git/bar --porcelain asdfasdfasd refs/heads/master:refs/remotes/origin/master &&
- test_must_fail grep -q Done .git/bar
+ ! grep -q Done .git/bar
'
test_expect_success 'push --porcelain rejected' '
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index a43339420..4241ea5b3 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -151,7 +151,7 @@ test_expect_success 'clone --mirror does not repeat tags' '
git clone --mirror src mirror2 &&
(cd mirror2 &&
git show-ref 2> clone.err > clone.out) &&
- test_must_fail grep Duplicate mirror2/clone.err &&
+ ! grep Duplicate mirror2/clone.err &&
grep some-tag mirror2/clone.out
'
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 5e5370feb..8c2c6eaef 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -407,7 +407,7 @@ test_expect_success 'good merge base when good and bad are siblings' '
test_i18ngrep "merge base must be tested" my_bisect_log.txt &&
grep $HASH4 my_bisect_log.txt &&
git bisect good > my_bisect_log.txt &&
- test_must_fail grep "merge base must be tested" my_bisect_log.txt &&
+ ! grep "merge base must be tested" my_bisect_log.txt &&
grep $HASH6 my_bisect_log.txt &&
git bisect reset
'
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 63d36fb28..0fe7e58cf 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -602,7 +602,7 @@ test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToT
test_config mergetool.myecho.trustExitCode true &&
test_must_fail git merge master &&
git mergetool --no-prompt --tool myecho -- both >actual &&
- test_must_fail grep ^\./both_LOCAL_ actual >/dev/null &&
+ ! grep ^\./both_LOCAL_ actual >/dev/null &&
grep /both_LOCAL_ actual >/dev/null &&
git reset --hard master >/dev/null 2>&1
'
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 3dc4a3454..0f398dd16 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -50,7 +50,7 @@ test_no_confirm () {
--smtp-server="$(pwd)/fake.sendmail" \
$@ \
$patches >stdout &&
- test_must_fail grep "Send this email" stdout &&
+ ! grep "Send this email" stdout &&
>no_confirm_okay
}
diff --git a/t/t9117-git-svn-init-clone.sh b/t/t9117-git-svn-init-clone.sh
index 69a675052..044f65e91 100755
--- a/t/t9117-git-svn-init-clone.sh
+++ b/t/t9117-git-svn-init-clone.sh
@@ -55,7 +55,7 @@ test_expect_success 'clone to target directory with --stdlayout' '
test_expect_success 'init without -s/-T/-b/-t does not warn' '
test ! -d trunk &&
git svn init "$svnrepo"/project/trunk trunk 2>warning &&
- test_must_fail grep -q prefix warning &&
+ ! grep -q prefix warning &&
rm -rf trunk &&
rm -f warning
'
@@ -63,7 +63,7 @@ test_expect_success 'init without -s/-T/-b/-t does not warn' '
test_expect_success 'clone without -s/-T/-b/-t does not warn' '
test ! -d trunk &&
git svn clone "$svnrepo"/project/trunk 2>warning &&
- test_must_fail grep -q prefix warning &&
+ ! grep -q prefix warning &&
rm -rf trunk &&
rm -f warning
'
@@ -86,7 +86,7 @@ EOF
test_expect_success 'init with -s/-T/-b/-t assumes --prefix=origin/' '
test ! -d project &&
git svn init -s "$svnrepo"/project project 2>warning &&
- test_must_fail grep -q prefix warning &&
+ ! grep -q prefix warning &&
test_svn_configured_prefix "origin/" &&
rm -rf project &&
rm -f warning
@@ -95,7 +95,7 @@ test_expect_success 'init with -s/-T/-b/-t assumes --prefix=origin/' '
test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '
test ! -d project &&
git svn clone -s "$svnrepo"/project 2>warning &&
- test_must_fail grep -q prefix warning &&
+ ! grep -q prefix warning &&
test_svn_configured_prefix "origin/" &&
rm -rf project &&
rm -f warning
@@ -104,7 +104,7 @@ test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '
test_expect_success 'init with -s/-T/-b/-t and --prefix "" still works' '
test ! -d project &&
git svn init -s "$svnrepo"/project project --prefix "" 2>warning &&
- test_must_fail grep -q prefix warning &&
+ ! grep -q prefix warning &&
test_svn_configured_prefix "" &&
rm -rf project &&
rm -f warning
@@ -113,7 +113,7 @@ test_expect_success 'init with -s/-T/-b/-t and --prefix "" still works' '
test_expect_success 'clone with -s/-T/-b/-t and --prefix "" still works' '
test ! -d project &&
git svn clone -s "$svnrepo"/project --prefix "" 2>warning &&
- test_must_fail grep -q prefix warning &&
+ ! grep -q prefix warning &&
test_svn_configured_prefix "" &&
rm -rf project &&
rm -f warning
diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh
index 0fe231280..798bf2b67 100755
--- a/t/t9813-git-p4-preserve-users.sh
+++ b/t/t9813-git-p4-preserve-users.sh
@@ -126,13 +126,13 @@ test_expect_success 'not preserving user with mixed authorship' '
grep "git author charlie@example.com does not match" &&
make_change_by_user usernamefile3 alice alice@example.com &&
- git p4 commit |\
- test_must_fail grep "git author.*does not match" &&
+ git p4 commit >actual 2>&1 &&
+ ! grep "git author.*does not match" actual &&
git config git-p4.skipUserNameCheck true &&
make_change_by_user usernamefile3 Charlie charlie@example.com &&
- git p4 commit |\
- test_must_fail grep "git author.*does not match" &&
+ git p4 commit >actual 2>&1 &&
+ ! grep "git author.*does not match" actual &&
p4_check_commit_author usernamefile3 alice
)
diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index c89992cf9..e7e0268e9 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -141,7 +141,7 @@ test_expect_success 'detect copies' '
git diff-tree -r -C HEAD &&
git p4 submit &&
p4 filelog //depot/file8 &&
- p4 filelog //depot/file8 | test_must_fail grep -q "branch from" &&
+ ! p4 filelog //depot/file8 | grep -q "branch from" &&
echo "file9" >>file2 &&
git commit -a -m "Differentiate file2" &&
@@ -154,7 +154,7 @@ test_expect_success 'detect copies' '
git config git-p4.detectCopies true &&
git p4 submit &&
p4 filelog //depot/file9 &&
- p4 filelog //depot/file9 | test_must_fail grep -q "branch from" &&
+ ! p4 filelog //depot/file9 | grep -q "branch from" &&
echo "file10" >>file2 &&
git commit -a -m "Differentiate file2" &&
@@ -202,7 +202,7 @@ test_expect_success 'detect copies' '
git config git-p4.detectCopies $(($level + 2)) &&
git p4 submit &&
p4 filelog //depot/file12 &&
- p4 filelog //depot/file12 | test_must_fail grep -q "branch from" &&
+ ! p4 filelog //depot/file12 | grep -q "branch from" &&
echo "file13" >>file2 &&
git commit -a -m "Differentiate file2" &&
--
2.11.0
^ 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