* 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
* 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 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: [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
* [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
* [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] 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
* 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 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 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
* 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 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
* 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
* [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
* 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
* 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
* [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
* [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
* 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
* 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: 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] 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: [PATCH 11/12] receive-pack: treat namespace .have lines like alternates
From: Lukas Fleischer @ 2017-01-27 17:45 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <xmqqa8aec40a.fsf@gitster.mtv.corp.google.com>
On Wed, 25 Jan 2017 at 20:51:17, Junio C Hamano wrote:
> [...]
> > diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> > index 8f8762e4a..c55e2f993 100644
> > --- a/builtin/receive-pack.c
> > +++ b/builtin/receive-pack.c
> > @@ -251,8 +251,9 @@ static void show_ref(const char *path, const unsigned char *sha1)
> [...]
> > if (ref_is_hidden(path, path_full))
> [...]
> This is an unrelated tangent, but there may want to be a knob to
> make the code return here without even showing, to make the
> advertisement even smaller and also to stop miniscule information
> leakage? If the namespaced multiple projects are totally unrelated
> (i.e. "My sysadmin gave me a write access only to this single
> directory, so I am using the namespace feature to host these three
> projects that have nothing to do with each other"), showing objects
> of other namespaces will buy us nothing and the user is better off
> without this code showing these refs as ".have".
I think this is already possible using receive.hideRefs (which causes
the ref_is_hidden() branch above to return if applicable).
Having support for suppressing .have lines corresponding to different
namespaces was actually the reason I implemented 78a766ab6 (hideRefs:
add support for matching full refs, 2015-11-03). We have been using
namespaces for hosting the package Git repositories of the Arch Linux
User Repository [1] with a shared object storage for several months now.
See [2] for *some* technical details on how things are implemented; the
last section explains how the hideRefs mechanism can be used to limit
ref advertisement to the "active" namespace.
Regards,
Lukas
[1] https://aur.archlinux.org/
[2] https://git.archlinux.org/aurweb.git/plain/doc/git-interface.txt
^ permalink raw reply
* Re: [PATCH v2 1/1] reset: support the --stdin option
From: Jeff King @ 2017-01-27 17:54 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano, Jakub Narębski
In-Reply-To: <alpine.DEB.2.20.1701271832270.3469@virtualbox>
On Fri, Jan 27, 2017 at 06:34:46PM +0100, Johannes Schindelin wrote:
> > 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.
I think with most Unix tools people tend to use the non-z forms until
they break, and then switch to the z form. :)
And in that sense, this transparently Just Works because the output will
often come from another git tool, which will quote as appropriate.
> 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.
Thanks. I think the path/pathspec thing was the more important of the
two suggestions.
-Peff
^ permalink raw reply
* Re: [PATCH v3] t/Makefile: add a rule to re-run previously-failed tests
From: Johannes Schindelin @ 2017-01-27 17:21 UTC (permalink / raw)
To: Jeff King
Cc: git, Junio C Hamano, Sverre Rabbelier,
Ævar Arnfjörð Bjarmason, Matthieu Moy
In-Reply-To: <20170127170713.kn35br4xsdco7xth@sigill.intra.peff.net>
Hi Peff,
On Fri, 27 Jan 2017, Jeff King wrote:
> 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/
Fixed in v4,
Johannes
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox