* Re: [PATCH] fixup! worktree move: new command
From: Junio C Hamano @ 2017-01-27 17:51 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <alpine.DEB.2.20.1701271150200.3469@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> always been discussed on the mailing list, I would like to kindly ask you
> to please add this patch to the nd/worktree-move branch for the time being
> (i.e. until Duy responds),
The tip of 'pu' (or anything beyond the tip of 'jch') is not always
expected to pass test or even build, and unless I know the OP is
leaky or usually slow, a "fixup!" that directly addresses the OP
tends to be left for the OP to pick up [*1*] to avoid "ah, you sent
a reroll but I already had one squashed locally", which is confusing
to both myself and OP (and adds burden to me).
It seems that OP is slower to respond than his usual, so I'll add it
to the tip of the topic so that it won't be lost.
[Footnote]
*1* It still is used on my end to leave a mental note to myself that
the topic is expected/waiting to be rerolled, but that is not
something you can read in "git log --first-parent master..pu".
^ permalink raw reply
* Re: show all merge conflicts
From: Jeff King @ 2017-01-27 17:51 UTC (permalink / raw)
To: Michael Spiegel; +Cc: git
In-Reply-To: <CANwu5-o=3p8QfWo9wQvok=UZESRVtF3Uxb3nEMZVv8AvkKYYGw@mail.gmail.com>
On Fri, Jan 27, 2017 at 11:56:08AM -0500, Michael Spiegel wrote:
> I'm trying to determine whether a merge required a conflict to resolve
> after the merge has occurred. The git book has some advice
> (https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging) to use
> `git show` on the merge commit or use `git log --cc -p -1`. These
> strategies work when the merge conflict was resolved with a change
> that is different from either parent. When the conflict is resolved
> with a change that is the same as one of the parents, then these
> commands are indistinguishable from a merge that did not conflict. Is
> it possible to distinguish between a conflict-free merge and a merge
> conflict that is resolved by with the changes from one the parents?
No. You'd have to replay the merge to know if it would have had
conflicts.
There was a patch series a few years ago that added a new diff-mode to
do exactly that, and show the diff against what was resolved. It had a
few issues (I don't remember exactly what) and never got merged.
Certainly one complication is that you don't know exactly _how_ the
merge was done in the first place (e.g., which merge strategy, which
custom merge drivers were in effect, etc). But in general, replaying
with a standard merge-recursive would get you most of what you want to
know.
I've done this manually sometimes when digging into erroneous merges
(e.g., somebody accidentally runs "git reset -- <paths>" in the middle
of a merge and throws away some changes.
You should be able to do:
git checkout $merge^1
git merge $merge^2
git diff -R $merge
to see what the original resolution did.
-Peff
^ permalink raw reply
* Re: [PATCH v3 4/4] urlmatch: allow globbing for the URL host part
From: Junio C Hamano @ 2017-01-27 17:45 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Patrick Steinhardt, Philip Oakley
In-Reply-To: <20170127062142.GA760@pks-pc>
Patrick Steinhardt <patrick.steinhardt@elego.de> writes:
>> This is probably a useful improvement.
>>
>> Having said that, when I mentioned "glob", I meant to also support
>> something like this:
>>
>> https://www[1-4].ibm.com/
>
> The problem with additional extended syntax like proposed by you
> is that we would indeed need an escaping mechanism here.
True. I think a true shell globbing is overkill (so is regexp) and
just a simple wildcarding with '*' would be a good first step that
is easy to explain and later extend as needed.
Thanks.
^ permalink raw reply
* Re: SubmittingPatches: drop temporal reference for PGP signing
From: Junio C Hamano @ 2017-01-27 17:43 UTC (permalink / raw)
To: Cornelius Weig
Cc: Philip Oakley, Stefan Beller, Johannes Sixt,
bitte.keine.werbung.einwerfen, git
In-Reply-To: <60e9abdf-cc37-33d8-e7eb-8a3370ffe1cc@tngtech.com>
Cornelius Weig <cornelius.weig@tngtech.com> writes:
> -Do not PGP sign your patch, -at least for now-. Most likely, your (...)
> +Do not PGP sign your patch. Most likely, your maintainer or other (...)
It has been quite a while since we wrote that "at least for now", so
it probably makes sense to drop it.
>> Maybe even s/by signing off/by adding your Signed-off-by:/ to be sure
>> that the reader knows that it is _their certification_ that is being
>> sought. Even if it does double up on the 'your'.
>
> I don't think doubling on the 'your' will make the heading clearer. The
> main intention of this change is to avoid mixups with pgp-signing and
> that would IMHO not improve by that.
> Besides, I find the colon in the heading a bit awkward. Is the following
> version as expressive as with the colon?
>
> -(5) Sign your work
> +(5) Certify your work by adding Signed-off-by
I am leaning towards agreeing with Philip, and my gut feeling says
the presense of the colon, would help a lot. It would visually
click to readers what we are talking about.
At the same time, I think ending a section header with a colon would
be seen as "awkward". I would have written it more like this to
avoid it:
... by adding "Signed-off-by: " line
I personally do not mind having "your" there, either, but I can see
that a section header wants to be kept shorter.
^ permalink raw reply
* [PATCH v3 0/1] Support `git reset --stdin`
From: Johannes Schindelin @ 2017-01-27 17:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Jeff King
In-Reply-To: <cover.1485520718.git.johannes.schindelin@gmx.de>
This feature was missing, and made it cumbersome for third-party
tools to reset a lot of paths in one go.
Support for --stdin has been added, following builtin/checkout-index.c's
example.
Changes since v2:
- the documentation clarifies that --stdin does not treat the input as
pathspecs
- the code now uses struct argv_array instead of rolling its own
Johannes Schindelin (1):
reset: support the --stdin option
Documentation/git-reset.txt | 10 ++++++++++
builtin/reset.c | 47 ++++++++++++++++++++++++++++++++++++++++++++-
t/t7107-reset-stdin.sh | 33 +++++++++++++++++++++++++++++++
3 files changed, 89 insertions(+), 1 deletion(-)
create mode 100755 t/t7107-reset-stdin.sh
base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe
Published-As: https://github.com/dscho/git/releases/tag/reset-stdin-v3
Fetch-It-Via: git fetch https://github.com/dscho/git reset-stdin-v3
Interdiff vs v2:
diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index abb71bb805..d319ed9b20 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -100,7 +100,8 @@ OPTIONS
--stdin::
Instead of taking list of paths from the command line,
- read list of paths from the standard input. Paths are
+ read list of paths from the standard input. The paths are
+ read verbatim, i.e. not handled as pathspecs. Paths are
separated by LF (i.e. one path per line) by default.
-z::
diff --git a/builtin/reset.c b/builtin/reset.c
index 1d3075b7ee..fe7723c179 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -23,6 +23,7 @@
#include "cache-tree.h"
#include "strbuf.h"
#include "quote.h"
+#include "argv-array.h"
static const char * const git_reset_usage[] = {
N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
@@ -271,8 +272,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
{
int reset_type = NONE, update_ref_status = 0, quiet = 0;
int patch_mode = 0, nul_term_line = 0, read_from_stdin = 0, unborn;
- char **stdin_paths = NULL;
- int stdin_nr = 0, stdin_alloc = 0;
+ struct argv_array stdin_paths = ARGV_ARRAY_INIT;
const char *rev;
struct object_id oid;
struct pathspec pathspec;
@@ -325,18 +325,15 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
die(_("line is badly quoted"));
strbuf_swap(&buf, &unquoted);
}
- ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
- stdin_paths[stdin_nr++] = xstrdup(buf.buf);
+ argv_array_push(&stdin_paths, buf.buf);
strbuf_reset(&buf);
}
strbuf_release(&unquoted);
strbuf_release(&buf);
- ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
- stdin_paths[stdin_nr++] = NULL;
flags |= PATHSPEC_LITERAL_PATH;
parse_pathspec(&pathspec, 0, flags, prefix,
- (const char **)stdin_paths);
+ stdin_paths.argv);
} else if (nul_term_line)
die(_("-z requires --stdin"));
@@ -431,11 +428,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (!pathspec.nr)
remove_branch_state();
- if (stdin_paths) {
- while (stdin_nr)
- free(stdin_paths[--stdin_nr]);
- free(stdin_paths);
- }
+ argv_array_clear(&stdin_paths);
return update_ref_status;
}
--
2.11.1.windows.prerelease.2.9.g3014b57
^ permalink raw reply
* [PATCH v3 1/1] reset: support the --stdin option
From: Johannes Schindelin @ 2017-01-27 17:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Jeff King
In-Reply-To: <cover.1485538197.git.johannes.schindelin@gmx.de>
Just like with other Git commands, this option makes it read the paths
from the standard input. It comes in handy when resetting many, many
paths at once and wildcards are not an option (e.g. when the paths are
generated by a tool).
Note: we first parse the entire list and perform the actual reset action
only in a second phase. Not only does this make things simpler, it also
helps performance, as do_diff_cache() traverses the index and the
(sorted) pathspecs in simultaneously to avoid unnecessary lookups.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Documentation/git-reset.txt | 10 ++++++++++
builtin/reset.c | 47 ++++++++++++++++++++++++++++++++++++++++++++-
t/t7107-reset-stdin.sh | 33 +++++++++++++++++++++++++++++++
3 files changed, 89 insertions(+), 1 deletion(-)
create mode 100755 t/t7107-reset-stdin.sh
diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 25432d9257..d319ed9b20 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -10,6 +10,7 @@ SYNOPSIS
[verse]
'git reset' [-q] [<tree-ish>] [--] <paths>...
'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]
+'git reset' [-q] [--stdin [-z]] [<tree-ish>]
'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
DESCRIPTION
@@ -97,6 +98,15 @@ OPTIONS
--quiet::
Be quiet, only report errors.
+--stdin::
+ Instead of taking list of paths from the command line,
+ read list of paths from the standard input. The paths are
+ read verbatim, i.e. not handled as pathspecs. Paths are
+ separated by LF (i.e. one path per line) by default.
+
+-z::
+ Only meaningful with `--stdin`; paths are separated with
+ NUL character instead of LF.
EXAMPLES
--------
diff --git a/builtin/reset.c b/builtin/reset.c
index 8ab915bfcb..fe7723c179 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -21,10 +21,14 @@
#include "parse-options.h"
#include "unpack-trees.h"
#include "cache-tree.h"
+#include "strbuf.h"
+#include "quote.h"
+#include "argv-array.h"
static const char * const git_reset_usage[] = {
N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
N_("git reset [-q] [<tree-ish>] [--] <paths>..."),
+ N_("git reset [-q] [--stdin [-z]] [<tree-ish>]"),
N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),
NULL
};
@@ -267,7 +271,8 @@ static int reset_refs(const char *rev, const struct object_id *oid)
int cmd_reset(int argc, const char **argv, const char *prefix)
{
int reset_type = NONE, update_ref_status = 0, quiet = 0;
- int patch_mode = 0, unborn;
+ int patch_mode = 0, nul_term_line = 0, read_from_stdin = 0, unborn;
+ struct argv_array stdin_paths = ARGV_ARRAY_INIT;
const char *rev;
struct object_id oid;
struct pathspec pathspec;
@@ -286,6 +291,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
OPT_BOOL('N', "intent-to-add", &intent_to_add,
N_("record only the fact that removed paths will be added later")),
+ OPT_BOOL('z', NULL, &nul_term_line,
+ N_("paths are separated with NUL character")),
+ OPT_BOOL(0, "stdin", &read_from_stdin,
+ N_("read paths from <stdin>")),
OPT_END()
};
@@ -295,6 +304,40 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
PARSE_OPT_KEEP_DASHDASH);
parse_args(&pathspec, argv, prefix, patch_mode, &rev);
+ if (read_from_stdin) {
+ strbuf_getline_fn getline_fn = nul_term_line ?
+ strbuf_getline_nul : strbuf_getline_lf;
+ int flags = PATHSPEC_PREFER_FULL |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP;
+ struct strbuf buf = STRBUF_INIT;
+ struct strbuf unquoted = STRBUF_INIT;
+
+ if (patch_mode)
+ die(_("--stdin is incompatible with --patch"));
+
+ if (pathspec.nr)
+ die(_("--stdin is incompatible with path arguments"));
+
+ while (getline_fn(&buf, stdin) != EOF) {
+ if (!nul_term_line && buf.buf[0] == '"') {
+ strbuf_reset(&unquoted);
+ if (unquote_c_style(&unquoted, buf.buf, NULL))
+ die(_("line is badly quoted"));
+ strbuf_swap(&buf, &unquoted);
+ }
+ argv_array_push(&stdin_paths, buf.buf);
+ strbuf_reset(&buf);
+ }
+ strbuf_release(&unquoted);
+ strbuf_release(&buf);
+
+ flags |= PATHSPEC_LITERAL_PATH;
+ parse_pathspec(&pathspec, 0, flags, prefix,
+ stdin_paths.argv);
+
+ } else if (nul_term_line)
+ die(_("-z requires --stdin"));
+
unborn = !strcmp(rev, "HEAD") && get_sha1("HEAD", oid.hash);
if (unborn) {
/* reset on unborn branch: treat as reset to empty tree */
@@ -385,5 +428,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (!pathspec.nr)
remove_branch_state();
+ argv_array_clear(&stdin_paths);
+
return update_ref_status;
}
diff --git a/t/t7107-reset-stdin.sh b/t/t7107-reset-stdin.sh
new file mode 100755
index 0000000000..997dc42dd2
--- /dev/null
+++ b/t/t7107-reset-stdin.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+
+test_description='reset --stdin'
+
+. ./test-lib.sh
+
+test_expect_success 'reset --stdin' '
+ test_commit hello &&
+ git rm hello.t &&
+ test -z "$(git ls-files hello.t)" &&
+ echo hello.t | git reset --stdin &&
+ test hello.t = "$(git ls-files hello.t)"
+'
+
+test_expect_success 'reset --stdin -z' '
+ test_commit world &&
+ git rm hello.t world.t &&
+ test -z "$(git ls-files hello.t world.t)" &&
+ printf world.tQworld.tQhello.tQ | q_to_nul | git reset --stdin -z &&
+ printf "hello.t\nworld.t\n" >expect &&
+ git ls-files >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--stdin requires --mixed' '
+ echo hello.t >list &&
+ test_must_fail git reset --soft --stdin <list &&
+ test_must_fail git reset --hard --stdin <list &&
+ git reset --mixed --stdin <list
+'
+
+test_done
+
--
2.11.1.windows.prerelease.2.9.g3014b57
^ permalink raw reply related
* Re: [PATCH v2 1/1] reset: support the --stdin option
From: Johannes Schindelin @ 2017-01-27 17:34 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, Jakub Narębski
In-Reply-To: <20170127170422.lvpghp6jdud37zxx@sigill.intra.peff.net>
Hi Peff,
On Fri, 27 Jan 2017, Jeff King wrote:
> On Fri, Jan 27, 2017 at 01:38:55PM +0100, Johannes Schindelin wrote:
>
> A few minor suggestions:
>
> > +--stdin::
> > + Instead of taking list of paths from the command line,
> > + read list of paths from the standard input. Paths are
> > + separated by LF (i.e. one path per line) by default.
> > +
> > +-z::
> > + Only meaningful with `--stdin`; paths are separated with
> > + NUL character instead of LF.
>
> Is it worth clarifying that these are paths, not pathspecs? The word
> "paths" is used to refer to the pathspecs on the command-line elsewhere
> in the document.
>
> It might also be worth mentioning the quoting rules for the non-z case.
I think this would be overkill. In reality, --stdin without -z does not
make much sense, anyway.
If you feel strongly about it, I encourage you to submit a follow-up
patch.
The rest of your suggestions have been implemented in v3.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH v2] git-p4: Fix git-p4.mapUser on Windows
From: Junio C Hamano @ 2017-01-27 17:33 UTC (permalink / raw)
To: Luke Diamand, Lars Schneider; +Cc: git, George Vanburgh
In-Reply-To: <01020159d4ea6861-fce45d61-8be5-49b0-ab4e-d161b9429795-000000@eu-west-1.amazonses.com>
George Vanburgh <george@vanburgh.me> writes:
> From: George Vanburgh <gvanburgh@bloomberg.net>
>
> When running git-p4 on Windows, with multiple git-p4.mapUser entries in
> git config - no user mappings are applied to the generated repository.
> ...
> Using splitlines solves this issue, by splitting config on all
> typical delimiters ('\n', '\r\n' etc.)
Luke, Lars, this version seems to be in line with the conclusion of
your earlier reviews, e.g.
<CAE5ih7_+Vc9oqKdjo8h2vgZPup4pto9wd=sBb=W6hCs4tuW2Jg@mail.gmail.com>
Even though it looks OK to my eyes, I'll wait for Acks or further
refinement suggestions from either of you two before acting on this
patch.
Thanks.
> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>
> ---
> git-p4.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index f427bf6..b66f68b 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -656,7 +656,7 @@ def gitConfigInt(key):
> def gitConfigList(key):
> if not _gitConfig.has_key(key):
> s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
> - _gitConfig[key] = s.strip().split(os.linesep)
> + _gitConfig[key] = s.strip().splitlines()
> if _gitConfig[key] == ['']:
> _gitConfig[key] = []
> return _gitConfig[key]
>
> --
> https://github.com/git/git/pull/319
^ permalink raw reply
* [PATCH v4] t/Makefile: add a rule to re-run previously-failed tests
From: Johannes Schindelin @ 2017-01-27 17:21 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Sverre Rabbelier,
Ævar Arnfjörð Bjarmason, Matthieu Moy
In-Reply-To: <85044791cfcba35c1ad3d8138051f3f075cb0646.1485526641.git.johannes.schindelin@gmx.de>
This patch automates the process of determining which tests failed
previously and re-running them.
While developing patch series, it is a good practice to run the test
suite from time to time, just to make sure that obvious bugs are caught
early. With complex patch series, it is common to run `make -j15 -k
test`, i.e. run the tests in parallel and *not* stop at the first
failing test but continue. This has the advantage of identifying
possibly multiple problems in one big test run.
It is particularly important to reduce the turn-around time thusly on
Windows, where the test suite spends 45 minutes on the computer on which
this patch was developed.
It is the most convenient way to determine which tests failed after
running the entire test suite, in parallel, to look for left-over "trash
directory.t*" subdirectories in the t/ subdirectory. However, those
directories might live outside t/ when overridden using the
--root=<directory> option, to which the Makefile has no access. The next
best method is to grep explicitly for failed tests in the test-results/
directory, which the Makefile *can* access.
Please note that the often-recommended `prove` tool requires Perl, and
that opens a whole new can of worms on Windows. As no native Windows Perl
comes with Subversion bindings, we have to use a Perl in Git for Windows
that uses the POSIX emulation layer named MSYS2 (which is a portable
version of Cygwin). When using this emulation layer under stress, e.g.
when running massively-parallel tests, unexplicable crashes occur quite
frequently, and instead of having a solution to the original problem, the
developer now has an additional, quite huge problem. For that reason, this
developer rejected `prove` as a solution and went with this patch instead.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/failing-tests-v4
Fetch-It-Via: git fetch https://github.com/dscho/git failing-tests-v4
t/Makefile | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/t/Makefile b/t/Makefile
index d613935f14..1bb06c36f2 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -35,6 +35,12 @@ all: $(DEFAULT_TEST_TARGET)
test: pre-clean $(TEST_LINT)
$(MAKE) aggregate-results-and-cleanup
+failed:
+ @failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \
+ grep -l '^failed [1-9]' *.counts | \
+ sed -n 's/\.counts$$/.sh/p') && \
+ test -z "$$failed" || $(MAKE) $$failed
+
prove: pre-clean $(TEST_LINT)
@echo "*** prove ***"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
$(MAKE) clean-except-prove-cache
base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe
--
2.11.1.windows.prerelease.2.9.g3014b57
^ permalink raw reply related
* show all merge conflicts
From: Michael Spiegel @ 2017-01-27 16:56 UTC (permalink / raw)
To: git
Hi folks,
I'm trying to determine whether a merge required a conflict to resolve
after the merge has occurred. The git book has some advice
(https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging) to use
`git show` on the merge commit or use `git log --cc -p -1`. These
strategies work when the merge conflict was resolved with a change
that is different from either parent. When the conflict is resolved
with a change that is the same as one of the parents, then these
commands are indistinguishable from a merge that did not conflict. Is
it possible to distinguish between a conflict-free merge and a merge
conflict that is resolved by with the changes from one the parents?
Thanks,
--Michael
^ permalink raw reply
* Re: [PATCH v3] t/Makefile: add a rule to re-run previously-failed tests
From: Jeff King @ 2017-01-27 17:07 UTC (permalink / raw)
To: Johannes Schindelin
Cc: git, Junio C Hamano, Sverre Rabbelier,
Ævar Arnfjörð Bjarmason, Matthieu Moy
In-Reply-To: <85044791cfcba35c1ad3d8138051f3f075cb0646.1485526641.git.johannes.schindelin@gmx.de>
On Fri, Jan 27, 2017 at 03:17:36PM +0100, Johannes Schindelin wrote:
> This patch automates the process of determinig which tests failed
> previously and re-running them.
s/determinig/determining/
Patch otherwise looks good, and I'm happy to be rid of the sed
complexity from v2.
-Peff
^ permalink raw reply
* Re: [PATCH v2 1/1] reset: support the --stdin option
From: Jeff King @ 2017-01-27 17:04 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano, Jakub Narębski
In-Reply-To: <3b0bad26045ed175f40f050e15b65cb6a8f2368c.1485520718.git.johannes.schindelin@gmx.de>
On Fri, Jan 27, 2017 at 01:38:55PM +0100, Johannes Schindelin wrote:
> Just like with other Git commands, this option makes it read the paths
> from the standard input. It comes in handy when resetting many, many
> paths at once and wildcards are not an option (e.g. when the paths are
> generated by a tool).
>
> Note: we first parse the entire list and perform the actual reset action
> only in a second phase. Not only does this make things simpler, it also
> helps performance, as do_diff_cache() traverses the index and the
> (sorted) pathspecs in simultaneously to avoid unnecessary lookups.
This looks OK to me. At first I wondered if using PATHSPEC_LITERAL_PATH
was consistent with other "--stdin" tools. I think it mostly is (or at
least consistent with checkout-index). The exception is "rev-list
--stdin", but that's probably fine. It is taking rev-list arguments in
the first place, not a list of paths.
A few minor suggestions:
> +--stdin::
> + Instead of taking list of paths from the command line,
> + read list of paths from the standard input. Paths are
> + separated by LF (i.e. one path per line) by default.
> +
> +-z::
> + Only meaningful with `--stdin`; paths are separated with
> + NUL character instead of LF.
Is it worth clarifying that these are paths, not pathspecs? The word
"paths" is used to refer to the pathspecs on the command-line elsewhere
in the document.
It might also be worth mentioning the quoting rules for the non-z case.
> @@ -267,7 +270,9 @@ static int reset_refs(const char *rev, const struct object_id *oid)
> int cmd_reset(int argc, const char **argv, const char *prefix)
> {
> int reset_type = NONE, update_ref_status = 0, quiet = 0;
> - int patch_mode = 0, unborn;
> + int patch_mode = 0, nul_term_line = 0, read_from_stdin = 0, unborn;
> + char **stdin_paths = NULL;
> + int stdin_nr = 0, stdin_alloc = 0;
This list is a good candidate for an argv_array, I think. So:
struct argv_array stdin_paths = ARGV_ARRAY_INIT;
> + ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
> + stdin_paths[stdin_nr++] = xstrdup(buf.buf);
argv_array_push(&stdin_paths, buf.buf);
> + ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
> + stdin_paths[stdin_nr++] = NULL;
This goes away because argv_array handles it for you.
> + if (stdin_paths) {
> + while (stdin_nr)
> + free(stdin_paths[--stdin_nr]);
> + free(stdin_paths);
> + }
argv_array_clear(&stdin_paths);
> @@ -295,6 +304,43 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
> PARSE_OPT_KEEP_DASHDASH);
> parse_args(&pathspec, argv, prefix, patch_mode, &rev);
>
> + if (read_from_stdin) {
> + strbuf_getline_fn getline_fn = nul_term_line ?
> + strbuf_getline_nul : strbuf_getline_lf;
> + int flags = PATHSPEC_PREFER_FULL |
> + PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP;
You set two flags here...
> + ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
> + stdin_paths[stdin_nr++] = NULL;
> + flags |= PATHSPEC_LITERAL_PATH;
> + parse_pathspec(&pathspec, 0, flags, prefix,
> + (const char **)stdin_paths);
...and then one more here. They all seem to be set unconditionally, and
we never look at "flags" between the two lines. I think it would be more
obvious to set them all in the same place.
> + } else if (nul_term_line)
> + die(_("-z requires --stdin"));
> +
Hmm, there's our brace question coming up again. :)
> diff --git a/t/t7107-reset-stdin.sh b/t/t7107-reset-stdin.sh
> new file mode 100755
> index 0000000000..997dc42dd2
> --- /dev/null
> +++ b/t/t7107-reset-stdin.sh
> @@ -0,0 +1,33 @@
> +#!/bin/sh
> +
> +test_description='reset --stdin'
Here are a few things we might want to test that I didn't see covered:
- feeding path X does not reset path Y
- de-quoting in the non-z case
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] use absolute_pathdup()
From: René Scharfe @ 2017-01-27 15:39 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git List, Junio C Hamano
In-Reply-To: <alpine.DEB.2.20.1701271121120.3469@virtualbox>
Hi Dscho,
Am 27.01.2017 um 11:21 schrieb Johannes Schindelin:
> On Thu, 26 Jan 2017, René Scharfe wrote:
>> Apply the symantic patch for converting callers that duplicate the
>
> s/symantic/semantic/
thank you! I wonder where this came from. And where my spellchecker
went without as much as a farewell. Reinstalled it now..
René
^ permalink raw reply
* [PATCH v3] t/Makefile: add a rule to re-run previously-failed tests
From: Johannes Schindelin @ 2017-01-27 14:17 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Sverre Rabbelier,
Ævar Arnfjörð Bjarmason, Matthieu Moy
In-Reply-To: <0dfa96b17edfe84ba19c7e57fe0b017c77943e0c.1472478285.git.johannes.schindelin@gmx.de>
This patch automates the process of determinig which tests failed
previously and re-running them.
While developing patch series, it is a good practice to run the test
suite from time to time, just to make sure that obvious bugs are caught
early. With complex patch series, it is common to run `make -j15 -k
test`, i.e. run the tests in parallel and *not* stop at the first
failing test but continue. This has the advantage of identifying
possibly multiple problems in one big test run.
It is particularly important to reduce the turn-around time thusly on
Windows, where the test suite spends 45 minutes on the computer on which
this patch was developed.
It is the most convenient way to determine which tests failed after
running the entire test suite, in parallel, to look for left-over "trash
directory.t*" subdirectories in the t/ subdirectory. However, those
directories might live outside t/ when overridden using the
--root=<directory> option, to which the Makefile has no access. The next
best method is to grep explicitly for failed tests in the test-results/
directory, which the Makefile *can* access.
Please note that the often-recommended `prove` tool requires Perl, and
that opens a whole new can of worms on Windows. As no native Windows Perl
comes with Subversion bindings, we have to use a Perl in Git for Windows
that uses the POSIX emulation layer named MSYS2 (which is a portable
version of Cygwin). When using this emulation layer under stress, e.g.
when running massively-parallel tests, unexplicable crashes occur quite
frequently, and instead of having a solution to the original problem, the
developer now has an additional, quite huge problem. For that reason, this
developer rejected `prove` as a solution and went with this patch instead.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/failing-tests-v3
Fetch-It-Via: git fetch https://github.com/dscho/git failing-tests-v3
Interdiff vs v2:
diff --git a/t/Makefile b/t/Makefile
index 8aa6a72a70..1bb06c36f2 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -37,9 +37,8 @@ test: pre-clean $(TEST_LINT)
failed:
@failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \
- grep -l '^failed [1-9]' $$(ls -t *.counts | \
- sed 'G;h;/^\(t[^.]*\)-[0-9]*\..*\n\1-[0-9]*\./d;P;d') | \
- sed -n 's/-[0-9]*\.counts$$/.sh/p') && \
+ grep -l '^failed [1-9]' *.counts | \
+ sed -n 's/\.counts$$/.sh/p') && \
test -z "$$failed" || $(MAKE) $$failed
prove: pre-clean $(TEST_LINT)
t/Makefile | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/t/Makefile b/t/Makefile
index d613935f14..1bb06c36f2 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -35,6 +35,12 @@ all: $(DEFAULT_TEST_TARGET)
test: pre-clean $(TEST_LINT)
$(MAKE) aggregate-results-and-cleanup
+failed:
+ @failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \
+ grep -l '^failed [1-9]' *.counts | \
+ sed -n 's/\.counts$$/.sh/p') && \
+ test -z "$$failed" || $(MAKE) $$failed
+
prove: pre-clean $(TEST_LINT)
@echo "*** prove ***"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
$(MAKE) clean-except-prove-cache
base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe
--
2.11.1.windows.prerelease.2.9.g3014b57
^ permalink raw reply related
* Re: [PATCH] test-lib: on FreeBSD, look for unzip(1) in /usr/local/bin/
From: Johannes Schindelin @ 2017-01-27 14:03 UTC (permalink / raw)
To: Eric Wong; +Cc: git, Junio C Hamano
In-Reply-To: <20160721171025.GA30979@plume>
Hi Junio,
On Thu, 21 Jul 2016, Eric Wong wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> wrote:
> > The common work-around is to install Info-Zip on FreeBSD, into
> > /usr/local/bin/.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Thanks, t5003 now works out-of-the-box.
> Tested with Info-ZIP unzip installed and uninstalled.
>
> Tested-by: Eric Wong <e@80x24.org>
Did you forget about this?
Ciao,
Johannes
^ permalink raw reply
* [PATCH] help: correct behavior for is_executable on Windows
From: Johannes Schindelin @ 2017-01-27 13:50 UTC (permalink / raw)
To: git; +Cc: Heiko Voigt, Junio C Hamano
From: Heiko Voigt <hvoigt@hvoigt.net>
The previous implementation said that the filesystem information on
Windows is not reliable to determine whether a file is executable. To
gather this information it was peeking into the first two bytes of a
file to see whether it looks executable.
Apart from the fact that on Windows executables are defined as such by
their extension (and Git has special code to support "executing" scripts
when they have a she-bang line) it leads to serious performance
problems: not only do we have to open many files now, it gets even
slower when a virus scanner is running.
See the following measurements (in seconds) as an example, where we
execute a simple program that simply lists the directory contents and
calls open() on every listed file:
With virus scanner running (coldcache):
$ ./a.exe /libexec/git-core/
before open (git-add.exe): 0.000000
after open (git-add.exe): 0.412873
before open (git-annotate.exe): 0.000175
after open (git-annotate.exe): 0.397925
before open (git-apply.exe): 0.000243
after open (git-apply.exe): 0.399996
before open (git-archive.exe): 0.000147
after open (git-archive.exe): 0.397783
before open (git-bisect--helper.exe): 0.000160
after open (git-bisect--helper.exe): 0.397700
before open (git-blame.exe): 0.000160
after open (git-blame.exe): 0.399136
...
With virus scanner running (hotcache):
$ ./a.exe /libexec/git-core/
before open (git-add.exe): 0.000000
after open (git-add.exe): 0.000325
before open (git-annotate.exe): 0.000229
after open (git-annotate.exe): 0.000177
before open (git-apply.exe): 0.000167
after open (git-apply.exe): 0.000150
before open (git-archive.exe): 0.000154
after open (git-archive.exe): 0.000156
before open (git-bisect--helper.exe): 0.000132
after open (git-bisect--helper.exe): 0.000180
before open (git-blame.exe): 0.000718
after open (git-blame.exe): 0.000724
...
With this patch I get:
$ time git help git
Launching default browser to display HTML ...
real 0m8.723s
user 0m0.000s
sys 0m0.000s
and without
$ time git help git
Launching default browser to display HTML ...
real 1m37.734s
user 0m0.000s
sys 0m0.031s
both tests with cold cache and giving the machine some time to settle
down after restart.
Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/help-is-exe-v1
Fetch-It-Via: git fetch https://github.com/dscho/git help-is-exe-v1
help.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/help.c b/help.c
index 53e2a67e00..bc6cd19cf3 100644
--- a/help.c
+++ b/help.c
@@ -105,7 +105,22 @@ static int is_executable(const char *name)
return 0;
#if defined(GIT_WINDOWS_NATIVE)
-{ /* cannot trust the executable bit, peek into the file instead */
+ /*
+ * On Windows there is no executable bit. The file extension
+ * indicates whether it can be run as an executable, and Git
+ * has special-handling to detect scripts and launch them
+ * through the indicated script interpreter. We test for the
+ * file extension first because virus scanners may make
+ * it quite expensive to open many files.
+ */
+ if (ends_with(name, ".exe"))
+ return S_IXUSR;
+
+{
+ /*
+ * Now that we know it does not have an executable extension,
+ * peek into the file instead.
+ */
char buf[3] = { 0 };
int n;
int fd = open(name, O_RDONLY);
@@ -113,8 +128,8 @@ static int is_executable(const char *name)
if (fd >= 0) {
n = read(fd, buf, 2);
if (n == 2)
- /* DOS executables start with "MZ" */
- if (!strcmp(buf, "#!") || !strcmp(buf, "MZ"))
+ /* look for a she-bang */
+ if (!strcmp(buf, "#!"))
st.st_mode |= S_IXUSR;
close(fd);
}
base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe
--
2.11.1.windows.prerelease.2.9.g3014b57
^ permalink raw reply related
* [PATCH v2 0/1] Support `git reset --stdin`
From: Johannes Schindelin @ 2017-01-27 12:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Jeff King
In-Reply-To: <cover.1476202100.git.johannes.schindelin@gmx.de>
This feature was missing, and made it cumbersome for third-party
tools to reset a lot of paths in one go.
Support for --stdin has been added, following builtin/checkout-index.c's
example.
Changes since v1:
- adjusted commit message to explain why we read everything before
resetting
- fixed synopsis (`--stdin` does not work with `-- <path>...`)
- avoid handling patch_mode twice
- use PATHSPEC_LITERAL_PATH to avoid interpreting input as wildcards
Johannes Schindelin (1):
reset: support the --stdin option
Documentation/git-reset.txt | 9 ++++++++
builtin/reset.c | 54 ++++++++++++++++++++++++++++++++++++++++++++-
t/t7107-reset-stdin.sh | 33 +++++++++++++++++++++++++++
3 files changed, 95 insertions(+), 1 deletion(-)
create mode 100755 t/t7107-reset-stdin.sh
base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe
Published-As: https://github.com/dscho/git/releases/tag/reset-stdin-v2
Fetch-It-Via: git fetch https://github.com/dscho/git reset-stdin-v2
Interdiff vs v1:
diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 533ef69f91..abb71bb805 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -8,8 +8,9 @@ git-reset - Reset current HEAD to the specified state
SYNOPSIS
--------
[verse]
-'git reset' [-q] [--stdin [-z]] [<tree-ish>] [--] <paths>...
+'git reset' [-q] [<tree-ish>] [--] <paths>...
'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]
+'git reset' [-q] [--stdin [-z]] [<tree-ish>]
'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
DESCRIPTION
diff --git a/builtin/reset.c b/builtin/reset.c
index 6de3936aed..1d3075b7ee 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -26,7 +26,8 @@
static const char * const git_reset_usage[] = {
N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
- N_("git reset [-q] [--stdin [-z]] [<tree-ish>] [--] <paths>..."),
+ N_("git reset [-q] [<tree-ish>] [--] <paths>..."),
+ N_("git reset [-q] [--stdin [-z]] [<tree-ish>]"),
N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),
NULL
};
@@ -317,9 +318,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (pathspec.nr)
die(_("--stdin is incompatible with path arguments"));
- if (patch_mode)
- flags |= PATHSPEC_PREFIX_ORIGIN;
-
while (getline_fn(&buf, stdin) != EOF) {
if (!nul_term_line && buf.buf[0] == '"') {
strbuf_reset(&unquoted);
@@ -336,8 +334,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
stdin_paths[stdin_nr++] = NULL;
+ flags |= PATHSPEC_LITERAL_PATH;
parse_pathspec(&pathspec, 0, flags, prefix,
(const char **)stdin_paths);
+
} else if (nul_term_line)
die(_("-z requires --stdin"));
--
2.11.1.windows.prerelease.2.9.g3014b57
^ permalink raw reply
* [PATCH v2 1/1] reset: support the --stdin option
From: Johannes Schindelin @ 2017-01-27 12:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Jeff King
In-Reply-To: <cover.1485520718.git.johannes.schindelin@gmx.de>
Just like with other Git commands, this option makes it read the paths
from the standard input. It comes in handy when resetting many, many
paths at once and wildcards are not an option (e.g. when the paths are
generated by a tool).
Note: we first parse the entire list and perform the actual reset action
only in a second phase. Not only does this make things simpler, it also
helps performance, as do_diff_cache() traverses the index and the
(sorted) pathspecs in simultaneously to avoid unnecessary lookups.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Documentation/git-reset.txt | 9 ++++++++
builtin/reset.c | 54 ++++++++++++++++++++++++++++++++++++++++++++-
t/t7107-reset-stdin.sh | 33 +++++++++++++++++++++++++++
3 files changed, 95 insertions(+), 1 deletion(-)
create mode 100755 t/t7107-reset-stdin.sh
diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 25432d9257..abb71bb805 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -10,6 +10,7 @@ SYNOPSIS
[verse]
'git reset' [-q] [<tree-ish>] [--] <paths>...
'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]
+'git reset' [-q] [--stdin [-z]] [<tree-ish>]
'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
DESCRIPTION
@@ -97,6 +98,14 @@ OPTIONS
--quiet::
Be quiet, only report errors.
+--stdin::
+ Instead of taking list of paths from the command line,
+ read list of paths from the standard input. Paths are
+ separated by LF (i.e. one path per line) by default.
+
+-z::
+ Only meaningful with `--stdin`; paths are separated with
+ NUL character instead of LF.
EXAMPLES
--------
diff --git a/builtin/reset.c b/builtin/reset.c
index 8ab915bfcb..1d3075b7ee 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -21,10 +21,13 @@
#include "parse-options.h"
#include "unpack-trees.h"
#include "cache-tree.h"
+#include "strbuf.h"
+#include "quote.h"
static const char * const git_reset_usage[] = {
N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
N_("git reset [-q] [<tree-ish>] [--] <paths>..."),
+ N_("git reset [-q] [--stdin [-z]] [<tree-ish>]"),
N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),
NULL
};
@@ -267,7 +270,9 @@ static int reset_refs(const char *rev, const struct object_id *oid)
int cmd_reset(int argc, const char **argv, const char *prefix)
{
int reset_type = NONE, update_ref_status = 0, quiet = 0;
- int patch_mode = 0, unborn;
+ int patch_mode = 0, nul_term_line = 0, read_from_stdin = 0, unborn;
+ char **stdin_paths = NULL;
+ int stdin_nr = 0, stdin_alloc = 0;
const char *rev;
struct object_id oid;
struct pathspec pathspec;
@@ -286,6 +291,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
OPT_BOOL('N', "intent-to-add", &intent_to_add,
N_("record only the fact that removed paths will be added later")),
+ OPT_BOOL('z', NULL, &nul_term_line,
+ N_("paths are separated with NUL character")),
+ OPT_BOOL(0, "stdin", &read_from_stdin,
+ N_("read paths from <stdin>")),
OPT_END()
};
@@ -295,6 +304,43 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
PARSE_OPT_KEEP_DASHDASH);
parse_args(&pathspec, argv, prefix, patch_mode, &rev);
+ if (read_from_stdin) {
+ strbuf_getline_fn getline_fn = nul_term_line ?
+ strbuf_getline_nul : strbuf_getline_lf;
+ int flags = PATHSPEC_PREFER_FULL |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP;
+ struct strbuf buf = STRBUF_INIT;
+ struct strbuf unquoted = STRBUF_INIT;
+
+ if (patch_mode)
+ die(_("--stdin is incompatible with --patch"));
+
+ if (pathspec.nr)
+ die(_("--stdin is incompatible with path arguments"));
+
+ while (getline_fn(&buf, stdin) != EOF) {
+ if (!nul_term_line && buf.buf[0] == '"') {
+ strbuf_reset(&unquoted);
+ if (unquote_c_style(&unquoted, buf.buf, NULL))
+ die(_("line is badly quoted"));
+ strbuf_swap(&buf, &unquoted);
+ }
+ ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
+ stdin_paths[stdin_nr++] = xstrdup(buf.buf);
+ strbuf_reset(&buf);
+ }
+ strbuf_release(&unquoted);
+ strbuf_release(&buf);
+
+ ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
+ stdin_paths[stdin_nr++] = NULL;
+ flags |= PATHSPEC_LITERAL_PATH;
+ parse_pathspec(&pathspec, 0, flags, prefix,
+ (const char **)stdin_paths);
+
+ } else if (nul_term_line)
+ die(_("-z requires --stdin"));
+
unborn = !strcmp(rev, "HEAD") && get_sha1("HEAD", oid.hash);
if (unborn) {
/* reset on unborn branch: treat as reset to empty tree */
@@ -385,5 +431,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (!pathspec.nr)
remove_branch_state();
+ if (stdin_paths) {
+ while (stdin_nr)
+ free(stdin_paths[--stdin_nr]);
+ free(stdin_paths);
+ }
+
return update_ref_status;
}
diff --git a/t/t7107-reset-stdin.sh b/t/t7107-reset-stdin.sh
new file mode 100755
index 0000000000..997dc42dd2
--- /dev/null
+++ b/t/t7107-reset-stdin.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+
+test_description='reset --stdin'
+
+. ./test-lib.sh
+
+test_expect_success 'reset --stdin' '
+ test_commit hello &&
+ git rm hello.t &&
+ test -z "$(git ls-files hello.t)" &&
+ echo hello.t | git reset --stdin &&
+ test hello.t = "$(git ls-files hello.t)"
+'
+
+test_expect_success 'reset --stdin -z' '
+ test_commit world &&
+ git rm hello.t world.t &&
+ test -z "$(git ls-files hello.t world.t)" &&
+ printf world.tQworld.tQhello.tQ | q_to_nul | git reset --stdin -z &&
+ printf "hello.t\nworld.t\n" >expect &&
+ git ls-files >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--stdin requires --mixed' '
+ echo hello.t >list &&
+ test_must_fail git reset --soft --stdin <list &&
+ test_must_fail git reset --hard --stdin <list &&
+ git reset --mixed --stdin <list
+'
+
+test_done
+
--
2.11.1.windows.prerelease.2.9.g3014b57
^ permalink raw reply related
* Re: [PATCH] fixup! worktree move: new command
From: Johannes Schindelin @ 2017-01-27 10:53 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <4f4ae057cd4d72d5b945a856deacd921fb5e7977.1485349447.git.johannes.schindelin@gmx.de>
Hi Junio,
On Wed, 25 Jan 2017, Johannes Schindelin wrote:
> This is required for the test to pass on Windows, where $TRASH_DIRECTORY
> is a POSIX path, while Git works with Windows paths instead. Using
> `$(pwd)` is the common workaround: it reports a Windows path (while `$PWD`
> would report the POSIX equivalent which, however, would only be understood
> by shell and Perl scripts).
>
> Duy, if you re-roll the `worktree-move` patch series, would you terribly
> mind squashing this in?
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
Given that you add all kinds of SQUASH patches in `pu` that have not even
always been discussed on the mailing list, I would like to kindly ask you
to please add this patch to the nd/worktree-move branch for the time being
(i.e. until Duy responds), so that I am not continuously pestered by
failing Continuous Integration builds about a problem for which I have
provided a fix already.
Thanks,
Johannes
^ permalink raw reply
* Re: [PATCH v2 1/1] status: be prepared for not-yet-started interactive rebase
From: Johannes Schindelin @ 2017-01-27 10:49 UTC (permalink / raw)
To: Stefan Beller
Cc: git@vger.kernel.org, Junio C Hamano, Matthieu Moy,
Guillaume Pages
In-Reply-To: <CAGZ79kYLFJYPQu5KSv3hG+_eavO9BHkxHjpVOEs63Nn6Hu1gTg@mail.gmail.com>
Hi Stefan,
On Thu, 26 Jan 2017, Stefan Beller wrote:
> On Thu, Jan 26, 2017 at 8:08 AM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
> > - if (!f)
> > + if (!f) {
> > + if (errno == ENOENT)
> > + return -1;
> > die_errno("Could not open file %s for reading",
> > git_path("%s", fname));
>
> While at it, fix the translation with die_errno(_(..),..) ?
That is not the purpose of my patch. But feel free to offer a follow-up
patch!
Ciao,
Johannes
^ permalink raw reply
* Re: SubmittingPatches: drop temporal reference for PGP signing
From: Cornelius Weig @ 2017-01-27 10:49 UTC (permalink / raw)
To: Philip Oakley, Junio C Hamano
Cc: Stefan Beller, Johannes Sixt, bitte.keine.werbung.einwerfen, git
In-Reply-To: <4B89512D54614F09817EA9901A8B625D@PhilipOakley>
On 01/26/2017 09:58 PM, Philip Oakley wrote:
> From: "Junio C Hamano" <gitster@pobox.com>
>> Cornelius Weig <cornelius.weig@tngtech.com> writes:
>>
>>> How about something along these lines? Does the forward reference
>>> break the main line of thought too severly?
>>
>> I find it a bit distracting for those who know PGP signing has
>> nothing to do with signing off your patch, but I think that is OK
>> because they are not the primary target audience of this part of the
>> document.
>
> Agreed. I this case the target audience was those weren't aware of that.
Yes, maybe the forward reference does give a wrong hint about a
connection between sign-off and pgp-signing. However, I would still vote
for the following change suggested by sbeller@google.com:
-Do not PGP sign your patch, -at least for now-. Most likely, your (...)
+Do not PGP sign your patch. Most likely, your maintainer or other (...)
>
> Maybe even s/by signing off/by adding your Signed-off-by:/ to be sure
> that the reader knows that it is _their certification_ that is being
> sought. Even if it does double up on the 'your'.
>
I don't think doubling on the 'your' will make the heading clearer. The
main intention of this change is to avoid mixups with pgp-signing and
that would IMHO not improve by that.
Besides, I find the colon in the heading a bit awkward. Is the following
version as expressive as with the colon?
-(5) Sign your work
+(5) Certify your work by adding Signed-off-by
^ permalink raw reply
* Re: [PATCH v2 3/3] connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config
From: Johannes Schindelin @ 2017-01-27 10:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Segev Finer, Jeff King
In-Reply-To: <xmqqo9yt4o5i.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Thu, 26 Jan 2017, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > diff --git a/connect.c b/connect.c
> > index 9f750eacb6..7b4437578b 100644
> > --- a/connect.c
> > +++ b/connect.c
> > @@ -691,6 +691,24 @@ static const char *get_ssh_command(void)
> > return NULL;
> > }
> >
> > +static int handle_ssh_variant(int *port_option, int *needs_batch)
> > +{
> > + const char *variant;
> > +
> > + if (!(variant = getenv("GIT_SSH_VARIANT")) &&
> > + git_config_get_string_const("ssh.variant", &variant))
> > + return 0;
> > +
> > + if (!strcmp(variant, "plink") || !strcmp(variant, "putty"))
> > + *port_option = 'P';
> > + else if (!strcmp(variant, "tortoiseplink")) {
> > + *port_option = 'P';
> > + *needs_batch = 1;
> > + }
> > +
> > + return 1;
> > +}
>
> Between handle and get I do not think there is strong reason to
> favor one over the other.
That is correct. "handle" and "get" are two very beautiful words, and none
of them deserves to take a back seat behind the other.
In this case, "get" is inappropriate, though, as the function does not
return the ssh variant, nor does it assign the ssh variant to any variable
to which any of its argument points.
Will try to find the time to address the other issues soon,
Johannes
^ permalink raw reply
* [PATCH v4 5/5] urlmatch: allow globbing for the URL host part
From: Patrick Steinhardt @ 2017-01-27 10:32 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Junio C Hamano, Patrick Steinhardt,
Philip Oakley
In-Reply-To: <cover.1485512626.git.patrick.steinhardt@elego.de>
The URL matching function computes for two URLs whether they match not.
The match is performed by splitting up the URL into different parts and
then doing an exact comparison with the to-be-matched URL.
The main user of `urlmatch` is the configuration subsystem. It allows to
set certain configurations based on the URL which is being connected to
via keys like `http.<url>.*`. A common use case for this is to set
proxies for only some remotes which match the given URL. Unfortunately,
having exact matches for all parts of the URL can become quite tedious
in some setups. Imagine for example a corporate network where there are
dozens or even hundreds of subdomains, which would have to be configured
individually.
Allow users to write an asterisk '*' in place of any 'host' or
'subdomain' label as part of the host name. For example,
"http.https://*.example.com.proxy" sets "http.proxy" for all direct
subdomains of "https://example.com", e.g. "https://foo.example.com", but
not "https://foo.bar.example.com".
Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
Helped-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/config.txt | 5 +++-
t/t1300-repo-config.sh | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
urlmatch.c | 49 +++++++++++++++++++++++++++++---
3 files changed, 121 insertions(+), 5 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 506431267..078e9b490 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1914,7 +1914,10 @@ http.<url>.*::
must match exactly between the config key and the URL.
. Host/domain name (e.g., `example.com` in `https://example.com/`).
- This field must match exactly between the config key and the URL.
+ This field must match between the config key and the URL. It is
+ possible to specify a `*` as part of the host name to match all subdomains
+ at this level. `https://*.example.com/` for example would match
+ `https://foo.example.com/`, but not `https://foo.bar.example.com/`.
. Port number (e.g., `8080` in `http://example.com:8080/`).
This field must match exactly between the config key and the URL.
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 6c844d519..052f12021 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1187,6 +1187,18 @@ test_expect_success 'urlmatch favors more specific URLs' '
cookieFile = /tmp/user.txt
[http "https://averylonguser@example.com/"]
cookieFile = /tmp/averylonguser.txt
+ [http "https://preceding.example.com"]
+ cookieFile = /tmp/preceding.txt
+ [http "https://*.example.com"]
+ cookieFile = /tmp/wildcard.txt
+ [http "https://*.example.com/wildcardwithsubdomain"]
+ cookieFile = /tmp/wildcardwithsubdomain.txt
+ [http "https://trailing.example.com"]
+ cookieFile = /tmp/trailing.txt
+ [http "https://user@*.example.com/"]
+ cookieFile = /tmp/wildcardwithuser.txt
+ [http "https://sub.example.com/"]
+ cookieFile = /tmp/sub.txt
EOF
echo http.cookiefile /tmp/root.txt >expect &&
@@ -1207,6 +1219,66 @@ test_expect_success 'urlmatch favors more specific URLs' '
echo http.cookiefile /tmp/subdirectory.txt >expect &&
git config --get-urlmatch HTTP https://averylonguser@example.com/subdirectory >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/preceding.txt >expect &&
+ git config --get-urlmatch HTTP https://preceding.example.com >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/wildcard.txt >expect &&
+ git config --get-urlmatch HTTP https://wildcard.example.com >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/sub.txt >expect &&
+ git config --get-urlmatch HTTP https://sub.example.com/wildcardwithsubdomain >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/trailing.txt >expect &&
+ git config --get-urlmatch HTTP https://trailing.example.com >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/sub.txt >expect &&
+ git config --get-urlmatch HTTP https://user@sub.example.com >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'urlmatch with wildcard' '
+ cat >.git/config <<-\EOF &&
+ [http]
+ sslVerify
+ [http "https://*.example.com"]
+ sslVerify = false
+ cookieFile = /tmp/cookie.txt
+ EOF
+
+ test_expect_code 1 git config --bool --get-urlmatch doesnt.exist https://good.example.com >actual &&
+ test_must_be_empty actual &&
+
+ echo true >expect &&
+ git config --bool --get-urlmatch http.SSLverify https://example.com >actual &&
+ test_cmp expect actual &&
+
+ echo true >expect &&
+ git config --bool --get-urlmatch http.SSLverify https://good-example.com >actual &&
+ test_cmp expect actual &&
+
+ echo true >expect &&
+ git config --bool --get-urlmatch http.sslverify https://deep.nested.example.com >actual &&
+ test_cmp expect actual &&
+
+ echo false >expect &&
+ git config --bool --get-urlmatch http.sslverify https://good.example.com >actual &&
+ test_cmp expect actual &&
+
+ {
+ echo http.cookiefile /tmp/cookie.txt &&
+ echo http.sslverify false
+ } >expect &&
+ git config --get-urlmatch HTTP https://good.example.com >actual &&
+ test_cmp expect actual &&
+
+ echo http.sslverify >expect &&
+ git config --get-urlmatch HTTP https://more.example.com.au >actual &&
test_cmp expect actual
'
diff --git a/urlmatch.c b/urlmatch.c
index f35d00a6e..bb5267732 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -63,6 +63,49 @@ static int append_normalized_escapes(struct strbuf *buf,
return 1;
}
+static const char *end_of_token(const char *s, int c, size_t n)
+{
+ const char *next = memchr(s, c, n);
+ if (!next)
+ next = s + n;
+ return next;
+}
+
+static int match_host(const struct url_info *url_info,
+ const struct url_info *pattern_info)
+{
+ const char *url = url_info->url + url_info->host_off;
+ const char *pat = pattern_info->url + pattern_info->host_off;
+ int url_len = url_info->host_len;
+ int pat_len = pattern_info->host_len;
+
+ while (url_len && pat_len) {
+ const char *url_next = end_of_token(url, '.', url_len);
+ const char *pat_next = end_of_token(pat, '.', pat_len);
+
+ if (pat_next == pat + 1 && pat[0] == '*')
+ /* wildcard matches anything */
+ ;
+ else if ((pat_next - pat) == (url_next - url) &&
+ !memcmp(url, pat, url_next - url))
+ /* the components are the same */
+ ;
+ else
+ return 0; /* found an unmatch */
+
+ if (url_next < url + url_len)
+ url_next++;
+ url_len -= url_next - url;
+ url = url_next;
+ if (pat_next < pat + pat_len)
+ pat_next++;
+ pat_len -= pat_next - pat;
+ pat = pat_next;
+ }
+
+ return (!url_len && !pat_len);
+}
+
static char *url_normalize_1(const char *url, struct url_info *out_info, char allow_globs)
{
/*
@@ -467,9 +510,7 @@ static int match_urls(const struct url_info *url,
}
/* check the host */
- if (url_prefix->host_len != url->host_len ||
- strncmp(url->url + url->host_off,
- url_prefix->url + url_prefix->host_off, url->host_len))
+ if (!match_host(url, url_prefix))
return 0; /* host names do not match */
/* check the port */
@@ -528,7 +569,7 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)
struct url_info norm_info;
config_url = xmemdupz(key, dot - key);
- norm_url = url_normalize(config_url, &norm_info);
+ norm_url = url_normalize_1(config_url, &norm_info, 1);
free(config_url);
if (!norm_url)
return 0;
--
2.11.0
^ permalink raw reply related
* [PATCH v4 3/5] urlmatch: split host and port fields in `struct url_info`
From: Patrick Steinhardt @ 2017-01-27 10:32 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Junio C Hamano, Patrick Steinhardt,
Philip Oakley
In-Reply-To: <cover.1485512626.git.patrick.steinhardt@elego.de>
The `url_info` structure contains information about a normalized URL
with the URL's components being represented by different fields. The
host and port part though are to be accessed by the same `host` field,
so that getting the host and/or port separately becomes more involved
than really necessary.
To make the port more readily accessible, split up the host and port
fields. Namely, the `host_len` will not include the port length anymore
and a new `port_off` field has been added which includes the offset to
the port, if available.
The only user of these fields is `url_normalize_1`. This change makes it
easier later on to treat host and port differently when introducing
globs for domains.
Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
---
urlmatch.c | 16 ++++++++++++----
urlmatch.h | 9 +++++----
2 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/urlmatch.c b/urlmatch.c
index d350478c0..e328905eb 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -104,7 +104,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
struct strbuf norm;
size_t spanned;
size_t scheme_len, user_off=0, user_len=0, passwd_off=0, passwd_len=0;
- size_t host_off=0, host_len=0, port_len=0, path_off, path_len, result_len;
+ size_t host_off=0, host_len=0, port_off=0, port_len=0, path_off, path_len, result_len;
const char *slash_ptr, *at_ptr, *colon_ptr, *path_start;
char *result;
@@ -263,6 +263,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
return NULL;
}
strbuf_addch(&norm, ':');
+ port_off = norm.len;
strbuf_add(&norm, url, slash_ptr - url);
port_len = slash_ptr - url;
}
@@ -270,7 +271,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
url = slash_ptr;
}
if (host_off)
- host_len = norm.len - host_off;
+ host_len = norm.len - host_off - (port_len ? port_len + 1 : 0);
/*
@@ -378,6 +379,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
out_info->passwd_len = passwd_len;
out_info->host_off = host_off;
out_info->host_len = host_len;
+ out_info->port_off = port_off;
out_info->port_len = port_len;
out_info->path_off = path_off;
out_info->path_len = path_len;
@@ -464,11 +466,17 @@ static int match_urls(const struct url_info *url,
usermatched = 1;
}
- /* check the host and port */
+ /* check the host */
if (url_prefix->host_len != url->host_len ||
strncmp(url->url + url->host_off,
url_prefix->url + url_prefix->host_off, url->host_len))
- return 0; /* host names and/or ports do not match */
+ return 0; /* host names do not match */
+
+ /* check the port */
+ if (url_prefix->port_len != url->port_len ||
+ strncmp(url->url + url->port_off,
+ url_prefix->url + url_prefix->port_off, url->port_len))
+ return 0; /* ports do not match */
/* check the path */
pathmatchlen = url_match_prefix(
diff --git a/urlmatch.h b/urlmatch.h
index 528862adc..0ea812b03 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -18,11 +18,12 @@ struct url_info {
size_t passwd_len; /* length of passwd; if passwd_off != 0 but
passwd_len == 0, an empty passwd was given */
size_t host_off; /* offset into url to start of host name (0 => none) */
- size_t host_len; /* length of host name; this INCLUDES any ':portnum';
+ size_t host_len; /* length of host name;
* file urls may have host_len == 0 */
- size_t port_len; /* if a portnum is present (port_len != 0), it has
- * this length (excluding the leading ':') at the
- * end of the host name (always 0 for file urls) */
+ size_t port_off; /* offset into url to start of port number (0 => none) */
+ size_t port_len; /* if a portnum is present (port_off != 0), it has
+ * this length (excluding the leading ':') starting
+ * from port_off (always 0 for file urls) */
size_t path_off; /* offset into url to the start of the url path;
* this will always point to a '/' character
* after the url has been normalized */
--
2.11.0
^ permalink raw reply related
* [PATCH v4 4/5] urlmatch: include host and port in urlmatch length
From: Patrick Steinhardt @ 2017-01-27 10:32 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Junio C Hamano, Patrick Steinhardt,
Philip Oakley
In-Reply-To: <cover.1485512626.git.patrick.steinhardt@elego.de>
In order to be able to rank positive matches by `urlmatch`, we inspect
the path length and user part to decide whether a match is better than
another match. As all other parts are matched exactly between both URLs,
this is the right thing to do right now.
In the future, though, we want to introduce wild cards for the domain
part. When doing this, though, it does not make sense anymore to only
compare the path lengths. Instead, we also want to compare the domain
lengths to determine which of both URLs matches the host part more
closely.
Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
---
t/t1300-repo-config.sh | 33 ++++++++++++++++++++++++++++
urlmatch.c | 59 +++++++++++++++++++++++++++++---------------------
urlmatch.h | 3 ++-
3 files changed, 69 insertions(+), 26 deletions(-)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 923bfc5a2..6c844d519 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1177,6 +1177,39 @@ test_expect_success 'urlmatch' '
test_cmp expect actual
'
+test_expect_success 'urlmatch favors more specific URLs' '
+ cat >.git/config <<-\EOF &&
+ [http "https://example.com/"]
+ cookieFile = /tmp/root.txt
+ [http "https://example.com/subdirectory"]
+ cookieFile = /tmp/subdirectory.txt
+ [http "https://user@example.com/"]
+ cookieFile = /tmp/user.txt
+ [http "https://averylonguser@example.com/"]
+ cookieFile = /tmp/averylonguser.txt
+ EOF
+
+ echo http.cookiefile /tmp/root.txt >expect &&
+ git config --get-urlmatch HTTP https://example.com >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/subdirectory.txt >expect &&
+ git config --get-urlmatch HTTP https://example.com/subdirectory >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/subdirectory.txt >expect &&
+ git config --get-urlmatch HTTP https://example.com/subdirectory/nested >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/user.txt >expect &&
+ git config --get-urlmatch HTTP https://user@example.com/ >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/subdirectory.txt >expect &&
+ git config --get-urlmatch HTTP https://averylonguser@example.com/subdirectory >actual &&
+ test_cmp expect actual
+'
+
# good section hygiene
test_expect_failure 'unsetting the last key in a section removes header' '
cat >.git/config <<-\EOF &&
diff --git a/urlmatch.c b/urlmatch.c
index e328905eb..f35d00a6e 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -426,7 +426,7 @@ static size_t url_match_prefix(const char *url,
static int match_urls(const struct url_info *url,
const struct url_info *url_prefix,
- int *exactusermatch)
+ struct urlmatch_item *match)
{
/*
* url_prefix matches url if the scheme, host and port of url_prefix
@@ -445,8 +445,8 @@ static int match_urls(const struct url_info *url,
* contained a user name or false if url_prefix did not have a
* user name. If there is no match *exactusermatch is left untouched.
*/
- int usermatched = 0;
- int pathmatchlen;
+ char usermatched = 0;
+ size_t pathmatchlen;
if (!url || !url_prefix || !url->url || !url_prefix->url)
return 0;
@@ -483,22 +483,38 @@ static int match_urls(const struct url_info *url,
url->url + url->path_off,
url_prefix->url + url_prefix->path_off,
url_prefix->url_len - url_prefix->path_off);
+ if (!pathmatchlen)
+ return 0; /* paths do not match */
- if (pathmatchlen && exactusermatch)
- *exactusermatch = usermatched;
- return pathmatchlen;
+ if (match) {
+ match->hostmatch_len = url_prefix->host_len;
+ match->pathmatch_len = pathmatchlen;
+ match->user_matched = usermatched;
+ }
+
+ return 1;
+}
+
+static int cmp_matches(const struct urlmatch_item *a,
+ const struct urlmatch_item *b)
+{
+ if (a->hostmatch_len != b->hostmatch_len)
+ return a->hostmatch_len < b->hostmatch_len ? -1 : 1;
+ if (a->pathmatch_len != b->pathmatch_len)
+ return a->pathmatch_len < b->pathmatch_len ? -1 : 1;
+ if (a->user_matched != b->user_matched)
+ return b->user_matched ? -1 : 1;
+ return 0;
}
int urlmatch_config_entry(const char *var, const char *value, void *cb)
{
struct string_list_item *item;
struct urlmatch_config *collect = cb;
- struct urlmatch_item *matched;
+ struct urlmatch_item matched;
struct url_info *url = &collect->url;
const char *key, *dot;
struct strbuf synthkey = STRBUF_INIT;
- size_t matched_len = 0;
- int user_matched = 0;
int retval;
if (!skip_prefix(var, collect->section, &key) || *(key++) != '.') {
@@ -516,9 +532,9 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)
free(config_url);
if (!norm_url)
return 0;
- matched_len = match_urls(url, &norm_info, &user_matched);
+ retval = match_urls(url, &norm_info, &matched);
free(norm_url);
- if (!matched_len)
+ if (!retval)
return 0;
key = dot + 1;
}
@@ -528,24 +544,17 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)
item = string_list_insert(&collect->vars, key);
if (!item->util) {
- matched = xcalloc(1, sizeof(*matched));
- item->util = matched;
+ item->util = xcalloc(1, sizeof(matched));
} else {
- matched = item->util;
- /*
- * Is our match shorter? Is our match the same
- * length, and without user while the current
- * candidate is with user? Then we cannot use it.
- */
- if (matched_len < matched->matched_len ||
- ((matched_len == matched->matched_len) &&
- (!user_matched && matched->user_matched)))
+ if (cmp_matches(&matched, item->util) <= 0)
+ /*
+ * Our match is worse than the old one,
+ * we cannot use it.
+ */
return 0;
- /* Otherwise, replace it with this one. */
}
- matched->matched_len = matched_len;
- matched->user_matched = user_matched;
+ memcpy(item->util, &matched, sizeof(matched));
strbuf_addstr(&synthkey, collect->section);
strbuf_addch(&synthkey, '.');
strbuf_addstr(&synthkey, key);
diff --git a/urlmatch.h b/urlmatch.h
index 0ea812b03..37ee5da85 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -34,7 +34,8 @@ struct url_info {
extern char *url_normalize(const char *, struct url_info *);
struct urlmatch_item {
- size_t matched_len;
+ size_t hostmatch_len;
+ size_t pathmatch_len;
char user_matched;
};
--
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