Re: [PATCH] test-string-list.c: Fix some sparse warnings
On 09/19/2012 09:07 PM, Ramsay Jones wrote: > Michael Haggerty wrote: >> Is there some documentation about how to run sparse on the git codebase? >> I naively tried "make sparse" and ended up with zillions of errors like >> >> /usr/include/unistd.h:288:54: error: attribute '__leaf__': unknown attribute >> /usr/include/unistd.h:294:6: error: attribute '__leaf__': unknown attribute >> /usr/include/unistd.h:298:6: error: attribute '__leaf__': unknown attribute >> /usr/include/unistd.h:306:6: error: attribute '__leaf__': unknown attribute >> /usr/include/unistd.h:338:18: error: attribute '__leaf__': unknown attribute >> /usr/include/unistd.h:347:6: error: attribute '__leaf__': unknown attribute >> /usr/include/unistd.h:418:36: error: attribute '__leaf__': unknown attribute >> /usr/include/unistd.h:423:50: error: attribute '__leaf__': unknown attribute > > Yep, "make sparse" is the correct way to run sparse over git. > > This looks like you are running sparse on a 64-bit system. I have heard that > it has (or *had*) problems running on 64-bit systems. Unfortunately, I am > currently confined to 32-bit. (I'm looking at getting a new laptop soon, > before > Windows 8 causes boot-time problems, so I will then have the same problem!) > > How did you obtain/build/install sparse? The current release (v0.4.4) was > released about Nov 2011 and I think you need a more up to date version. > i.e. you need to build the latest, directly from the sparse repo. Yes, I'm running 64-bit Ubuntu 12.04 "precise". I installed sparse from the Ubuntu "multiverse" repository. It is package version 0.4.3+20110419-1 in Ubuntu's notation. Thanks very much for all the info. I hadn't heard of sparse before and thought that using it might help me avoid submitting patches with problems like the ones you detected. It does seem promising! But since it seems a bit fiddly to get it running, and even then has some problems, it doesn't sound like the simple pre-submit checklist item that I had imagined. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Documentation: Document signature showing options
The pretty formats for GPG signatures were introduced but never
documented. Use the documentation from the commit that introduced them.
Do the same for the --show-signature option added to git log and
friends.
Signed-off-by: Stephen Boyd
---
I had to google for --show-signature one too many times.
Documentation/pretty-formats.txt | 3 +++
Documentation/pretty-options.txt | 4
2 files changed, 7 insertions(+)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index e3d8a83..d9edded 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -130,6 +130,9 @@ The placeholders are:
- '%b': body
- '%B': raw body (unwrapped subject and body)
- '%N': commit notes
+- '%GG': raw verification message from GPG for a signed commit
+- '%G?': show either "G" for Good or "B" for Bad for a signed commit
+- '%GS': show the name of the signer for a signed commit
- '%gD': reflog selector, e.g., `refs/stash@{1}`
- '%gd': shortened reflog selector, e.g., `stash@{1}`
- '%gn': reflog identity name
diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 2a3dc86..5e49942 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -66,3 +66,7 @@ being displayed. Examples: "--notes=foo" will show only notes
from
--[no-]standard-notes::
These options are deprecated. Use the above --notes/--no-notes
options instead.
+
+--show-signature::
+ Check the validity of a signed commit object by passing the signature
+ to `gpg --verify` and show the output.
--
1.7.12.1.382.gb0576a6.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/6] Color skipped tests blue
On Thu, Sep 20, 2012 at 6:48 AM, Johannes Sixt wrote: > Am 9/19/2012 22:24, schrieb Adam Spiers: >> skip) >> - tput bold; tput setaf 2;; # bold green >> + tput setaf 4;;# blue > > It's unreadable on black background. Keep it bold; that works on both > black and white background. Whilst my preference aligns with yours in this particular case, we are now on a slippery slope, since these days terminal colors are infinitely configurable, and some heretics even choose backgrounds which are neither black nor white ;-) I don't want to trigger a long discussion or end up spamming the list with lots of different color scheme patches in an attempt to please everyone. So bold blue will be my last version of this patch. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 3/3] Color skipped tests bold blue
Skipped tests indicate incomplete test coverage. Whilst this is not a test failure or other error, it's still not complete success, so according to the universal traffic lights coloring scheme, yellow/brown seems more suitable than green. However, it's more informational than cautionary, so instead we use blue which is a universal color for information signs. Bold blue should work better on both black and white backgrounds than normal blue. Signed-off-by: Adam Spiers --- t/test-lib.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 5293830..5ef87d4 100755 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -182,13 +182,13 @@ then error) tput bold; tput setaf 1;; # bold red skip) - tput bold; tput setaf 2;; # bold green + tput bold; tput setaf 4;; # bold blue warn) tput bold; tput setaf 3;; # bold yellow pass) tput setaf 2;;# green info) - tput setaf 3;;# brown + tput setaf 3;;# yellow/brown *) test -n "$quiet" && return;; esac -- 1.7.12.147.g6d168f4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] Color skipped tests bold blue
Hi Adam. On 09/20/2012 11:08 AM, Adam Spiers wrote: > Skipped tests indicate incomplete test coverage. Whilst this is > not a test failure or other error, it's still not complete > success, so according to the universal traffic lights coloring > scheme, yellow/brown seems more suitable than green. However, > it's more informational than cautionary, so instead we use blue > which is a universal color for information signs. Bold blue > should work better on both black and white backgrounds than > normal blue. > A very minor nit (feel free to ignore it): IMHO, it should be nice to state explicitly in the commit message that blue is already used by other testsuite-related software to highlight skipped tests; you can report the examples of at least Automake, Autotest and prove -- and extra kudos if you find further examples ;-) Thanks, Stefano -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] log --oneline: put decoration at the end of the line
On Thu, Sep 20, 2012 at 6:42 AM, Jeff King wrote: > If you are particular about the exact format, how about using > --format="%h%d %s" instead? > > Obviously Duy could do the same to achieve his format, but I think there > is still value in considering what the default for --oneline should be. Yeah I was wondering if a new default would make sense. I tried --pretty=format:"%h %s %d" but the colors were lost. --decorate uses more than one color so %C* does not immitate the colorful output we have with --decorate. Maybe I should add %Cd as "colored %d" and update my alias to use it. And perhaps a specifier to enable right alignment. It may help make better use of wide terminal screen. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] New pretty format color specifiers %C+ and %C-
On Thu, Sep 20, 2012 at 5:43 PM, Nguyen Thai Ngoc Duy wrote: > I tried --pretty=format:"%h %s %d" but the colors were lost. > --decorate uses more than one color so %C* does not immitate the > colorful output we have with --decorate. Maybe I should add %Cd as > "colored %d" and update my alias to use it. Here goes. I'm now a happy user with --pretty='format:%C+%h %s %d'. Somebody might want to add coloring to sig placeholder too. Nguyễn Thái Ngọc Duy (2): pretty: share code between format_decoration and show_decorations pretty: support placeholders %C+ and %C- Documentation/pretty-formats.txt | 2 ++ log-tree.c | 55 log-tree.h | 3 +++ pretty.c | 30 +- 4 files changed, 50 insertions(+), 40 deletions(-) -- 1.7.12.1.383.gda6001e.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] pretty: share code between format_decoration and show_decorations
This also adds color support to format_decoration()
Signed-off-by: Nguyễn Thái Ngọc Duy
---
log-tree.c | 55 +--
log-tree.h | 3 +++
pretty.c | 19 +--
3 files changed, 37 insertions(+), 40 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index c894930..b8cea3f 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -174,36 +174,47 @@ static void show_children(struct rev_info *opt, struct
commit *commit, int abbre
}
}
-void show_decorations(struct rev_info *opt, struct commit *commit)
+void format_decoration(struct strbuf *sb,
+ const struct commit *commit,
+ int use_color)
{
- const char *prefix;
- struct name_decoration *decoration;
+ const char *prefix = " (";
+ struct name_decoration *d;
const char *color_commit =
- diff_get_color_opt(&opt->diffopt, DIFF_COMMIT);
+ diff_get_color(use_color, DIFF_COMMIT);
const char *color_reset =
- decorate_get_color_opt(&opt->diffopt, DECORATION_NONE);
+ decorate_get_color(use_color, DECORATION_NONE);
+
+ load_ref_decorations(DECORATE_SHORT_REFS);
+ d = lookup_decoration(&name_decoration, &commit->object);
+ if (!d)
+ return;
+ while (d) {
+ strbuf_addstr(sb, prefix);
+ strbuf_addstr(sb, decorate_get_color(use_color, d->type));
+ if (d->type == DECORATION_REF_TAG)
+ strbuf_addstr(sb, "tag: ");
+ strbuf_addstr(sb, d->name);
+ strbuf_addstr(sb, color_reset);
+ strbuf_addstr(sb, color_commit);
+ prefix = ", ";
+ d = d->next;
+ }
+ if (prefix[0] == ',')
+ strbuf_addch(sb, ')');
+}
+
+void show_decorations(struct rev_info *opt, struct commit *commit)
+{
+ struct strbuf sb = STRBUF_INIT;
if (opt->show_source && commit->util)
printf("\t%s", (char *) commit->util);
if (!opt->show_decorations)
return;
- decoration = lookup_decoration(&name_decoration, &commit->object);
- if (!decoration)
- return;
- prefix = " (";
- while (decoration) {
- printf("%s", prefix);
- fputs(decorate_get_color_opt(&opt->diffopt, decoration->type),
- stdout);
- if (decoration->type == DECORATION_REF_TAG)
- fputs("tag: ", stdout);
- printf("%s", decoration->name);
- fputs(color_reset, stdout);
- fputs(color_commit, stdout);
- prefix = ", ";
- decoration = decoration->next;
- }
- putchar(')');
+ format_decoration(&sb, commit, opt->diffopt.use_color);
+ fputs(sb.buf, stdout);
+ strbuf_release(&sb);
}
/*
diff --git a/log-tree.h b/log-tree.h
index f5ac238..10c2682 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -13,6 +13,9 @@ int log_tree_diff_flush(struct rev_info *);
int log_tree_commit(struct rev_info *, struct commit *);
int log_tree_opt_parse(struct rev_info *, const char **, int);
void show_log(struct rev_info *opt);
+void format_decoration(struct strbuf *sb,
+ const struct commit *commit,
+ int use_color);
void show_decorations(struct rev_info *opt, struct commit *commit);
void log_write_email_headers(struct rev_info *opt, struct commit *commit,
const char **subject_p,
diff --git a/pretty.c b/pretty.c
index 8b1ea9f..e910679 100644
--- a/pretty.c
+++ b/pretty.c
@@ -764,23 +764,6 @@ static void parse_commit_message(struct
format_commit_context *c)
c->commit_message_parsed = 1;
}
-static void format_decoration(struct strbuf *sb, const struct commit *commit)
-{
- struct name_decoration *d;
- const char *prefix = " (";
-
- load_ref_decorations(DECORATE_SHORT_REFS);
- d = lookup_decoration(&name_decoration, &commit->object);
- while (d) {
- strbuf_addstr(sb, prefix);
- prefix = ", ";
- strbuf_addstr(sb, d->name);
- d = d->next;
- }
- if (prefix[0] == ',')
- strbuf_addch(sb, ')');
-}
-
static void strbuf_wrap(struct strbuf *sb, size_t pos,
size_t width, size_t indent1, size_t indent2)
{
@@ -1005,7 +988,7 @@ static size_t format_commit_one(struct strbuf *sb, const
char *placeholder,
strbuf_addstr(sb, get_revision_mark(NULL, commit));
return 1;
case 'd':
- format_decoration(sb, commit);
+ format_decoration(sb, commit, 0);
return 1;
case 'g': /* reflog info */
switch(placeholder[1]) {
--
1.7.12.1.383.gda6001e.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the
[PATCH 2/2] pretty: support placeholders %C+ and %C-
%C+ tells the next specifiers that color is preferred. %C- the
opposite. So far only %H, %h and %d support coloring.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
Documentation/pretty-formats.txt | 2 ++
pretty.c | 13 -
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index e3d8a83..6e287d6 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -142,6 +142,8 @@ The placeholders are:
- '%Cblue': switch color to blue
- '%Creset': reset color
- '%C(...)': color specification, as described in color.branch.* config option
+- '%C+': enable coloring on the following placeholders if supported
+- '%C-': disable coloring on the following placeholders
- '%m': left, right or boundary mark
- '%n': newline
- '%%': a raw '%'
diff --git a/pretty.c b/pretty.c
index e910679..b1cec71 100644
--- a/pretty.c
+++ b/pretty.c
@@ -623,6 +623,7 @@ struct format_commit_context {
unsigned commit_header_parsed:1;
unsigned commit_message_parsed:1;
unsigned commit_signature_parsed:1;
+ unsigned use_color:1;
struct {
char *gpg_output;
char good_bad;
@@ -885,6 +886,12 @@ static size_t format_commit_one(struct strbuf *sb, const
char *placeholder,
"--pretty format", color);
strbuf_addstr(sb, color);
return end - placeholder + 1;
+ } else if (placeholder[1] == '+') {
+ c->use_color = 1;
+ return 2;
+ } else if (placeholder[1] == '-') {
+ c->use_color = 0;
+ return 2;
}
if (!prefixcmp(placeholder + 1, "red")) {
strbuf_addstr(sb, GIT_COLOR_RED);
@@ -945,13 +952,17 @@ static size_t format_commit_one(struct strbuf *sb, const
char *placeholder,
switch (placeholder[0]) {
case 'H': /* commit hash */
+ strbuf_addstr(sb, diff_get_color(c->use_color, DIFF_COMMIT));
strbuf_addstr(sb, sha1_to_hex(commit->object.sha1));
+ strbuf_addstr(sb, diff_get_color(c->use_color, DIFF_RESET));
return 1;
case 'h': /* abbreviated commit hash */
+ strbuf_addstr(sb, diff_get_color(c->use_color, DIFF_COMMIT));
if (add_again(sb, &c->abbrev_commit_hash))
return 1;
strbuf_addstr(sb, find_unique_abbrev(commit->object.sha1,
c->pretty_ctx->abbrev));
+ strbuf_addstr(sb, diff_get_color(c->use_color, DIFF_RESET));
c->abbrev_commit_hash.len = sb->len - c->abbrev_commit_hash.off;
return 1;
case 'T': /* tree hash */
@@ -988,7 +999,7 @@ static size_t format_commit_one(struct strbuf *sb, const
char *placeholder,
strbuf_addstr(sb, get_revision_mark(NULL, commit));
return 1;
case 'd':
- format_decoration(sb, commit, 0);
+ format_decoration(sb, commit, c->use_color);
return 1;
case 'g': /* reflog info */
switch(placeholder[1]) {
--
1.7.12.1.383.gda6001e.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/2] pretty: support right alignment
And this is a for-fun patch that adds %| to right align everything
after that. I'm ignoring problems with line wrapping, i18n and so
on. "%C+%h %s%|%d" looks quite nice. I'm not sure how much useful it
is beyond --oneline though. It looks something like this
cc543b2 pretty: support placeholders %C+ and %C-
(HEAD, master)
da6001e pretty: share code between format_decoration and show_decorations
b0576a6 Update draft release notes to 1.8.0
(origin/master, origin/HEAD)
3d7535e Merge branch 'jc/maint-log-grep-all-match'
06e211a Merge branch 'jc/make-static'
8db3865 Merge branch 'pw/p4-submit-conflicts'
3387423 Merge branch 'mv/cherry-pick-s'
d71abd9 Merge branch 'nd/fetch-status-alignment'
3c7d509 Sync with 1.7.12.1
304b7d9 Git 1.7.12.1
(tag: v1.7.12.1, origin/maint)
39e2e02 Merge branch 'er/doc-fast-import-done' into maint
8ffc331 Merge branch 'jk/config-warn-on-inaccessible-paths' into maint
01f7d7f Doc: Improve shallow depth wording
8093ae8 Documentation/git-filter-branch: Move note about effect of removing
commits
-- 8< --
diff --git a/pretty.c b/pretty.c
index b1cec71..6e96f83 100644
--- a/pretty.c
+++ b/pretty.c
@@ -624,6 +624,7 @@ struct format_commit_context {
unsigned commit_message_parsed:1;
unsigned commit_signature_parsed:1;
unsigned use_color:1;
+ unsigned right_alignment:1;
struct {
char *gpg_output;
char good_bad;
@@ -645,6 +646,8 @@ struct format_commit_context {
struct chunk abbrev_tree_hash;
struct chunk abbrev_parent_hashes;
size_t wrap_start;
+
+ struct strbuf *right_sb;
};
static int add_again(struct strbuf *sb, struct chunk *chunk)
@@ -944,6 +947,10 @@ static size_t format_commit_one(struct strbuf *sb, const
char *placeholder,
return end - placeholder + 1;
} else
return 0;
+
+ case '|':
+ c->right_alignment = 1;
+ return 1;
}
/* these depend on the commit */
@@ -1099,9 +1106,44 @@ static size_t format_commit_one(struct strbuf *sb, const
char *placeholder,
return 0; /* unknown placeholder */
}
+static void right_align(struct strbuf *sb,
+ struct format_commit_context *c,
+ int flush)
+{
+ const char *p;
+ int llen, rlen, len, total = term_columns() - 1;
+ if (!c->right_alignment)
+ return;
+ p = strchr(c->right_sb->buf, '\n');
+ if (!p && flush)
+ p = c->right_sb->buf + c->right_sb->len;
+ if (!p)
+ return;
+
+ c->right_alignment = 0;
+ len = p - c->right_sb->buf;
+ if (!len)
+ return;
+ if (total > 110)
+ total = 110;
+ rlen = utf8_strnwidth(c->right_sb->buf, len);
+ p = strrchr(sb->buf, '\n');
+ if (!p)
+ p = sb->buf;
+ else
+ p++;
+ llen = utf8_strwidth(p);
+ strbuf_addf(sb, "%*s",
+ total - llen + (len - rlen),
+ c->right_sb->buf);
+ strbuf_reset(c->right_sb);
+}
+
static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
void *context)
{
+ struct format_commit_context *c = context;
+ struct strbuf *real_sb;
int consumed;
size_t orig_len;
enum {
@@ -1127,10 +1169,13 @@ static size_t format_commit_item(struct strbuf *sb,
const char *placeholder,
if (magic != NO_MAGIC)
placeholder++;
+ if (c->right_alignment && c->right_sb) {
+ real_sb = sb;
+ sb = c->right_sb;
+ }
+
orig_len = sb->len;
consumed = format_commit_one(sb, placeholder, context);
- if (magic == NO_MAGIC)
- return consumed;
if ((orig_len == sb->len) && magic == DEL_LF_BEFORE_EMPTY) {
while (sb->len && sb->buf[sb->len - 1] == '\n')
@@ -1141,7 +1186,13 @@ static size_t format_commit_item(struct strbuf *sb,
const char *placeholder,
else if (magic == ADD_SP_BEFORE_NON_EMPTY)
strbuf_insert(sb, orig_len, " ", 1);
}
- return consumed + 1;
+
+ if (real_sb)
+ right_align(real_sb, c, 0);
+
+ if (magic != NO_MAGIC)
+ consumed++;
+ return consumed;
}
static size_t userformat_want_item(struct strbuf *sb, const char *placeholder,
@@ -1180,12 +1231,14 @@ void format_commit_message(const struct commit *commit,
struct format_commit_context context;
static const char utf8[] = "UTF-8";
const char *output_enc = pretty_ctx->output_encoding;
+ struct strbuf right_sb = STRBUF_INIT;
memset(&context, 0, sizeof(context));
context.co
Re: [PATCH v5 3/3] Color skipped tests bold blue
Stefano Lattarini writes: > Hi Adam. > > On 09/20/2012 11:08 AM, Adam Spiers wrote: >> Skipped tests indicate incomplete test coverage. Whilst this is >> not a test failure or other error, it's still not complete >> success, so according to the universal traffic lights coloring >> scheme, yellow/brown seems more suitable than green. However, >> it's more informational than cautionary, so instead we use blue >> which is a universal color for information signs. Bold blue >> should work better on both black and white backgrounds than >> normal blue. >> > A very minor nit (feel free to ignore it): IMHO, it should be nice > to state explicitly in the commit message that blue is already used > by other testsuite-related software to highlight skipped tests; you > can report the examples of at least Automake, Autotest and prove -- > and extra kudos if you find further examples ;-) Thanks. Will tweak the proposed log message to include the above when queuing the updated patch. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert "retry request without query when info/refs?query fails"
On Wed, Sep 19, 2012 at 11:29:34PM -0700, Junio C Hamano wrote: > "Shawn O. Pearce" writes: > > > This reverts commit 703e6e76a14825e5b0c960d525f34e607154b4f7. > > > > Retrying without the query parameter was added as a workaround > > for a single broken HTTP server at git.debian.org[1]. The server > > was misconfigured to route every request with a query parameter > > into gitweb.cgi. Admins fixed the server's configuration within > > 16 hours of the bug report to the Git mailing list, but we still > > patched Git with this fallback and have been paying for it since. > > As the consequence of the above, the only two things we know about > the servers in the wild are (1) a misconfiguration that requires > this retry was once made, so it is not very unlikely others did the > same misconfiguration, and (2) those unknown number of servers have > been happily serving the current clients because the workaround > patch have been hiding the misconfiguration ever since. The misconfiguration was pretty wild in this case. I'd be much more worried about stupidly non-compliant servers that will not serve "foo/bar" when asked for "foo/bar?key=value". > But as long as the failure diagnosis from updated clients that > revert this workaround is sufficient to allow such misconfigured > servers, I think it is OK. We might see a large number of small > people having to run around and fix the configuration as a fallout, > though. I think Shawn's revert is the right thing to do. But it is not complete without the manual workaround. I'm putting that patch together now and should have it out in a few minutes. > > Most Git hosting services configure the smart HTTP protocol and the > > retry logic confuses users when there is a transient HTTP error as > > Git dropped the real error from the smart HTTP request. Removing the > > retry makes root causes easier to identify. > > Does that hold true also for dumb only small people installations? > They are the ones that need more help than the large installations > staffed sufficiently and run smart http gateway. For the most part, yes. They will get a useful error out of the smart request if there is a transient error, the repo does not exist, etc. The real fallout is the people who are hitting a broken or misconfigured server and may get a confusing error code (in the one case we know about, it was a 404, but it really could be anything, depending on the exact nature of the misconfiguration). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/2] pretty: support right alignment
Nguyen Thai Ngoc Duy writes: > And this is a for-fun patch that adds %| to right align everything > after that. I'm ignoring problems with line wrapping, i18n and so > on. "%C+%h %s%|%d" looks quite nice. I'm not sure how much useful it > is beyond --oneline though. It looks something like this > ... > [oneline output ellided] > ... I think this is a great feature at the conceptual level, and you know "but" is coming ;-). - Shouldn't it be "everything from there until the end of the current line" than "everything after that"? - How is the display width determined and is it fixed once it gets computed? - How does this interact with the wrapped output? Should it? - I am wondering if somebody ever want to do this with a follow-up patch: Left %h%|Center %cd%|Right %ad Is %| a sensible choice for "flush right"? I am wondering if it makes more sense to make %|, %< and %> as "multi-column introducer" (the example defines output with three columns) that also tells how text inside each column is flushed inside the column, e.g. %>col 1 right flushed%|col 2 centered%< col 3 left flushed or something like that (we may want explicit "column width" specifiers if we were to do this kind of thing). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] pretty: support placeholders %C+ and %C-
Nguyễn Thái Ngọc Duy writes: > %C+ tells the next specifiers that color is preferred. %C- the > opposite. So far only %H, %h and %d support coloring. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > Documentation/pretty-formats.txt | 2 ++ > pretty.c | 13 - > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/Documentation/pretty-formats.txt > b/Documentation/pretty-formats.txt > index e3d8a83..6e287d6 100644 > --- a/Documentation/pretty-formats.txt > +++ b/Documentation/pretty-formats.txt > @@ -142,6 +142,8 @@ The placeholders are: > - '%Cblue': switch color to blue > - '%Creset': reset color > - '%C(...)': color specification, as described in color.branch.* config > option > +- '%C+': enable coloring on the following placeholders if supported > +- '%C-': disable coloring on the following placeholders OK, so typically you replace some format placeholder "%?" in your format string with "%C+%?%C-", because you cannot get away with replacing it with "%C+%? and other things in the format you do not know if they support coloring%C-". If that is the case, does it really make sense to have %C-? It smells as if it makes more sense to make _all_ %? placeholder reset the effect of %C+ after they are done (even the ones that they themselves do not color their own output elements), so that you can mechanically replace "%?" with "%C+%?". I dunno. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Improve legibility of test_expect_code output
Adam Spiers writes: > On Thu, Sep 20, 2012 at 1:06 AM, Junio C Hamano wrote: >> Adam Spiers writes: >> >>> --- >> >> No explanation why this is a good idea, nor sign-off? > > I realised I forgot the sign-off seconds after sending :-( > > Isn't it completely self-explanatory? e.g. > > test_expect_code: command exited with 0, we wanted 128 git foo bar > > clearly makes more sense than > > test_expect_code: command exited with 0, we wanted 128 from: git foo bar test_expect_code: command exited with 0, we wanted 128: git foo bar would be shorter and equally legible, I would think. In any case, the proposed commit log message should have explained these differences in the first place so that I or others do not have to ask. Do you want this queued on top of your other series, or as an independent change? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: fix shell expansion of items
Jeff King writes:
> On Thu, Sep 20, 2012 at 04:15:15AM +0200, Felipe Contreras wrote:
>
>> As reported by Jeroen Meijer[1]; the current code doesn't deal properly
>> with items (tags, branches, etc.) that have ${} in them because they get
>> expaned by bash while using compgen.
>>
>> A simple solution is to quote the items so they get expanded properly
>> (\$\{\}).
>>
>> In order to achieve that I took bash-completion's quote() function,
>> which is rather simple, and renamed it to __git_quote() as per Jeff
>> King's suggestion.
>>
>> Solves the original problem for me.
>
> Me too. Thanks.
Thanks, both. Will queue.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Improve legibility of test_expect_code output
On Thu, Sep 20, 2012 at 5:50 PM, Junio C Hamano wrote: > Do you want this queued on top of your other series, or as an > independent change? Independent please. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] smart http toggle switch fails"
On Thu, Sep 20, 2012 at 12:24:56PM -0400, Jeff King wrote: > I think Shawn's revert is the right thing to do. But it is not complete > without the manual workaround. I'm putting that patch together now and > should have it out in a few minutes. And here it is. This goes on top of Shawn's revert patch (it might be nice to mention the commit id of that in the commit message of the second patch. I couldn't do so because it is not yet in your repo). [1/2]: remote-curl: rename is_http variable [2/2]: remote-curl: let users turn off smart http -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] remote-curl: rename is_http variable
We don't actually care whether the connection is http or
not; what we care about is whether it might be smart http.
Rename the variable to be more accurate, which will make it
easier to later make smart-http optional.
Signed-off-by: Jeff King
---
remote-curl.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index 2359f59..c0b98cc 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -95,7 +95,7 @@ static struct discovery* discover_refs(const char *service)
struct strbuf buffer = STRBUF_INIT;
struct discovery *last = last_discovery;
char *refs_url;
- int http_ret, is_http = 0;
+ int http_ret, maybe_smart = 0;
if (last && !strcmp(service, last->service))
return last;
@@ -103,7 +103,7 @@ static struct discovery* discover_refs(const char *service)
strbuf_addf(&buffer, "%sinfo/refs", url);
if (!prefixcmp(url, "http://";) || !prefixcmp(url, "https://";)) {
- is_http = 1;
+ maybe_smart = 1;
if (!strchr(url, '?'))
strbuf_addch(&buffer, '?');
else
@@ -131,7 +131,7 @@ static struct discovery* discover_refs(const char *service)
last->buf_alloc = strbuf_detach(&buffer, &last->len);
last->buf = last->buf_alloc;
- if (is_http && 5 <= last->len && last->buf[4] == '#') {
+ if (maybe_smart && 5 <= last->len && last->buf[4] == '#') {
/* smart HTTP response; validate that the service
* pkt-line matches our request.
*/
--
1.7.11.7.15.g085c6bd
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] remote-curl: let users turn off smart http
Usually there is no need for users to specify whether an
http remote is smart or dumb; the protocol is designed so
that a single initial request is made, and the client can
determine the server's capability from the response.
However, some misconfigured dumb-only servers may not like
the initial request by a smart client, as it contains a
query string. Until recently, commit 703e6e7 worked around
this by making a second request. However, that commit was
recently reverted due to its side effect of masking the
initial request's error code.
This patch takes a different approach to the workaround. We
assume that the common case is that the server is either
smart-http or a reasonably configured dumb-http. If that is
not the case, we provide both a per-remote config option and
an environment variable with which the user can manually
work around the issue.
Signed-off-by: Jeff King
---
I added the config item as remote.foo.smarthttp. You could also allow
"http.$url.smart" (and just "http.smart", for that matter), which could
be more flexible if you have multiple remotes pointing to the same
broken server. However, it is also more complex to use, and is a lot
more code. Since we don't know if any such servers even exist, I tried
to give the minimal escape hatch, and we can easily build more features
on it later if people complain.
Documentation/config.txt | 11 +++
remote-curl.c| 3 ++-
remote.c | 3 +++
remote.h | 1 +
t/t5551-http-fetch.sh| 17 +
5 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6416cae..651b23c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1871,6 +1871,17 @@ remote..uploadpack::
The default program to execute on the remote side when fetching. See
option \--upload-pack of linkgit:git-fetch-pack[1].
+remote..smartHttp::
+ If true, this remote will attempt to use git's smart http
+ protocol when making remote http requests. Normally git sends an
+ initial smart-http request, and falls back to the older "dumb"
+ protocol if the server does not claim to support the smart
+ protocol. However, some misconfigured dumb-only servers may
+ produce confusing results for the initial request. Setting this
+ option to false disables the initial smart request, which can
+ workaround problems with such servers. You should not generally
+ need to set this. Defaults to `true`.
+
remote..tagopt::
Setting this value to \--no-tags disables automatic tag following when
fetching from remote . Setting it to \--tags will fetch every
diff --git a/remote-curl.c b/remote-curl.c
index c0b98cc..8829bfb 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -102,7 +102,8 @@ static struct discovery* discover_refs(const char *service)
free_discovery(last);
strbuf_addf(&buffer, "%sinfo/refs", url);
- if (!prefixcmp(url, "http://";) || !prefixcmp(url, "https://";)) {
+ if ((!prefixcmp(url, "http://";) || !prefixcmp(url, "https://";)) &&
+git_env_bool("GIT_SMART_HTTP", remote->smart_http)) {
maybe_smart = 1;
if (!strchr(url, '?'))
strbuf_addch(&buffer, '?');
diff --git a/remote.c b/remote.c
index 04fd9ea..a334390 100644
--- a/remote.c
+++ b/remote.c
@@ -152,6 +152,7 @@ static struct remote *make_remote(const char *name, int len)
ret->name = xstrndup(name, len);
else
ret->name = xstrdup(name);
+ ret->smart_http = 1;
return ret;
}
@@ -453,6 +454,8 @@ static int handle_config(const char *key, const char
*value, void *cb)
key, value);
} else if (!strcmp(subkey, ".vcs")) {
return git_config_string(&remote->foreign_vcs, key, value);
+ } else if (!strcmp(subkey, ".smarthttp")) {
+ remote->smart_http = git_config_bool(key, value);
}
return 0;
}
diff --git a/remote.h b/remote.h
index 251d8fd..9031d18 100644
--- a/remote.h
+++ b/remote.h
@@ -40,6 +40,7 @@ struct remote {
int fetch_tags;
int skip_default_update;
int mirror;
+ int smart_http;
const char *receivepack;
const char *uploadpack;
diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index 2db5c35..48173ed 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -129,6 +129,23 @@ test_expect_success 'clone from auth-only-for-push
repository' '
test_cmp expect actual
'
+test_expect_success 'disable dumb http on server' '
+ git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
+ config http.getanyfile false
+'
+
+test_expect_success 'GIT_SMART_HTTP can disable smart http' '
+ (GIT_SMART_HTTP=0 &&
+export GIT_SMART_HTTP &&
+cd clone &&
+test_must_fail gi
Re: [PATCH] Disable dumb HTTP fallback with GIT_CURL_FALLBACK=0
On Wed, Sep 19, 2012 at 10:57:35PM -0700, Shawn O. Pearce wrote: > > I have been looking into this recently, as well. GitHub does not allow > > dumb http at all these days, > > Interesting that GitHub doesn't support dumb transfer either. Our objects are still in regular repos, and is served by fairly stock git. The main show-stopper is that we share objects via alternates, and we aggressively repack the alternates repos. So a dumb client would end up being quite inefficient. We also toyed with having mixed public/private fork networks at one point, which would obviously necessitate disabling dumb access. But we gave it up as too risky, as you can do all sorts of weird tricks to convince git to disclose information about what's in the private repos (e.g., you can reliably find out which sha1s exist, and you can lie about which sha1s you have to ask git to create deltas against them). Doing a shared-object store right would require marking which repo "knows" which objects (I assume that's what Google Code does, but I don't remember if Dave talked about that aspect at last year's GitTogether). > > so transient errors on the initial smart > > contact can cause us to fall back to dumb, > > Transient errors is actually what is leading me down this path. We see > about 0.0455% of our requests to the Android hosting service as these > dumb fallbacks. This means the client never got a proper smart service > reply. Server logs suggest we sent a valid response, so I am > suspecting transient proxy errors, but its hard to debug because > clients discard the error. Yup, we see the same thing. I've tracked a few down manually to actual things like gateway timeouts on our reverse proxy. > > and end up reporting a > > totally useless 403 Forbidden error. > > Today I posted a change to JGit [1] to make this 406 Not Acceptable as > I think it better matches the description of the failure. It also no > longer reuses the common 403 Forbidden error that a server might > choose to return if a request was given with invalid credentials. That might be worth doing for git-http-backend, too. It might even make sense for the git client to recognize the 406 (and possibly the 403) and print a more useful message. > > 1. If both smart and dumb requests fail, report the error for the > > smart request. Now that smart-http clients are common, I'd expect > > most http servers to be smart these days. Of course I don't have > > any sort of numbers to back this up (nor am I sure how to get them; > > obviously big sites like GitHub and Google Code do a lot of > > traffic, but who knows how many one-off repo-on-a-generic-web-host > > sites still exist?). > > I suspect there are still a number of servers that rely on dumb > protocol to host repositories because its very simple to setup. > Breaking support for this wouldn't be a good idea. I don't think it would break on most servers, though. Even for a dumb server, the initial error will be a useful one. It's only the weirdly-configured ones where you get wildly different results depending on whether the query string is there. In other words, it is really no worse than reverting 703e6e76, and it might be better. In the common case, you get a better error message. In the broken-server case, we still try the fallback. So it will keep working on a broken server without any manual intervention, whereas reverting and adding a manual escape hatch means the user has to do something. BUT. I still think it's better to revert 703e6e76, because I really do think the broken-server case is an extreme enough minority that it is not even worth wasting the time of clients to make a second request (especially because the first request may very well have failed because of a network error that causes a long timeout, and the user then has to wait double to find out what the error is). Of course, I have no actual data aside from reading the original thread that led to 703e6e76, and the fact that nobody else mentioned it (not during the time when it was broken, and not even after, when people often still complain because they haven't upgraded yet). But who knows? Maybe I will eat my words, and we will end up getting that data in the form of complaints. :) We can always switch to fallback-but-prefer-the-initial-error then. And we'll have more data on exactly how the misconfigured servers behave. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Improve legibility of test_expect_code output
Adam Spiers writes: > On Thu, Sep 20, 2012 at 5:50 PM, Junio C Hamano wrote: >> Do you want this queued on top of your other series, or as an >> independent change? > > Independent please. With a sign-off? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] pretty: support placeholders %C+ and %C-
Junio C Hamano writes: > Nguyễn Thái Ngọc Duy writes: > >> %C+ tells the next specifiers that color is preferred. %C- the >> opposite. So far only %H, %h and %d support coloring. >> >> Signed-off-by: Nguyễn Thái Ngọc Duy >> --- >> Documentation/pretty-formats.txt | 2 ++ >> pretty.c | 13 - >> 2 files changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/pretty-formats.txt >> b/Documentation/pretty-formats.txt >> index e3d8a83..6e287d6 100644 >> --- a/Documentation/pretty-formats.txt >> +++ b/Documentation/pretty-formats.txt >> @@ -142,6 +142,8 @@ The placeholders are: >> - '%Cblue': switch color to blue >> - '%Creset': reset color >> - '%C(...)': color specification, as described in color.branch.* config >> option >> +- '%C+': enable coloring on the following placeholders if supported >> +- '%C-': disable coloring on the following placeholders > > OK, so typically you replace some format placeholder "%?" in your > format string with "%C+%?%C-", because you cannot get away with > replacing it with "%C+%? and other things in the format you do not > know if they support coloring%C-". > > If that is the case, does it really make sense to have %C-? > > It smells as if it makes more sense to make _all_ %? placeholder > reset the effect of %C+ after they are done (even the ones that they > themselves do not color their own output elements), so that you can > mechanically replace "%?" with "%C+%?". > > I dunno. Thinking about this a bit more, perhaps we would want a generic mechanism to give parameters to various %? placeholders. This is not limited to "I can do color but there is no mechanism for the user to tell me that I should do color" %H, %h and %d may want to say. An obvious and immediate example is that %h might want to be told how many hexdigits it should use. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] remote-curl: let users turn off smart http
Jeff King writes: > I added the config item as remote.foo.smarthttp. You could also allow > "http.$url.smart" (and just "http.smart", for that matter), which could > be more flexible if you have multiple remotes pointing to the same > broken server. What would the user experience be when we introduce "even smarter" http server protocol extension? Will we add remote.foo.starterhttp? Perhaps remote.$name.httpvariants = [smart] [dumb] to allow users to say "smart only", "dumb only", or "smart and/or dumb" might be more code but less burden on the users. The code obviously looks correct, and the documentation reads fine. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Improve legibility of test_expect_code output
On Thu, Sep 20, 2012 at 6:45 PM, Junio C Hamano wrote: > Adam Spiers writes: >> On Thu, Sep 20, 2012 at 5:50 PM, Junio C Hamano wrote: >>> Do you want this queued on top of your other series, or as an >>> independent change? >> >> Independent please. > > With a sign-off? Yep. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv8] clone --single: limit the fetch refspec to fetched branch
After running "git clone --single", the resulting repository has the
usual default "+refs/heads/*:refs/remotes/origin/*" wildcard fetch
refspec installed, which means that a subsequent "git fetch" will
end up grabbing all the other branches.
Update the fetch refspec to cover only the singly cloned ref instead
to correct this.
That means:
If "--single" is used without "--branch" or "--mirror", the
fetch refspec covers the branch on which remote's HEAD points to.
If "--single" is used with "--branch", it'll cover only the branch
specified in the "--branch" option.
If "--single" is combined with "--mirror", then it'll cover all
refs of the cloned repository.
If "--single" is used with "--branch" that specifies a tag, then
it'll cover only the ref for this tag.
Signed-off-by: Ralf Thielow
---
changes in v8:
- remove unnecessary strbuf_reset
- remove test from v7 which has shown that tags get fetched when cloning
- add test to show that tags will not being updated when using simple "git
clone"
- change position of the tag in the setup, not directly in the test
Documentation/git-clone.txt | 15 +++--
builtin/clone.c | 65 ++
t/t5709-clone-refspec.sh| 156
3 Dateien geändert, 218 Zeilen hinzugefügt(+), 18 Zeilen entfernt(-)
create mode 100755 t/t5709-clone-refspec.sh
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index c1ddd4c..6d98ef3 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -29,7 +29,8 @@ currently active branch.
After the clone, a plain `git fetch` without arguments will update
all the remote-tracking branches, and a `git pull` without
arguments will in addition merge the remote master branch into the
-current master branch, if any.
+current master branch, if any (this is untrue when "--single-branch"
+is given; see below).
This default configuration is achieved by creating references to
the remote branch heads under `refs/remotes/origin` and
@@ -152,9 +153,10 @@ objects from the source repository into a pack in the
cloned repository.
-b ::
Instead of pointing the newly created HEAD to the branch pointed
to by the cloned repository's HEAD, point to `` branch
- instead. `--branch` can also take tags and treat them like
- detached HEAD. In a non-bare repository, this is the branch
- that will be checked out.
+ instead. In a non-bare repository, this is the branch that will
+ be checked out.
+ `--branch` can also take tags and detaches the HEAD at that commit
+ in the resulting repository.
--upload-pack ::
-u ::
@@ -193,6 +195,11 @@ objects from the source repository into a pack in the
cloned repository.
clone with the `--depth` option, this is the default, unless
`--no-single-branch` is given to fetch the histories near the
tips of all branches.
+ Further fetches into the resulting repository will only update the
+ remote tracking branch for the branch this option was used for the
+ initial cloning. If the HEAD at the remote did not point at any
+ branch when `--single-branch` clone was made, no remote tracking
+ branch is created.
--recursive::
--recurse-submodules::
diff --git a/builtin/clone.c b/builtin/clone.c
index 5e8f3ba..047239c 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -610,6 +610,54 @@ static void write_config(struct string_list *config)
}
}
+static void write_refspec_config(const char* src_ref_prefix,
+ const struct ref* our_head_points_at,
+ const struct ref* remote_head_points_at, struct strbuf*
branch_top)
+{
+ struct strbuf key = STRBUF_INIT;
+ struct strbuf value = STRBUF_INIT;
+
+ if (option_mirror || !option_bare) {
+ if (option_single_branch && !option_mirror) {
+ if (option_branch) {
+ if (strstr(our_head_points_at->name,
"refs/tags/"))
+ strbuf_addf(&value, "+%s:%s",
our_head_points_at->name,
+ our_head_points_at->name);
+ else
+ strbuf_addf(&value, "+%s:%s%s",
our_head_points_at->name,
+ branch_top->buf, option_branch);
+ } else if (remote_head_points_at) {
+ strbuf_addf(&value, "+%s:%s%s",
remote_head_points_at->name,
+ branch_top->buf,
+
skip_prefix(remote_head_points_at->name, "refs/heads/"));
+ }
+ /*
+* otherwise, the next "git fetch" will
+* simply fetch from HEAD without updating
+* any remote tracking branch, which is w
Re: [PATCH 2/2] remote-curl: let users turn off smart http
On Thu, Sep 20, 2012 at 10:53:15AM -0700, Junio C Hamano wrote: > Jeff King writes: > > > I added the config item as remote.foo.smarthttp. You could also allow > > "http.$url.smart" (and just "http.smart", for that matter), which could > > be more flexible if you have multiple remotes pointing to the same > > broken server. > > What would the user experience be when we introduce "even smarter" > http server protocol extension? Will we add remote.foo.starterhttp? I would hope that it would actually be negotiated reliably at the protocol level so we do not have to deal with this mess again. > Perhaps > > remote.$name.httpvariants = [smart] [dumb] > > to allow users to say "smart only", "dumb only", or "smart and/or > dumb" might be more code but less burden on the users. I don't mind that format if we are going that direction, but is there anybody who actually wants to say "smart only?" -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: fix shell expansion of items
Hi,
On Wed, Sep 19, 2012 at 09:46:08PM -0400, Jeff King wrote:
> On Thu, Sep 20, 2012 at 04:15:15AM +0200, Felipe Contreras wrote:
>
> > As reported by Jeroen Meijer[1]; the current code doesn't deal properly
> > with items (tags, branches, etc.) that have ${} in them because they get
> > expaned by bash while using compgen.
> >
> > A simple solution is to quote the items so they get expanded properly
> > (\$\{\}).
> >
> > In order to achieve that I took bash-completion's quote() function,
> > which is rather simple, and renamed it to __git_quote() as per Jeff
> > King's suggestion.
> >
> > Solves the original problem for me.
>
> Me too. Thanks.
While it solves the original problem, it seems to break refs
completion, as demonstrated by the following POC test:
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 92d7eb47..fab63b95 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -228,4 +228,11 @@ test_expect_success 'general options plus command' '
test_completion "git --no-replace-objects check" "checkout "
'
+test_expect_success 'basic refs completion' '
+ touch file &&
+ git add file &&
+ git commit -m initial &&
+ test_completion "git branch m" "master "
+'
+
test_done
--
1.7.12.1.438.g7dfa67b
which fails with:
--- expected2012-09-20 18:05:23.857752925 +
+++ out 2012-09-20 18:05:23.877752925 +
@@ -1 +1 @@
-master
+
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: fix shell expansion of items
On Thu, Sep 20, 2012 at 08:11:52PM +0200, SZEDER Gábor wrote: > > > In order to achieve that I took bash-completion's quote() function, > > > which is rather simple, and renamed it to __git_quote() as per Jeff > > > King's suggestion. > > > > > > Solves the original problem for me. > > > > Me too. Thanks. > > While it solves the original problem, it seems to break refs > completion, as demonstrated by the following POC test: > > diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh > index 92d7eb47..fab63b95 100755 > --- a/t/t9902-completion.sh > +++ b/t/t9902-completion.sh > @@ -228,4 +228,11 @@ test_expect_success 'general options plus command' ' > test_completion "git --no-replace-objects check" "checkout " > ' > > +test_expect_success 'basic refs completion' ' > + touch file && > + git add file && > + git commit -m initial && > + test_completion "git branch m" "master " > +' Hmm. I notice that Felipe's patch wraps the _whole_ input to __gitcomp_nl in single quotes. So if there are multiple completions we would end up with: 'one two quo\'ted three' I wonder if that is OK to feed to compgen -W, or if it wants to expand it line-by-line. Just guessing at this point, though. -Peff > + > test_done > -- > 1.7.12.1.438.g7dfa67b > > > which fails with: > > --- expected2012-09-20 18:05:23.857752925 + > +++ out 2012-09-20 18:05:23.877752925 + > @@ -1 +1 @@ > -master > + > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] remote-curl: let users turn off smart http
Jeff King writes: > On Thu, Sep 20, 2012 at 10:53:15AM -0700, Junio C Hamano wrote: > >> Jeff King writes: >> >> > I added the config item as remote.foo.smarthttp. You could also allow >> > "http.$url.smart" (and just "http.smart", for that matter), which could >> > be more flexible if you have multiple remotes pointing to the same >> > broken server. >> >> What would the user experience be when we introduce "even smarter" >> http server protocol extension? Will we add remote.foo.starterhttp? > > I would hope that it would actually be negotiated reliably at the > protocol level so we do not have to deal with this mess again. The original dumb vs smart was supposed to be "negotiated reliably at the protocol level", no? Yet we need this band-aid, so... >> Perhaps >> >> remote.$name.httpvariants = [smart] [dumb] >> >> to allow users to say "smart only", "dumb only", or "smart and/or >> dumb" might be more code but less burden on the users. > > I don't mind that format if we are going that direction, but is there > anybody who actually wants to say "smart only?" With 703e6e7 reverted, we take a failure from the initial smart request to mean the server is simply not serving, so "smart only" to fail quickly without trying dumb fallback is not needed. "smart only" to say "I wouldn't want to talk to dumb-only server---I do not have infinite amount of time, and I'd rather try another server" is still a possibility, but likely not worth supporting. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: The GitTogether
On 19.09.2012 15:43, Michael Haggerty wrote: Is there any news about the proposed gatherings? I would be quite interested in attending the developer meeting. October is just around the corner...what's up? I'm also very much interested in attending a gathering Berlin, though preferably not in the first week of October. As I'm a local, I could probably also help with finding a location if necessary. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: fix shell expansion of items
On Thu, Sep 20, 2012 at 8:21 PM, Jeff King wrote: > On Thu, Sep 20, 2012 at 08:11:52PM +0200, SZEDER Gábor wrote: > >> > > In order to achieve that I took bash-completion's quote() function, >> > > which is rather simple, and renamed it to __git_quote() as per Jeff >> > > King's suggestion. >> > > >> > > Solves the original problem for me. >> > >> > Me too. Thanks. >> >> While it solves the original problem, it seems to break refs >> completion, as demonstrated by the following POC test: >> >> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh >> index 92d7eb47..fab63b95 100755 >> --- a/t/t9902-completion.sh >> +++ b/t/t9902-completion.sh >> @@ -228,4 +228,11 @@ test_expect_success 'general options plus command' ' >> test_completion "git --no-replace-objects check" "checkout " >> ' >> >> +test_expect_success 'basic refs completion' ' >> + touch file && >> + git add file && >> + git commit -m initial && >> + test_completion "git branch m" "master " >> +' > > Hmm. I notice that Felipe's patch wraps the _whole_ input to > __gitcomp_nl in single quotes. Wasn't there a patch series that added tests for __gitcomp_nl to catch these issues? -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 01/14] Update directory listing API doc to match code
7c4c97c0ac turned the flags in struct dir_struct into a single bitfield variable, but forgot to update this document. Signed-off-by: Adam Spiers --- Documentation/technical/api-directory-listing.txt | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt index add6f43..0356d25 100644 --- a/Documentation/technical/api-directory-listing.txt +++ b/Documentation/technical/api-directory-listing.txt @@ -17,24 +17,24 @@ options are: The name of the file to be read in each directory for excluded files (typically `.gitignore`). -`collect_ignored`:: +`flags`:: - Include paths that are to be excluded in the result. + A bit-field of options: -`show_ignored`:: +`DIR_SHOW_IGNORED`::: The traversal is for finding just ignored files, not unignored files. -`show_other_directories`:: +`DIR_SHOW_OTHER_DIRECTORIES`::: Include a directory that is not tracked. -`hide_empty_directories`:: +`DIR_HIDE_EMPTY_DIRECTORIES`::: Do not include a directory that is not tracked and is empty. -`no_gitlinks`:: +`DIR_NO_GITLINKS`::: If set, recurse into a directory that looks like a git directory. Otherwise it is shown as a directory. -- 1.7.12.147.g6d168f4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/14] new git check-ignore sub-command
This is a re-vamp of my original check-ignore series, which aims to address all the feedback which was raised in the first round of reviews. The most notable changes are the CLI options and output formats as suggested by Junio and Nguyễn; now there are three levels of verbosity: --quiet, default, and --verbose. -z also now affects the output and so is now compatible with the --stdin optin. Some commits have been broken into smaller pieces to facilitate easier reviews, and based on an earlier discussion, three exclude functions have been given an 'is_' prefix to clarify their boolean nature. The helper functions extracted from these three now have more meaningful names rather than just a '_1' suffix. Other minor issues, such as inconsistent coding style, have been fixed, and the modification to the output text in add.c has been scrapped. It has been rebased on the latest master, and passed a full test run. Adam Spiers (14): Update directory listing API doc to match code Improve documentation and comments regarding directory traversal API Rename cryptic 'which' variable to more consistent name Rename path_excluded() to is_path_excluded() Rename excluded_from_list() to is_excluded_from_list() Rename excluded() to is_excluded() Refactor is_excluded_from_list() Refactor is_excluded() Refactor is_path_excluded() For each exclude pattern, store information about where it came from Refactor treat_gitlinks() Extract some useful pathspec handling code from builtin/add.c into a library Provide free_directory() for reclaiming dir_struct memory Add git-check-ignore sub-command .gitignore| 1 + Documentation/git-check-ignore.txt| 85 Documentation/gitignore.txt | 6 +- Documentation/technical/api-directory-listing.txt | 23 +- Makefile | 3 + attr.c| 2 +- builtin.h | 1 + builtin/add.c | 84 +--- builtin/check-ignore.c| 167 ++ builtin/clean.c | 2 +- builtin/ls-files.c| 5 +- command-list.txt | 1 + contrib/completion/git-completion.bash| 1 + dir.c | 191 +-- dir.h | 47 +- git.c | 1 + pathspec.c| 97 pathspec.h| 6 + t/t0007-ignores.sh| 587 ++ t/t9902-completion.sh | 24 +- unpack-trees.c| 10 +- 21 files changed, 1182 insertions(+), 162 deletions(-) create mode 100644 Documentation/git-check-ignore.txt create mode 100644 builtin/check-ignore.c create mode 100644 pathspec.c create mode 100644 pathspec.h create mode 100755 t/t0007-ignores.sh -- 1.7.12.147.g6d168f4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 05/14] Rename excluded_from_list() to is_excluded_from_list()
Continue adopting clearer names for exclude functions. This 'is_*'
naming pattern for functions returning booleans was discussed here:
http://thread.gmane.org/gmane.comp.version-control.git/204661/focus=204924
Signed-off-by: Adam Spiers
---
dir.c | 11 ++-
dir.h | 4 ++--
unpack-trees.c | 8 +---
3 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/dir.c b/dir.c
index dad1582..8f262f6 100644
--- a/dir.c
+++ b/dir.c
@@ -514,9 +514,9 @@ static void prep_exclude(struct dir_struct *dir, const char
*base, int baselen)
/* Scan the list and let the last match determine the fate.
* Return 1 for exclude, 0 for include and -1 for undecided.
*/
-int excluded_from_list(const char *pathname,
- int pathlen, const char *basename, int *dtype,
- struct exclude_list *el)
+int is_excluded_from_list(const char *pathname,
+ int pathlen, const char *basename, int *dtype,
+ struct exclude_list *el)
{
int i;
@@ -596,8 +596,9 @@ static int excluded(struct dir_struct *dir, const char
*pathname, int *dtype_p)
prep_exclude(dir, pathname, basename-pathname);
for (st = EXC_CMDL; st <= EXC_FILE; st++) {
- switch (excluded_from_list(pathname, pathlen, basename,
- dtype_p, &dir->exclude_list[st])) {
+ switch (is_excluded_from_list(pathname, pathlen,
+ basename, dtype_p,
+ &dir->exclude_list[st])) {
case 0:
return 0;
case 1:
diff --git a/dir.h b/dir.h
index 41a5e32..6904a35 100644
--- a/dir.h
+++ b/dir.h
@@ -98,8 +98,8 @@ extern int within_depth(const char *name, int namelen, int
depth, int max_depth)
extern int fill_directory(struct dir_struct *dir, const char **pathspec);
extern int read_directory(struct dir_struct *, const char *path, int len,
const char **pathspec);
-extern int excluded_from_list(const char *pathname, int pathlen, const char
*basename,
- int *dtype, struct exclude_list *el);
+extern int is_excluded_from_list(const char *pathname, int pathlen, const char
*basename,
+int *dtype, struct exclude_list *el);
struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char
*pathname, int len);
/*
diff --git a/unpack-trees.c b/unpack-trees.c
index 724f69b..56127c9 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -837,7 +837,8 @@ static int clear_ce_flags_dir(struct cache_entry **cache,
int nr,
{
struct cache_entry **cache_end;
int dtype = DT_DIR;
- int ret = excluded_from_list(prefix, prefix_len, basename, &dtype, el);
+ int ret = is_excluded_from_list(prefix, prefix_len,
+ basename, &dtype, el);
prefix[prefix_len++] = '/';
@@ -856,7 +857,7 @@ static int clear_ce_flags_dir(struct cache_entry **cache,
int nr,
* with ret (iow, we know in advance the incl/excl
* decision for the entire directory), clear flag here without
* calling clear_ce_flags_1(). That function will call
-* the expensive excluded_from_list() on every entry.
+* the expensive is_excluded_from_list() on every entry.
*/
return clear_ce_flags_1(cache, cache_end - cache,
prefix, prefix_len,
@@ -939,7 +940,8 @@ static int clear_ce_flags_1(struct cache_entry **cache, int
nr,
/* Non-directory */
dtype = ce_to_dtype(ce);
- ret = excluded_from_list(ce->name, ce_namelen(ce), name,
&dtype, el);
+ ret = is_excluded_from_list(ce->name, ce_namelen(ce),
+ name, &dtype, el);
if (ret < 0)
ret = defval;
if (ret > 0)
--
1.7.12.147.g6d168f4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/14] Rename excluded() to is_excluded()
Continue adopting clearer names for exclude functions. This is_*
naming pattern for functions returning booleans was discussed here:
http://thread.gmane.org/gmane.comp.version-control.git/204661/focus=204924
Signed-off-by: Adam Spiers
---
attr.c | 2 +-
dir.c | 10 +-
dir.h | 4 ++--
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/attr.c b/attr.c
index 887a9ae..fd33ff9 100644
--- a/attr.c
+++ b/attr.c
@@ -270,7 +270,7 @@ static struct match_attr *parse_attr_line(const char *line,
const char *src,
* (reading the file from top to bottom), .gitattribute of the root
* directory (again, reading the file from top to bottom) down to the
* current directory, and then scan the list backwards to find the first match.
- * This is exactly the same as what excluded() does in dir.c to deal with
+ * This is exactly the same as what is_excluded() does in dir.c to deal with
* .gitignore
*/
diff --git a/dir.c b/dir.c
index 8f262f6..b381b1b 100644
--- a/dir.c
+++ b/dir.c
@@ -587,7 +587,7 @@ int is_excluded_from_list(const char *pathname,
return -1; /* undecided */
}
-static int excluded(struct dir_struct *dir, const char *pathname, int *dtype_p)
+static int is_excluded(struct dir_struct *dir, const char *pathname, int
*dtype_p)
{
int pathlen = strlen(pathname);
int st;
@@ -637,7 +637,7 @@ int is_path_excluded(struct path_exclude_check *check,
/*
* we allow the caller to pass namelen as an optimization; it
* must match the length of the name, as we eventually call
-* excluded() on the whole name string.
+* is_excluded() on the whole name string.
*/
if (namelen < 0)
namelen = strlen(name);
@@ -654,7 +654,7 @@ int is_path_excluded(struct path_exclude_check *check,
if (ch == '/') {
int dt = DT_DIR;
- if (excluded(check->dir, path->buf, &dt))
+ if (is_excluded(check->dir, path->buf, &dt))
return 1;
}
strbuf_addch(path, ch);
@@ -663,7 +663,7 @@ int is_path_excluded(struct path_exclude_check *check,
/* An entry in the index; cannot be a directory with subentries */
strbuf_setlen(path, 0);
- return excluded(check->dir, name, dtype);
+ return is_excluded(check->dir, name, dtype);
}
static struct dir_entry *dir_entry_new(const char *pathname, int len)
@@ -963,7 +963,7 @@ static enum path_treatment treat_one_path(struct dir_struct
*dir,
const struct path_simplify *simplify,
int dtype, struct dirent *de)
{
- int exclude = excluded(dir, path->buf, &dtype);
+ int exclude = is_excluded(dir, path->buf, &dtype);
if (exclude && (dir->flags & DIR_COLLECT_IGNORED)
&& exclude_matches_pathspec(path->buf, path->len, simplify))
dir_add_ignored(dir, path->buf, path->len);
diff --git a/dir.h b/dir.h
index 6904a35..009c9df 100644
--- a/dir.h
+++ b/dir.h
@@ -103,8 +103,8 @@ extern int is_excluded_from_list(const char *pathname, int
pathlen, const char *
struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char
*pathname, int len);
/*
- * The excluded() API is meant for callers that check each level of leading
- * directory hierarchies with excluded() to avoid recursing into excluded
+ * The is_excluded() API is meant for callers that check each level of leading
+ * directory hierarchies with is_excluded() to avoid recursing into excluded
* directories. Callers that do not do so should use this API instead.
*/
struct path_exclude_check {
--
1.7.12.147.g6d168f4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 07/14] Refactor is_excluded_from_list()
The excluded function uses a new helper function called
last_exclude_matching_from_list() to perform the inner loop over all of
the exclude patterns. The helper just tells us whether the path is
included, excluded, or undecided.
However, it may be useful to know _which_ pattern was triggered. So
let's pass out the entire exclude match, which contains the status
information we were already passing out.
Further patches can make use of this.
This is a modified forward port of a patch from 2009 by Jeff King:
http://article.gmane.org/gmane.comp.version-control.git/108815
Signed-off-by: Adam Spiers
---
dir.c | 41 ++---
1 file changed, 30 insertions(+), 11 deletions(-)
diff --git a/dir.c b/dir.c
index b381b1b..70b7d7e 100644
--- a/dir.c
+++ b/dir.c
@@ -511,22 +511,26 @@ static void prep_exclude(struct dir_struct *dir, const
char *base, int baselen)
dir->basebuf[baselen] = '\0';
}
-/* Scan the list and let the last match determine the fate.
- * Return 1 for exclude, 0 for include and -1 for undecided.
+/*
+ * Scan the given exclude list in reverse to see whether pathname
+ * should be ignored. The first match (i.e. the last on the list), if
+ * any, determines the fate. Returns the exclude_list element which
+ * matched, or NULL for undecided.
*/
-int is_excluded_from_list(const char *pathname,
- int pathlen, const char *basename, int *dtype,
- struct exclude_list *el)
+static struct exclude *last_exclude_matching_from_list(const char *pathname,
+ int pathlen,
+ const char *basename,
+ int *dtype,
+ struct exclude_list *el)
{
int i;
if (!el->nr)
- return -1; /* undefined */
+ return NULL;/* undefined */
for (i = el->nr - 1; 0 <= i; i--) {
struct exclude *x = el->excludes[i];
const char *name, *exclude = x->pattern;
- int to_exclude = x->to_exclude;
int namelen, prefix = x->nowildcardlen;
if (x->flags & EXC_FLAG_MUSTBEDIR) {
@@ -540,14 +544,14 @@ int is_excluded_from_list(const char *pathname,
/* match basename */
if (prefix == x->patternlen) {
if (!strcmp_icase(exclude, basename))
- return to_exclude;
+ return x;
} else if (x->flags & EXC_FLAG_ENDSWITH) {
if (x->patternlen - 1 <= pathlen &&
!strcmp_icase(exclude + 1, pathname +
pathlen - x->patternlen + 1))
- return to_exclude;
+ return x;
} else {
if (fnmatch_icase(exclude, basename, 0) == 0)
- return to_exclude;
+ return x;
}
continue;
}
@@ -582,8 +586,23 @@ int is_excluded_from_list(const char *pathname,
}
if (!namelen || !fnmatch_icase(exclude, name, FNM_PATHNAME))
- return to_exclude;
+ return x;
}
+ return NULL; /* undecided */
+}
+
+/*
+ * Scan the list and let the last match determine the fate.
+ * Return 1 for exclude, 0 for include and -1 for undecided.
+ */
+int is_excluded_from_list(const char *pathname,
+ int pathlen, const char *basename, int *dtype,
+ struct exclude_list *el)
+{
+ struct exclude *exclude;
+ exclude = last_exclude_matching_from_list(pathname, pathlen, basename,
dtype, el);
+ if (exclude)
+ return exclude->to_exclude;
return -1; /* undecided */
}
--
1.7.12.147.g6d168f4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 09/14] Refactor is_path_excluded()
In a similar way to the previous commit, this extracts a new helper
function last_exclude_matching_path() which return the last
exclude_list element which matched, or NULL if no match was found.
is_path_excluded() becomes a wrapper around this, and just returns 0
or 1 depending on whether any matching exclude_list element was found.
This allows callers to find out _why_ a given path was excluded,
rather than just whether it was or not, paving the way for a new git
sub-command which allows users to test their exclude lists from the
command line.
Signed-off-by: Adam Spiers
---
dir.c | 47 ++-
dir.h | 3 +++
2 files changed, 41 insertions(+), 9 deletions(-)
diff --git a/dir.c b/dir.c
index 3e2161e..904a7f3 100644
--- a/dir.c
+++ b/dir.c
@@ -651,6 +651,7 @@ void path_exclude_check_init(struct path_exclude_check
*check,
struct dir_struct *dir)
{
check->dir = dir;
+ check->exclude = NULL;
strbuf_init(&check->path, 256);
}
@@ -660,18 +661,21 @@ void path_exclude_check_clear(struct path_exclude_check
*check)
}
/*
- * Is this name excluded? This is for a caller like show_files() that
- * do not honor directory hierarchy and iterate through paths that are
- * possibly in an ignored directory.
+ * For each subdirectory in name, starting with the top-most, checks
+ * to see if that subdirectory is excluded, and if so, returns the
+ * corresponding exclude structure. Otherwise, checks whether name
+ * itself (which is presumably a file) is excluded.
*
* A path to a directory known to be excluded is left in check->path to
* optimize for repeated checks for files in the same excluded directory.
*/
-int is_path_excluded(struct path_exclude_check *check,
-const char *name, int namelen, int *dtype)
+struct exclude *last_exclude_matching_path(struct path_exclude_check *check,
+ const char *name, int namelen,
+ int *dtype)
{
int i;
struct strbuf *path = &check->path;
+ struct exclude *exclude;
/*
* we allow the caller to pass namelen as an optimization; it
@@ -681,11 +685,17 @@ int is_path_excluded(struct path_exclude_check *check,
if (namelen < 0)
namelen = strlen(name);
+ /*
+* If path is non-empty, and name is equal to path or a
+* subdirectory of path, name should be excluded, because
+* it's inside a directory which is already known to be
+* excluded and was previously left in check->path.
+*/
if (path->len &&
path->len <= namelen &&
!memcmp(name, path->buf, path->len) &&
(!name[path->len] || name[path->len] == '/'))
- return 1;
+ return check->exclude;
strbuf_setlen(path, 0);
for (i = 0; name[i]; i++) {
@@ -693,8 +703,12 @@ int is_path_excluded(struct path_exclude_check *check,
if (ch == '/') {
int dt = DT_DIR;
- if (is_excluded(check->dir, path->buf, &dt))
- return 1;
+ exclude = last_exclude_matching(check->dir,
+ path->buf, &dt);
+ if (exclude) {
+ check->exclude = exclude;
+ return exclude;
+ }
}
strbuf_addch(path, ch);
}
@@ -702,7 +716,22 @@ int is_path_excluded(struct path_exclude_check *check,
/* An entry in the index; cannot be a directory with subentries */
strbuf_setlen(path, 0);
- return is_excluded(check->dir, name, dtype);
+ return last_exclude_matching(check->dir, name, dtype);
+}
+
+/*
+ * Is this name excluded? This is for a caller like show_files() that
+ * do not honor directory hierarchy and iterate through paths that are
+ * possibly in an ignored directory.
+ */
+int is_path_excluded(struct path_exclude_check *check,
+ const char *name, int namelen, int *dtype)
+{
+ struct exclude *exclude =
+ last_exclude_matching_path(check, name, namelen, dtype);
+ if (exclude)
+ return exclude->to_exclude;
+ return 0;
}
static struct dir_entry *dir_entry_new(const char *pathname, int len)
diff --git a/dir.h b/dir.h
index 009c9df..19beddb 100644
--- a/dir.h
+++ b/dir.h
@@ -109,10 +109,13 @@ struct dir_entry *dir_add_ignored(struct dir_struct *dir,
const char *pathname,
*/
struct path_exclude_check {
struct dir_struct *dir;
+ struct exclude *exclude;
struct strbuf path;
};
extern void path_exclude_check_init(struct path_exclude_check *, struct
dir_struct *);
extern void path_exclude_check_clear(struct path_exclude_check *);
+extern struct exclude *last_exclude_ma
[PATCH v2 08/14] Refactor is_excluded()
In a similar way to the previous commit, this extracts a new helper
function last_exclude_matching() which returns the last exclude_list
element which matched, or NULL if no match was found. is_excluded()
becomes a wrapper around this, and just returns 0 or 1 depending on
whether any matching exclude_list element was found.
This allows callers to find out _why_ a given path was excluded,
rather than just whether it was or not, paving the way for a new git
sub-command which allows users to test their exclude lists from the
command line.
Signed-off-by: Adam Spiers
---
dir.c | 38 +-
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/dir.c b/dir.c
index 70b7d7e..3e2161e 100644
--- a/dir.c
+++ b/dir.c
@@ -606,24 +606,44 @@ int is_excluded_from_list(const char *pathname,
return -1; /* undecided */
}
-static int is_excluded(struct dir_struct *dir, const char *pathname, int
*dtype_p)
+/*
+ * Loads the exclude lists for the directory containing pathname, then
+ * scans all exclude lists to determine whether pathname is excluded.
+ * Returns the exclude_list element which matched, or NULL for
+ * undecided.
+ */
+static struct exclude *last_exclude_matching(struct dir_struct *dir,
+const char *pathname,
+int *dtype_p)
{
int pathlen = strlen(pathname);
int st;
+ struct exclude *exclude;
const char *basename = strrchr(pathname, '/');
basename = (basename) ? basename+1 : pathname;
prep_exclude(dir, pathname, basename-pathname);
for (st = EXC_CMDL; st <= EXC_FILE; st++) {
- switch (is_excluded_from_list(pathname, pathlen,
- basename, dtype_p,
- &dir->exclude_list[st])) {
- case 0:
- return 0;
- case 1:
- return 1;
- }
+ exclude = last_exclude_matching_from_list(
+ pathname, pathlen, basename, dtype_p,
+ &dir->exclude_list[st]);
+ if (exclude)
+ return exclude;
}
+ return NULL;
+}
+
+/*
+ * Loads the exclude lists for the directory containing pathname, then
+ * scans all exclude lists to determine whether pathname is excluded.
+ * Returns 1 if true, otherwise 0.
+ */
+static int is_excluded(struct dir_struct *dir, const char *pathname, int
*dtype_p)
+{
+ struct exclude *exclude =
+ last_exclude_matching(dir, pathname, dtype_p);
+ if (exclude)
+ return exclude->to_exclude;
return 0;
}
--
1.7.12.147.g6d168f4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 10/14] For each exclude pattern, store information about where it came from
For exclude patterns read in from files, the filename is stored together
with the corresponding line number (counting starting at 1).
For exclude patterns provided on the command line, the sequence number
is negative, with counting starting at -1, so for example the 2nd
pattern provided via --exclude would be numbered -2. This allows any
future consumers of that data to easily distinguish between exclude
patterns from files vs. from the CLI.
Signed-off-by: Adam Spiers
---
builtin/clean.c| 2 +-
builtin/ls-files.c | 3 ++-
dir.c | 25 +++--
dir.h | 5 -
4 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 69c1cda..893dd7a 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -99,7 +99,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
for (i = 0; i < exclude_list.nr; i++)
add_exclude(exclude_list.items[i].string, "", 0,
- &dir.exclude_list[EXC_CMDL]);
+ &dir.exclude_list[EXC_CMDL], "--exclude option",
-(i+1));
pathspec = get_pathspec(prefix, argv);
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index d226456..64b1969 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -35,6 +35,7 @@ static int error_unmatch;
static char *ps_matched;
static const char *with_tree;
static int exc_given;
+static int exclude_args;
static const char *tag_cached = "";
static const char *tag_unmerged = "";
@@ -423,7 +424,7 @@ static int option_parse_exclude(const struct option *opt,
struct exclude_list *list = opt->value;
exc_given = 1;
- add_exclude(arg, "", 0, list);
+ add_exclude(arg, "", 0, list, "--exclude option", --exclude_args);
return 0;
}
diff --git a/dir.c b/dir.c
index 904a7f3..705546f 100644
--- a/dir.c
+++ b/dir.c
@@ -311,7 +311,7 @@ static int no_wildcard(const char *string)
}
void add_exclude(const char *string, const char *base,
-int baselen, struct exclude_list *el)
+int baselen, struct exclude_list *el, const char *src, int
srcpos)
{
struct exclude *x;
size_t len;
@@ -341,6 +341,8 @@ void add_exclude(const char *string, const char *base,
x->base = base;
x->baselen = baselen;
x->flags = flags;
+ x->src = src;
+ x->srcpos = srcpos;
if (!strchr(string, '/'))
x->flags |= EXC_FLAG_NODIR;
x->nowildcardlen = simple_length(string);
@@ -393,7 +395,7 @@ int add_excludes_from_file_to_list(const char *fname,
int check_index)
{
struct stat st;
- int fd, i;
+ int fd, i, lineno = 1;
size_t size = 0;
char *buf, *entry;
@@ -438,8 +440,9 @@ int add_excludes_from_file_to_list(const char *fname,
if (buf[i] == '\n') {
if (entry != buf + i && entry[0] != '#') {
buf[i - (i && buf[i-1] == '\r')] = 0;
- add_exclude(entry, base, baselen, el);
+ add_exclude(entry, base, baselen, el, fname,
lineno);
}
+ lineno++;
entry = buf + i + 1;
}
}
@@ -474,8 +477,10 @@ static void prep_exclude(struct dir_struct *dir, const
char *base, int baselen)
!strncmp(dir->basebuf, base, stk->baselen))
break;
dir->exclude_stack = stk->prev;
- while (stk->exclude_ix < el->nr)
- free(el->excludes[--el->nr]);
+ while (stk->exclude_ix < el->nr) {
+ struct exclude *exclude = el->excludes[--el->nr];
+ free(exclude);
+ }
free(stk->filebuf);
free(stk);
}
@@ -502,7 +507,15 @@ static void prep_exclude(struct dir_struct *dir, const
char *base, int baselen)
memcpy(dir->basebuf + current, base + current,
stk->baselen - current);
strcpy(dir->basebuf + stk->baselen, dir->exclude_per_dir);
- add_excludes_from_file_to_list(dir->basebuf,
+
+ /* dir->basebuf gets reused by the traversal, but we
+* need fname to remain unchanged to ensure the src
+* member of each struct exclude correctly back-references
+* its source file.
+*/
+ char *fname = strdup(dir->basebuf);
+
+ add_excludes_from_file_to_list(fname,
dir->basebuf, stk->baselen,
&stk->filebuf, el, 1);
dir->exclude_stack = stk;
diff --git a/dir.h b/dir.h
index 19beddb..ebb0367 100644
--- a/dir.h
+++ b/dir.h
@@ -31,6 +31,9 @@ struct exclude_list {
[PATCH v2 11/14] Refactor treat_gitlinks()
Extract the body of the for loop in treat_gitlinks() into a separate
treat_gitlink() function so that it can be reused elsewhere. This
paves the way for a new check-ignore sub-command.
Signed-off-by: Adam Spiers
---
builtin/add.c | 49 +++--
1 file changed, 31 insertions(+), 18 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 075312a..b4ec5cd 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -153,31 +153,44 @@ static char *prune_directory(struct dir_struct *dir,
const char **pathspec, int
return seen;
}
-static void treat_gitlinks(const char **pathspec)
+/*
+ * Check whether path refers to a submodule, or something inside a
+ * submodule. If the former, returns the path with any trailing slash
+ * stripped. If the latter, dies with an error message.
+ */
+const char *treat_gitlink(const char *path)
{
- int i;
-
- if (!pathspec || !*pathspec)
- return;
-
+ int i, path_len = strlen(path);
for (i = 0; i < active_nr; i++) {
struct cache_entry *ce = active_cache[i];
if (S_ISGITLINK(ce->ce_mode)) {
- int len = ce_namelen(ce), j;
- for (j = 0; pathspec[j]; j++) {
- int len2 = strlen(pathspec[j]);
- if (len2 <= len || pathspec[j][len] != '/' ||
- memcmp(ce->name, pathspec[j], len))
- continue;
- if (len2 == len + 1)
- /* strip trailing slash */
- pathspec[j] = xstrndup(ce->name, len);
- else
- die (_("Path '%s' is in submodule
'%.*s'"),
- pathspec[j], len, ce->name);
+ int ce_len = ce_namelen(ce);
+ if (path_len <= ce_len || path[ce_len] != '/' ||
+ memcmp(ce->name, path, ce_len))
+ /* path does not refer to this
+* submodule or anything inside it */
+ continue;
+ if (path_len == ce_len + 1) {
+ /* path refers to submodule;
+* strip trailing slash */
+ return xstrndup(ce->name, ce_len);
+ } else {
+ die (_("Path '%s' is in submodule '%.*s'"),
+path, ce_len, ce->name);
}
}
}
+ return path;
+}
+
+void treat_gitlinks(const char **pathspec)
+{
+ if (!pathspec || !*pathspec)
+ return;
+
+ int i;
+ for (i = 0; pathspec[i]; i++)
+ pathspec[i] = treat_gitlink(pathspec[i]);
}
static void refresh(int verbose, const char **pathspec)
--
1.7.12.147.g6d168f4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 12/14] Extract some useful pathspec handling code from builtin/add.c into a library
This is in preparation for reuse by a new git check-ignore command.
Signed-off-by: Adam Spiers
---
Makefile | 2 ++
builtin/add.c | 95 ++---
pathspec.c| 97 +++
pathspec.h| 6
4 files changed, 108 insertions(+), 92 deletions(-)
create mode 100644 pathspec.c
create mode 100644 pathspec.h
diff --git a/Makefile b/Makefile
index a49d1db..9a4bf9e 100644
--- a/Makefile
+++ b/Makefile
@@ -649,6 +649,7 @@ LIB_H += pack-revindex.h
LIB_H += pack.h
LIB_H += parse-options.h
LIB_H += patch-ids.h
+LIB_H += pathspec.h
LIB_H += pkt-line.h
LIB_H += progress.h
LIB_H += prompt.h
@@ -769,6 +770,7 @@ LIB_OBJS += parse-options-cb.o
LIB_OBJS += patch-delta.o
LIB_OBJS += patch-ids.o
LIB_OBJS += path.o
+LIB_OBJS += pathspec.o
LIB_OBJS += pkt-line.o
LIB_OBJS += preload-index.o
LIB_OBJS += pretty.o
diff --git a/builtin/add.c b/builtin/add.c
index b4ec5cd..c3def9c 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -6,6 +6,7 @@
#include "cache.h"
#include "builtin.h"
#include "dir.h"
+#include "pathspec.h"
#include "exec_cmd.h"
#include "cache-tree.h"
#include "run-command.h"
@@ -97,39 +98,6 @@ int add_files_to_cache(const char *prefix, const char
**pathspec, int flags)
return !!data.add_errors;
}
-static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
-{
- int num_unmatched = 0, i;
-
- /*
-* Since we are walking the index as if we were walking the directory,
-* we have to mark the matched pathspec as seen; otherwise we will
-* mistakenly think that the user gave a pathspec that did not match
-* anything.
-*/
- for (i = 0; i < specs; i++)
- if (!seen[i])
- num_unmatched++;
- if (!num_unmatched)
- return;
- for (i = 0; i < active_nr; i++) {
- struct cache_entry *ce = active_cache[i];
- match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen);
- }
-}
-
-static char *find_used_pathspec(const char **pathspec)
-{
- char *seen;
- int i;
-
- for (i = 0; pathspec[i]; i++)
- ; /* just counting */
- seen = xcalloc(i, 1);
- fill_pathspec_matches(pathspec, seen, i);
- return seen;
-}
-
static char *prune_directory(struct dir_struct *dir, const char **pathspec,
int prefix)
{
char *seen;
@@ -153,46 +121,6 @@ static char *prune_directory(struct dir_struct *dir, const
char **pathspec, int
return seen;
}
-/*
- * Check whether path refers to a submodule, or something inside a
- * submodule. If the former, returns the path with any trailing slash
- * stripped. If the latter, dies with an error message.
- */
-const char *treat_gitlink(const char *path)
-{
- int i, path_len = strlen(path);
- for (i = 0; i < active_nr; i++) {
- struct cache_entry *ce = active_cache[i];
- if (S_ISGITLINK(ce->ce_mode)) {
- int ce_len = ce_namelen(ce);
- if (path_len <= ce_len || path[ce_len] != '/' ||
- memcmp(ce->name, path, ce_len))
- /* path does not refer to this
-* submodule or anything inside it */
- continue;
- if (path_len == ce_len + 1) {
- /* path refers to submodule;
-* strip trailing slash */
- return xstrndup(ce->name, ce_len);
- } else {
- die (_("Path '%s' is in submodule '%.*s'"),
-path, ce_len, ce->name);
- }
- }
- }
- return path;
-}
-
-void treat_gitlinks(const char **pathspec)
-{
- if (!pathspec || !*pathspec)
- return;
-
- int i;
- for (i = 0; pathspec[i]; i++)
- pathspec[i] = treat_gitlink(pathspec[i]);
-}
-
static void refresh(int verbose, const char **pathspec)
{
char *seen;
@@ -210,23 +138,6 @@ static void refresh(int verbose, const char **pathspec)
free(seen);
}
-static const char **validate_pathspec(int argc, const char **argv, const char
*prefix)
-{
- const char **pathspec = get_pathspec(prefix, argv);
-
- if (pathspec) {
- const char **p;
- for (p = pathspec; *p; p++) {
- if (has_symlink_leading_path(*p, strlen(*p))) {
- int len = prefix ? strlen(prefix) : 0;
- die(_("'%s' is beyond a symbolic link"), *p +
len);
- }
- }
- }
-
- return pathspec;
-}
-
int run_add_interactive(const char *revision, const char *patch_mode,
const char
[PATCH v2 03/14] Rename cryptic 'which' variable to more consistent name
'el' is only *slightly* less cryptic, but is already used as the
variable name for a struct exclude_list pointer in numerous other
places, so this reduces the number of cryptic variable names in use by
one :-)
Signed-off-by: Adam Spiers
---
dir.c | 10 +-
dir.h | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/dir.c b/dir.c
index 98d1995..91e57d9 100644
--- a/dir.c
+++ b/dir.c
@@ -311,7 +311,7 @@ static int no_wildcard(const char *string)
}
void add_exclude(const char *string, const char *base,
-int baselen, struct exclude_list *which)
+int baselen, struct exclude_list *el)
{
struct exclude *x;
size_t len;
@@ -346,8 +346,8 @@ void add_exclude(const char *string, const char *base,
x->nowildcardlen = simple_length(string);
if (*string == '*' && no_wildcard(string+1))
x->flags |= EXC_FLAG_ENDSWITH;
- ALLOC_GROW(which->excludes, which->nr + 1, which->alloc);
- which->excludes[which->nr++] = x;
+ ALLOC_GROW(el->excludes, el->nr + 1, el->alloc);
+ el->excludes[el->nr++] = x;
}
static void *read_skip_worktree_file_from_index(const char *path, size_t *size)
@@ -389,7 +389,7 @@ int add_excludes_from_file_to_list(const char *fname,
const char *base,
int baselen,
char **buf_p,
- struct exclude_list *which,
+ struct exclude_list *el,
int check_index)
{
struct stat st;
@@ -438,7 +438,7 @@ int add_excludes_from_file_to_list(const char *fname,
if (buf[i] == '\n') {
if (entry != buf + i && entry[0] != '#') {
buf[i - (i && buf[i-1] == '\r')] = 0;
- add_exclude(entry, base, baselen, which);
+ add_exclude(entry, base, baselen, el);
}
entry = buf + i + 1;
}
diff --git a/dir.h b/dir.h
index a226fbc..549a187 100644
--- a/dir.h
+++ b/dir.h
@@ -117,10 +117,10 @@ extern int path_excluded(struct path_exclude_check *,
const char *, int namelen,
extern int add_excludes_from_file_to_list(const char *fname, const char *base,
int baselen,
- char **buf_p, struct exclude_list
*which, int check_index);
+ char **buf_p, struct exclude_list
*el, int check_index);
extern void add_excludes_from_file(struct dir_struct *, const char *fname);
extern void add_exclude(const char *string, const char *base,
- int baselen, struct exclude_list *which);
+ int baselen, struct exclude_list *el);
extern void free_excludes(struct exclude_list *el);
extern int file_exists(const char *);
--
1.7.12.147.g6d168f4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 13/14] Provide free_directory() for reclaiming dir_struct memory
Signed-off-by: Adam Spiers
---
Documentation/technical/api-directory-listing.txt | 2 ++
dir.c | 23 +--
dir.h | 1 +
3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/Documentation/technical/api-directory-listing.txt
b/Documentation/technical/api-directory-listing.txt
index 944fc39..e339c18 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -79,4 +79,6 @@ marked. If you to exclude files, make sure you have loaded
index first.
* Use `dir.entries[]`.
+* Call `free_directory()` when none of the contained elements are no longer in
use.
+
(JC)
diff --git a/dir.c b/dir.c
index 705546f..a04739c 100644
--- a/dir.c
+++ b/dir.c
@@ -456,6 +456,12 @@ void add_excludes_from_file(struct dir_struct *dir, const
char *fname)
die("cannot use %s as an exclude file", fname);
}
+static void free_exclude_stack(struct exclude_stack *stk)
+{
+ free(stk->filebuf);
+ free(stk);
+}
+
/*
* Loads the per-directory exclude list for the substring of base
* which has a char length of baselen.
@@ -481,8 +487,7 @@ static void prep_exclude(struct dir_struct *dir, const char
*base, int baselen)
struct exclude *exclude = el->excludes[--el->nr];
free(exclude);
}
- free(stk->filebuf);
- free(stk);
+ free_exclude_stack(stk);
}
/* Read from the parent directories and push them down. */
@@ -1473,3 +1478,17 @@ void free_pathspec(struct pathspec *pathspec)
free(pathspec->items);
pathspec->items = NULL;
}
+
+void free_directory(struct dir_struct *dir)
+{
+ int st;
+ for (st = EXC_CMDL; st <= EXC_FILE; st++)
+ free_excludes(&dir->exclude_list[st]);
+
+ struct exclude_stack *prev, *stk = dir->exclude_stack;
+ while (stk) {
+ prev = stk->prev;
+ free_exclude_stack(stk);
+ stk = prev;
+ }
+}
diff --git a/dir.h b/dir.h
index ebb0367..7da29da 100644
--- a/dir.h
+++ b/dir.h
@@ -128,6 +128,7 @@ extern void add_excludes_from_file(struct dir_struct *,
const char *fname);
extern void add_exclude(const char *string, const char *base,
int baselen, struct exclude_list *el, const char *src,
int srcpos);
extern void free_excludes(struct exclude_list *el);
+extern void free_directory(struct dir_struct *dir);
extern int file_exists(const char *);
extern int is_inside_dir(const char *dir);
--
1.7.12.147.g6d168f4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/14] Rename path_excluded() to is_path_excluded()
Start adopting clearer names for exclude functions. This 'is_*'
naming pattern for functions returning booleans was agreed here:
http://thread.gmane.org/gmane.comp.version-control.git/204661/focus=204924
Signed-off-by: Adam Spiers
---
builtin/add.c | 2 +-
builtin/ls-files.c | 2 +-
dir.c | 4 ++--
dir.h | 2 +-
unpack-trees.c | 2 +-
5 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index e664100..075312a 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -454,7 +454,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
&& !file_exists(pathspec[i])) {
if (ignore_missing) {
int dtype = DT_UNKNOWN;
- if (path_excluded(&check, pathspec[i],
-1, &dtype))
+ if (is_path_excluded(&check,
pathspec[i], -1, &dtype))
dir_add_ignored(&dir,
pathspec[i], strlen(pathspec[i]));
} else
die(_("pathspec '%s' did not match any
files"),
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index b5434af..d226456 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -203,7 +203,7 @@ static void show_ru_info(void)
static int ce_excluded(struct path_exclude_check *check, struct cache_entry
*ce)
{
int dtype = ce_to_dtype(ce);
- return path_excluded(check, ce->name, ce_namelen(ce), &dtype);
+ return is_path_excluded(check, ce->name, ce_namelen(ce), &dtype);
}
static void show_files(struct dir_struct *dir)
diff --git a/dir.c b/dir.c
index 91e57d9..dad1582 100644
--- a/dir.c
+++ b/dir.c
@@ -627,8 +627,8 @@ void path_exclude_check_clear(struct path_exclude_check
*check)
* A path to a directory known to be excluded is left in check->path to
* optimize for repeated checks for files in the same excluded directory.
*/
-int path_excluded(struct path_exclude_check *check,
- const char *name, int namelen, int *dtype)
+int is_path_excluded(struct path_exclude_check *check,
+const char *name, int namelen, int *dtype)
{
int i;
struct strbuf *path = &check->path;
diff --git a/dir.h b/dir.h
index 549a187..41a5e32 100644
--- a/dir.h
+++ b/dir.h
@@ -113,7 +113,7 @@ struct path_exclude_check {
};
extern void path_exclude_check_init(struct path_exclude_check *, struct
dir_struct *);
extern void path_exclude_check_clear(struct path_exclude_check *);
-extern int path_excluded(struct path_exclude_check *, const char *, int
namelen, int *dtype);
+extern int is_path_excluded(struct path_exclude_check *, const char *, int
namelen, int *dtype);
extern int add_excludes_from_file_to_list(const char *fname, const char *base,
int baselen,
diff --git a/unpack-trees.c b/unpack-trees.c
index 6d96366..724f69b 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1373,7 +1373,7 @@ static int check_ok_to_remove(const char *name, int len,
int dtype,
return 0;
if (o->dir &&
- path_excluded(o->path_exclude_check, name, -1, &dtype))
+ is_path_excluded(o->path_exclude_check, name, -1, &dtype))
/*
* ce->name is explicitly excluded, so it is Ok to
* overwrite it.
--
1.7.12.147.g6d168f4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 14/14] Add git-check-ignore sub-command
This works in a similar manner to git-check-attr. Some code was reused from add.c by refactoring out into pathspec.c. Thanks to Jeff King and Junio C Hamano for the idea: http://thread.gmane.org/gmane.comp.version-control.git/108671/focus=108815 Signed-off-by: Adam Spiers --- .gitignore | 1 + Documentation/git-check-ignore.txt | 85 + Documentation/gitignore.txt| 6 +- Makefile | 1 + builtin.h | 1 + builtin/check-ignore.c | 167 ++ command-list.txt | 1 + contrib/completion/git-completion.bash | 1 + git.c | 1 + t/t0007-ignores.sh | 587 + t/t9902-completion.sh | 24 +- 11 files changed, 861 insertions(+), 14 deletions(-) create mode 100644 Documentation/git-check-ignore.txt create mode 100644 builtin/check-ignore.c create mode 100755 t/t0007-ignores.sh diff --git a/.gitignore b/.gitignore index a188a82..11cd975 100644 --- a/.gitignore +++ b/.gitignore @@ -22,6 +22,7 @@ /git-bundle /git-cat-file /git-check-attr +/git-check-ignore /git-check-ref-format /git-checkout /git-checkout-index diff --git a/Documentation/git-check-ignore.txt b/Documentation/git-check-ignore.txt new file mode 100644 index 000..2de4e17 --- /dev/null +++ b/Documentation/git-check-ignore.txt @@ -0,0 +1,85 @@ +git-check-ignore(1) += + +NAME + +git-check-ignore - Debug gitignore / exclude files + + +SYNOPSIS + +[verse] +'git check-ignore' [options] pathname... +'git check-ignore' [options] --stdin < + +DESCRIPTION +--- + +For each pathname given via the command-line or from a file via +`--stdin`, this command will list the first exclude pattern found (if +any) which explicitly excludes or includes that pathname. Note that +within any given exclude file, later patterns take precedence over +earlier ones, so any matching pattern which this command outputs may +not be the one you would immediately expect. + +OPTIONS +--- +-q, --quiet:: + Don't output anything, just set exit status. This is only + valid with a single pathname. + +-v, --verbose:: + Also output details about the matching pattern (if any) + for each given pathname. + +--stdin:: + Read file names from stdin instead of from the command-line. + +-z:: + The output format is modified to be machine-parseable (see + below). If `--stdin` is also given, input paths are separated + with a NUL character instead of a linefeed character. + +OUTPUT +-- + +By default, any of the given pathnames which match an ignore pattern +will be output, one per line. If no pattern matches a given path, +nothing will be output for that path; this means that path will not be +ignored. + +If `--verbose` is specified, the output is a series of lines of the form: + + + + is the path of a file being queried, is the +matching pattern, is the pattern's source file, and +is the line number of the pattern within that source. If the pattern +contained a `!` prefix or `/` suffix, it will be preserved in the +output. will be an absolute path when referring to the file +configured by `core.excludesfile`, or relative to the repository root +when referring to `.git/info/exclude` or a per-directory exclude file. + +If `-z` is specified, the output is a series of lines of the form: + +EXIT STATUS +--- + +0:: + One or more of the provided paths is ignored. + +1:: + None of the provided paths are ignored. + +128:: + A fatal error was encountered. + +SEE ALSO + +linkgit:gitignore[5] +linkgit:gitconfig[5] +linkgit:git-ls-files[5] + +GIT +--- +Part of the linkgit:git[1] suite diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt index c1f692a..7ba16fe 100644 --- a/Documentation/gitignore.txt +++ b/Documentation/gitignore.txt @@ -155,8 +155,10 @@ The second .gitignore prevents git from ignoring SEE ALSO -linkgit:git-rm[1], linkgit:git-update-index[1], -linkgit:gitrepository-layout[5] +linkgit:git-rm[1], +linkgit:git-update-index[1], +linkgit:gitrepository-layout[5], +linkgit:git-check-ignore[1] GIT --- diff --git a/Makefile b/Makefile index 9a4bf9e..2d78b83 100644 --- a/Makefile +++ b/Makefile @@ -834,6 +834,7 @@ BUILTIN_OBJS += builtin/branch.o BUILTIN_OBJS += builtin/bundle.o BUILTIN_OBJS += builtin/cat-file.o BUILTIN_OBJS += builtin/check-attr.o +BUILTIN_OBJS += builtin/check-ignore.o BUILTIN_OBJS += builtin/check-ref-format.o BUILTIN_OBJS += builtin/checkout-index.o BUILTIN_OBJS += builtin/checkout.o diff --git a/builtin.h b/builtin.h index 95116b8..7519f81 100644 --- a/builtin.h +++ b/builtin.h @@ -55,6 +55,7 @@ extern int cmd_cat_file(int argc, const char **argv, const char *prefix); extern int cmd_checkout(int argc, const char **argv, const char *prefi
[PATCH v2 02/14] Improve documentation and comments regarding directory traversal API
>From the perspective of a newcomer to the codebase, the directory
traversal API has a few potentially confusing properties. These
comments clarify a few key aspects and will hopefully make it easier
to understand for other newcomers in the future.
Signed-off-by: Adam Spiers
---
Documentation/technical/api-directory-listing.txt | 9 +---
dir.c | 8 ++-
dir.h | 26 +--
3 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/Documentation/technical/api-directory-listing.txt
b/Documentation/technical/api-directory-listing.txt
index 0356d25..944fc39 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -9,8 +9,11 @@ Data structure
--
`struct dir_struct` structure is used to pass directory traversal
-options to the library and to record the paths discovered. The notable
-options are:
+options to the library and to record the paths discovered. A single
+`struct dir_struct` is used regardless of whether or not the traversal
+recursively descends into subdirectories.
+
+The notable options are:
`exclude_per_dir`::
@@ -39,7 +42,7 @@ options are:
If set, recurse into a directory that looks like a git
directory. Otherwise it is shown as a directory.
-The result of the enumeration is left in these fields::
+The result of the enumeration is left in these fields:
`entries[]`::
diff --git a/dir.c b/dir.c
index 4868339..98d1995 100644
--- a/dir.c
+++ b/dir.c
@@ -2,6 +2,8 @@
* This handles recursive filename detection with exclude
* files, index knowledge etc..
*
+ * See Documentation/technical/api-directory-listing.txt
+ *
* Copyright (C) Linus Torvalds, 2005-2006
* Junio Hamano, 2005-2006
*/
@@ -451,6 +453,10 @@ void add_excludes_from_file(struct dir_struct *dir, const
char *fname)
die("cannot use %s as an exclude file", fname);
}
+/*
+ * Loads the per-directory exclude list for the substring of base
+ * which has a char length of baselen.
+ */
static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
{
struct exclude_list *el;
@@ -461,7 +467,7 @@ static void prep_exclude(struct dir_struct *dir, const char
*base, int baselen)
(baselen + strlen(dir->exclude_per_dir) >= PATH_MAX))
return; /* too long a path -- ignore */
- /* Pop the ones that are not the prefix of the path being checked. */
+ /* Pop the directories that are not the prefix of the path being
checked. */
el = &dir->exclude_list[EXC_DIRS];
while ((stk = dir->exclude_stack) != NULL) {
if (stk->baselen <= baselen &&
diff --git a/dir.h b/dir.h
index 893465a..a226fbc 100644
--- a/dir.h
+++ b/dir.h
@@ -1,6 +1,8 @@
#ifndef DIR_H
#define DIR_H
+/* See Documentation/technical/api-directory-listing.txt */
+
#include "strbuf.h"
struct dir_entry {
@@ -12,6 +14,12 @@ struct dir_entry {
#define EXC_FLAG_ENDSWITH 4
#define EXC_FLAG_MUSTBEDIR 8
+/*
+ * Each .gitignore file will be parsed into patterns which are then
+ * appended to the relevant exclude_list (either EXC_DIRS or
+ * EXC_FILE). exclude_lists are also used to represent the list of
+ * --exclude values passed via CLI args (EXC_CMDL).
+ */
struct exclude_list {
int nr;
int alloc;
@@ -26,9 +34,15 @@ struct exclude_list {
} **excludes;
};
+/*
+ * The contents of the per-directory exclude files are lazily read on
+ * demand and then cached in memory, one per exclude_stack struct, in
+ * order to avoid opening and parsing each one every time that
+ * directory is traversed.
+ */
struct exclude_stack {
- struct exclude_stack *prev;
- char *filebuf;
+ struct exclude_stack *prev; /* the struct exclude_stack for the parent
directory */
+ char *filebuf; /* remember pointer to per-directory exclude file
contents so we can free() */
int baselen;
int exclude_ix;
};
@@ -59,6 +73,14 @@ struct dir_struct {
#define EXC_DIRS 1
#define EXC_FILE 2
+ /*
+* Temporary variables which are used during loading of the
+* per-directory exclude lists.
+*
+* exclude_stack points to the top of the exclude_stack, and
+* basebuf contains the full path to the current
+* (sub)directory in the traversal.
+*/
struct exclude_stack *exclude_stack;
char basebuf[PATH_MAX];
};
--
1.7.12.147.g6d168f4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff across submodules
Am 20.09.2012 00:31, schrieb Junio C Hamano:
> Junio C Hamano writes:
>
>> I also suspect that you do not have to change "git diff" at all to
>> show the patch recursively by using the attribute mechanism (look in
>> Documentation/gitattributes.text for a string GIT_EXTERNAL_DIFF).
>> It might be just as simple as doing this:
>>
>> echo >.gitattributes "/lib/frotz diff=subrecurse"
>> git config diff.subrecurse.command $HOME/bin/diff-subrecurse
>> cat >$HOME/bin/diff-subrecurse <<\-EOF
>> #!/bin/sh
>> path=$1 old_hex=$3 new_hex=$6
>> unset GIT_DIR
>> cd "$path" || exit 1
>> git diff "$old_hex" "$new_hex"
>> EOF
>> chmod +x $HOME/bin/diff-subrecurse
>>
>> The corner cases like "new submodule", "removed submodule" are left
>> as an exercise to the reader ;-)
>
> It turns out that essentially the above outline I concocted in my
> MUA is usable almost as-is.
>
> Here is what I ended up with.
>
> * In .git/config of the superproject, I added this:
>
> [diff "submodule-recurse"]
> command = src/bin/diff-submodule-recurse
>
> * In the superproject, src/bin/diff-submodule-recurse has this
>(this is probably whitespace damaged---the lines must be indented
>by HT for the here document to correctly work):
>
> #!/bin/sh
> # $1 $2 $3 $4 $5 $6 $7
> # path old-file old-hex old-mode new-file new-hex new-mode
>
> case "$#,$4,$7" in
> 7,16,16) ;;
> *) echo "diff --git a/$1 b/$1"
> echo "(punt)"
> exit
> ;;
> esac
> unset GIT_DIR
> cd "$1" || {
> cat <<-\EOF
> diff --git a/$1 b/$1
> (cannot chdir to $1)
> -Subproject commit $3
> +Subproject commit $6
> EOF
> }
> git --no-pager diff --src-prefix="s/$1/" --dst-prefix="m/$1/" "$3"
> "$6"
>
> * In .gitattributes of the superproject, I have this:
>
> /var diff=submodule-recurse
>
> The superproject in this case is a repository to control what I have
> in my $HOME directory (e.g. it has src/dot/Makefile that builds and
> installs the appropriate dotfiles, src/bin/Makefile that builds and
> installs to $HOME/bin, etc.), and one subdirectory, 'var', is a
> submodule that is only cloned to some but not all machines I clone
> this superproject to.
>
> With this setting, things like
>
> $ git diff HEAD~20
>
> show differences with recursion into the var/ submodule just fine.
That's pretty cool! Even though diff options like --stat and --name-only
still won't take into account what happened inside the submodule this
approach makes it possible to see the diff recursively. Wouldn't it make
sense add this script to contrib (after teaching it new and removed
submodules and documenting its use in a few lines after the shebang)?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation: Document signature showing options
Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] grep.c: mark private file-scope symbols as static
Junio C Hamano wrote:
> Ramsay Jones writes:
>
>> Heh, so I obviously didn't see this before sending the patch yesterday! :-D
>>
>> Yes, this solves the problem addressed by yesterday's patch, so please
>> ignore that. However, this tickles sparse to complain as well ... ;-)
>>
>> New patch on it's way.
>
> Are you sure the patch you are responding to really "tickles
> sparse"?
Yes.
> You have another grep.c patch timestamped two minutes after the
> message I am responding to, and as far as I can see, it is a subset
> of the patch you are responding to with the message I am responding
> to.
Hmm, that sentence has too many twists for me! :-D
Let me see if I can clear up the misunderstanding:
- the "patch from yesterday" (18-09-2021) fixed a complaint from
sparse regarding symbol 'dump_grep_expression'. This was before
I had seen this patch email, or the resulting commit 07a7d656.
Since my "patch from yesterday" is a strict subset of your patch,
then your patch also fixes the complaint from sparse regarding
the 'dump_grep_expression' symbol.
- this patch (commit 07a7d656) causes sparse to complain about the
symbols 'grep_source_load' and 'grep_source_is_binary'. The new
patch from me ("timestamped two minutes after ...") also titled
"grep.c: Fix some sparse warnings" on 19-09-2012 at 7:04 PM, is
*not* a subset of commit 07a7d656. This patch addresses the new
sparse warnings regarding 'grep_source_load' and
'grep_source_is_binary'.
Hopefully that addresses the confusion! ;-)
HTH
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] remote-curl: let users turn off smart http
On Thu, Sep 20, 2012 at 11:36:34AM -0700, Junio C Hamano wrote: > >> What would the user experience be when we introduce "even smarter" > >> http server protocol extension? Will we add remote.foo.starterhttp? > > > > I would hope that it would actually be negotiated reliably at the > > protocol level so we do not have to deal with this mess again. > > The original dumb vs smart was supposed to be "negotiated reliably > at the protocol level", no? Yet we need this band-aid, so... I started to write much more in my original response, but deleted it as being too wordy. I guess I will have to rewrite it now. :) The difference is that jumping from dumb to smart had to give context clues at the HTTP layer. That is, by sending a query string, the client sends a single bit that tells the server "I understand smart http", and the server responds with output that indicates it also understands. We had to embed this in the HTTP layer, because the previous iteration wasn't running any custom git code at all. Whereas if we were to enhance the protocol again, it would probably _still_ begin with the same type of initial query, but we would negotiate more at the git-protocol level. And there we are in charge of how the implementation responds and handles backwards compatibility. This has already happened to some degree. We have added new capabilities at the git-protocol level, and it worked without these problems. It's not a "new protocol", but it is a backwards-compatible enhancement. And it's the likely mode for new enhancements in the future. It's possible we could have something drastically different in the future that does not even start with the same initial git conversation. But even then, I think we'd do it with a new "git-upload-pack2" service tag, or git:// and ssh access would be left behind. > >> Perhaps > >> > >> remote.$name.httpvariants = [smart] [dumb] > >> > >> to allow users to say "smart only", "dumb only", or "smart and/or > >> dumb" might be more code but less burden on the users. > > > > I don't mind that format if we are going that direction, but is there > > anybody who actually wants to say "smart only?" > > With 703e6e7 reverted, we take a failure from the initial smart > request to mean the server is simply not serving, so "smart only" to > fail quickly without trying dumb fallback is not needed. "smart > only" to say "I wouldn't want to talk to dumb-only server---I do not > have infinite amount of time, and I'd rather try another server" is > still a possibility, but likely not worth supporting. Yes. I do still need to resurrect my fetch-a-bundle-by-http code, which could also be covered by such a switch. But I guess I am just not sure if there is any point in spending effort to implement toggles that nobody has actually asked for. I'm also a little iffy on it because we would be inventing new config syntax. I don't think we want to split the list across multiple config items (which makes our usual later-config-overwrites-earlier rules behave badly). So what is the value format? Is it a whitespace-delimited case-insensitive list completely specifying the transports allowed? What happens if a new value is added. Do people who have said "smart" not get the new value, even though all they really wanted to say was "not dumb"? What about people who write "bundle smart" because their new version of git understands it, but then have old versions of git barf on it? Most of our current config is very toggle-oriented, and I'm not sure there is precedent for an option exactly like this. We can try to come up with answers to those questions, but I don't think doing it is as simple as just changing a few lines of code to support !dumb and !smart modes. I'm half-tempted to just drop the config entirely, leave GIT_SMART_HTTP=false as an escape hatch, and see if anybody even cares. At least then we're not promising support for a config option that we may want to change later. What do you want to do? -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff across submodules
Jens Lehmann writes:
> That's pretty cool! Even though diff options like --stat and --name-only
> still won't take into account what happened inside the submodule this
> approach makes it possible to see the diff recursively. Wouldn't it make
> sense add this script to contrib (after teaching it new and removed
> submodules and documenting its use in a few lines after the shebang)?
A few things somebody may want to work on while doing that "few
lines of documentation" I know about are:
* From the core side, pass options that are releavant when
generating patch (i.e. with p) in environment variables to the
external diff script;
* Not using "s/$1" and "m/$1" as prefix; instead, pass src/dst
prefix values (i.e. s/ and m/) from the core side in environment
variables, and make the external diff script itself aware of
possibly nested submodules, e.g.
SUBMODULE_PATH="${SUBMODULE_PATH}$1"
export SUBMODULE_PATH
exec git --no-pager diff -p \
--src-prefix="$SRC_PREFIX/$SUBMODULE_PATH" \
--dst-prefix="$DST_PREFIX/$SUBMODULE_PATH" "$3" "$6"
After people gain sufficient experience with it, as the next step,
we can think about how to handle --stat and other options when we
are run without -p (currently the attribute mechanism would not
trigger) and then we can call the result a native "diff that
recurses into submodules" that people can use without setting up the
attributes based mechanism.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/6] Refactor mechanics of testing in a sub test-lib
Adam Spiers writes:
> This will allow us to test the test framework more thoroughly
> without disrupting the top-level test metrics.
>
> Signed-off-by: Adam Spiers
> ---
> t/t-basic.sh | 67
>
> 1 file changed, 29 insertions(+), 38 deletions(-)
>
> diff --git a/t/t-basic.sh b/t/t-basic.sh
> index c6b42de..029e3bd 100755
> --- a/t/t-basic.sh
> +++ b/t/t-basic.sh
> @@ -55,39 +55,49 @@ test_expect_failure 'pretend we have a known breakage' '
> false
> '
>
> -test_expect_success 'pretend we have fixed a known breakage (run in sub
> test-lib)' "
> - mkdir passing-todo &&
> - (cd passing-todo &&
> - cat >passing-todo.sh <<-EOF &&
> +run_sub_test_lib_test () {
> + name="$1" descr="$2" # stdin is body of test code
> + mkdir $name &&
> + (cd $name &&
> + cat >$name.sh <<-EOF &&
> #!$SHELL_PATH
>
> - test_description='A passing TODO test
> + test_description='$descr (run in sub test-lib)
>
> This is run in a sub test-lib so that we do not get incorrect
> passing metrics
> '
>
> # Point to the t/test-lib.sh, which isn't in ../ as usual
> - TEST_DIRECTORY=\"$TEST_DIRECTORY\"
> - . \"\$TEST_DIRECTORY\"/test-lib.sh
> -
> - test_expect_failure 'pretend we have fixed a known breakage' '
> - :
> - '
> + TEST_DIRECTORY="$TEST_DIRECTORY"
> + . "\$TEST_DIRECTORY"/test-lib.sh
> + EOF
The quoting of $TEST_DIRECTORY in the assignment does not look
correct (imagine a path with a double quote in it).
Removing the assignment and instead exporting TEST_DIRECTORY before
calling name.sh may be a reasonable fix, than trying to quotemeta
the value of $TEST_DIRECTORY here.
I'll re-queue this series in 'pu' with fixes and retitles; please
eyeball them before submitting a reroll.
b465316 tests: paint unexpectedly fixed known breakages in bold red
7214717 tests: test the test framework more thoroughly
03c772a [SQUASH] t/t-basic.sh: quoting of TEST_DIRECTORY is screwed up
99fe0af tests: refactor mechanics of testing in a sub test-lib
6af90bf tests: paint skipped tests in bold blue
0b87581 tests: test number comes first in 'not ok $count - $message'
1c55079 tests: paint known breakages in bold yellow
The third one from the tip looks like the following, to re-indent to
make it readable and then minimally fix the quoting.
Thanks.
t/t-basic.sh | 50 +++---
1 file changed, 27 insertions(+), 23 deletions(-)
diff --git a/t/t-basic.sh b/t/t-basic.sh
index ee78e68..c3345a9 100755
--- a/t/t-basic.sh
+++ b/t/t-basic.sh
@@ -56,33 +56,37 @@ test_expect_failure 'pretend we have a known breakage' '
'
run_sub_test_lib_test () {
- name="$1" descr="$2" # stdin is body of test code
+ name="$1" descr="$2" # stdin is the body of the test code
mkdir $name &&
- (cd $name &&
- cat >$name.sh <<-EOF &&
- #!$SHELL_PATH
-
- test_description='$descr (run in sub test-lib)
-
- This is run in a sub test-lib so that we do not get incorrect
- passing metrics
- '
-
- # Point to the t/test-lib.sh, which isn't in ../ as usual
- TEST_DIRECTORY="$TEST_DIRECTORY"
- . "\$TEST_DIRECTORY"/test-lib.sh
- EOF
- cat >>$name.sh &&
- chmod +x $name.sh &&
- ./$name.sh >out 2>err)
+ (
+ cd $name &&
+ cat >$name.sh <<-EOF &&
+ #!$SHELL_PATH
+
+ test_description='$descr (run in sub test-lib)
+
+ This is run in a sub test-lib so that we do not get incorrect
+ passing metrics
+ '
+
+ # Point to the t/test-lib.sh, which isn't in ../ as usual
+ . "\$TEST_DIRECTORY"/test-lib.sh
+ EOF
+ cat >>$name.sh &&
+ chmod +x $name.sh &&
+ export TEST_DIRECTORY &&
+ ./$name.sh >out 2>err
+ )
}
check_sub_test_lib_test () {
- name="$1" # stdin is expected output from the test
- (cd $name &&
- ! test -s err &&
- sed -e 's/^> //' -e 's/Z$//' >expect &&
- test_cmp expect out)
+ name="$1" # stdin is the expected output from the test
+ (
+ cd $name &&
+ ! test -s err &&
+ sed -e 's/^> //' -e 's/Z$//' >expect &&
+ test_cmp expect out
+ )
}
test_expect_success 'pretend we have fixed a known breakage' "
--
1.7.12.1.389.g3dff30b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] remote-curl: let users turn off smart http
Jeff King writes: > I'm half-tempted to just drop the config entirely, leave > GIT_SMART_HTTP=false as an escape hatch, and see if anybody even cares. Sounds like a very attractive minimalistic way to go forward. We can always add per-remote configuration when we find it necessary, but once we add support, we cannot easily yank it out. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv8] clone --single: limit the fetch refspec to fetched branch
Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/14] new git check-ignore sub-command
Adam Spiers writes: > Adam Spiers (14): > Update directory listing API doc to match code > Improve documentation and comments regarding directory traversal API > Rename cryptic 'which' variable to more consistent name > Rename path_excluded() to is_path_excluded() > Rename excluded_from_list() to is_excluded_from_list() > Rename excluded() to is_excluded() > Refactor is_excluded_from_list() > Refactor is_excluded() > Refactor is_path_excluded() > For each exclude pattern, store information about where it came from > Refactor treat_gitlinks() > Extract some useful pathspec handling code from builtin/add.c into a > library > Provide free_directory() for reclaiming dir_struct memory > Add git-check-ignore sub-command Please retitle these to have a short "prefix: " that names a specific area the series intends to touch. I retitled your other series to share "test :" as their common prefix. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] remote-curl: let users turn off smart http
On Thu, Sep 20, 2012 at 02:15:20PM -0700, Junio C Hamano wrote:
> Jeff King writes:
>
> > I'm half-tempted to just drop the config entirely, leave
> > GIT_SMART_HTTP=false as an escape hatch, and see if anybody even cares.
>
> Sounds like a very attractive minimalistic way to go forward. We
> can always add per-remote configuration when we find it necessary,
> but once we add support, we cannot easily yank it out.
Like this?
-- >8 --
Subject: [PATCH] remote-curl: let users turn off smart http
Usually there is no need for users to specify whether an
http remote is smart or dumb; the protocol is designed so
that a single initial request is made, and the client can
determine the server's capability from the response.
However, some misconfigured dumb-only servers may not like
the initial request by a smart client, as it contains a
query string. Until recently, commit 703e6e7 worked around
this by making a second request. However, that commit was
recently reverted due to its side effect of masking the
initial request's error code.
Since git has had that workaround for several years, we
don't know exactly how many such misconfigured servers are
out there. The reversion of 703e6e7 assumes they are rare
enough not to worry about. Still, that reversion leaves
somebody who does run into such a server with no escape
hatch at all. Let's give them an environment variable they
can tweak to perform the "dumb" request.
This is intentionally not a documented interface. It's
overly simple and is really there for debugging in case
somebody does complain about git not working with their
server. A real user-facing interface would entail a
per-remote or per-URL config variable.
Signed-off-by: Jeff King
---
remote-curl.c | 3 ++-
t/t5551-http-fetch.sh | 12
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/remote-curl.c b/remote-curl.c
index c0b98cc..7b19ebb 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -102,7 +102,8 @@ static struct discovery* discover_refs(const char *service)
free_discovery(last);
strbuf_addf(&buffer, "%sinfo/refs", url);
- if (!prefixcmp(url, "http://";) || !prefixcmp(url, "https://";)) {
+ if ((!prefixcmp(url, "http://";) || !prefixcmp(url, "https://";)) &&
+git_env_bool("GIT_SMART_HTTP", 1)) {
maybe_smart = 1;
if (!strchr(url, '?'))
strbuf_addch(&buffer, '?');
diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index 2db5c35..8427943 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -129,6 +129,18 @@ test_expect_success 'clone from auth-only-for-push
repository' '
test_cmp expect actual
'
+test_expect_success 'disable dumb http on server' '
+ git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
+ config http.getanyfile false
+'
+
+test_expect_success 'GIT_SMART_HTTP can disable smart http' '
+ (GIT_SMART_HTTP=0 &&
+export GIT_SMART_HTTP &&
+cd clone &&
+test_must_fail git fetch)
+'
+
test -n "$GIT_TEST_LONG" && test_set_prereq EXPENSIVE
test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '
--
1.7.11.7.15.g085c6bd
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 10/14] For each exclude pattern, store information about where it came from
Adam Spiers writes:
> void add_exclude(const char *string, const char *base,
> - int baselen, struct exclude_list *el)
> + int baselen, struct exclude_list *el, const char *src, int
> srcpos)
> {
> struct exclude *x;
> size_t len;
> @@ -341,6 +341,8 @@ void add_exclude(const char *string, const char *base,
> x->base = base;
> x->baselen = baselen;
> x->flags = flags;
> + x->src = src;
> + x->srcpos = srcpos;
Hrm, don't all elements "x" in "el" share the same "src", even if
their srcpos may be different?
> if (!strchr(string, '/'))
> x->flags |= EXC_FLAG_NODIR;
> x->nowildcardlen = simple_length(string);
> @@ -393,7 +395,7 @@ int add_excludes_from_file_to_list(const char *fname,
> int check_index)
> {
> struct stat st;
> - int fd, i;
> + int fd, i, lineno = 1;
> size_t size = 0;
> char *buf, *entry;
>
> @@ -438,8 +440,9 @@ int add_excludes_from_file_to_list(const char *fname,
> if (buf[i] == '\n') {
> if (entry != buf + i && entry[0] != '#') {
> buf[i - (i && buf[i-1] == '\r')] = 0;
> - add_exclude(entry, base, baselen, el);
> + add_exclude(entry, base, baselen, el, fname,
> lineno);
> }
> + lineno++;
> entry = buf + i + 1;
> }
> }
> @@ -474,8 +477,10 @@ static void prep_exclude(struct dir_struct *dir, const
> char *base, int baselen)
> !strncmp(dir->basebuf, base, stk->baselen))
> break;
> dir->exclude_stack = stk->prev;
> - while (stk->exclude_ix < el->nr)
> - free(el->excludes[--el->nr]);
> + while (stk->exclude_ix < el->nr) {
> + struct exclude *exclude = el->excludes[--el->nr];
> + free(exclude);
> + }
> free(stk->filebuf);
> free(stk);
> }
> @@ -502,7 +507,15 @@ static void prep_exclude(struct dir_struct *dir, const
> char *base, int baselen)
> memcpy(dir->basebuf + current, base + current,
> stk->baselen - current);
> strcpy(dir->basebuf + stk->baselen, dir->exclude_per_dir);
> - add_excludes_from_file_to_list(dir->basebuf,
> +
> + /* dir->basebuf gets reused by the traversal, but we
> + * need fname to remain unchanged to ensure the src
> + * member of each struct exclude correctly back-references
> + * its source file.
> + */
> + char *fname = strdup(dir->basebuf);
/*
* We try to format our multi-line comments
* like this.
*
* By the way, who owns x->src and who is responsible for
* freeing it when the exclude-stack is popped to make them
* no longer necessary?
*
* Oh, by the way, that is a decl-after-statement.
*/
> +
> + add_excludes_from_file_to_list(fname,
> dir->basebuf, stk->baselen,
> &stk->filebuf, el, 1);
> dir->exclude_stack = stk;
> diff --git a/dir.h b/dir.h
> index 19beddb..ebb0367 100644
> --- a/dir.h
> +++ b/dir.h
> @@ -31,6 +31,9 @@ struct exclude_list {
> int baselen;
> int to_exclude;
> int flags;
> + const char *src;
> + int srcpos; /* counting starts from 1 for line numbers in
> ignore files,
> +and from -1 decrementing for patterns from CLI
> (--exclude) */
> } **excludes;
> };
>
> @@ -123,7 +126,7 @@ extern int add_excludes_from_file_to_list(const char
> *fname, const char *base, i
> char **buf_p, struct exclude_list
> *el, int check_index);
> extern void add_excludes_from_file(struct dir_struct *, const char *fname);
> extern void add_exclude(const char *string, const char *base,
> - int baselen, struct exclude_list *el);
> + int baselen, struct exclude_list *el, const char *src,
> int srcpos);
> extern void free_excludes(struct exclude_list *el);
> extern int file_exists(const char *);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/14] new git check-ignore sub-command
Junio C Hamano writes: > Adam Spiers writes: > >> Adam Spiers (14): >> Update directory listing API doc to match code >> Improve documentation and comments regarding directory traversal API >> Rename cryptic 'which' variable to more consistent name >> Rename path_excluded() to is_path_excluded() >> Rename excluded_from_list() to is_excluded_from_list() >> Rename excluded() to is_excluded() >> Refactor is_excluded_from_list() >> Refactor is_excluded() >> Refactor is_path_excluded() >> For each exclude pattern, store information about where it came from >> Refactor treat_gitlinks() >> Extract some useful pathspec handling code from builtin/add.c into a >> library >> Provide free_directory() for reclaiming dir_struct memory >> Add git-check-ignore sub-command > > Please retitle these to have a short "prefix: " that names a > specific area the series intends to touch. I retitled your other > series to share "test :" as their common prefix. Just to clarify, I think most of them can say "dir.c: ". I saw quite a few decl-after-statement in new code. Please fix them. As to the "who owns x->src and when is it freed" question, it may make sense to give el a "filename" field (command line and other special cases would get whatever value you deem appropriate, like NULL or ""), have x->src point at that field when you queue many x's to the el in add_exc_from_file_to_list(). Then when you pop an element in the exclude_stack, you can free el->filename to plug a potential leak. Also I do not see why you need to have the strdup() in the caller of add_excludes_from_file_to_list(). If you need to keep it stable because you are copying it away in exclude or excludde_list, wouldn't it make more sense to do that at the beginning of the callee, i.e. add_excludes_from_file_to_list() function? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git archive --format zip utf-8 issues
Am 18.09.2012 23:12, schrieb Junio C Hamano: René Scharfe writes: WindowsInfo-ZIP unzip 7-Zip PeaZip builtin Linux msysgit Windows 7-Zip 9.20 0 0 4626 43 43 PeaZip 4.7.1 win64 0 0 4626 42 42 Info-ZIP zip 3.0 Linux 0 0 72 0 43 43 Info-ZIP zip 3.0 Windows 45 45 n/a 0 43 43 It is kind of surprising that "Windows builtin" has very poor score extracting from the output of Zip tools running on Windows (I am looking at 46, 46 and n/a over there). If you tell it to create an archive from its disk and then extract from it, I wonder what would happen. I didn't include it as a packer because it refused to archive the pangrams directory due to illegal characters in one of the filenames. When I just tried a bit harder, I had to delete all but 14 files with Latin script, accents etc. before I could zip the directory. I'll include these results in the next round. It uses codepage 850 on my system (MSDOS Latin 1). I don't expect this to be portable. Does this result mean that practically nobody uses Zip archive with exotic letters in paths on that platform? I am not talking about developers and savvy people who know where to download third-party Zip archivers and how to install them. I am imagining a grandma who received an archive full of photos of her grandchild in her Outlook Express or GMail inbox, clicked the attachment to download it, and is trying to view the photo inside. Not necessarily. Photos often have names like img_0123.jpg etc., which are handled just fine. And all family members probably use the same codepage on their computers, so they're less likely to run into this problem. By the way, I found this bug asking for codepage support in unzip: https://bugs.launchpad.net/ubuntu/+source/unzip/+bug/580961 Multiple codepages seem to be used for ZIP files in the wild, none of them are supported by unzip on Linux, which only accepts ASCII or UTF-8. René -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Disable dumb HTTP fallback with GIT_CURL_FALLBACK=0
On Thu, Sep 20, 2012 at 10:24 AM, Jeff King wrote: > On Wed, Sep 19, 2012 at 10:57:35PM -0700, Shawn O. Pearce wrote: > >> > so transient errors on the initial smart >> > contact can cause us to fall back to dumb, >> >> Transient errors is actually what is leading me down this path. We see >> about 0.0455% of our requests to the Android hosting service as these >> dumb fallbacks. This means the client never got a proper smart service >> reply. Server logs suggest we sent a valid response, so I am >> suspecting transient proxy errors, but its hard to debug because >> clients discard the error. > > Yup, we see the same thing. I've tracked a few down manually to actual > things like gateway timeouts on our reverse proxy. Our reverse proxies also fail sometimes. :-) But right now I am seeing failures in libcurl's SSL connection that may also be causing the smart connection failures. For example this trace, where libcurl was just not able to connect to respond to the 401 with a password. I suspect what is happening is the SSL session dropped out of cache on our servers, and libcurl couldn't reuse the existing SSL session. Instead of discarding the bad session and retrying, Git aborts. I'm willing to bet modern browsers just discard the bad session and start a new one, because clients can't assume the remote server will be able to remember their session forever. * Couldn't find host android.googlesource.com in the .netrc file; using defaults * About to connect() to android.googlesource.com port 443 (#0) * Trying 2607:f8b0:400e:c02::52... * Connected to android.googlesource.com (2607:f8b0:400e:c02::52) port 443 (#0) * successfully set certificate verify locations: * CAfile: none CApath: /etc/ssl/certs * SSL connection using RC4-SHA * Server certificate: *subject: C=US; ST=California; L=Mountain View; O=Google Inc; CN=*.googlecode.com *start date: 2012-08-16 12:25:39 GMT *expire date: 2013-06-07 19:43:27 GMT *subjectAltName: android.googlesource.com matched *issuer: C=US; O=Google Inc; CN=Google Internet Authority *SSL certificate verify ok. > GET /a/platform/tools/build/info/refs?service=git-upload-pack HTTP/1.1 User-Agent: git/1.7.12.1.1.g9b7ccb3 Host: android.googlesource.com Accept: */* Pragma: no-cache * The requested URL returned error: 401 * Closing connection #0 * Couldn't find host android.googlesource.com in the .netrc file; using defaults * About to connect() to android.googlesource.com port 443 (#0) * Trying 2607:f8b0:400e:c02::52... * Connected to android.googlesource.com (2607:f8b0:400e:c02::52) port 443 (#0) * successfully set certificate verify locations: * CAfile: none CApath: /etc/ssl/certs * SSL re-using session ID * Unknown SSL protocol error in connection to android.googlesource.com:443 * Expire cleared * Closing connection #0 error: Unknown SSL protocol error in connection to android.googlesource.com:443 while accessing https://android.googlesource.com/a/platform/tools/build/info/refs?service=git-upload-pack fatal: HTTP request failed -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/14] new git check-ignore sub-command
On Thu, Sep 20, 2012 at 10:43 PM, Junio C Hamano wrote: > Junio C Hamano writes: >> Adam Spiers writes: >>> Adam Spiers (14): >>> Update directory listing API doc to match code >>> Improve documentation and comments regarding directory traversal API >>> Rename cryptic 'which' variable to more consistent name >>> Rename path_excluded() to is_path_excluded() >>> Rename excluded_from_list() to is_excluded_from_list() >>> Rename excluded() to is_excluded() >>> Refactor is_excluded_from_list() >>> Refactor is_excluded() >>> Refactor is_path_excluded() >>> For each exclude pattern, store information about where it came from >>> Refactor treat_gitlinks() >>> Extract some useful pathspec handling code from builtin/add.c into a >>> library >>> Provide free_directory() for reclaiming dir_struct memory >>> Add git-check-ignore sub-command >> >> Please retitle these to have a short "prefix: " that names a >> specific area the series intends to touch. I retitled your other >> series to share "test :" as their common prefix. > > Just to clarify, I think most of them can say "dir.c: ". Sure, I can do that, but shouldn't this convention be documented in SubmittingPatches? > I saw quite a few decl-after-statement in new code. Please fix > them. Again, I can do that no problem, but again this convention is undocumented and I am not psychic ;-) I see that a patch was provided 5 years ago to document this, but was apparently never pulled in: http://thread.gmane.org/gmane.comp.version-control.git/47843/focus=48015 It would save everybody's time if these things are documented, since then they wouldn't create noise during the review process. I also see in the same thread that a patch to add -Wdeclaration-after-statement to CFLAGS was also offered but never pulled in, presumably on the stated grounds that that option was relatively recent 5 years ago. But wouldn't it be trivial to do gcc -v --help 2>&1 | grep declaration-after-statement at build-time? I'm also curious to know why this convention exists. Are people really still compiling git with compilers which don't support C99? > As to the "who owns x->src and when is it freed" question, it may > make sense to give el a "filename" field (command line and other > special cases would get whatever value you deem appropriate, like > NULL or ""), have x->src point at that field when you > queue many x's to the el in add_exc_from_file_to_list(). Then when > you pop an element in the exclude_stack, you can free el->filename > to plug a potential leak. > > Also I do not see why you need to have the strdup() in the caller of > add_excludes_from_file_to_list(). If you need to keep it stable > because you are copying it away in exclude or excludde_list, > wouldn't it make more sense to do that at the beginning of the > callee, i.e. add_excludes_from_file_to_list() function? Thanks, I'll take a look at these suggestions tomorrow. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git diff-tree -r -C output inexact sometimes
Hello A colleague of mine discovered an inconsistency in the functioning of git diff-tree -r -C in specific conditions. As tenuous as these conditions might seem (once you run the script in attachment and analyse its output), please rest assured that it comes from a real-life case. Running the script in attachment produces a git repository in which were operated a large number of file renames, in which many of the renamed files (in this particular case all) have the same content but different names. The commit data from the renaming operation (last commit in the script- generated history) is inexactly rendered by the command git diff-tree -r -C master The logical result is correctly produced by the more restricted command git diff-tree -r -M master IMO for this particular last commit both the above commands should return the same result. Note that reducing i or j in the generator script attached below makes the bug dissapear. Thanks a lot for your attention. -- Cristian Tibirna KDE developer .. tibi...@kde.org .. http://www.kde.org generate_git_tree.sh Description: application/shellscript
Re: [PATCH v2 00/14] new git check-ignore sub-command
Adam Spiers writes: > Sure, I can do that, but shouldn't this convention be documented in > SubmittingPatches? People must have learned this by imitating what senior contributors send to the list, but the "[Subject] area: title" thing does not appear in that document. I agree it should (patches welcome). >> I saw quite a few decl-after-statement in new code. Please fix >> them. > > Again, I can do that no problem, but again this convention is > undocumented and I am not psychic. Yeah, when there is no code that does decl-after-statement, with the "imitate surrounding code" rule alone, I agree that it is a bit hard to tell we do not allow it (as opposed to seeing a construct is often used and assuming that the construct is permitted, which is much easier). > I see that a patch was provided > 5 years ago to document this, but was apparently never pulled in: > > http://thread.gmane.org/gmane.comp.version-control.git/47843/focus=48015 I just read SubmittingPatches again and looked for 1a as found in that patch, and it is there. > I also see in the same thread that a patch to add > -Wdeclaration-after-statement to CFLAGS was also offered but never > pulled in, There is no guarantee your CC would understand it; you don't even know if it is a gcc in the first place. > I'm also curious to know why this convention exists. Are people > really still compiling git with compilers which don't support C99? See 6d62c98 (Makefile: Change the default compiler from "gcc" to "cc", 2011-12-20). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Disable dumb HTTP fallback with GIT_CURL_FALLBACK=0
On Thu, Sep 20, 2012 at 04:05:03PM -0700, Shawn O. Pearce wrote: > But right now I am seeing failures in libcurl's SSL connection that > may also be causing the smart connection failures. For example this > trace, where libcurl was just not able to connect to respond to the > 401 with a password. I suspect what is happening is the SSL session > dropped out of cache on our servers, and libcurl couldn't reuse the > existing SSL session. Instead of discarding the bad session and > retrying, Git aborts. I'm willing to bet modern browsers just discard > the bad session and start a new one, because clients can't assume the > remote server will be able to remember their session forever. That's something I haven't seen. But then, I don't usually see the client side; I just see the fallback dumb fetch in our logs, and have occasionally followed up. Is there a long pause while the user is typing their password? > * SSL re-using session ID > * Unknown SSL protocol error in connection to android.googlesource.com:443 > * Expire cleared > * Closing connection #0 > error: Unknown SSL protocol error in connection to > android.googlesource.com:443 while accessing > https://android.googlesource.com/a/platform/tools/build/info/refs?service=git-upload-pack > fatal: HTTP request failed You could try turning off CURLOPT_SSL_SESSIONID_CACHE and seeing if that improves it. Of course, it is probably hard to reproduce, so it would be tough to know if that helped or not. It would also be nice if you could dump more information on the error from the ssl library (I typically build curl against openssl; I wonder if it could be related to using gnutls or something). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] nicer receive-pack errors over http
While we are on the subject of http user experience, I thought I'd mention this patch to route more errors from index-pack back to the user. We're not doing it yet at GitHub, but I plan to apply it soon. The first patch is a cleanup and minor bug fix. The second is the interesting one. The third is purely cosmetic, and doesn't need to be tied to the others. [1/3]: receive-pack: redirect unpack-objects stdout to /dev/null [2/3]: receive-pack: send pack-processing stderr over sideband [3/3]: receive-pack: drop "n/a" on unpacker errors -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] receive-pack: redirect unpack-objects stdout to /dev/null
The unpack-objects command should not generally produce any
output on stdout. However, if it's given extra input after
the packfile, it will spew the remainder to stdout. When
called by receive-pack, this means we will break protocol,
since our stdout is connected to the remote send-pack.
Signed-off-by: Jeff King
---
I've never actually seen this bug in practice, but I noticed it while
auditing the outputs of receive-pack's children recently.
builtin/receive-pack.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 9145f1a..5ba0c98 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -815,6 +815,7 @@ static const char *unpack(void)
if (ntohl(hdr.hdr_entries) < unpack_limit) {
int code, i = 0;
+ struct child_process child;
const char *unpacker[5];
unpacker[i++] = "unpack-objects";
if (quiet)
@@ -823,7 +824,11 @@ static const char *unpack(void)
unpacker[i++] = "--strict";
unpacker[i++] = hdr_arg;
unpacker[i++] = NULL;
- code = run_command_v_opt(unpacker, RUN_GIT_CMD);
+ memset(&child, 0, sizeof(child));
+ child.argv = unpacker;
+ child.no_stdout = 1;
+ child.git_cmd = 1;
+ code = run_command(&child);
if (!code)
return NULL;
return "unpack-objects abnormal exit";
--
1.7.11.7.15.g085c6bd
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] receive-pack: send pack-processing stderr over sideband
Receive-pack invokes either unpack-objects or index-pack to
handle the incoming pack. However, we do not redirect the
stderr of the sub-processes at all, so it is never seen by
the client. From the initial thread adding sideband support,
which is here:
http://thread.gmane.org/gmane.comp.version-control.git/139471
it is clear that some messages are specifically kept off the
sideband (with the assumption that they are of interest only
to an administrator, not the client). The stderr of the
subprocesses is mentioned in the thread, but it's unclear if
they are included in that group, or were simply forgotten.
However, there are a few good reasons to show them to the
client:
1. In many cases, they are directly about the incoming
packfile (e.g., fsck warnings with --strict, corruption
in the packfile, etc). Without these messages, the
client just gets "unpacker error" with no extra useful
diagnosis.
2. No matter what the cause, we are probably better off
showing the errors to the client. If the client and the
server admin are not the same entity, it is probably
much easier for the client to cut-and-paste the errors
they see than for the admin to try to dig them out of a
log and correlate them with a particular session.
3. Users of the ssh transport typically already see these
stderr messages, as the remote's stderr is copied
literally by ssh. This brings other transports (http,
and push-over-git if you are crazy enough to enable it)
more in line with ssh. As a bonus for ssh users,
because the messages are now fed through the sideband
and printed by the local git, they will have "remote:"
prepended and be properly interleaved with any local
output to stderr.
Signed-off-by: Jeff King
---
This one is logically independent of the first patch, but relies
textually on the conversion of unpack-objects to use a separate
child_process.
builtin/receive-pack.c | 26 --
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 5ba0c98..ac679ab 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -795,7 +795,7 @@ static const char *pack_lockfile;
static const char *pack_lockfile;
-static const char *unpack(void)
+static const char *unpack(int err_fd)
{
struct pack_header hdr;
const char *hdr_err;
@@ -827,6 +827,7 @@ static const char *unpack(void)
memset(&child, 0, sizeof(child));
child.argv = unpacker;
child.no_stdout = 1;
+ child.err = err_fd;
child.git_cmd = 1;
code = run_command(&child);
if (!code)
@@ -853,6 +854,7 @@ static const char *unpack(void)
memset(&ip, 0, sizeof(ip));
ip.argv = keeper;
ip.out = -1;
+ ip.err = err_fd;
ip.git_cmd = 1;
status = start_command(&ip);
if (status) {
@@ -869,6 +871,26 @@ static const char *unpack(void)
}
}
+static const char *unpack_with_sideband(void)
+{
+ struct async muxer;
+ const char *ret;
+
+ if (!use_sideband)
+ return unpack(0);
+
+ memset(&muxer, 0, sizeof(muxer));
+ muxer.proc = copy_to_sideband;
+ muxer.in = -1;
+ if (start_async(&muxer))
+ return NULL;
+
+ ret = unpack(muxer.in);
+
+ finish_async(&muxer);
+ return ret;
+}
+
static void report(struct command *commands, const char *unpack_status)
{
struct command *cmd;
@@ -966,7 +988,7 @@ int cmd_receive_pack(int argc, const char **argv, const
char *prefix)
const char *unpack_status = NULL;
if (!delete_only(commands))
- unpack_status = unpack();
+ unpack_status = unpack_with_sideband();
execute_commands(commands, unpack_status);
if (pack_lockfile)
unlink_or_warn(pack_lockfile);
--
1.7.11.7.15.g085c6bd
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] receive-pack: drop "n/a" on unpacker errors
The output from git push currently looks like this:
$ git push dest HEAD
fatal: [some message from index-pack]
error: unpack failed: index-pack abnormal exit
To dest
! [remote rejected] HEAD -> master (n/a (unpacker error))
That n/a is meant to be "the per-ref status is not
available" but the nested parentheses just make it look
ugly. Let's turn the final line into just:
! [remote rejected] HEAD -> master (unpacker error)
Signed-off-by: Jeff King
---
Maybe it is just me, but I have always found the "n/a" and extra
parentheses ugly and unnecessary. But obviously others may differ.
It doesn't really come up that often, since index-pack failing usually
implies a git bug. But with transfer.fsckObjects turn on, it is more
common.
I don't think there should be any backwards compatibility issues with
changing this. The "reason" field sent back by receive-pack has always
been a free-form human-readable string.
I also dislike the "index-pack abnormal exit" message. Again, when
index-pack really crashes, it's fine, but it can die due to bogus
objects, too, in which case it might be nice to have a more
human-readable message.
builtin/receive-pack.c | 2 +-
t/t5504-fetch-receive-strict.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ac679ab..ff781fe 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -695,7 +695,7 @@ static void execute_commands(struct command *commands,
const char *unpacker_erro
if (unpacker_error) {
for (cmd = commands; cmd; cmd = cmd->next)
- cmd->error_string = "n/a (unpacker error)";
+ cmd->error_string = "unpacker error";
return;
}
diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
index 35ec294..69ee13c 100755
--- a/t/t5504-fetch-receive-strict.sh
+++ b/t/t5504-fetch-receive-strict.sh
@@ -89,7 +89,7 @@ To dst
cat >exp
Re: [PATCH v2 14/14] Add git-check-ignore sub-command
Am 9/20/2012 21:46, schrieb Adam Spiers: > test_expect_success 'general options plus command' ' > - test_completion "git --version check" "checkout " && > - test_completion "git --paginate check" "checkout " && > - test_completion "git --git-dir=foo check" "checkout " && > - test_completion "git --bare check" "checkout " && > + test_completion "git --version checko" "checkout " && > + test_completion "git --paginate checko" "checkout " && > + test_completion "git --git-dir=foo checko" "checkout " && > + test_completion "git --bare checko" "checkout " && > ... I find this worrysome. Is check-ignore, being a debugging aid, so important that it must be autocompleted? -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff-tree -r -C output inexact sometimes
On Thu, Sep 20, 2012 at 11:20:31PM -0400, Cristian Tibirna wrote: > Running the script in attachment produces a git repository in which were > operated a large number of file renames, in which many of the renamed files > (in this particular case all) have the same content but different names. > > The commit data from the renaming operation (last commit in the script- > generated history) is inexactly rendered by the command > > git diff-tree -r -C master > > The logical result is correctly produced by the more restricted command > > git diff-tree -r -M master > > IMO for this particular last commit both the above commands should return the > same result. Interesting. I get the same results from both commands. But I did have to munge your script, as my "rename" command does not seem to work like the one you expect in your script. So I may have misinterpreted the intent of it. However, I would not be surprised if one could conduct a situation in which "-C" and "-M" produced different results. Since the content of all the files is the same, git has to make a guess about which files match up based on their filenames. The current heuristic is very stupid and just tries to match basenames (e.g., moving "foo/Makefile" to "bar/Makefile" is a better match than moving the same content to "bar/foo.c"). But in this case, the basenames don't match at all. By using "-C", we will typically have more rename sources available, and we may therefore process the possible pairs in a different order. Since our name heuristic is largely useless, our results depend on that order. I think the real solution is to improve the name heuristic. Something like an edit distance would make more sense (though I think it is not as simple as an edit distance across the whole pathname, as moving a basename across directories should probably be preferred to changing the filename inside a directory). Largely I think nobody has cared much because this only comes up when you move multiple identical files. Quite often there is a minor difference even between very similar files, and that is enough to come up with sane results. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/6] Color skipped tests blue
On Wed, Sep 19, 2012 at 09:24:23PM +0100, Adam Spiers wrote: > t/test-lib.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 5293830..78c88c2 100755 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -182,13 +182,13 @@ then > error) > tput bold; tput setaf 1;; # bold red > skip) > - tput bold; tput setaf 2;; # bold green > + tput setaf 4;;# blue > warn) > tput bold; tput setaf 3;; # bold yellow > pass) > tput setaf 2;;# green > info) > - tput setaf 3;;# brown > + tput setaf 3;;# yellow/brown I happened to be running a test script with "-v" earlier today, and I noticed that the "expecting success..." dump of the test contents is also yellow. By your new rules, shouldn't it be blue? I think it is matching the "info" type, which from the discussion should be blue, no? Maybe it is just my terminal. I see it is labeled as "brown" here, but it looks very yellow (and I am using the stock xterm colors. According to: https://en.wikipedia.org/wiki/ANSI_colors It looks it really is brown on some platforms. I'm not sure if it is worth worrying about. I don't really want to get into configurable colors just for the test-suite output. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
