* [PATCH 1/2] ls-files: optionally recurse into submodules
From: Brandon Williams @ 2016-09-21 22:42 UTC (permalink / raw)
To: git; +Cc: Brandon Williams
In-Reply-To: <CAKoko1qch_odsEWba0rtCv-DWO0ABS2yprnwGPCgyT6-7H-LdQ@mail.gmail.com>
Allow ls-files to recognize submodules in order to retrieve a list of
files from a repository's submodules. This is done by forking off a
process to recursively call ls-files on all submodules. Also added a
submodule-prefix command in order to prepend paths to child processes.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Documentation/git-ls-files.txt | 11 +++-
builtin/ls-files.c | 61 +++++++++++++++++++++
t/t3007-ls-files-recurse-submodules.sh | 99 ++++++++++++++++++++++++++++++++++
3 files changed, 170 insertions(+), 1 deletion(-)
create mode 100755 t/t3007-ls-files-recurse-submodules.sh
diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index 0d933ac..09e4449 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -18,7 +18,9 @@ SYNOPSIS
[--exclude-per-directory=<file>]
[--exclude-standard]
[--error-unmatch] [--with-tree=<tree-ish>]
- [--full-name] [--abbrev] [--] [<file>...]
+ [--full-name] [--recurse-submodules]
+ [--submodule-prefix=<path>]
+ [--abbrev] [--] [<file>...]
DESCRIPTION
-----------
@@ -137,6 +139,13 @@ a space) at the start of each line:
option forces paths to be output relative to the project
top directory.
+--recurse-submodules::
+ Recursively calls ls-files on each submodule in the repository.
+ Currently there is only support for the --cached mode.
+
+--submodule-prefix=<path>::
+ Prepend the provided path to the output of each file
+
--abbrev[=<n>]::
Instead of showing the full 40-byte hexadecimal object
lines, show only a partial prefix.
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 00ea91a..ffd9ea6 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -14,6 +14,7 @@
#include "resolve-undo.h"
#include "string-list.h"
#include "pathspec.h"
+#include "run-command.h"
static int abbrev;
static int show_deleted;
@@ -28,6 +29,8 @@ static int show_valid_bit;
static int line_terminator = '\n';
static int debug_mode;
static int show_eol;
+static const char *submodule_prefix;
+static int recurse_submodules;
static const char *prefix;
static int max_prefix_len;
@@ -68,6 +71,21 @@ static void write_eolinfo(const struct cache_entry *ce, const char *path)
static void write_name(const char *name)
{
/*
+ * NEEDSWORK: To make this thread-safe, full_name would have to be owned
+ * by the caller.
+ *
+ * full_name get reused across output lines to minimize the allocation
+ * churn.
+ */
+ static struct strbuf full_name = STRBUF_INIT;
+ if (submodule_prefix && *submodule_prefix) {
+ strbuf_reset(&full_name);
+ strbuf_addstr(&full_name, submodule_prefix);
+ strbuf_addstr(&full_name, name);
+ name = full_name.buf;
+ }
+
+ /*
* With "--full-name", prefix_len=0; this caller needs to pass
* an empty string in that case (a NULL is good for "").
*/
@@ -152,6 +170,26 @@ static void show_killed_files(struct dir_struct *dir)
}
}
+/**
+ * Recursively call ls-files on a submodule
+ */
+static void show_gitlink(const struct cache_entry *ce)
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+ int status;
+
+ argv_array_push(&cp.args, "ls-files");
+ argv_array_push(&cp.args, "--recurse-submodules");
+ argv_array_pushf(&cp.args, "--submodule-prefix=%s%s/",
+ submodule_prefix ? submodule_prefix : "",
+ ce->name);
+ cp.git_cmd = 1;
+ cp.dir = ce->name;
+ status = run_command(&cp);
+ if (status)
+ exit(status);
+}
+
static void show_ce_entry(const char *tag, const struct cache_entry *ce)
{
int len = max_prefix_len;
@@ -163,6 +201,10 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)
len, ps_matched,
S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode)))
return;
+ if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) {
+ show_gitlink(ce);
+ return;
+ }
if (tag && *tag && show_valid_bit &&
(ce->ce_flags & CE_VALID)) {
@@ -468,6 +510,10 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
{ OPTION_SET_INT, 0, "full-name", &prefix_len, NULL,
N_("make the output relative to the project top directory"),
PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL },
+ OPT_STRING(0, "submodule-prefix", &submodule_prefix,
+ N_("path"), N_("prepend <path> to each file")),
+ OPT_BOOL(0, "recurse-submodules", &recurse_submodules,
+ N_("recurse through submodules")),
OPT_BOOL(0, "error-unmatch", &error_unmatch,
N_("if any <file> is not in the index, treat this as an error")),
OPT_STRING(0, "with-tree", &with_tree, N_("tree-ish"),
@@ -519,6 +565,21 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
if (require_work_tree && !is_inside_work_tree())
setup_work_tree();
+ if (recurse_submodules &&
+ (show_stage || show_deleted || show_others || show_unmerged ||
+ show_killed || show_modified || show_resolve_undo ||
+ show_valid_bit || show_tag || show_eol))
+ die("ls-files --recurse-submodules can only be used in "
+ "--cached mode");
+
+ if (recurse_submodules && error_unmatch)
+ die("ls-files --recurse-submodules does not support "
+ "--error-unmatch");
+
+ if (recurse_submodules && argc)
+ die("ls-files --recurse-submodules does not support path "
+ "arguments");
+
parse_pathspec(&pathspec, 0,
PATHSPEC_PREFER_CWD |
PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh
new file mode 100755
index 0000000..caf3815
--- /dev/null
+++ b/t/t3007-ls-files-recurse-submodules.sh
@@ -0,0 +1,99 @@
+#!/bin/sh
+
+test_description='Test ls-files recurse-submodules feature
+
+This test verifies the recurse-submodules feature correctly lists files from
+submodules.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup directory structure and submodules' '
+ echo a >a &&
+ mkdir b &&
+ echo b >b/b &&
+ git add a b &&
+ git commit -m "add a and b" &&
+ git init submodule &&
+ echo c >submodule/c &&
+ git -C submodule add c &&
+ git -C submodule commit -m "add c" &&
+ git submodule add ./submodule &&
+ git commit -m "added submodule"
+'
+
+test_expect_success 'ls-files correctly outputs files in submodule' '
+ cat >expect <<-\EOF &&
+ .gitmodules
+ a
+ b/b
+ submodule/c
+ EOF
+
+ git ls-files --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'ls-files does not output files not added to a repo' '
+ cat >expect <<-\EOF &&
+ .gitmodules
+ a
+ b/b
+ submodule/c
+ EOF
+
+ echo a >not_added &&
+ echo b >b/not_added &&
+ echo c >submodule/not_added &&
+ git ls-files --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'ls-files recurses more than 1 level' '
+ cat >expect <<-\EOF &&
+ .gitmodules
+ a
+ b/b
+ submodule/.gitmodules
+ submodule/c
+ submodule/subsub/d
+ EOF
+
+ git init submodule/subsub &&
+ echo d >submodule/subsub/d &&
+ git -C submodule/subsub add d &&
+ git -C submodule/subsub commit -m "add d" &&
+ git -C submodule submodule add ./subsub &&
+ git -C submodule commit -m "added subsub" &&
+ git ls-files --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules does not support using path arguments' '
+ test_must_fail git ls-files --recurse-submodules b 2>actual &&
+ test_i18ngrep "does not support path arguments" actual
+'
+
+test_expect_success '--recurse-submodules does not support --error-unmatch' '
+ test_must_fail git ls-files --recurse-submodules --error-unmatch 2>actual &&
+ test_i18ngrep "does not support --error-unmatch" actual
+'
+
+test_incompatible_with_recurse_submodules () {
+ test_expect_success "--recurse-submodules and $1 are incompatible" "
+ test_must_fail git ls-files --recurse-submodules $1 2>actual &&
+ test_i18ngrep 'can only be used in --cached mode' actual
+ "
+}
+
+test_incompatible_with_recurse_submodules -v
+test_incompatible_with_recurse_submodules -t
+test_incompatible_with_recurse_submodules --deleted
+test_incompatible_with_recurse_submodules --modified
+test_incompatible_with_recurse_submodules --others
+test_incompatible_with_recurse_submodules --stage
+test_incompatible_with_recurse_submodules --killed
+test_incompatible_with_recurse_submodules --unmerged
+test_incompatible_with_recurse_submodules --eol
+
+test_done
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* Re: v2.10.0: ls-tree exit status is always 0, this differs from ls(1)
From: Steffen Nurpmeso @ 2016-09-21 22:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqeg4d6l7k.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> wrote:
|Steffen Nurpmeso <steffen@sdaoden.eu> writes:
...
|Sorry, but I did not notice that there was an attached patch when I
|was reading your response for the first time. Risk of using an
|attachment to e-mail ;-)
|
|I think this issue does not need a separate bullet point. The
|existing text says:
..
|and what caused your surprise is already covered by the first bullet
|point, if the reader knows what "patterns to match" means in Git's
|command line tools; it just needs to be extended to be more
|meaningful to those who don't, I think.
|
|How about rewriting the first bullet point like so instead:
|
| - the behaviour is different from that of "/bin/ls" in that the
| '<path>' are actually patterns to match, e.g. so specifying
| directory name (without `-r`) will behave differently, the order
| of the arguments does not matter, and a '<path>' argument that
| does not match any path is not an error (i.e. if there is no
| path that matches any pattern, nothing is shown in the output).
Not an error would have been an enlightenment to me.
But now i'm even getting nervous to read about patterns.
We have patterns for tags/remotes/branches, author/committer/grep
patterns, (most of those, maybe all today, with fixed string,
extended or basic regex), the git-grep patterns ("leading paths
match and glob(7) patterns are supported"). Is that all?
I would assume glob-style for ls-tree:
?0[steffen@wales ]$ git ls-tree HEAD `ls mime*`
100644 blob ee47419c209da789b606ab6d979c22f4ae632712 mime.c
100644 blob 0cfe3766bd5f035eac06b728a4f63224455e13ca mime.types
100644 blob 7d890df7553522691ed09f266ea7f9effb6a2f4e mime_enc.c
100644 blob 430e300d9a8887c5cd48d1cc63034168e47e9721 mime_param.c
100644 blob 0338a46d3247ea00b5bcedb2d82ff30fe5d18d48 mime_parse.c
100644 blob d62fa8defae27240a5ce81ad2239dd7f94b6c5c5 mime_types.c
?0[steffen@wales ]$ git ls-tree HEAD 'mime*'
?0[steffen@wales ]$ git ls-tree HEAD 'mime.*'
No, ls-tree is not part of what i use every day, "Git's command
line tools" is (too) wide afield, in that sense.
Thank you (also in general, for git), and ciao from a country with
a pretty real autumn,
--steffen
^ permalink raw reply
* Re: [PATCH 2/2] ls-files: add pathspec matching for submodules
From: Junio C Hamano @ 2016-09-21 22:53 UTC (permalink / raw)
To: Brandon Williams; +Cc: git
In-Reply-To: <1474495472-94190-2-git-send-email-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> Pathspecs can be a bit tricky when trying to apply them to submodules.
> The main challenge is that the pathspecs will be with respect to the
> super module and not with respect to paths in the submodule. The
> approach this patch takes is to pass in the identical pathspec from the
> super module to the submodule in addition to the submodule-prefix, which
> is the path from the root of the super module to the submodule, and then
> we can compare an entry in the submodule prepended with the
> submodule-prefix to the pathspec in order to determine if there is a
> match.
>
> This patch also permits the pathspec logic to perform a prefix match against
> submodules since a pathspec could refer to a file inside of a submodule.
> Due to limitations in the wildmatch logic, a prefix match is only done
> literally. If any wildcard character is encountered we'll simply punt
> and produce a false positive match. More accurate matching will be done
> once inside the submodule. This is due to the super module not knowing
> what files could exist in the submodule.
Sounds sensible. Just a minor nit in terminology, but I think we
fairly consistently say "a superproject contains submodules" (run
"git grep -E 'super *(module|project)'").
I'd suggest s/super module/superproject/ for consistency.
> diff --git a/dir.c b/dir.c
> index 0ea235f..9df6d36 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -207,8 +207,9 @@ int within_depth(const char *name, int namelen,
> return 1;
> }
>
> -#define DO_MATCH_EXCLUDE 1
> -#define DO_MATCH_DIRECTORY 2
> +#define DO_MATCH_EXCLUDE (1<<0)
> +#define DO_MATCH_DIRECTORY (1<<1)
> +#define DO_MATCH_SUBMODULE (1<<2)
>
> /*
> * Does 'match' match the given name?
> @@ -283,6 +284,29 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
> item->nowildcard_len - prefix))
> return MATCHED_FNMATCH;
>
> + /* Perform checks to see if "name" is a super set of the pathspec */
> + if (flags & DO_MATCH_SUBMODULE) {
> + /* Check if the name is a literal prefix of the pathspec */
> + if ((item->match[namelen] == '/') &&
> + !ps_strncmp(item, match, name, namelen))
> + return MATCHED_RECURSIVELY;
An example of this test would be to match pathspec "sub/file" with
submodule path "sub"?
item->match[namelen] is accessed without checking if item->match[]
is long enough here; shouldn't item->len be checked before doing
that?
> + /*
> + * Here is where we would perform a wildmatch to check if
> + * "name" can be matched as a directory (or a prefix) against
> + * the pathspec. Since wildmatch doesn't have this capability
> + * at the present we have to punt and say that it is a match,
> + * esentially returning a false positive (as long as "name"
> + * matches upto the first wild character).
> + * The submodules themselves will be able to perform more
> + * accurate matching to determine if the pathspec matches.
> + */
> + if (item->nowildcard_len < item->len &&
> + !ps_strncmp(item, match, name,
> + item->nowildcard_len - prefix))
> + return MATCHED_RECURSIVELY;
An example of this test would be to match pathspec "su?/file" with
submodule path "sub", where the substring up to nowildcard_len is
the leading literal string "su" that must match with the path (in
other words, a path "sib" will not match "su?/file").
> + }
> +
Hmph, isn't this the one that is allowed produce false positive but
cannot afford to give any false negative? It feels a bit strange
that the code checks two cases where we can positively say that it
is worth descending into, and falling through would give "no this
will never match". That sounds like invitation for false negatives.
IOW, I would have expected
if (flags & DO_MATCH_SUBMODULE) {
if (may match in this case)
return MATCHED_RECURSIVE;
if (may match in this other case)
return MATCHED_RECURSIVE;
...
if (obviously cannot match in this case)
return 0;
if (obviously cannot match in this other case)
return 0;
/* otherwise we cannot say */
return MATCHED_RECURSIVELY;
}
as the general code structure.
Fully spelled out,
if (flags & DO_MATCH_SUBMODULE) {
/* Check if the name is a literal prefix of the pathspec */
if (namelen < item->len &&
item->match[namelen] == '/' &&
!ps_strncmp(item, match, name, namelen))
return MATCHED_RECURSIVE;
/* Does the literal leading part have chance of matching? */
if (item->nowildcard_len < item->len &&
namelen <= item->nowildcard_len &&
ps_strncmp(item, match, name, namelen))
return 0; /* no way "su?/file" can match "sib" */
/* Otherwise we cannot say */
return MATCHED_RECURSIVELY;
}
or something like that. There may be other "obviously cannot match"
cases we may want to add further.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/2] ls-files: adding support for submodules
From: Junio C Hamano @ 2016-09-21 23:13 UTC (permalink / raw)
To: Brandon Williams; +Cc: git
In-Reply-To: <CAKoko1qch_odsEWba0rtCv-DWO0ABS2yprnwGPCgyT6-7H-LdQ@mail.gmail.com>
Brandon Williams <bmwill@google.com> writes:
> yes you mentioned this and I meant to change that before sending it out.
> Looks like it slipped through have slipped through.
I already fixed it up locally when I sent the reply, but thanks for
resending (which assures me that your local copy is up-to-date and I
do not have to worry about having to repeat me in the future, if
this ever needs further rerolling ;-).
^ permalink raw reply
* Re: [PATCH 2/2] ls-files: add pathspec matching for submodules
From: Brandon Williams @ 2016-09-21 23:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqtwd86f0q.fsf@gitster.mtv.corp.google.com>
On Wed, Sep 21, 2016 at 3:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Sounds sensible. Just a minor nit in terminology, but I think we
> fairly consistently say "a superproject contains submodules" (run
> "git grep -E 'super *(module|project)'").
>
> I'd suggest s/super module/superproject/ for consistency.
Will do.
> An example of this test would be to match pathspec "sub/file" with
> submodule path "sub"?
Yep, I believe there's a test for that case
> item->match[namelen] is accessed without checking if item->match[]
> is long enough here; shouldn't item->len be checked before doing
> that?
Oh right! Good catch.
>
> Hmph, isn't this the one that is allowed produce false positive but
> cannot afford to give any false negative? It feels a bit strange
> that the code checks two cases where we can positively say that it
> is worth descending into, and falling through would give "no this
> will never match". That sounds like invitation for false negatives.
>
> IOW, I would have expected
>
> if (flags & DO_MATCH_SUBMODULE) {
> if (may match in this case)
> return MATCHED_RECURSIVE;
> if (may match in this other case)
> return MATCHED_RECURSIVE;
> ...
> if (obviously cannot match in this case)
> return 0;
> if (obviously cannot match in this other case)
> return 0;
> /* otherwise we cannot say */
> return MATCHED_RECURSIVELY;
> }
>
> as the general code structure.
>
> Fully spelled out,
>
> if (flags & DO_MATCH_SUBMODULE) {
> /* Check if the name is a literal prefix of the pathspec */
> if (namelen < item->len &&
> item->match[namelen] == '/' &&
> !ps_strncmp(item, match, name, namelen))
> return MATCHED_RECURSIVE;
>
> /* Does the literal leading part have chance of matching? */
> if (item->nowildcard_len < item->len &&
> namelen <= item->nowildcard_len &&
> ps_strncmp(item, match, name, namelen))
> return 0; /* no way "su?/file" can match "sib" */
>
> /* Otherwise we cannot say */
> return MATCHED_RECURSIVELY;
> }
>
> or something like that. There may be other "obviously cannot match"
> cases we may want to add further.
>
> Thanks.
You're right it should be structured the other way.
^ permalink raw reply
* [PATCH 2/2 v2] ls-files: add pathspec matching for submodules
From: Brandon Williams @ 2016-09-21 23:28 UTC (permalink / raw)
To: git; +Cc: Brandon Williams
In-Reply-To: <CAKoko1oacXxrSMZBvfM9X6iGDq+KcUUZnUrD2qD3X8+ze8vUXg@mail.gmail.com>
Pathspecs can be a bit tricky when trying to apply them to submodules.
The main challenge is that the pathspecs will be with respect to the
superproject and not with respect to paths in the submodule. The
approach this patch takes is to pass in the identical pathspec from the
superproject to the submodule in addition to the submodule-prefix, which
is the path from the root of the superproject to the submodule, and then
we can compare an entry in the submodule prepended with the
submodule-prefix to the pathspec in order to determine if there is a
match.
This patch also permits the pathspec logic to perform a prefix match against
submodules since a pathspec could refer to a file inside of a submodule.
Due to limitations in the wildmatch logic, a prefix match is only done
literally. If any wildcard character is encountered we'll simply punt
and produce a false positive match. More accurate matching will be done
once inside the submodule. This is due to the superproject not knowing
what files could exist in the submodule.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
builtin/ls-files.c | 132 ++++++++++++++++++++-------------
dir.c | 46 +++++++++++-
dir.h | 4 +
t/t3007-ls-files-recurse-submodules.sh | 114 ++++++++++++++++++++++++++--
4 files changed, 234 insertions(+), 62 deletions(-)
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index ffd9ea6..fa4029e 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -177,12 +177,34 @@ static void show_gitlink(const struct cache_entry *ce)
{
struct child_process cp = CHILD_PROCESS_INIT;
int status;
+ int i;
argv_array_push(&cp.args, "ls-files");
argv_array_push(&cp.args, "--recurse-submodules");
argv_array_pushf(&cp.args, "--submodule-prefix=%s%s/",
submodule_prefix ? submodule_prefix : "",
ce->name);
+ /* add options */
+ if (show_eol)
+ argv_array_push(&cp.args, "--eol");
+ if (show_valid_bit)
+ argv_array_push(&cp.args, "-v");
+ if (show_stage)
+ argv_array_push(&cp.args, "--stage");
+ if (show_cached)
+ argv_array_push(&cp.args, "--cached");
+ if (debug_mode)
+ argv_array_push(&cp.args, "--debug");
+
+ /*
+ * Pass in the original pathspec args. The submodule will be
+ * responsible for prepending the 'submodule_prefix' prior to comparing
+ * against the pathspec for matches.
+ */
+ argv_array_push(&cp.args, "--");
+ for (i = 0; i < pathspec.nr; i++)
+ argv_array_push(&cp.args, pathspec.items[i].original);
+
cp.git_cmd = 1;
cp.dir = ce->name;
status = run_command(&cp);
@@ -192,57 +214,62 @@ static void show_gitlink(const struct cache_entry *ce)
static void show_ce_entry(const char *tag, const struct cache_entry *ce)
{
+ struct strbuf name = STRBUF_INIT;
int len = max_prefix_len;
+ if (submodule_prefix)
+ strbuf_addstr(&name, submodule_prefix);
+ strbuf_addstr(&name, ce->name);
if (len >= ce_namelen(ce))
die("git ls-files: internal error - cache entry not superset of prefix");
- if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce),
- len, ps_matched,
- S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode)))
- return;
- if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) {
+ if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
+ submodule_path_match(&pathspec, name.buf, ps_matched)) {
show_gitlink(ce);
- return;
- }
+ } else if (match_pathspec(&pathspec, name.buf, name.len,
+ len, ps_matched,
+ S_ISDIR(ce->ce_mode) ||
+ S_ISGITLINK(ce->ce_mode))) {
+ if (tag && *tag && show_valid_bit &&
+ (ce->ce_flags & CE_VALID)) {
+ static char alttag[4];
+ memcpy(alttag, tag, 3);
+ if (isalpha(tag[0]))
+ alttag[0] = tolower(tag[0]);
+ else if (tag[0] == '?')
+ alttag[0] = '!';
+ else {
+ alttag[0] = 'v';
+ alttag[1] = tag[0];
+ alttag[2] = ' ';
+ alttag[3] = 0;
+ }
+ tag = alttag;
+ }
- if (tag && *tag && show_valid_bit &&
- (ce->ce_flags & CE_VALID)) {
- static char alttag[4];
- memcpy(alttag, tag, 3);
- if (isalpha(tag[0]))
- alttag[0] = tolower(tag[0]);
- else if (tag[0] == '?')
- alttag[0] = '!';
- else {
- alttag[0] = 'v';
- alttag[1] = tag[0];
- alttag[2] = ' ';
- alttag[3] = 0;
+ if (!show_stage) {
+ fputs(tag, stdout);
+ } else {
+ printf("%s%06o %s %d\t",
+ tag,
+ ce->ce_mode,
+ find_unique_abbrev(ce->sha1,abbrev),
+ ce_stage(ce));
+ }
+ write_eolinfo(ce, ce->name);
+ write_name(ce->name);
+ if (debug_mode) {
+ const struct stat_data *sd = &ce->ce_stat_data;
+
+ printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec);
+ printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec);
+ printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino);
+ printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid);
+ printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags);
}
- tag = alttag;
}
- if (!show_stage) {
- fputs(tag, stdout);
- } else {
- printf("%s%06o %s %d\t",
- tag,
- ce->ce_mode,
- find_unique_abbrev(ce->sha1,abbrev),
- ce_stage(ce));
- }
- write_eolinfo(ce, ce->name);
- write_name(ce->name);
- if (debug_mode) {
- const struct stat_data *sd = &ce->ce_stat_data;
-
- printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec);
- printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec);
- printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino);
- printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid);
- printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags);
- }
+ strbuf_release(&name);
}
static void show_ru_info(void)
@@ -566,27 +593,28 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
setup_work_tree();
if (recurse_submodules &&
- (show_stage || show_deleted || show_others || show_unmerged ||
- show_killed || show_modified || show_resolve_undo ||
- show_valid_bit || show_tag || show_eol))
- die("ls-files --recurse-submodules can only be used in "
- "--cached mode");
+ (show_deleted || show_others || show_unmerged ||
+ show_killed || show_modified || show_resolve_undo))
+ die("ls-files --recurse-submodules unsupported mode");
if (recurse_submodules && error_unmatch)
die("ls-files --recurse-submodules does not support "
"--error-unmatch");
- if (recurse_submodules && argc)
- die("ls-files --recurse-submodules does not support path "
- "arguments");
-
parse_pathspec(&pathspec, 0,
PATHSPEC_PREFER_CWD |
PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
prefix, argv);
- /* Find common prefix for all pathspec's */
- max_prefix = common_prefix(&pathspec);
+ /*
+ * Find common prefix for all pathspec's
+ * This is used as a performance optimization which unfortunately cannot
+ * be done when recursing into submodules
+ */
+ if (recurse_submodules)
+ max_prefix = NULL;
+ else
+ max_prefix = common_prefix(&pathspec);
max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
/* Treat unmatching pathspec elements as errors */
diff --git a/dir.c b/dir.c
index 0ea235f..28e9736 100644
--- a/dir.c
+++ b/dir.c
@@ -207,8 +207,9 @@ int within_depth(const char *name, int namelen,
return 1;
}
-#define DO_MATCH_EXCLUDE 1
-#define DO_MATCH_DIRECTORY 2
+#define DO_MATCH_EXCLUDE (1<<0)
+#define DO_MATCH_DIRECTORY (1<<1)
+#define DO_MATCH_SUBMODULE (1<<2)
/*
* Does 'match' match the given name?
@@ -283,6 +284,32 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
item->nowildcard_len - prefix))
return MATCHED_FNMATCH;
+ /* Perform checks to see if "name" is a super set of the pathspec */
+ if (flags & DO_MATCH_SUBMODULE) {
+ /* name is a literal prefix of the pathspec */
+ if ((namelen < matchlen) &&
+ (match[namelen] == '/') &&
+ !ps_strncmp(item, match, name, namelen))
+ return MATCHED_RECURSIVELY;
+
+ /* name" doesn't match up to the first wild character */
+ if (item->nowildcard_len < item->len &&
+ ps_strncmp(item, match, name,
+ item->nowildcard_len - prefix))
+ return 0;
+
+ /*
+ * Here is where we would perform a wildmatch to check if
+ * "name" can be matched as a directory (or a prefix) against
+ * the pathspec. Since wildmatch doesn't have this capability
+ * at the present we have to punt and say that it is a match,
+ * potentially returning a false positive
+ * The submodules themselves will be able to perform more
+ * accurate matching to determine if the pathspec matches.
+ */
+ return MATCHED_RECURSIVELY;
+ }
+
return 0;
}
@@ -386,6 +413,21 @@ int match_pathspec(const struct pathspec *ps,
return negative ? 0 : positive;
}
+/**
+ * Check if a submodule is a superset of the pathspec
+ */
+int submodule_path_match(const struct pathspec *ps,
+ const char *submodule_name,
+ char *seen)
+{
+ int matched = do_match_pathspec(ps, submodule_name,
+ strlen(submodule_name),
+ 0, seen,
+ DO_MATCH_DIRECTORY |
+ DO_MATCH_SUBMODULE);
+ return matched;
+}
+
int report_path_error(const char *ps_matched,
const struct pathspec *pathspec,
const char *prefix)
diff --git a/dir.h b/dir.h
index da1a858..97c83bb 100644
--- a/dir.h
+++ b/dir.h
@@ -304,6 +304,10 @@ extern int git_fnmatch(const struct pathspec_item *item,
const char *pattern, const char *string,
int prefix);
+extern int submodule_path_match(const struct pathspec *ps,
+ const char *submodule_name,
+ char *seen);
+
static inline int ce_path_match(const struct cache_entry *ce,
const struct pathspec *pathspec,
char *seen)
diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh
index caf3815..ca79fda 100755
--- a/t/t3007-ls-files-recurse-submodules.sh
+++ b/t/t3007-ls-files-recurse-submodules.sh
@@ -69,9 +69,111 @@ test_expect_success 'ls-files recurses more than 1 level' '
test_cmp expect actual
'
-test_expect_success '--recurse-submodules does not support using path arguments' '
- test_must_fail git ls-files --recurse-submodules b 2>actual &&
- test_i18ngrep "does not support path arguments" actual
+test_expect_success '--recurse-submodules and pathspecs setup' '
+ echo e >submodule/subsub/e.txt &&
+ git -C submodule/subsub add e.txt &&
+ git -C submodule/subsub commit -m "adding e.txt" &&
+ echo f >submodule/f.TXT &&
+ echo g >submodule/g.txt &&
+ git -C submodule add f.TXT g.txt &&
+ git -C submodule commit -m "add f and g" &&
+ echo h >h.txt &&
+ mkdir sib &&
+ echo sib >sib/file &&
+ git add h.txt sib/file &&
+ git commit -m "add h and sib/file" &&
+ git init sub &&
+ echo sub >sub/file &&
+ git -C sub add file &&
+ git -C sub commit -m "add file" &&
+ git submodule add ./sub &&
+ git commit -m "added sub" &&
+
+ cat >expect <<-\EOF &&
+ .gitmodules
+ a
+ b/b
+ h.txt
+ sib/file
+ sub/file
+ submodule/.gitmodules
+ submodule/c
+ submodule/f.TXT
+ submodule/g.txt
+ submodule/subsub/d
+ submodule/subsub/e.txt
+ EOF
+
+ git ls-files --recurse-submodules >actual &&
+ test_cmp expect actual &&
+ cat actual &&
+ git ls-files --recurse-submodules "*" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules and pathspecs' '
+ cat >expect <<-\EOF &&
+ h.txt
+ submodule/g.txt
+ submodule/subsub/e.txt
+ EOF
+
+ git ls-files --recurse-submodules "*.txt" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules and pathspecs' '
+ cat >expect <<-\EOF &&
+ h.txt
+ submodule/f.TXT
+ submodule/g.txt
+ submodule/subsub/e.txt
+ EOF
+
+ git ls-files --recurse-submodules ":(icase)*.txt" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules and pathspecs' '
+ cat >expect <<-\EOF &&
+ h.txt
+ submodule/f.TXT
+ submodule/g.txt
+ EOF
+
+ git ls-files --recurse-submodules ":(icase)*.txt" ":(exclude)submodule/subsub/*" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules and pathspecs' '
+ cat >expect <<-\EOF &&
+ sub/file
+ EOF
+
+ git ls-files --recurse-submodules "sub" >actual &&
+ test_cmp expect actual &&
+ git ls-files --recurse-submodules "sub/" >actual &&
+ test_cmp expect actual &&
+ git ls-files --recurse-submodules "sub/file" >actual &&
+ test_cmp expect actual &&
+ git ls-files --recurse-submodules "su*/file" >actual &&
+ test_cmp expect actual &&
+ git ls-files --recurse-submodules "su?/file" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules and pathspecs' '
+ cat >expect <<-\EOF &&
+ sib/file
+ sub/file
+ EOF
+
+ git ls-files --recurse-submodules "s??/file" >actual &&
+ test_cmp expect actual &&
+ git ls-files --recurse-submodules "s???file" >actual &&
+ test_cmp expect actual &&
+ git ls-files --recurse-submodules "s*file" >actual &&
+ test_cmp expect actual
'
test_expect_success '--recurse-submodules does not support --error-unmatch' '
@@ -82,18 +184,14 @@ test_expect_success '--recurse-submodules does not support --error-unmatch' '
test_incompatible_with_recurse_submodules () {
test_expect_success "--recurse-submodules and $1 are incompatible" "
test_must_fail git ls-files --recurse-submodules $1 2>actual &&
- test_i18ngrep 'can only be used in --cached mode' actual
+ test_i18ngrep 'unsupported mode' actual
"
}
-test_incompatible_with_recurse_submodules -v
-test_incompatible_with_recurse_submodules -t
test_incompatible_with_recurse_submodules --deleted
test_incompatible_with_recurse_submodules --modified
test_incompatible_with_recurse_submodules --others
-test_incompatible_with_recurse_submodules --stage
test_incompatible_with_recurse_submodules --killed
test_incompatible_with_recurse_submodules --unmerged
-test_incompatible_with_recurse_submodules --eol
test_done
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* What's cooking in git.git (Sep 2016, #06; Wed, 21)
From: Junio C Hamano @ 2016-09-21 23:31 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'. The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.
Quite a lot of topics have graduated to 'master'. The tip of 'next'
has been rewound and rebuilt. Accumulated fixes since v2.10.0 are
now almost ready to spawn the first maintenance update v2.10.1 but
not quite yet.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[Graduated to "master"]
* bw/pathspec-remove-unused-extern-decl (2016-09-13) 1 commit
(merged to 'next' on 2016-09-15 at c5b281b)
+ pathspec: remove unnecessary function prototypes
Code cleanup.
* et/add-chmod-x (2016-09-12) 1 commit
(merged to 'next' on 2016-09-15 at c81abae)
+ add: document the chmod option
(this branch is used by tg/add-chmod+x-fix.)
"git add --chmod=+x" added recently lacked documentation, which has
been corrected.
* ew/http-do-not-forget-to-call-curl-multi-remove-handle (2016-09-13) 3 commits
(merged to 'next' on 2016-09-15 at 696acb7)
+ http: always remove curl easy from curlm session on release
+ http: consolidate #ifdefs for curl_multi_remove_handle
+ http: warn on curl_multi_add_handle failures
The http transport (with curl-multi option, which is the default
these days) failed to remove curl-easy handle from a curlm session,
which led to unnecessary API failures.
* jk/delta-base-cache (2016-09-12) 1 commit
(merged to 'next' on 2016-09-15 at 1e35f8d)
+ add_delta_base_cache: use list_for_each_safe
Recently we updated the code to manage the in-core cache that holds
objects that have recently been used to reconstitute other objects
that are stored as deltas against them, but the update used an
incorrect API function to manage the list of these objects. This
has been fixed.
This is a last-minute fix to a topic that graduated to 'master'
post 2.10 release.
* jk/patch-ids-no-merges (2016-09-12) 2 commits
(merged to 'next' on 2016-09-15 at 14bb3a0)
+ patch-ids: refuse to compute patch-id for merge commit
+ patch-ids: turn off rename detection
"git log --cherry-pick" used to include merge commits as candidates
to be matched up with other commits, resulting a lot of wasted time.
The patch-id generation logic has been updated to ignore merges to
avoid the wastage.
* jk/rebase-i-drop-ident-check (2016-07-29) 1 commit
(merged to 'next' on 2016-08-14 at 6891bcd)
+ rebase-interactive: drop early check for valid ident
Even when "git pull --rebase=preserve" (and the underlying "git
rebase --preserve") can complete without creating any new commit
(i.e. fast-forwards), it still insisted on having a usable ident
information (read: user.email is set correctly), which was less
than nice. As the underlying commands used inside "git rebase"
would fail with a more meaningful error message and advice text
when the bogus ident matters, this extra check was removed.
* jk/reduce-gc-aggressive-depth (2016-08-11) 1 commit
(merged to 'next' on 2016-08-11 at 6810c6f)
+ gc: default aggressive depth to 50
"git gc --aggressive" used to limit the delta-chain length to 250,
which is way too deep for gaining additional space savings and is
detrimental for runtime performance. The limit has been reduced to
50.
* jk/setup-sequence-update (2016-09-13) 16 commits
(merged to 'next' on 2016-09-15 at 4df8399)
+ t1007: factor out repeated setup
+ init: reset cached config when entering new repo
+ init: expand comments explaining config trickery
+ config: only read .git/config from configured repos
+ test-config: setup git directory
+ t1302: use "git -C"
+ pager: handle early config
+ pager: use callbacks instead of configset
+ pager: make pager_program a file-local static
+ pager: stop loading git_default_config()
+ pager: remove obsolete comment
+ diff: always try to set up the repository
+ diff: handle --no-index prefixes consistently
+ diff: skip implicit no-index check when given --no-index
+ patch-id: use RUN_SETUP_GENTLY
+ hash-object: always try to set up the git repository
(this branch is used by nd/init-core-worktree-in-multi-worktree-world.)
There were numerous corner cases in which the configuration files
are read and used or not read at all depending on the directory a
Git command was run, leading to inconsistent behaviour. The code
to set-up repository access at the beginning of a Git process has
been updated to fix them.
* js/cat-file-filters (2016-09-11) 4 commits
(merged to 'next' on 2016-09-15 at a231380)
+ cat-file: support --textconv/--filters in batch mode
+ cat-file --textconv/--filters: allow specifying the path separately
+ cat-file: introduce the --filters option
+ cat-file: fix a grammo in the man page
Even though "git hash-objects", which is a tool to take an
on-filesystem data stream and put it into the Git object store,
allowed to perform the "outside-world-to-Git" conversions (e.g.
end-of-line conversions and application of the clean-filter), and
it had the feature on by default from very early days, its reverse
operation "git cat-file", which takes an object from the Git object
store and externalize for the consumption by the outside world,
lacked an equivalent mechanism to run the "Git-to-outside-world"
conversion. The command learned the "--filters" option to do so.
* jt/accept-capability-advertisement-when-fetching-from-void (2016-09-09) 3 commits
(merged to 'next' on 2016-09-15 at 1cd9f9a)
+ connect: advertized capability is not a ref
+ connect: tighten check for unexpected early hang up
+ tests: move test_lazy_prereq JGIT to test-lib.sh
JGit can show a fake ref "capabilities^{}" to "git fetch" when it
does not advertise any refs, but "git fetch" was not prepared to
see such an advertisement. When the other side disconnects without
giving any ref advertisement, we used to say "there may not be a
repository at that URL", but we may have seen other advertisement
like "shallow" and ".have" in which case we definitely know that a
repository is there. The code to detect this case has also been
updated.
* jt/format-patch-base-info-above-sig (2016-09-15) 1 commit
(merged to 'next' on 2016-09-15 at 3da5c68)
+ format-patch: show base info before email signature
"git format-patch --base=..." feature that was recently added
showed the base commit information after "-- " e-mail signature
line, which turned out to be inconvenient. The base information
has been moved above the signature line.
* ks/pack-objects-bitmap (2016-09-12) 2 commits
(merged to 'next' on 2016-09-15 at e0600bd)
+ pack-objects: use reachability bitmap index when generating non-stdout pack
+ pack-objects: respect --local/--honor-pack-keep/--incremental when bitmap is in use
Some codepaths in "git pack-objects" were not ready to use an
existing pack bitmap; now they are and as the result they have
become faster.
* ks/perf-build-with-autoconf (2016-09-15) 1 commit
(merged to 'next' on 2016-09-15 at 261878d)
+ t/perf/run: copy config.mak.autogen & friends to build area
Performance tests done via "t/perf" did not use the same set of
build configuration if the user relied on autoconf generated
configuration.
* mr/vcs-svn-printf-ulong (2016-09-14) 1 commit
(merged to 'next' on 2016-09-15 at cc8ef53)
+ vcs-svn/fast_export: fix timestamp fmt specifiers
Code cleanup.
* rs/checkout-some-states-are-const (2016-09-13) 1 commit
(merged to 'next' on 2016-09-15 at 19f219b)
+ checkout: constify parameters of checkout_stage() and checkout_merged()
Code cleanup.
* rs/pack-sort-with-llist-mergesort (2016-09-13) 1 commit
(merged to 'next' on 2016-09-15 at 45159f5)
+ sha1_file: use llist_mergesort() for sorting packs
Code cleanup.
* rs/strbuf-remove-fix (2016-09-13) 1 commit
(merged to 'next' on 2016-09-15 at 5f64556)
+ strbuf: use valid pointer in strbuf_remove()
Code cleanup.
* rs/unpack-trees-reduce-file-scope-global (2016-09-13) 1 commit
(merged to 'next' on 2016-09-15 at cd16435)
+ unpack-trees: pass checkout state explicitly to check_updates()
Code cleanup.
* rs/xdiff-merge-overlapping-hunks-for-W-context (2016-09-14) 1 commit
(merged to 'next' on 2016-09-15 at eaa85ab)
+ xdiff: fix merging of hunks with -W context and -u context
"git diff -W" output needs to extend the context backward to
include the header line of the current function and also forward to
include the body of the entire current function up to the header
line of the next one. This process may have to merge to adjacent
hunks, but the code forgot to do so in some cases.
* va/i18n (2016-09-15) 11 commits
(merged to 'next' on 2016-09-15 at 2b3d368)
+ i18n: update-index: mark warnings for translation
+ i18n: show-branch: mark plural strings for translation
+ i18n: show-branch: mark error messages for translation
+ i18n: receive-pack: mark messages for translation
+ notes: spell first word of error messages in lowercase
+ i18n: notes: mark error messages for translation
+ i18n: merge-recursive: mark verbose message for translation
+ i18n: merge-recursive: mark error messages for translation
+ i18n: config: mark error message for translation
+ i18n: branch: mark option description for translation
+ i18n: blame: mark error messages for translation
(this branch is used by va/i18n-more.)
More i18n.
--------------------------------------------------
[New Topics]
* ep/doc-check-ref-format-example (2016-09-21) 1 commit
- git-check-ref-format.txt: fixup documentation
A shell script example in check-ref-format documentation has been
fixed.
Will merge to 'next'.
* js/regexec-buf (2016-09-21) 3 commits
- regex: use regexec_buf()
- regex: add regexec_buf() that can work on a non NUL-terminated string
- regex: -G<pattern> feeds a non NUL-terminated string to regexec() and fails
Some codepaths in "git diff" used regexec(3) on a buffer that was
mmap(2)ed, which may not have a terminating NUL, leading to a read
beyond the end of the mapped region. This was fixed by introducing
a regexec_buf() helper that takes a <ptr,len> pair with REG_STARTEND
extension.
Waiting for an Ack to minor tweaks.
cf. <cover.1474482164.git.johannes.schindelin@gmx.de>
* jt/format-patch-rfc (2016-09-21) 1 commit
- format-patch: add "--rfc" for the common case of [RFC PATCH]
In some projects it is common to use "[RFC PATCH]" as the subject
prefix for a patch meant for discussion rather than application. A
new option "--rfc" was a short-hand for "--subject-prefix=RFC PATCH"
to help the participants of such projects.
Will merge to 'next'.
* ls/travis-homebrew-path-fix (2016-09-21) 1 commit
- travis-ci: ask homebrew for the its path instead of hardcoding it
The procedure to build Git on Mac OS X for Travis CI hardcoded the
internal directory structure we assumed HomeBrew uses, which was a
no-no. The procedure has been updated to ask HomeBrew things we
need to know to fix this.
Will merge to 'next'.
* nd/init-core-worktree-in-multi-worktree-world (2016-09-21) 3 commits
- init: reuse original_git_dir in set_git_dir_init()
- init: do not set core.worktree more often than necessary
- init: correct re-initialization from a linked worktree
"git init" tried to record core.worktree in the repository's
'config' file when GIT_WORK_TREE environment variable was set and
it was different from where GIT_DIR appears as ".git" at its top,
but the logic was faulty when .git is a "gitdir:" file that points
at the real place, causing trouble in working trees that are
managed by "git worktree". This has been corrected.
The second one seems to need a bit more polishing.
cf. <xmqqd1jx854z.fsf@gitster.mtv.corp.google.com>
--------------------------------------------------
[Stalled]
* jc/bundle (2016-03-03) 6 commits
- index-pack: --clone-bundle option
- Merge branch 'jc/index-pack' into jc/bundle
- bundle v3: the beginning
- bundle: keep a copy of bundle file name in the in-core bundle header
- bundle: plug resource leak
- bundle doc: 'verify' is not about verifying the bundle
The beginning of "split bundle", which could be one of the
ingredients to allow "git clone" traffic off of the core server
network to CDN.
While I think it would make it easier for people to experiment and
build on if the topic is merged to 'next', I am at the same time a
bit reluctant to merge an unproven new topic that introduces a new
file format, which we may end up having to support til the end of
time. It is likely that to support a "prime clone from CDN", it
would need a lot more than just "these are the heads and the pack
data is over there", so this may not be sufficient.
Will discard.
* jc/blame-reverse (2016-06-14) 2 commits
- blame: dwim "blame --reverse OLD" as "blame --reverse OLD.."
- blame: improve diagnosis for "--reverse NEW"
It is a common mistake to say "git blame --reverse OLD path",
expecting that the command line is dwimmed as if asking how lines
in path in an old revision OLD have survived up to the current
commit.
Has been waiting for positive responses without seeing any.
Will discard.
* jc/attr (2016-05-25) 18 commits
- attr: support quoting pathname patterns in C style
- attr: expose validity check for attribute names
- attr: add counted string version of git_attr()
- attr: add counted string version of git_check_attr()
- attr: retire git_check_attrs() API
- attr: convert git_check_attrs() callers to use the new API
- attr: convert git_all_attrs() to use "struct git_attr_check"
- attr: (re)introduce git_check_attr() and struct git_attr_check
- attr: rename function and struct related to checking attributes
- attr.c: plug small leak in parse_attr_line()
- attr.c: tighten constness around "git_attr" structure
- attr.c: simplify macroexpand_one()
- attr.c: mark where #if DEBUG ends more clearly
- attr.c: complete a sentence in a comment
- attr.c: explain the lack of attr-name syntax check in parse_attr()
- attr.c: update a stale comment on "struct match_attr"
- attr.c: use strchrnul() to scan for one line
- commit.c: use strchrnul() to scan for one line
(this branch is used by jc/attr-more, sb/pathspec-label and sb/submodule-default-paths.)
The attributes API has been updated so that it can later be
optimized using the knowledge of which attributes are queried.
I wanted to polish this topic further to make the attribute
subsystem thread-ready, but because other topics depend on this
topic and they do not (yet) need it to be thread-ready.
As the authors of topics that depend on this seem not in a hurry,
let's discard this and dependent topics and restart them some other
day.
Will discard.
* jc/attr-more (2016-06-09) 8 commits
- attr.c: outline the future plans by heavily commenting
- attr.c: always pass check[] to collect_some_attrs()
- attr.c: introduce empty_attr_check_elems()
- attr.c: correct ugly hack for git_all_attrs()
- attr.c: rename a local variable check
- fixup! d5ad6c13
- attr.c: pass struct git_attr_check down the callchain
- attr.c: add push_stack() helper
(this branch uses jc/attr; is tangled with sb/pathspec-label and sb/submodule-default-paths.)
The beginning of long and tortuous journey to clean-up attribute
subsystem implementation.
Needs to be redone.
Will discard.
* sb/submodule-default-paths (2016-06-20) 5 commits
- completion: clone can recurse into submodules
- clone: add --init-submodule=<pathspec> switch
- submodule update: add `--init-default-path` switch
- Merge branch 'sb/pathspec-label' into sb/submodule-default-paths
- Merge branch 'jc/attr' into sb/submodule-default-paths
(this branch uses jc/attr and sb/pathspec-label; is tangled with jc/attr-more.)
Allow specifying the set of submodules the user is interested in on
the command line of "git clone" that clones the superproject.
Will discard.
* sb/pathspec-label (2016-06-03) 6 commits
- pathspec: disable preload-index when attribute pathspec magic is in use
- pathspec: allow escaped query values
- pathspec: allow querying for attributes
- pathspec: move prefix check out of the inner loop
- pathspec: move long magic parsing out of prefix_pathspec
- Documentation: fix a typo
(this branch is used by sb/submodule-default-paths; uses jc/attr; is tangled with jc/attr-more.)
The pathspec mechanism learned ":(attr:X)$pattern" pathspec magic
to limit paths that match $pattern further by attribute settings.
The preload-index mechanism is disabled when the new pathspec magic
is in use (at least for now), because the attribute subsystem is
not thread-ready.
Will discard.
* mh/connect (2016-06-06) 10 commits
- connect: [host:port] is legacy for ssh
- connect: move ssh command line preparation to a separate function
- connect: actively reject git:// urls with a user part
- connect: change the --diag-url output to separate user and host
- connect: make parse_connect_url() return the user part of the url as a separate value
- connect: group CONNECT_DIAG_URL handling code
- connect: make parse_connect_url() return separated host and port
- connect: re-derive a host:port string from the separate host and port variables
- connect: call get_host_and_port() earlier
- connect: document why we sometimes call get_port after get_host_and_port
Rewrite Git-URL parsing routine (hopefully) without changing any
behaviour.
It has been two months without any support. We may want to discard
this.
* sb/bisect (2016-04-15) 22 commits
. SQUASH???
. bisect: get back halfway shortcut
. bisect: compute best bisection in compute_relevant_weights()
. bisect: use a bottom-up traversal to find relevant weights
. bisect: prepare for different algorithms based on find_all
. bisect: rename count_distance() to compute_weight()
. bisect: make total number of commits global
. bisect: introduce distance_direction()
. bisect: extract get_distance() function from code duplication
. bisect: use commit instead of commit list as arguments when appropriate
. bisect: replace clear_distance() by unique markers
. bisect: use struct node_data array instead of int array
. bisect: get rid of recursion in count_distance()
. bisect: make algorithm behavior independent of DEBUG_BISECT
. bisect: make bisect compile if DEBUG_BISECT is set
. bisect: plug the biggest memory leak
. bisect: add test for the bisect algorithm
. t6030: generalize test to not rely on current implementation
. t: use test_cmp_rev() where appropriate
. t/test-lib-functions.sh: generalize test_cmp_rev
. bisect: allow 'bisect run' if no good commit is known
. bisect: write about `bisect next` in documentation
The internal algorithm used in "git bisect" to find the next commit
to check has been optimized greatly.
Was expecting a reroll, but now pb/bisect topic starts removinging
more and more parts from git-bisect.sh, this needs to see a fresh
reroll.
Will discard.
cf. <1460294354-7031-1-git-send-email-s-beyer@gmx.net>
* sg/completion-updates (2016-02-28) 21 commits
. completion: cache the path to the repository
. completion: extract repository discovery from __gitdir()
. completion: don't guard git executions with __gitdir()
. completion: consolidate silencing errors from git commands
. completion: don't use __gitdir() for git commands
. completion: respect 'git -C <path>'
. completion: fix completion after 'git -C <path>'
. completion: don't offer commands when 'git --opt' needs an argument
. rev-parse: add '--absolute-git-dir' option
. completion: list short refs from a remote given as a URL
. completion: don't list 'HEAD' when trying refs completion outside of a repo
. completion: list refs from remote when remote's name matches a directory
. completion: respect 'git --git-dir=<path>' when listing remote refs
. completion: fix most spots not respecting 'git --git-dir=<path>'
. completion: ensure that the repository path given on the command line exists
. completion tests: add tests for the __git_refs() helper function
. completion tests: check __gitdir()'s output in the error cases
. completion tests: consolidate getting path of current working directory
. completion tests: make the $cur variable local to the test helper functions
. completion tests: don't add test cruft to the test repository
. completion: improve __git_refs()'s in-code documentation
Has been waiting for a reroll for too long.
cf. <1456754714-25237-1-git-send-email-szeder@ira.uka.de>
Will discard.
* ec/annotate-deleted (2015-11-20) 1 commit
- annotate: skip checking working tree if a revision is provided
Usability fix for annotate-specific "<file> <rev>" syntax with deleted
files.
Has been waiting for a review for too long without seeing anything.
Will discard.
* dk/gc-more-wo-pack (2016-01-13) 4 commits
- gc: clean garbage .bitmap files from pack dir
- t5304: ensure non-garbage files are not deleted
- t5304: test .bitmap garbage files
- prepare_packed_git(): find more garbage
Follow-on to dk/gc-idx-wo-pack topic, to clean up stale
.bitmap and .keep files.
Has been waiting for a reroll for too long.
cf. <xmqq60ypbeng.fsf@gitster.mtv.corp.google.com>
Will discard.
* jc/diff-b-m (2015-02-23) 5 commits
. WIPWIP
. WIP: diff-b-m
- diffcore-rename: allow easier debugging
- diffcore-rename.c: add locate_rename_src()
- diffcore-break: allow debugging
"git diff -B -M" produced incorrect patch when the postimage of a
completely rewritten file is similar to the preimage of a removed
file; such a resulting file must not be expressed as a rename from
other place.
The fix in this patch is broken, unfortunately.
Will discard.
--------------------------------------------------
[Cooking]
* mm/config-color-ui-default-to-auto (2016-09-16) 1 commit
- Documentation/config: default for color.* is color.ui
Documentation for individual configuration variables to control use
of color (like `color.grep`) said that their default value was
'false', instead of saying their default is taken from `color.ui`.
When we updated the default value for color.ui from 'false' to
'auto' quite a while ago, all of them broke. This has been
corrected.
Will merge to 'next'.
* rs/c-auto-resets-attributes (2016-09-19) 1 commit
- pretty: let %C(auto) reset all attributes
The pretty-format specifier used by the "log" family of commands
have "%C(auto)" to turn coloring of the output is taught to also
issue a color-reset sequence to the output.
Will merge to 'next'.
* rs/cocci (2016-09-15) 3 commits
- use strbuf_addstr() for adding constant strings to a strbuf, part 2
- add coccicheck make target
- contrib/coccinelle: fix semantic patch for oid_to_hex_r()
Code cleanup.
Will merge to 'next'.
* va/i18n-more (2016-09-21) 6 commits
- i18n: stash: mark messages for translation
- i18n: notes-merge: mark die messages for translation
- i18n: ident: mark hint for translation
- i18n: i18n: diff: mark die messages for translation
- i18n: connect: mark die messages for translation
- i18n: commit: mark message for translation
Even more i18n.
Will merge to 'next'.
* jt/mailinfo-fold-in-body-headers (2016-09-21) 3 commits
- mailinfo: handle in-body header continuations
- mailinfo: make is_scissors_line take plain char *
- mailinfo: separate in-body header processing
When "git format-patch --stdout" output is placed as an in-body
header and it used the RFC2822 header folding, "git am" failed to
notice and put the header line back into a single logical line.
The underlying "git mailinfo" was taught to handle this properly.
Will merge to 'next'.
* kd/mailinfo-quoted-string (2016-09-19) 2 commits
- mailinfo: unescape quoted-pair in header fields
- t5100-mailinfo: replace common path prefix with variable
An e-mail author named that spelled a backslash-quoted double quote
in the human readable part "My \"double quoted\" name" was not
unquoted correctly.
Waiting for the discussion to conclude.
cf. <20160920035710.qw2byl3qeqwih7t5@sigill.intra.peff.net>
* js/libify-require-clean-work-tree (2016-09-12) 5 commits
- wt-status: teach has_{unstaged,uncommitted}_changes() about submodules
- Export also the has_un{staged,committed}_changed() functions
- Make the require_clean_work_tree() function truly reusable
- pull: make code more similar to the shell script again
- pull: drop confusing prefix parameter of die_on_unclean_work_tree()
The require_clean_work_tree() helper was recreated in C when "git
pull" was rewritten from shell; the helper is now made available to
other callers in preparation for upcoming "rebase -i" work.
Waiting for comments.
Modulo a few minor nits, this looked almost ready.
cf. <xmqqtwdl2bhm.fsf@gitster.mtv.corp.google.com>
cf. <xmqqpoo92bdr.fsf@gitster.mtv.corp.google.com>
* tg/add-chmod+x-fix (2016-09-21) 6 commits
- t3700-add: do not check working tree file mode without POSIXPERM
- t3700-add: create subdirectory gently
- add: modify already added files when --chmod is given
- read-cache: introduce chmod_index_entry
- update-index: add test for chmod flags
- Merge branch 'ib/t3700-add-chmod-x-updates' into tg/add-chmod+x-fix
"git add --chmod=+x <pathspec>" added recently only toggled the
executable bit for paths that are either new or modified. This has
been corrected to flip the executable bit for all paths that match
the given pathspec.
Waiting for the discussion to conclude.
cf. <c3aefd9d-b794-21a1-619e-bce6a3c2cf47@kdbg.org>
* bw/ls-files-recurse-submodules (2016-09-21) 2 commits
- ls-files: add pathspec matching for submodules
- ls-files: optionally recurse into submodules
"git ls-files" learned "--recurse-submodules" option that can be
used to get a listing of tracked files across submodules (i.e. this
only works with "--cached" option, not for listing untracked or
ignored files). This would be a useful tool to sit on the upstream
side of a pipe that is read with xargs to work on all working tree
files from the top-level superproject.
Waiting for the discussion to conclude.
* ls/filter-process (2016-09-21) 11 commits
- convert: add filter.<driver>.process option
- convert: make apply_filter() adhere to standard Git error handling
- convert: modernize tests
- convert: quote filter names in error messages
- pkt-line: add functions to read/write flush terminated packet streams
- pkt-line: add packet_write_gently()
- pkt-line: add packet_flush_gently()
- pkt-line: add packet_write_fmt_gently()
- run-command: move check_pipe() from write_or_die to run_command
- pkt-line: extract set_packet_header()
- pkt-line: rename packet_write() to packet_write_fmt()
The smudge/clean filter API expect an external process is spawned
to filter the contents for each path that has a filter defined. A
new type of "process" filter API has been added to allow the first
request to run the filter for a path to spawn a single process, and
all filtering need is served by this single process for multiple
paths, reducing the process creation overhead.
Is this ready?
* hv/submodule-not-yet-pushed-fix (2016-09-15) 5 commits
. SQUASH??? -Wdecl-after-stmt
. use actual start hashes for submodule push check instead of local refs
. batch check whether submodule needs pushing into one call
- serialize collection of refs that contain submodule changes
- serialize collection of changed submodules
The code in "git push" to compute if any commit being pushed in the
superproject binds a commit in a submodule that hasn't been pushed
out was overly inefficient, making it unusable even for a small
project that does not have any submodule but have a reasonable
number of refs. This has been optimized.
The last two in the original series seem to break a few tests when
queued to 'pu'.
* rt/rebase-i-broken-insn-advise (2016-09-07) 1 commit
- rebase -i: improve advice on bad instruction lines
When "git rebase -i" is given a broken instruction, it told the
user to fix it with "--edit-todo", but didn't say what the step
after that was (i.e. "--continue").
Will hold.
Dscho's "rebase -i" hopefully will become available in 'pu', by
which time an equivalent of this fix would be ported to C. This is
queued merely as a reminder.
* nd/checkout-disambiguation (2016-09-21) 3 commits
- checkout: fix ambiguity check in subdir
- checkout.txt: document a common case that ignores ambiguation rules
- checkout: add some spaces between code and comment
"git checkout <word>" does not follow the usual disambiguation
rules when the <word> can be both a rev and a path, to allow
checking out a branch 'foo' in a project that happens to have a
file 'foo' in the working tree without having to disambiguate.
This was poorly documented and the check was incorrect when the
command was run from a subdirectory.
Will merge to 'next'.
* sg/fix-versioncmp-with-common-suffix (2016-09-08) 5 commits
- versioncmp: cope with common leading parts in versionsort.prereleaseSuffix
- versioncmp: pass full tagnames to swap_prereleases()
- t7004-tag: add version sort tests to show prerelease reordering issues
- t7004-tag: use test_config helper
- t7004-tag: delete unnecessary tags with test_when_finished
The prereleaseSuffix feature of version comparison that is used in
"git tag -l" did not correctly when two or more prereleases for the
same release were present (e.g. when 2.0, 2.0-beta1, and 2.0-beta2
are there and the code needs to compare 2.0-beta1 and 2.0-beta2).
Waiting for a reroll.
cf. <20160908223727.Horde.jVOOJ278ssZ3qkyjkmyqZD-@webmail.informatik.kit.edu>
* cp/completion-negative-refs (2016-08-24) 1 commit
- completion: support excluding refs
The command-line completion script (in contrib/) learned to
complete "git cmd ^mas<HT>" to complete the negative end of
reference to "git cmd ^master".
Needs review.
* sb/push-make-submodule-check-the-default (2016-08-24) 1 commit
- push: change submodule default to check
Turn the default of "push.recurseSubmodules" to "check".
Alas, this reveals that the "check" mode is too inefficient to use
in real projects, even in ones as small as git itself.
cf. <xmqqh9aaot49.fsf@gitster.mtv.corp.google.com>
* ak/curl-imap-send-explicit-scheme (2016-08-17) 1 commit
- imap-send: Tell cURL to use imap:// or imaps://
When we started cURL to talk to imap server when a new enough
version of cURL library is available, we forgot to explicitly add
imap(s):// before the destination. To some folks, that didn't work
and the library tried to make HTTP(s) requests instead.
Needs review and testing.
* mh/diff-indent-heuristic (2016-09-19) 8 commits
- blame: honor the diff heuristic options and config
- parse-options: add parse_opt_unknown_cb()
- diff: improve positioning of add/delete blocks in diffs
- xdl_change_compact(): introduce the concept of a change group
- recs_match(): take two xrecord_t pointers as arguments
- is_blank_line(): take a single xrecord_t as argument
- xdl_change_compact(): only use heuristic if group can't be matched
- xdl_change_compact(): fix compaction heuristic to adjust ixo
Output from "git diff" can be made easier to read by selecting
which lines are common and which lines are added/deleted
intelligently when the lines before and after the changed section
are the same. A command line option is added to help with the
experiment to find a good heuristics.
Will merge to 'next'.
* jk/pack-objects-optim-mru (2016-08-11) 4 commits
(merged to 'next' on 2016-09-21 at 97b919b)
+ pack-objects: use mru list when iterating over packs
+ pack-objects: break delta cycles before delta-search phase
+ sha1_file: make packed_object_info public
+ provide an initializer for "struct object_info"
Originally merged to 'next' on 2016-08-11
"git pack-objects" in a repository with many packfiles used to
spend a lot of time looking for/at objects in them; the accesses to
the packfiles are now optimized by checking the most-recently-used
packfile first.
Will hold to see if people scream.
* dp/autoconf-curl-ssl (2016-06-28) 1 commit
- ./configure.ac: detect SSL in libcurl using curl-config
The ./configure script generated from configure.ac was taught how
to detect support of SSL by libcurl better.
Needs review.
* jc/pull-rebase-ff (2016-07-28) 1 commit
- pull: fast-forward "pull --rebase=true"
"git pull --rebase", when there is no new commits on our side since
we forked from the upstream, should be able to fast-forward without
invoking "git rebase", but it didn't.
Needs a real log message and a few tests.
* ex/deprecate-empty-pathspec-as-match-all (2016-06-22) 1 commit
(merged to 'next' on 2016-09-21 at e19148e)
+ pathspec: warn on empty strings as pathspec
Originally merged to 'next' on 2016-07-13
An empty string used as a pathspec element has always meant
'everything matches', but it is too easy to write a script that
finds a path to remove in $path and run 'git rm "$paht"', which
ends up removing everything. Start warning about this use of an
empty string used for 'everything matches' and ask users to use a
more explicit '.' for that instead.
The hope is that existing users will not mind this change, and
eventually the warning can be turned into a hard error, upgrading
the deprecation into removal of this (mis)feature.
Will hold to see if people scream.
* nd/shallow-deepen (2016-06-13) 27 commits
- fetch, upload-pack: --deepen=N extends shallow boundary by N commits
- upload-pack: add get_reachable_list()
- upload-pack: split check_unreachable() in two, prep for get_reachable_list()
- t5500, t5539: tests for shallow depth excluding a ref
- clone: define shallow clone boundary with --shallow-exclude
- fetch: define shallow boundary with --shallow-exclude
- upload-pack: support define shallow boundary by excluding revisions
- refs: add expand_ref()
- t5500, t5539: tests for shallow depth since a specific date
- clone: define shallow clone boundary based on time with --shallow-since
- fetch: define shallow boundary with --shallow-since
- upload-pack: add deepen-since to cut shallow repos based on time
- shallow.c: implement a generic shallow boundary finder based on rev-list
- fetch-pack: use a separate flag for fetch in deepening mode
- fetch-pack.c: mark strings for translating
- fetch-pack: use a common function for verbose printing
- fetch-pack: use skip_prefix() instead of starts_with()
- upload-pack: move rev-list code out of check_non_tip()
- upload-pack: make check_non_tip() clean things up on error
- upload-pack: tighten number parsing at "deepen" lines
- upload-pack: use skip_prefix() instead of starts_with()
- upload-pack: move "unshallow" sending code out of deepen()
- upload-pack: remove unused variable "backup"
- upload-pack: move "shallow" sending code out of deepen()
- upload-pack: move shallow deepen code out of receive_needs()
- transport-helper.c: refactor set_helper_option()
- remote-curl.c: convert fetch_git() to use argv_array
The existing "git fetch --depth=<n>" option was hard to use
correctly when making the history of an existing shallow clone
deeper. A new option, "--deepen=<n>", has been added to make this
easier to use. "git clone" also learned "--shallow-since=<date>"
and "--shallow-exclude=<tag>" options to make it easier to specify
"I am interested only in the recent N months worth of history" and
"Give me only the history since that version".
Needs review.
Rerolled. What this topic attempts to achieve is worthwhile, I
would think.
* pb/bisect (2016-08-23) 27 commits
. bisect--helper: remove the dequote in bisect_start()
. bisect--helper: retire `--bisect-auto-next` subcommand
. bisect--helper: retire `--bisect-autostart` subcommand
. bisect--helper: retire `--check-and-set-terms` subcommand
. bisect--helper: retire `--bisect-write` subcommand
. bisect--helper: `bisect_replay` shell function in C
. bisect--helper: `bisect_log` shell function in C
. bisect--helper: retire `--write-terms` subcommand
. bisect--helper: retire `--check-expected-revs` subcommand
. bisect--helper: `bisect_state` & `bisect_head` shell function in C
. bisect--helper: `bisect_autostart` shell function in C
. bisect--helper: retire `--next-all` subcommand
. bisect--helper: retire `--bisect-clean-state` subcommand
. bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
. bisect--helper: `bisect_start` shell function partially in C
. bisect--helper: `get_terms` & `bisect_terms` shell function in C
. bisect--helper: `bisect_next_check` & bisect_voc shell function in C
. bisect--helper: `check_and_set_terms` shell function in C
. bisect--helper: `bisect_write` shell function in C
. bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
. bisect--helper: `bisect_reset` shell function in C
. wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
. t6030: explicitly test for bisection cleanup
. bisect--helper: `bisect_clean_state` shell function in C
. bisect--helper: `write_terms` shell function in C
. bisect: rewrite `check_term_format` shell function in C
. bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
GSoC "bisect" topic.
I'd prefer to see early part solidified so that reviews can focus
on the later part that is still in flux. We are almost there but
not quite yet.
* kn/ref-filter-branch-list (2016-05-17) 17 commits
- branch: implement '--format' option
- branch: use ref-filter printing APIs
- branch, tag: use porcelain output
- ref-filter: allow porcelain to translate messages in the output
- ref-filter: add `:dir` and `:base` options for ref printing atoms
- ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
- ref-filter: introduce symref_atom_parser() and refname_atom_parser()
- ref-filter: introduce refname_atom_parser_internal()
- ref-filter: make "%(symref)" atom work with the ':short' modifier
- ref-filter: add support for %(upstream:track,nobracket)
- ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
- ref-filter: introduce format_ref_array_item()
- ref-filter: move get_head_description() from branch.c
- ref-filter: modify "%(objectname:short)" to take length
- ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
- ref-filter: include reference to 'used_atom' within 'atom_value'
- ref-filter: implement %(if), %(then), and %(else) atoms
The code to list branches in "git branch" has been consolidated
with the more generic ref-filter API.
Rerolled.
Needs review.
* jc/merge-drop-old-syntax (2015-04-29) 1 commit
- merge: drop 'git merge <message> HEAD <commit>' syntax
Stop supporting "git merge <message> HEAD <commit>" syntax that has
been deprecated since October 2007, and issues a deprecation
warning message since v2.5.0.
It has been reported that git-gui still uses the deprecated syntax,
which needs to be fixed before this final step can proceed.
cf. <5671DB28.8020901@kdbg.org>
--------------------------------------------------
[Discarded]
* jn/fix-connect-unexpected-hangup-diag (2016-09-08) 1 commit
. connect: tighten check for unexpected early hang up
Now part of jt/accept-capability-advertisement-when-fetching-from-void
topic.
^ permalink raw reply
* [PATCH] verify_packfile: check pack validity before accessing data
From: Jeff King @ 2016-09-22 3:49 UTC (permalink / raw)
To: git
The verify_packfile() does not explicitly open the packfile;
instead, it starts with a sha1 checksum over the whole pack,
and relies on use_pack() to open the packfile as a side
effect.
If the pack cannot be opened for whatever reason (either
because its header information is corrupted, or perhaps
because a simultaneous repack deleted it), then use_pack()
will die(), as it has no way to return an error. This is not
ideal, as verify_packfile() otherwise tries to gently return
an error (this lets programs like git-fsck go on to check
other packs).
Instead, let's check is_pack_valid() up front, and return an
error if it fails. This will open the pack as a side effect,
and then use_pack() will later rely on our cached
descriptor, and avoid calling die().
Signed-off-by: Jeff King <peff@peff.net>
---
pack-check.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/pack-check.c b/pack-check.c
index d123846..c5c7763 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -57,11 +57,8 @@ static int verify_packfile(struct packed_git *p,
int err = 0;
struct idx_entry *entries;
- /* Note that the pack header checks are actually performed by
- * use_pack when it first opens the pack file. If anything
- * goes wrong during those checks then the call will die out
- * immediately.
- */
+ if (!is_pack_valid(p))
+ return error("packfile %s cannot be accessed", p->pack_name);
git_SHA1_Init(&ctx);
do {
--
2.10.0.482.gae5a597
^ permalink raw reply related
* Re: [PATCH] verify_packfile: check pack validity before accessing data
From: Jeff King @ 2016-09-22 4:05 UTC (permalink / raw)
To: git
In-Reply-To: <20160922034904.dm5okldfmgmin5d7@sigill.intra.peff.net>
On Wed, Sep 21, 2016 at 11:49:05PM -0400, Jeff King wrote:
> The verify_packfile() does not explicitly open the packfile;
> instead, it starts with a sha1 checksum over the whole pack,
> and relies on use_pack() to open the packfile as a side
> effect.
>
> If the pack cannot be opened for whatever reason (either
> because its header information is corrupted, or perhaps
> because a simultaneous repack deleted it), then use_pack()
> will die(), as it has no way to return an error. This is not
> ideal, as verify_packfile() otherwise tries to gently return
> an error (this lets programs like git-fsck go on to check
> other packs).
>
> Instead, let's check is_pack_valid() up front, and return an
> error if it fails. This will open the pack as a side effect,
> and then use_pack() will later rely on our cached
> descriptor, and avoid calling die().
I actually had an ulterior motive, but it didn't pan out. I
think this change is an improvement on its own, which is why I posted
it. But here's my ulterior motive, for reference.
The die() in question happens when use_pack() is asked to lazily open a
pack, but it fails. If this happens then the code in question is racy
with respect to somebody else running a repack, because the pack we are
looking for might go away, but we could find the object in another pack.
Since we cannot handle the retry at this level, callers of use_pack()
should generally use is_pack_valid() early to cache the descriptor (and
at that early stage, they can still bail to another pack if necessary).
See the comment in fill_pack_entry(), for example, or the discussion in
4c08018 (pack-objects: protect against disappearing packs, 2011-10-14).
So I wanted to know whether there were any code paths that failed to do
so, and just blindly rely on the lazy-open. Finding the races is
inherently hard, because you only catch them when somebody else is doing
a repack. But if we just _remove_ the lazy-load, then it becomes easy to
catch anybody relying on it. Like:
diff --git a/sha1_file.c b/sha1_file.c
index b9c1fa3..f3d7615 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1122,8 +1122,8 @@ unsigned char *use_pack(struct packed_git *p,
* hash, and the in_window function above wouldn't match
* don't allow an offset too close to the end of the file.
*/
- if (!p->pack_size && p->pack_fd == -1 && open_packed_git(p))
- die("packfile %s cannot be accessed", p->pack_name);
+ if (!p->pack_size && p->pack_fd == -1)
+ die("BUG: use_pack() called on unopened '%s'", p->pack_name);
if (offset > (p->pack_size - 20))
die("offset beyond end of packfile (truncated pack?)");
if (offset < 0)
@@ -1140,8 +1140,8 @@ unsigned char *use_pack(struct packed_git *p,
size_t window_align = packed_git_window_size / 2;
off_t len;
- if (p->pack_fd == -1 && open_packed_git(p))
- die("packfile %s cannot be accessed", p->pack_name);
+ if (p->pack_fd == -1)
+ die("BUG: use_pack() called on unopened '%s'", p->pack_name);
win = xcalloc(1, sizeof(*win));
win->offset = (offset / window_align) * window_align;
Running the test suite with the patch above revealed the issue in
verify_packfile (and with my patch, the test suite now passes, even with
this).
So I was hoping that we could convert these into assertions as above.
But the test suite passing does not quite tell the whole story. We might
still close a pack in the middle of an operation if we need to open
another one and are running against the system file descriptor limits.
That would only trigger in a repository with a large number of packs (or
a very low descriptor limit).
In such a case, we are relying on the lazy-load (and we _are_ racy!).
But the patch above would punish people on low-descriptor systems. It's
better to have an unlikely race and complete the request than to fail
consistently. :-/
For people who are running high-traffic servers, they just need to make
sure their file descriptor limit is reasonably high to avoid the race.
-Peff
^ permalink raw reply related
* Re: [PATCH 1/2] ls-files: adding support for submodules
From: Jeff King @ 2016-09-22 4:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Brandon Williams, git
In-Reply-To: <xmqqponw6e3x.fsf@gitster.mtv.corp.google.com>
On Wed, Sep 21, 2016 at 04:13:22PM -0700, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
>
> > yes you mentioned this and I meant to change that before sending it out.
> > Looks like it slipped through have slipped through.
>
> I already fixed it up locally when I sent the reply, but thanks for
> resending (which assures me that your local copy is up-to-date and I
> do not have to worry about having to repeat me in the future, if
> this ever needs further rerolling ;-).
While we are on the subject, the commit message also uses some past
tense:
Allow ls-files to recognize submodules in order to retrieve a list of
files from a repository's submodules. This is done by forking off a
process to recursively call ls-files on all submodules. Also added a
submodule-prefix command in order to prepend paths to child processes.
The final sentence should be "Also add...".
Since this final bit of logic was sufficiently non-obvious that it only
came about in v2, maybe it is worth describing a little more fully:
Also add a submodule-prefix option, which instructs the child
processes to prepend the prefix to each path they output. This makes
the output paths match what is on the filesystem (i.e., as if the
submodule boundaries were not there at all).
Should this option just be "--prefix", or maybe "--output-prefix"?
Submodules are the obvious use case here, but I could see somebody
adapting this for other uses (alternatively, if we _do_ want to keep it
just as an implementation detail for submodules, we should probably
discourage people in the documentation from using it themselves).
-Peff
^ permalink raw reply
* Re: [PATCH tg/add-chmod+x-fix 2/2] t3700-add: protect one --chmod=+x test with POSIXPERM
From: Johannes Sixt @ 2016-09-22 5:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Gummerer, Git Mailing List
In-Reply-To: <xmqq60pp6jor.fsf@gitster.mtv.corp.google.com>
Am 21.09.2016 um 23:12 schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
>
>> But I came to a different conclusion as I said in a message that
>> crossed yours. I hope Thomas can pick up the baton again.
>
> Yeah, our mails crossed, apparently, and I do agree with your
> reasoning. How about this, then?
>
> -- >8 --
> From: Johannes Sixt <j6t@kdbg.org>
> Date: Tue, 20 Sep 2016 08:18:25 +0200
> Subject: [PATCH] t3700-add: do not check working tree file mode without POSIXPERM
>
> A recently introduced test checks the result of 'git status' after
> setting the executable bit on a file. This check does not yield the
> expected result when the filesystem does not support the executable
> bit.
>
> What we care about is that a file added with "--chmod=+x" has
> executable bit in the index and that "--chmod=+x" (or any other
> options for that matter) does not muck with working tree files.
> The former is tested by other existing tests, so let's check the
> latter more explicitly and only under POSIXPERM prerequisite.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> t/t3700-add.sh | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/t/t3700-add.sh b/t/t3700-add.sh
> index 16ab2da..924a266 100755
> --- a/t/t3700-add.sh
> +++ b/t/t3700-add.sh
> @@ -361,13 +361,11 @@ test_expect_success 'git add --chmod=[+-]x changes index with already added file
> test_mode_in_index 100644 xfoo3
> '
>
> -test_expect_success 'file status is changed after git add --chmod=+x' '
> - echo "AM foo4" >expected &&
> +test_expect_success POSIXPERM 'git add --chmod=[+-]x does not change the working tree' '
> echo foo >foo4 &&
> git add foo4 &&
> git add --chmod=+x foo4 &&
> - git status -s foo4 >actual &&
> - test_cmp expected actual
> + ! test -x foo4
> '
>
> test_expect_success 'no file status change if no pathspec is given' '
>
That makes a lot of sense. Thank you so much!
-- Hannes
^ permalink raw reply
* [PATCH] clone: pass --progress decision to recursive submodules
From: Jeff King @ 2016-09-22 5:24 UTC (permalink / raw)
To: git; +Cc: Stefan Beller
When cloning with "--recursive", we'd generally expect
submodules to show progress reports if the main clone did,
too.
In older versions of git, this mostly worked out of the
box. Since we show progress by default when stderr is a tty,
and since the child clones inherit the parent stderr, then
both processes would come to the same decision by default.
If the parent clone was asked for "--quiet", we passed down
"--quiet" to the child. However, if stderr was not a tty and
the user specified "--progress", we did not propagate this
to the child.
That's a minor bug, but things got much worse when we
switched recently to submodule--helper's update_clone
command. With that change, the stderr of the child clones
are always connected to a pipe, and we never output
progress at all.
This patch teaches git-submodule and git-submodule--helper
how to pass down an explicit "--progress" flag when cloning.
The clone command then decides to propagate that flag based
on the cloning decision made earlier (which takes into
account isatty(2) of the parent process, existing --progress
or --quiet flags, etc). Since the child processes always run
without a tty on stderr, we don't have to worry about
passing an explicit "--no-progress"; it's the default for
them.
This fixes the recent loss of progress during recursive
clones. And as a bonus, it makes:
git clone --recursive --progress ... 2>&1 | cat
work by triggering progress explicitly in the children.
Signed-off-by: Jeff King <peff@peff.net>
---
I don't usually use submodules, but I happened to be testing something,
and my goto "this has a lot of submodules" repository happens to have
some pretty large submodules. So I had plenty of time to contemplate the
lack of progress reporting. :)
I checked with --jobs, too, and this should do the right thing; it
queues up the progress reports for submodules that aren't the
output_owner, and then dumps them all at once (so the progress meter
will appear to whiz by for those jobs until it catches up to the current
state, which is the only reasonable thing we can do, I think).
I imagine there are other code paths that want similar treatment, but I
didn't look into them. I'd assume "fetch" is one. I'm not sure if we do
parallel checkouts, but that's another potential.
builtin/clone.c | 16 ++++++++++++++--
builtin/submodule--helper.c | 18 +++++++++++++++---
git-submodule.sh | 5 +++++
3 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 404c5e8..28ce938 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -670,7 +670,7 @@ static void update_head(const struct ref *our, const struct ref *remote,
}
}
-static int checkout(void)
+static int checkout(int submodule_progress)
{
unsigned char sha1[20];
char *head;
@@ -734,6 +734,9 @@ static int checkout(void)
if (max_jobs != -1)
argv_array_pushf(&args, "--jobs=%d", max_jobs);
+ if (submodule_progress)
+ argv_array_push(&args, "--progress");
+
err = run_command_v_opt(args.argv, RUN_GIT_CMD);
argv_array_clear(&args);
}
@@ -841,6 +844,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
const char *src_ref_prefix = "refs/heads/";
struct remote *remote;
int err = 0, complete_refs_before_fetch = 1;
+ int submodule_progress;
struct refspec *refspec;
const char *fetch_pattern;
@@ -1099,6 +1103,14 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
update_head(our_head_points_at, remote_head, reflog_msg.buf);
+ /*
+ * We want to show progress for recursive submodule clones iff
+ * we did so for the main clone. But only the transport knows
+ * the final decision for this flag, so we need to rescue the value
+ * before we free the transport.
+ */
+ submodule_progress = transport->progress;
+
transport_unlock_pack(transport);
transport_disconnect(transport);
@@ -1108,7 +1120,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
}
junk_mode = JUNK_LEAVE_REPO;
- err = checkout();
+ err = checkout(submodule_progress);
strbuf_release(&reflog_msg);
strbuf_release(&branch_top);
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 7b8ddfe..d2f9d7d 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -443,7 +443,8 @@ static int module_name(int argc, const char **argv, const char *prefix)
}
static int clone_submodule(const char *path, const char *gitdir, const char *url,
- const char *depth, struct string_list *reference, int quiet)
+ const char *depth, struct string_list *reference,
+ int quiet, int progress)
{
struct child_process cp = CHILD_PROCESS_INIT;
@@ -451,6 +452,8 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
argv_array_push(&cp.args, "--no-checkout");
if (quiet)
argv_array_push(&cp.args, "--quiet");
+ if (progress)
+ argv_array_push(&cp.args, "--progress");
if (depth && *depth)
argv_array_pushl(&cp.args, "--depth", depth, NULL);
if (reference->nr) {
@@ -575,6 +578,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
{
const char *name = NULL, *url = NULL, *depth = NULL;
int quiet = 0;
+ int progress = 0;
FILE *submodule_dot_git;
char *p, *path = NULL, *sm_gitdir;
struct strbuf rel_path = STRBUF_INIT;
@@ -601,6 +605,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
N_("string"),
N_("depth for shallow clones")),
OPT__QUIET(&quiet, "Suppress output for cloning a submodule"),
+ OPT_BOOL(0, "progress", &progress,
+ N_("force cloning progress")),
OPT_END()
};
@@ -634,7 +640,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
prepare_possible_alternates(name, &reference);
- if (clone_submodule(path, sm_gitdir, url, depth, &reference, quiet))
+ if (clone_submodule(path, sm_gitdir, url, depth, &reference,
+ quiet, progress))
die(_("clone of '%s' into submodule path '%s' failed"),
url, path);
} else {
@@ -684,6 +691,7 @@ struct submodule_update_clone {
struct submodule_update_strategy update;
/* configuration parameters which are passed on to the children */
+ int progress;
int quiet;
int recommend_shallow;
struct string_list references;
@@ -702,7 +710,7 @@ struct submodule_update_clone {
int failed_clones_nr, failed_clones_alloc;
};
#define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
- SUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, STRING_LIST_INIT_DUP, \
+ SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, \
NULL, NULL, NULL, \
STRING_LIST_INIT_DUP, 0, NULL, 0, 0}
@@ -804,6 +812,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
child->err = -1;
argv_array_push(&child->args, "submodule--helper");
argv_array_push(&child->args, "clone");
+ if (suc->progress)
+ argv_array_push(&child->args, "--progress");
if (suc->quiet)
argv_array_push(&child->args, "--quiet");
if (suc->prefix)
@@ -950,6 +960,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)
OPT_BOOL(0, "recommend-shallow", &suc.recommend_shallow,
N_("whether the initial clone should follow the shallow recommendation")),
OPT__QUIET(&suc.quiet, N_("don't print cloning progress")),
+ OPT_BOOL(0, "progress", &suc.progress,
+ N_("force cloning progress")),
OPT_END()
};
diff --git a/git-submodule.sh b/git-submodule.sh
index a1cc71b..a024a13 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -44,6 +44,7 @@ update=
prefix=
custom_name=
depth=
+progress=
die_if_unmatched ()
{
@@ -498,6 +499,9 @@ cmd_update()
-q|--quiet)
GIT_QUIET=1
;;
+ --progress)
+ progress="--progress"
+ ;;
-i|--init)
init=1
;;
@@ -573,6 +577,7 @@ cmd_update()
{
git submodule--helper update-clone ${GIT_QUIET:+--quiet} \
+ ${progress:+"$progress"} \
${wt_prefix:+--prefix "$wt_prefix"} \
${prefix:+--recursive-prefix "$prefix"} \
${update:+--update "$update"} \
--
2.10.0.482.gae5a597
^ permalink raw reply related
* Re: [PATCH 1/2] ls-files: optionally recurse into submodules
From: Jeff King @ 2016-09-22 6:20 UTC (permalink / raw)
To: Brandon Williams; +Cc: git
In-Reply-To: <1474497772-97986-1-git-send-email-bmwill@google.com>
On Wed, Sep 21, 2016 at 03:42:52PM -0700, Brandon Williams wrote:
> @@ -68,6 +71,21 @@ static void write_eolinfo(const struct cache_entry *ce, const char *path)
> static void write_name(const char *name)
> {
> /*
> + * NEEDSWORK: To make this thread-safe, full_name would have to be owned
> + * by the caller.
I'm not sure if that is quite true. You could simply drop the "static"
here, and pay the malloc cost each time. But if we want to amortize the
allocation, then yeah, somebody else on the stack needs to own it.
That being said, I don't know if it is worth pointing this out as a
NEEDSWORK or not. Most of git is not thread-safe, so I think that's
generally the norm.
> + *
> + * full_name get reused across output lines to minimize the allocation
> + * churn.
Likewise, this kind of strbuf-reuse trickery is common in git. I don't
know if it's worth a comment or not (though I mind this one much less,
because it's a bit more subtle, being across multiple calls rather than
in a single loop).
(To be clear, I actually don't mind either _that_ much; usually the
problem is under-commenting, not over-commenting. I am just trying to
give style pointers since you are new to the project).
> + static struct strbuf full_name = STRBUF_INIT;
> + if (submodule_prefix && *submodule_prefix) {
> + strbuf_reset(&full_name);
> + strbuf_addstr(&full_name, submodule_prefix);
> + strbuf_addstr(&full_name, name);
> + name = full_name.buf;
> + }
I actually wonder if it would be more obvious if we hoisted out the
prefix copy, too. IOW, have a global:
/* used to assemble full names in place */
static struct strbuf prefixed_name = STRBUF_INIT;
static size_t prefixed_name_base;
then parse --submodule-prefix as:
strbuf_reset(&prefixed_name);
strbuf_addstr(&prefixed_name, arg);
prefixed_name_base = prefixed_name.len;
and then here do:
if (prefixed_name_base) {
strbuf_addstr(&prefixed_name, name);
name = prefixed_name.buf;
}
... do stuff with name ...
if (prefixed_name_base)
strbuf_setlen(&prefixed_name, prefixed_name_base);
I dunno. It is slightly more efficient (we do not memcpy the prefix over
and over), but I doubt that matters much in practice. I just wonder if
it makes the buffer reuse a bit more obvious.
> +/**
> + * Recursively call ls-files on a submodule
> + */
> +static void show_gitlink(const struct cache_entry *ce)
> +{
> + struct child_process cp = CHILD_PROCESS_INIT;
> + int status;
> +
> + argv_array_push(&cp.args, "ls-files");
> + argv_array_push(&cp.args, "--recurse-submodules");
> + argv_array_pushf(&cp.args, "--submodule-prefix=%s%s/",
> + submodule_prefix ? submodule_prefix : "",
> + ce->name);
> + cp.git_cmd = 1;
> + cp.dir = ce->name;
> + status = run_command(&cp);
> + if (status)
> + exit(status);
> +}
This doesn't propagate the parent argv at all. So if I run:
git ls-files -z --recurse-submodules
then the paths are all NUL-terminated in the parent, but
newline-terminated in the submodules. Oops.
I see in the second patch you make an effort to pass in some specific
options explicitly. In some ways that is safer (we do not accidentally
pass an argument that needs to be munged). But in some ways it is less
safe (as shown by "-z" above, anything we fail to pass in show_gitlink()
must be explicitly blocked in cmd_ls_files(), or we produce garbage
output). It also means that we have a bunch of boilerplate forwarding
the options along (e.g., your second patch lets through --stage, but why
not other ones like "--deleted"?).
So I dunno. I would have done it more like:
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 03c283e..2a3d65c 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -31,6 +31,7 @@ static int debug_mode;
static int show_eol;
static const char *submodule_prefix;
static int recurse_submodules;
+static struct argv_array recurse_argv = ARGV_ARRAY_INIT;
static const char *prefix;
static int max_prefix_len;
@@ -170,6 +171,32 @@ static void show_killed_files(struct dir_struct *dir)
}
}
+/*
+ * Copy all of the parent arguments, but omit --submodule-prefix, as
+ * we will be adding our own. Other arguments are assumed to behave
+ * reasonably in the submodule, or to be blocked explicitly in
+ * cmd_ls_files().
+ *
+ * We have to make a full copy here, because parse_options will munge our
+ * original.
+ */
+static void store_recurse_argv(const char **argv)
+{
+ for (; *argv; argv++) {
+ const char *arg = *argv;
+
+ /* Yikes, we're reinventing option parsing here. */
+ if (starts_with(arg, "--submodule-prefix="))
+ continue;
+ if (!strcmp(arg, "--submodule-prefix")) {
+ if (argv[1])
+ argv++;
+ continue;
+ }
+ argv_array_push(&recurse_argv, arg);
+ }
+}
+
/**
* Recursively call ls-files on a submodule
*/
@@ -177,9 +204,10 @@ static void show_gitlink(const struct cache_entry *ce)
{
struct child_process cp = CHILD_PROCESS_INIT;
int status;
+ int i;
- argv_array_push(&cp.args, "ls-files");
- argv_array_push(&cp.args, "--recurse-submodules");
+ for (i = 0; i < recurse_argv.argc; i++)
+ argv_array_push(&cp.args, recurse_argv.argv[i]);
argv_array_pushf(&cp.args, "--submodule-prefix=%s%s/",
submodule_prefix ? submodule_prefix : "",
ce->name);
@@ -535,6 +563,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
if (read_cache() < 0)
die("index file corrupt");
+ store_recurse_argv(argv);
+
argc = parse_options(argc, argv, prefix, builtin_ls_files_options,
ls_files_usage, 0);
el = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
@@ -565,13 +595,6 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
if (require_work_tree && !is_inside_work_tree())
setup_work_tree();
- if (recurse_submodules &&
- (show_stage || show_deleted || show_others || show_unmerged ||
- show_killed || show_modified || show_resolve_undo ||
- show_valid_bit || show_tag || show_eol))
- die("ls-files --recurse-submodules can only be used in "
- "--cached mode");
-
if (recurse_submodules && error_unmatch)
die("ls-files --recurse-submodules does not support "
"--error-unmatch");
which I admit is somewhat ugly, due to the "yikes" comment above. For
example, I think parse_options() will actually allow a unique prefix
like "--submodule-pre=foo/", which we would not catch. Or worse, we
would accidentally munge the pathspec in:
git ls-files --recurse-submodules -- --submodule-prefix=foo
(or any case where a non-option looked like "--submodule-prefix=".
Obviously those are pathological, but it's still nasty.
I think we _could_ actually avoid picking out the --submodule-prefix
argument entirely, and instead just add our own at the end to override
it.
I don't think it actually matters that much here. You _could_ get away
with even leaving the old --submodule-prefix in place, and just adding
our new one at the end to override it. That works, though it does mean
that your argv gets continually longer as you recurse modules. Or I
suppose another way would be to make the prefix option additive, and
then just literally add "--prefix-add=$name" at each level. The option
parser would then build up the real prefix from multiple prefix-add
options.
The final thing I could think of is that we could teach parse_options()
to record a canonicalized copy of all of the options in an argv_array.
So it would normalize "--submodule-pre foo" to "--submodule-prefix=foo",
and that makes our after-the-fact parsing much easier.
But like I said, I dunno. I'm on the fence on which approach is the
least ugly (including your original, to just forward specific options).
They all have their warts. :)
-Peff
^ permalink raw reply related
* Re: Bug: pager.<cmd> doesn't work well with editors
From: Jeff King @ 2016-09-22 6:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Anatoly Borodin, git
In-Reply-To: <xmqqponxb56a.fsf@gitster.mtv.corp.google.com>
On Wed, Sep 21, 2016 at 09:15:09AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > And this isn't really limited to the editor. It's more _annoying_ with
> > the editor, but really "pager.tag" does not make any sense to set right
> > now, because it is handled outside of the "tag" command entirely, and
> > doesn't know what mode the tag command will be running in.
>
> Stepping back even further, perhaps the whole pager.<cmd> was a bad
> interim move. For those who set "less" without "-F", being able to
> set pager.<cmd> to false may still be necessary, but I am wondering
> about setting it to true or a command string here.
>
> It did mean well and may have helped when "git <cmd>" that produces
> reams of output had not yet learned to auto-paginate as a stop-gap
> measure by allowing users to set pager.<cmd>, but I wonder if the
> ideal course of action was to identify (or "wait until people show
> their desire") individual operating modes of various commands and
> teach them to auto-paginate. For example, "tag -l" may be one of
> them that we would want to teach to.
I don't think it is a bad move overall. I use "pager.log" to pipe
through a specific command (that is different than I would use for other
commands).
So I like the idea of configurability; the problem is just that it is
happening at the wrong level. The individual commands should be in
charge of it, with something like:
/*
* See if pager.log is configured, falling back to "true" (due to the
* second argument). If so, and stdout is a tty, run the pager.
*
* This would be run at the top of cmd_log().
*/
setup_auto_pager("log", 1);
...
/*
* As above, but note that we run this in cmd_tag() only at the right
* moment. We'd probably actually flip the "0" here to a "1", but
* this represents the current default.
*/
if (mode_list)
setup_auto_pager("tag.list", 0);
That's a lot more boilerplate (each command needs to decide if and when
it should support the pager), but a one-liner in each spot is not so
bad. Both builtins and external could use the C interface, but it's
trickier for other languages to redirect their own stdout (we want the
pager to run as a separate process, but we also need to wait for it at
the end).
Though I suspect for most cases, external scripts are really paging the
output of some other command anyway. So it would be enough to provide
something like:
if git pager --query bisect.log
then
bisect_log | git pager bisect.log
else
bisect_log
fi
(you can lose the "--query" form if you don't mind _always_ piping
through "cat" when there's no pager in use, but that seems like a
pointless inefficiency).
So I think it's all workable, and for the most part would even remain
backwards compatible, with the exception that "pager.foo" would not work
for a third-party "git-foo" until the author updates it to call "git
pager".
I don't have a particular plan to work on it anytime soon, but maybe
somebody could pick it up as relatively low-hanging fruit.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
From: Jeff King @ 2016-09-22 6:49 UTC (permalink / raw)
To: Kevin Daudt; +Cc: Junio C Hamano, git
In-Reply-To: <20160921174550.GB27363@ikke.info>
On Wed, Sep 21, 2016 at 07:45:50PM +0200, Kevin Daudt wrote:
> On Wed, Sep 21, 2016 at 10:36:57AM -0700, Junio C Hamano wrote:
> > Kevin Daudt <me@ikke.info> writes:
> >
> > > On Mon, Sep 19, 2016 at 04:30:34PM -0700, Junio C Hamano wrote:
> > >>
> > >> * kd/mailinfo-quoted-string (2016-09-19) 2 commits
> > >> - mailinfo: unescape quoted-pair in header fields
> > >> - t5100-mailinfo: replace common path prefix with variable
> > >
> > > Is this good enough, or do you want me to look into the feedback from
> > > jeff?
> >
> > If you are talking about the simplified loop that deliberately sets
> > a rule that is looser than RFC, yes, I'd like to see you at least
> > consider the pros and cons of his approach, which looked nicer to my
> > brief reading of it.
> >
> > It is perfectly OK by me (it may not be so if you ask Peff) if you
> > decide that your version is better after doing so, though.
>
> Alright, I'll look into it.
Thanks. I am OK if we do not use my simplified version, but I think
there were some issues I noted with your last version.
-Peff
^ permalink raw reply
* [PATCH 0/3] update git cvs import documentation
From: Jeff King @ 2016-09-22 7:23 UTC (permalink / raw)
To: git; +Cc: esr
I dreamed a dream that I would never have to think about git-cvsimport
again. And yet. Somebody reported[1] that the link to cvsps in
gitcvs-migration is broken. While fixing it, I noticed a few other
out-of-date items. I haven't used any of these tools myself in years,
but hopefully these changes are minimal no-brainers.
[1/3]: docs/cvsimport: prefer cvs-fast-export to parsecvs
[2/3]: docs/cvs-migration: update link to cvsps homepage
[3/3]: docs/cvs-migration: mention cvsimport caveats
-Peff
[1] https://github.com/git/git-scm.com/issues/851
^ permalink raw reply
* [PATCH 1/3] docs/cvsimport: prefer cvs-fast-export to parsecvs
From: Jeff King @ 2016-09-22 7:25 UTC (permalink / raw)
To: git; +Cc: esr
In-Reply-To: <20160922072350.ivjrfuedodd2rezn@sigill.intra.peff.net>
parsecvs maintenance was taken over by ESR, and the name
changed to cvs-fast-export as it learned to support that
output format. Let's point to cvs-fast-export, as it should
have additional bug-fixes and be more convenient to use.
Signed-off-by: Jeff King <peff@peff.net>
---
No opinion on how this compares to cvs2git these days, so I made the
minimal change here. Anyone may feel free to debate it, but kindly
remove me from the Cc of any flamewars. :)
Documentation/git-cvsimport.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt
index 41207a2..de1ebed 100644
--- a/Documentation/git-cvsimport.txt
+++ b/Documentation/git-cvsimport.txt
@@ -22,7 +22,7 @@ DESCRIPTION
deprecated; it does not work with cvsps version 3 and later. If you are
performing a one-shot import of a CVS repository consider using
http://cvs2svn.tigris.org/cvs2git.html[cvs2git] or
-https://github.com/BartMassey/parsecvs[parsecvs].
+http://www.catb.org/esr/cvs-fast-export/[cvs-fast-export].
Imports a CVS repository into Git. It will either create a new
repository, or incrementally import into an existing one.
--
2.10.0.482.gae5a597
^ permalink raw reply related
* [PATCH 2/3] docs/cvs-migration: update link to cvsps homepage
From: Jeff King @ 2016-09-22 7:26 UTC (permalink / raw)
To: git; +Cc: esr
In-Reply-To: <20160922072350.ivjrfuedodd2rezn@sigill.intra.peff.net>
The old page gives a 404 now. Searching for "cvsps" via
Google returns a GitHub project page as the top hit.
Reported-by: Dan Pritts
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/gitcvs-migration.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/gitcvs-migration.txt b/Documentation/gitcvs-migration.txt
index b06e852..faee5c2 100644
--- a/Documentation/gitcvs-migration.txt
+++ b/Documentation/gitcvs-migration.txt
@@ -117,7 +117,7 @@ Importing a CVS archive
-----------------------
First, install version 2.1 or higher of cvsps from
-http://www.cobite.com/cvsps/[http://www.cobite.com/cvsps/] and make
+https://github.com/andreyvit/cvsps[https://github.com/andreyvit/cvsps] and make
sure it is in your path. Then cd to a checked out CVS working directory
of the project you are interested in and run linkgit:git-cvsimport[1]:
--
2.10.0.482.gae5a597
^ permalink raw reply related
* [PATCH 3/3] docs/cvs-migration: mention cvsimport caveats
From: Jeff King @ 2016-09-22 7:26 UTC (permalink / raw)
To: git; +Cc: esr
In-Reply-To: <20160922072350.ivjrfuedodd2rezn@sigill.intra.peff.net>
Back when this guide was written, cvsimport was the only
game in town. These days it is probably not the best option.
Rather than go into details, let's point people to the note
at the top of cvsimport which gives other options.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/gitcvs-migration.txt | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/gitcvs-migration.txt b/Documentation/gitcvs-migration.txt
index faee5c2..4c6143c 100644
--- a/Documentation/gitcvs-migration.txt
+++ b/Documentation/gitcvs-migration.txt
@@ -116,6 +116,10 @@ they create are writable and searchable by other group members.
Importing a CVS archive
-----------------------
+NOTE: These instructions use the `git-cvsimport` script which ships with
+git, but other importers may provide better results. See the note in
+linkgit:git-cvsimport[1] for other options.
+
First, install version 2.1 or higher of cvsps from
https://github.com/andreyvit/cvsps[https://github.com/andreyvit/cvsps] and make
sure it is in your path. Then cd to a checked out CVS working directory
--
2.10.0.482.gae5a597
^ permalink raw reply related
* Re: [PATCH v1] travis-ci: ask homebrew for the its path instead of hardcoding it
From: Lars Schneider @ 2016-09-22 9:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: stefan.naewe, git
In-Reply-To: <xmqqzin19pbj.fsf@gitster.mtv.corp.google.com>
> On 21 Sep 2016, at 18:42, Junio C Hamano <gitster@pobox.com> wrote:
>
> Lars Schneider <larsxschneider@gmail.com> writes:
>
>>> On 21 Sep 2016, at 11:31, stefan.naewe@atlas-elektronik.com wrote:
>>>
>>> In the Subject: s/the //
>>>
>>> Am 21.09.2016 um 10:45 schrieb larsxschneider@gmail.com:
>>>> From: Lars Schneider <larsxschneider@gmail.com>
>>>>
>>>> The TravisCI macOS build is broken because homebrew (a macOS depedency
>>>
>>> s/depedency/dependency/
>>
>> Thanks for spotting both errors!
>>
>> @Junio: Should I make a v2?
>
> No. osx before_install stuff was in there since the very beginning,
> i.e. 522354d7 ("Add Travis CI support", 2015-11-27), so I guess this
> needs to go to maint-2.7 and upwards, but I guess we should discourage
> people to stay on an older maintenance track forever, so let's do
> this only for 'maint' and upwards.
Sound good to me.
Thank you,
Lars
Minor nit:
If still possible and no trouble for you please do
`s/the //` on the subject line of 72fa5cd29f2a9249462215109dbf41b4a6c0c768 (in PU)
^ permalink raw reply
* Aw: Re: Re: Homebrew and Git
From: Jonas Thiel @ 2016-09-22 9:23 UTC (permalink / raw)
To: Jeff King; +Cc: John Keeping, Heiko Voigt, git
In-Reply-To: <20160921084841.phq7cfbagi5k7ku4@sigill.intra.peff.net>
Sorry for my late reply. Thanks for your support -- I really appreciate that.
@Jeff: Unfortunately, I do not know how to implement the patch you provided. Can you explain how to do that?
Thanks and best regards,
Jonas
> Gesendet: Mittwoch, 21. September 2016 um 10:48 Uhr
> Von: "Jeff King" <peff@peff.net>
> An: "John Keeping" <john@keeping.me.uk>
> Cc: "Heiko Voigt" <hvoigt@hvoigt.net>, "Jonas Thiel" <jonas.lierschied@gmx.de>, git@vger.kernel.org
> Betreff: Re: Re: Homebrew and Git
>
> On Tue, Sep 20, 2016 at 08:15:55PM +0100, John Keeping wrote:
>
> > > BTW, here is the callstack inlined from the crashreport:
> > >
> > > bsystem_platform.dylib 0x00007fff840db41c _platform_strchr$VARIANT$Haswell + 28
> > > 1 git 0x000000010ba1d3f4 ident_default_email + 801
> > > 2 git 0x000000010ba1d68f fmt_ident + 66
> > > 3 git 0x000000010ba4b495 files_log_ref_write + 175
> > > 4 git 0x000000010ba4b0a6 commit_ref_update + 106
> > > 5 git 0x000000010ba4c3a8 ref_transaction_commit + 468
> > > 6 git 0x000000010b994dd8 s_update_ref + 271
> > > 7 git 0x000000010b994556 fetch_refs + 1969
> > > 8 git 0x000000010b9935f2 fetch_one + 1913
> > > 9 git 0x000000010b992bc4 cmd_fetch + 549
> > > 10 git 0x000000010b9666c4 handle_builtin + 478
> > > 11 git 0x000000010b96602f main + 376
> > > 12 libdyld.dylib 0x00007fff834ef5ad start + 1
> > >
> > > Maybe someone else has an idea what might be causing this...
> >
> > The only strchr I can see that could be called here is in
> > canonical_name(), where it's called with addrinfo::ai_canonname.
>
> There's one in add_domainname(), too, but it can never be NULL (we could
> walk off the end of the buffer, but only if gethostname() lies to us
> about its result code, which seems unlikely). So I agree it's probably
> the call in canonical_name().
>
> > Searching for OS X and ai_canonname, leads me straight back to this
> > list, although 7 years ago! I think ident.c needs a fix similar to
> > commit 3e8a00a (daemon.c: fix segfault on OS X, 2009-04-27); from the
> > commit message there:
> >
> > On OS X (and maybe other unices), getaddrinfo(3) returns NULL
> > in the ai_canonname field if it's called with an IP address for
> > the hostname.
>
> Interesting. We are already prepared for failure from getaddrinfo()
> here, so probably:
>
> diff --git a/ident.c b/ident.c
> index e20a772..d17b5bd 100644
> --- a/ident.c
> +++ b/ident.c
> @@ -101,7 +101,7 @@ static int canonical_name(const char *host, struct strbuf *out)
> memset (&hints, '\0', sizeof (hints));
> hints.ai_flags = AI_CANONNAME;
> if (!getaddrinfo(host, NULL, &hints, &ai)) {
> - if (ai && strchr(ai->ai_canonname, '.')) {
> + if (ai && ai->ai_canonname && strchr(ai->ai_canonname, '.')) {
> strbuf_addstr(out, ai->ai_canonname);
> status = 0;
> }
>
> would be sufficient. Jonas, can you see if that patch helps?
>
> -Peff
>
^ permalink raw reply
* Re: [PATCH v2 2/3] init: do not set core.worktree more often than necessary
From: Duy Nguyen @ 2016-09-22 10:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Michael J Gruber, Max Nordlund
In-Reply-To: <xmqqd1jx854z.fsf@gitster.mtv.corp.google.com>
On Thu, Sep 22, 2016 at 1:44 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> @@ -314,6 +315,8 @@ static void create_object_directory(void)
>> int set_git_dir_init(const char *git_dir, const char *real_git_dir,
>> int exist_ok)
>> {
>> + original_git_dir = xstrdup(real_path(git_dir));
>> +
>> if (real_git_dir) {
>> struct stat st;
>
> The function being extern bothers me. The create_default_files()
> function, which is the only thing consumes this variable, is called
> only from init_db(), and I'd prefer to see some way to guarantee
> that everybody who calls init_db() calls set_git_dir_init()
> beforehand. Right now, cmd_init_db() and cmd_clone() are the only
> ones that call init_db() and they both call set_dir_git_init(); if a
> new caller starts calling init_db() and forgets to call the other
> one, that caller will be buggy.
>
> Perhaps a comment before init_db() to tell callers to always call
> the other one is the least thing necessary?
Good thinking. We could go a step further, baking it as assert() to
catch new/incorrect call sequences automatically.
Or we could combine the two functions init_db() and set_git_dir_init()
into one. I prefer this one, but having problem with finding a good
name for it because the new function would prepare $GIT_DIR for the
entire process and init the repository. Maybe enter_and_init_db(),
enter_and_init_repo()? If no good name is found, I'll go back to
either adding comment or assert().
--
Duy
^ permalink raw reply
* Re: v2.10.0: ls-tree exit status is always 0, this differs from ls(1)
From: Michael J Gruber @ 2016-09-22 11:36 UTC (permalink / raw)
To: Steffen Nurpmeso, Junio C Hamano, git
In-Reply-To: <20160921224616.GuR6adBwB%steffen@sdaoden.eu>
Steffen Nurpmeso venit, vidit, dixit 22.09.2016 00:46:
> Junio C Hamano <gitster@pobox.com> wrote:
> |Steffen Nurpmeso <steffen@sdaoden.eu> writes:
> ...
> |Sorry, but I did not notice that there was an attached patch when I
> |was reading your response for the first time. Risk of using an
> |attachment to e-mail ;-)
> |
> |I think this issue does not need a separate bullet point. The
> |existing text says:
> ..
> |and what caused your surprise is already covered by the first bullet
> |point, if the reader knows what "patterns to match" means in Git's
> |command line tools; it just needs to be extended to be more
> |meaningful to those who don't, I think.
> |
> |How about rewriting the first bullet point like so instead:
> |
> | - the behaviour is different from that of "/bin/ls" in that the
> | '<path>' are actually patterns to match, e.g. so specifying
> | directory name (without `-r`) will behave differently, the order
> | of the arguments does not matter, and a '<path>' argument that
> | does not match any path is not an error (i.e. if there is no
> | path that matches any pattern, nothing is shown in the output).
>
> Not an error would have been an enlightenment to me.
>
> But now i'm even getting nervous to read about patterns.
> We have patterns for tags/remotes/branches, author/committer/grep
> patterns, (most of those, maybe all today, with fixed string,
> extended or basic regex), the git-grep patterns ("leading paths
> match and glob(7) patterns are supported"). Is that all?
> I would assume glob-style for ls-tree:
>
> ?0[steffen@wales ]$ git ls-tree HEAD `ls mime*`
> 100644 blob ee47419c209da789b606ab6d979c22f4ae632712 mime.c
> 100644 blob 0cfe3766bd5f035eac06b728a4f63224455e13ca mime.types
> 100644 blob 7d890df7553522691ed09f266ea7f9effb6a2f4e mime_enc.c
> 100644 blob 430e300d9a8887c5cd48d1cc63034168e47e9721 mime_param.c
> 100644 blob 0338a46d3247ea00b5bcedb2d82ff30fe5d18d48 mime_parse.c
> 100644 blob d62fa8defae27240a5ce81ad2239dd7f94b6c5c5 mime_types.c
> ?0[steffen@wales ]$ git ls-tree HEAD 'mime*'
> ?0[steffen@wales ]$ git ls-tree HEAD 'mime.*'
>
> No, ls-tree is not part of what i use every day, "Git's command
> line tools" is (too) wide afield, in that sense.
>
> Thank you (also in general, for git), and ciao from a country with
> a pretty real autumn,
Maybe "git ls-files" is the command that you are looking for, really.
That and others have glob pathspec enabled by default, see "git help git".
"git ls-tree" does not understand globs nor pathspec magic. In fact, it
only matches on the first component of a path (complete matches).
Michael
^ permalink raw reply
* Request for large repo clone on slow intermittent connections
From: Aaron Gray @ 2016-09-22 11:54 UTC (permalink / raw)
To: Git Mailing List
I am having problems cloning a 2.1GB repo from googlesource
C:\Users\Aaron Gray\GitHub>git clone
https://chromium.googlesource.com/chromium/chromium
Cloning into 'chromium'...
remote: Sending approximately 2.11 GiB ...
error: fatal: The remote end hung up unexpectedly MiB | 2.74 MiB/s
fatal: RPC failed; curl 56 SSL read:
error:00000000:lib(0):func(0):reason(0), errno 10054
early EOF
fatal: index-pack failed
I am repeatedly getting the same result on a 36MBit connection
Hoping for a soulution.
Regards,
Aaron
^ permalink raw reply
* Re: v2.10.0: ls-tree exit status is always 0, this differs from ls(1)
From: Steffen Nurpmeso @ 2016-09-22 12:57 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <68354d78-fa7a-ee99-2e6e-7ffdcf1a568e@drmicha.warpmail.net>
Hello,
Michael J Gruber <git@drmicha.warpmail.net> wrote:
|Steffen Nurpmeso venit, vidit, dixit 22.09.2016 00:46:
|> Junio C Hamano <gitster@pobox.com> wrote:
|>|Steffen Nurpmeso <steffen@sdaoden.eu> writes:
...
|>|I think this issue does not need a separate bullet point. The
|>|existing text says:
|> ..
|>|and what caused your surprise is already covered by the first bullet
|>|point, if the reader knows what "patterns to match" means in Git's
...
|>|How about rewriting the first bullet point like so instead:
...
|>| of the arguments does not matter, and a '<path>' argument that
|>| does not match any path is not an error (i.e. if there is no
|>| path that matches any pattern, nothing is shown in the output).
|>
|> Not an error would have been an enlightenment to me.
...
|>
|> But now i'm even getting nervous to read about patterns.
...
|> We have patterns for tags/remotes/branches, author/committer/grep
|> patterns, (most of those, maybe all today, with fixed string,
|> extended or basic regex), the git-grep patterns ("leading paths
|> match and glob(7) patterns are supported"). Is that all?
|> I would assume glob-style for ls-tree:
...
|Maybe "git ls-files" is the command that you are looking for, really.
|
|That and others have glob pathspec enabled by default, see "git help git".
Please rollback all of that, i have only reported something that
seemed odd to me. What i really need is instead
if `git cat-file -e ${relbr}:NEWS 2>/dev/null`; then
and that is what i will end up with.
_But_, now that i am here again, "git help cat-file" says
-e
Suppress all output; instead exit with zero status if <object>
exists and is a valid object.
and
OUTPUT
...
If -e is specified, no output.
But this is not what happens if "output" includes stderr:
?0[steffen@wales ]$ git cat-file -e HEAD:NEWS
?0[steffen@wales ]$ git cat-file -e HEAD:NEWSS
fatal: Not a valid object name HEAD:NEWSS
?128[steffen@wales ]$
I would also not expect $?=128 as an counterpart to EXIT_SUCCESS=0
when performing a qualified "test" action, but EXIT_FAILURE=1 is
just an as-bad non-0 exit status code, anyway. To me
EX_NOINPUT=66 obtrudes itself as the right status, but my own
projects don't adhere to this from a-z or not at all, so what i am
talking about? I mean, some things take time and are eventually
and temporarily a bit odd, so what? That is just how it is. Even
Sparta declined some day, and then it crushed, iirc.
Thanks for git, just yesterday evening i did rebasing and cherry
picking and commit amending and garbage collection and it saved me
days of work, or, to be more exact, i never have been able to work
the way i would work before. Really.
Ciao.
--steffen
^ 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