Watchman/inotify support and other ways to speed up git status
Hi everyone, I am starting to investigate ways to speed up git status and other git commands for Booking.com (thanks to AEvar) and I'd be happy to discuss the current status or be pointed to relevant documentation or mailing list threads. >From the threads below ([0], [1], [2], [3], [4], [5], [6], [7], [8]) I understand that the status is roughly the following: - instead of working on inotify support it's better to work on using a cross platform tool like Watchman - instead of working on Watchman support it is better to work first on caching information in the index - git update-index --untracked-cache has been developed by Duy and others and merged to master in May 2015 to cache untracked status in the index; it is still considered experimental - git index-helper has been worked on by Duy but its status is not clear (at least to me) Is that correct? What are the possible/planned next steps in this area? improving --untracked-cache? git index-helper? watchman support? Thanks, Christian. [0] March 8 2015: [PATCH 00/24] nd/untracked-cache updates http://thread.gmane.org/gmane.comp.version-control.git/265053/ [1] November 11 2014: [RFC] On watchman support http://thread.gmane.org/gmane.comp.version-control.git/259399/ [2] October 27 2014:[PATCH 00/19] Untracked cache to speed up "git status" http://thread.gmane.org/gmane.comp.version-control.git/258766 [3] July 28 2014: [PATCH v3 0/9] Speed up cache loading time http://thread.gmane.org/gmane.comp.version-control.git/254314/ [4] May 7 2014: [PATCH 00/20] Untracked cache to speed up "git status" http://thread.gmane.org/gmane.comp.version-control.git/248306 [5] May 2 2014: Watchman support for git http://thread.gmane.org/gmane.comp.version-control.git/248004/ [6] March 10 2014: http://git.661346.n2.nabble.com/question-about-Facebook-makes-Mercurial-faster-than-Git-tt7605273.html#a7605280 [7] January 29 2014: http://git.661346.n2.nabble.com/inotify-support-nearly-there-tt7602739.html [8] January 12 2014: http://git.661346.n2.nabble.com/PATCH-0-6-inotify-support-tt7601877.html#a7603955 -- 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 2/2] blame: allow blame --reverse --first-parent when it makes sense
Max Kirillov writes:
> Do not die immediately when the two flags are specified. Instead
> check that the specified range is along first-parent chain. Exploit
> how prepare_revision_walk() handles first_parent_only flag: the commits
> outside of first-parent chain are either unknown (and do not have any
> children recorded) or appear as non-first parent of a commit along the
> first-parent chain.
>
> Since the check seems fragile, add test which verifies that blame dies
> in both cases.
It is not quite clear in what way the "check seems fragile".
It is either "correct" or "appears to have worked by chance and
nobody has any confidence that it would tell if 'it makes sense'
reliably", and the latter cannot be papered over with any number of
tests.
The logic you implemented feels solid to me, at least at a first
glance. What kind of gotchas are you worried about?
> Signed-off-by: Max Kirillov
> ---
> builtin/blame.c | 11 +--
> t/t8009-blame-reverse.sh | 7 ++-
> 2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 295ce92..27de544 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2692,8 +2692,6 @@ parse_done:
> }
> else if (contents_from)
> die("--contents and --children do not blend well.");
> - else if (revs.first_parent_only)
> - die("combining --first-parent and --reverse is not supported");
> else {
> final_commit_name = prepare_initial(&sb);
> sb.commits.compare = compare_commits_by_reverse_commit_date;
> @@ -2721,6 +2719,15 @@ parse_done:
> if (prepare_revision_walk(&revs))
> die(_("revision walk setup failed"));
>
> + if (reverse && revs.first_parent_only) {
> + struct commit_list *final_children =
> lookup_decoration(&revs.children,
> +
> &sb.final->object);
> + if (!final_children ||
> + hashcmp(final_children->item->parents->item->object.sha1,
> + sb.final->object.sha1))
> + die("--reverse --first-parent together require range along
> first-parent chain");
> + }
> +
> if (is_null_sha1(sb.final->object.sha1)) {
> o = sb.final->util;
> sb.final_buf = xmemdupz(o->file.ptr, o->file.size);
> diff --git a/t/t8009-blame-reverse.sh b/t/t8009-blame-reverse.sh
> index 9f40613..042863b 100755
> --- a/t/t8009-blame-reverse.sh
> +++ b/t/t8009-blame-reverse.sh
> @@ -24,11 +24,16 @@ test_expect_failure 'blame --reverse finds B1, not C1' '
> test_cmp expect actual
> '
>
> -test_expect_failure 'blame --reverse --first-parent finds A1' '
> +test_expect_success 'blame --reverse --first-parent finds A1' '
> git blame --porcelain --reverse --first-parent A0..A3 -- file.t
> >actual_full &&
> head -1 actual &&
> git rev-parse A1 >expect &&
> test_cmp expect actual
> '
>
> +test_expect_success 'blame --reverse --first-parse dies if no first parent
> chain' '
> + test_must_fail git blame --porcelain --reverse --first-parent B1..A3 --
> file.t &&
> + test_must_fail git blame --porcelain --reverse --first-parent B2..A3 --
> file.t
> + '
> +
> test_done
--
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 1/2] Add test to describe expectation of blame --reverse with branched history
Max Kirillov writes: > If history contains merges from feature branches, `blame --reverse` > reports not the commit when the line was actually edited, but head of > the last merged branch which was created before the edit. > > As a workaround, `blame --reverse --first-parent` could be used to find > the merge of branch containing the edit, but it was disabled in > 95a4fb0eac, because incorrectly specified range could produce in > unexpected and meaningless result. > > Add tests which describe ideal functionality with and without > `--first-parent`. > > Signed-off-by: Max Kirillov > --- > t/t8009-blame-reverse.sh | 34 ++ > 1 file changed, 34 insertions(+) > create mode 100755 t/t8009-blame-reverse.sh > > diff --git a/t/t8009-blame-reverse.sh b/t/t8009-blame-reverse.sh > new file mode 100755 > index 000..9f40613 > --- /dev/null > +++ b/t/t8009-blame-reverse.sh > @@ -0,0 +1,34 @@ > +#!/bin/sh > + > +test_description='git blame reverse' > +. ./test-lib.sh > + If you are going to dedicate the whole test script for this test, could you draw the topology of the commit DAG here, so that readers can more easily follow what is going on? > +test_expect_success setup ' > + test_commit A0 file.t line0 && > + test_commit A1 && > + git reset --hard A0 && > + test_commit B1 && > + test_commit B2 file.t line0changed && > + git reset --hard A1 && > + test_merge A2 B2 && > + git reset --hard A1 && > + test_commit C1 && > + git reset --hard A2 && > + test_merge A3 C1 > + ' Let me try. (1) for merges, an edge with '*' denotes the one to the first parent. (2) a commit that touches file.t are in capital c1---a3 /* // A0---a1--*a2 \ / b1---B2 Did I draw it correctly? > +test_expect_failure 'blame --reverse finds B1, not C1' ' > + git blame --porcelain --reverse A0..A3 -- file.t >actual_full && > + head -1 actual && "head -n 1" is more POSIXy way to spell this. Don't get cute with sXXX in sed script when the usual s/// suffices; the only effect of the cuteness is to waste readers' time, who have to wonder if there is something interesting going on. > + git rev-parse B1 >expect && > + test_cmp expect actual > + ' > + > +test_expect_failure 'blame --reverse --first-parent finds A1' ' > + git blame --porcelain --reverse --first-parent A0..A3 -- file.t > >actual_full && > + head -1 actual && > + git rev-parse A1 >expect && > + test_cmp expect actual > + ' > + > +test_done -- 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 v3 2/2] blame: allow blame --reverse --first-parent when it makes sense
Do not die immediately when the two flags are specified. Instead
check that the specified range is along first-parent chain. Exploit
how prepare_revision_walk() handles first_parent_only flag: the commits
outside of first-parent chain are either unknown (and do not have any
children recorded) or appear as non-first parent of a commit along the
first-parent chain.
Since the check seems fragile, add test which verifies that blame dies
in both cases.
Signed-off-by: Max Kirillov
---
builtin/blame.c | 11 +--
t/t8009-blame-reverse.sh | 7 ++-
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 295ce92..eb348f0 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2692,8 +2692,6 @@ parse_done:
}
else if (contents_from)
die("--contents and --children do not blend well.");
- else if (revs.first_parent_only)
- die("combining --first-parent and --reverse is not supported");
else {
final_commit_name = prepare_initial(&sb);
sb.commits.compare = compare_commits_by_reverse_commit_date;
@@ -2721,6 +2719,15 @@ parse_done:
if (prepare_revision_walk(&revs))
die(_("revision walk setup failed"));
+ if (reverse && revs.first_parent_only) {
+ struct commit_list *final_children =
lookup_decoration(&revs.children,
+
&sb.final->object);
+ if (!final_children ||
+ hashcmp(final_children->item->parents->item->object.sha1,
+ sb.final->object.sha1))
+ die("--reverse --first-parent together require range
along first-parent chain");
+ }
+
if (is_null_sha1(sb.final->object.sha1)) {
o = sb.final->util;
sb.final_buf = xmemdupz(o->file.ptr, o->file.size);
diff --git a/t/t8009-blame-reverse.sh b/t/t8009-blame-reverse.sh
index 9f40613..042863b 100755
--- a/t/t8009-blame-reverse.sh
+++ b/t/t8009-blame-reverse.sh
@@ -24,11 +24,16 @@ test_expect_failure 'blame --reverse finds B1, not C1' '
test_cmp expect actual
'
-test_expect_failure 'blame --reverse --first-parent finds A1' '
+test_expect_success 'blame --reverse --first-parent finds A1' '
git blame --porcelain --reverse --first-parent A0..A3 -- file.t
>actual_full &&
head -1 actual &&
git rev-parse A1 >expect &&
test_cmp expect actual
'
+test_expect_success 'blame --reverse --first-parse dies if no first parent
chain' '
+ test_must_fail git blame --porcelain --reverse --first-parent B1..A3 --
file.t &&
+ test_must_fail git blame --porcelain --reverse --first-parent B2..A3 --
file.t
+ '
+
test_done
--
2.3.4.2801.g3d0809b
--
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 v3 0/2] blame: allow blame --reverse --first-parent when it makes sense
Fix indentation. Max Kirillov (2): Add test to describe expectation of blame --reverse with branched history blame: allow blame --reverse --first-parent when it makes sense builtin/blame.c | 11 +-- t/t8009-blame-reverse.sh | 39 +++ 2 files changed, 48 insertions(+), 2 deletions(-) create mode 100755 t/t8009-blame-reverse.sh -- 2.3.4.2801.g3d0809b -- 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 v3 1/2] Add test to describe expectation of blame --reverse with branched history
If history contains merges from feature branches, `blame --reverse` reports not the commit when the line was actually edited, but head of the last merged branch which was created before the edit. As a workaround, `blame --reverse --first-parent` could be used to find the merge of branch containing the edit, but it was disabled in 95a4fb0eac, because incorrectly specified range could produce in unexpected and meaningless result. Add tests which describe ideal functionality with and without `--first-parent`. Signed-off-by: Max Kirillov --- t/t8009-blame-reverse.sh | 34 ++ 1 file changed, 34 insertions(+) create mode 100755 t/t8009-blame-reverse.sh diff --git a/t/t8009-blame-reverse.sh b/t/t8009-blame-reverse.sh new file mode 100755 index 000..9f40613 --- /dev/null +++ b/t/t8009-blame-reverse.sh @@ -0,0 +1,34 @@ +#!/bin/sh + +test_description='git blame reverse' +. ./test-lib.sh + +test_expect_success setup ' + test_commit A0 file.t line0 && + test_commit A1 && + git reset --hard A0 && + test_commit B1 && + test_commit B2 file.t line0changed && + git reset --hard A1 && + test_merge A2 B2 && + git reset --hard A1 && + test_commit C1 && + git reset --hard A2 && + test_merge A3 C1 + ' + +test_expect_failure 'blame --reverse finds B1, not C1' ' + git blame --porcelain --reverse A0..A3 -- file.t >actual_full && + head -1 actual && + git rev-parse B1 >expect && + test_cmp expect actual + ' + +test_expect_failure 'blame --reverse --first-parent finds A1' ' + git blame --porcelain --reverse --first-parent A0..A3 -- file.t >actual_full && + head -1 actual && + git rev-parse A1 >expect && + test_cmp expect actual + ' + +test_done -- 2.3.4.2801.g3d0809b -- 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] blame: allow blame --reverse --first-parent when it makes sense
On Wed, Oct 21, 2015 at 12:29:12AM -0400, Eric Sunshine wrote: > On Wed, Oct 21, 2015 at 12:08 AM, Max Kirillov wrote: >> Do not die immediately when the two flags are specified. Instead >> check that the specified range is along first-parent chain. Explioit > > s/Explioit/Exploit/ Fixed. Thanks. -- Max -- 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 0/2] blame: allow blame --reverse --first-parent when it makes sense
Fixed mistype in commit message. Max Kirillov (2): Add test to describe expectation of blame --reverse with branched history blame: allow blame --reverse --first-parent when it makes sense builtin/blame.c | 11 +-- t/t8009-blame-reverse.sh | 39 +++ 2 files changed, 48 insertions(+), 2 deletions(-) create mode 100755 t/t8009-blame-reverse.sh -- 2.3.4.2801.g3d0809b -- 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 2/2] blame: allow blame --reverse --first-parent when it makes sense
Do not die immediately when the two flags are specified. Instead
check that the specified range is along first-parent chain. Exploit
how prepare_revision_walk() handles first_parent_only flag: the commits
outside of first-parent chain are either unknown (and do not have any
children recorded) or appear as non-first parent of a commit along the
first-parent chain.
Since the check seems fragile, add test which verifies that blame dies
in both cases.
Signed-off-by: Max Kirillov
---
builtin/blame.c | 11 +--
t/t8009-blame-reverse.sh | 7 ++-
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 295ce92..27de544 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2692,8 +2692,6 @@ parse_done:
}
else if (contents_from)
die("--contents and --children do not blend well.");
- else if (revs.first_parent_only)
- die("combining --first-parent and --reverse is not supported");
else {
final_commit_name = prepare_initial(&sb);
sb.commits.compare = compare_commits_by_reverse_commit_date;
@@ -2721,6 +2719,15 @@ parse_done:
if (prepare_revision_walk(&revs))
die(_("revision walk setup failed"));
+ if (reverse && revs.first_parent_only) {
+ struct commit_list *final_children =
lookup_decoration(&revs.children,
+
&sb.final->object);
+ if (!final_children ||
+ hashcmp(final_children->item->parents->item->object.sha1,
+ sb.final->object.sha1))
+ die("--reverse --first-parent together require range along
first-parent chain");
+ }
+
if (is_null_sha1(sb.final->object.sha1)) {
o = sb.final->util;
sb.final_buf = xmemdupz(o->file.ptr, o->file.size);
diff --git a/t/t8009-blame-reverse.sh b/t/t8009-blame-reverse.sh
index 9f40613..042863b 100755
--- a/t/t8009-blame-reverse.sh
+++ b/t/t8009-blame-reverse.sh
@@ -24,11 +24,16 @@ test_expect_failure 'blame --reverse finds B1, not C1' '
test_cmp expect actual
'
-test_expect_failure 'blame --reverse --first-parent finds A1' '
+test_expect_success 'blame --reverse --first-parent finds A1' '
git blame --porcelain --reverse --first-parent A0..A3 -- file.t
>actual_full &&
head -1 actual &&
git rev-parse A1 >expect &&
test_cmp expect actual
'
+test_expect_success 'blame --reverse --first-parse dies if no first parent
chain' '
+ test_must_fail git blame --porcelain --reverse --first-parent B1..A3 --
file.t &&
+ test_must_fail git blame --porcelain --reverse --first-parent B2..A3 --
file.t
+ '
+
test_done
--
2.3.4.2801.g3d0809b
--
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 1/2] Add test to describe expectation of blame --reverse with branched history
If history contains merges from feature branches, `blame --reverse` reports not the commit when the line was actually edited, but head of the last merged branch which was created before the edit. As a workaround, `blame --reverse --first-parent` could be used to find the merge of branch containing the edit, but it was disabled in 95a4fb0eac, because incorrectly specified range could produce in unexpected and meaningless result. Add tests which describe ideal functionality with and without `--first-parent`. Signed-off-by: Max Kirillov --- t/t8009-blame-reverse.sh | 34 ++ 1 file changed, 34 insertions(+) create mode 100755 t/t8009-blame-reverse.sh diff --git a/t/t8009-blame-reverse.sh b/t/t8009-blame-reverse.sh new file mode 100755 index 000..9f40613 --- /dev/null +++ b/t/t8009-blame-reverse.sh @@ -0,0 +1,34 @@ +#!/bin/sh + +test_description='git blame reverse' +. ./test-lib.sh + +test_expect_success setup ' + test_commit A0 file.t line0 && + test_commit A1 && + git reset --hard A0 && + test_commit B1 && + test_commit B2 file.t line0changed && + git reset --hard A1 && + test_merge A2 B2 && + git reset --hard A1 && + test_commit C1 && + git reset --hard A2 && + test_merge A3 C1 + ' + +test_expect_failure 'blame --reverse finds B1, not C1' ' + git blame --porcelain --reverse A0..A3 -- file.t >actual_full && + head -1 actual && + git rev-parse B1 >expect && + test_cmp expect actual + ' + +test_expect_failure 'blame --reverse --first-parent finds A1' ' + git blame --porcelain --reverse --first-parent A0..A3 -- file.t >actual_full && + head -1 actual && + git rev-parse A1 >expect && + test_cmp expect actual + ' + +test_done -- 2.3.4.2801.g3d0809b -- 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
Simultaneous Interpreting into Chinese
Dear git, I am honored to introduce Asian Language Solutions to you as a representative of ALS. Asian Langauge Solutions focuses on providing Asian language solutions to all of you around the world. From translation to interpretation, we are experts in every section we are involved in. Our areas of expertise include law, education, business, literature, engineering, technical, agriculture,environment, gneral articles, etc. Besides, with our own equipments(BOSCH II) for interpreting use, we could offer you one-stop service in interpreting. If you are interested in our services and want to know more please contact me at i...@asianlanguagesolutions.com best wishes, Allen
[PATCH v4 00/35] libify mailinfo and call it directly from am
The change in the endgame since v3 ($gmane/279832) is almost none
(interdiff attached at the end), so I won't be sending the patches
themselves. The bulk out of the miniscule interdiff comes from the
fifth "plug strbuf leak" patch.
The patches have been reordered to make the structure of the series
clearer:
Preliminary fixes:
mailinfo: remove a no-op call convert_to_utf8(it, "")
mailinfo: fold decode_header_bq() into decode_header()
mailinfo: fix an off-by-one error in the boundary stack
mailinfo: explicitly close file handle to the patch output
mailinfo: plug strbuf leak during continuation line handling
Group related things together and remove forward declarations:
mailinfo: move handle_boundary() lower
mailinfo: move read_one_header_line() closer to its callers
mailinfo: move check_header() after the helpers it uses
mailinfo: move cleanup_space() before its users
mailinfo: move definition of MAX_HDR_PARSED closer to its use
Preliminary logic clean-up:
mailinfo: get rid of function-local static states
mailinfo: do not let handle_body() touch global "line" directly
mailinfo: do not let handle_boundary() touch global "line" directly
mailinfo: do not let find_boundary() touch global "line" directly
mailinfo: move global "line" into mailinfo() function
Getting rid of the globals:
mailinfo: introduce "struct mailinfo" to hold globals
mailinfo: move keep_subject & keep_non_patch_bracket to struct mailinfo
mailinfo: move global "FILE *fin, *fout" to struct mailinfo
mailinfo: move filter/header stage to struct mailinfo
mailinfo: move patch_lines to struct mailinfo
mailinfo: move add_message_id and message_id to struct mailinfo
mailinfo: move use_scissors and use_inbody_headers to struct mailinfo
mailinfo: move metainfo_charset to struct mailinfo
mailinfo: move check for metainfo_charset to convert_to_utf8()
mailinfo: move transfer_encoding to struct mailinfo
mailinfo: move charset to struct mailinfo
mailinfo: move cmitmsg and patchfile to struct mailinfo
mailinfo: move [ps]_hdr_data to struct mailinfo
mailinfo: move content/content_top to struct mailinfo
Libify:
mailinfo: handle_commit_msg() shouldn't be called after finding patchbreak
mailinfo: keep the parsed log message in a strbuf
mailinfo: libify
Lifting the error handling to the caller:
mailinfo: handle charset conversion errors in the caller
mailinfo: remove calls to exit() and die() deep in the callchain
Endgame:
am: make direct call to mailinfo
Makefile |1 +
builtin/am.c | 42 ++-
builtin/mailinfo.c | 1055 +---
mailinfo.c | 1037 +++
mailinfo.h | 41 ++
5 files changed, 1122 insertions(+), 1054 deletions(-)
create mode 100644 mailinfo.c
create mode 100644 mailinfo.h
--
2.6.2-377-g450896c
(Interdiff)
diff --git a/mailinfo.c b/mailinfo.c
index 49eaa99..e157ca6 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -729,6 +729,8 @@ static int is_rfc2822_header(const struct strbuf *line)
static int read_one_header_line(struct strbuf *line, FILE *in)
{
+ struct strbuf continuation = STRBUF_INIT;
+
/* Get the first part of the line. */
if (strbuf_getline(line, in, '\n'))
return 0;
@@ -750,7 +752,6 @@ static int read_one_header_line(struct strbuf *line, FILE
*in)
*/
for (;;) {
int peek;
- struct strbuf continuation = STRBUF_INIT;
peek = fgetc(in); ungetc(peek, in);
if (peek != ' ' && peek != '\t')
@@ -761,6 +762,7 @@ static int read_one_header_line(struct strbuf *line, FILE
*in)
strbuf_rtrim(&continuation);
strbuf_addbuf(line, &continuation);
}
+ strbuf_release(&continuation);
return 1;
}
diff --git a/mailinfo.h b/mailinfo.h
index 5bf257d..93776a7 100644
--- a/mailinfo.h
+++ b/mailinfo.h
@@ -14,7 +14,7 @@ struct mailinfo {
int keep_non_patch_brackets_in_subject;
int add_message_id;
int use_scissors;
- int use_inbody_headers; /* defaults to 1 */
+ int use_inbody_headers;
const char *metainfo_charset;
struct strbuf *content[MAX_BOUNDARIES];
--
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] fix flaky untracked-cache test
Looks good to me, Ack. Test run with 74301d6 + my TravisCI patch: https://travis-ci.org/larsxschneider/git/builds/86702932 ... on Linux it failed in 1/2 cases after 53min ... on OSX it failed in 2/2 cases after 6min Test run with 74301d6 + my TravisCI patch + David's t7063 patch: https://travis-ci.org/larsxschneider/git/builds/86707133 .. on Linux it failed in 0/2 cases after 77min ...on OSX it failed in 0/2 cases after 48min (no error, CI system timed out, click on the builds to see detailed log output) Cheers, Lars On 19 Oct 2015, at 21:48, David Turner wrote: > Dirty the test worktree's root directory, as the test expects. > > When testing the untracked-cache, we previously assumed that checking > out master would be sufficient to mark the mtime of the worktree's > root directory as racily-dirty. But sometimes, the checkout would > happen at 12345.999 seconds and the status at 12346.001 seconds, > meaning that the worktree's root directory would not be racily-dirty. > And since it was not truly dirty, occasionally the test would fail. > By making the root truly dirty, the test will always succeed. > > Tested by running a few hundred times. > > Signed-off-by: David Turner > --- > t/t7063-status-untracked-cache.sh | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/t/t7063-status-untracked-cache.sh > b/t/t7063-status-untracked-cache.sh > index 37a24c1..0e8d0d4 100755 > --- a/t/t7063-status-untracked-cache.sh > +++ b/t/t7063-status-untracked-cache.sh > @@ -412,7 +412,9 @@ test_expect_success 'create/modify files, some of which > are gitignored' ' > echo two bis >done/two && > echo three >done/three && # three is gitignored > echo four >done/four && # four is gitignored at a higher level > - echo five >done/five # five is not gitignored > + echo five >done/five && # five is not gitignored > + echo test >base && #we need to ensure that the root dir is touched > + rm base > ' > > test_expect_success 'test sparse status with untracked cache' ' > -- > 2.4.2.644.g97b850b-twtrsrc > -- 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 8/8] git submodule update: Have a dedicated helper for cloning
On Wed, Oct 21, 2015 at 2:23 PM, Junio C Hamano wrote:
> Stefan Beller writes:
>
>> I'd like to counter your argument with quoting code from update_clone
>> method:
>>
>> run_processes_parallel(1, get_next_task, start_failure,
>> task_finished, &pp);
>>
>> if (pp.print_unmatched) {
>> printf("#unmatched\n");
>> return 1;
>> }
>>
>> for_each_string_list_item(item, &pp.projectlines) {
>> utf8_fprintf(stdout, "%s", item->string);
>> }
>>
>> So we do already all the cloning first, and then once we did all of that
>> we just put out all accumulated lines of text. (It was harder to come up with
>> a sufficient file name than just storing it in memory. I don't think
>> memory is an
>> issue here, only a few bytes per submodule. So even 1000 submodules would
>> consume maybe 100kB)
>
> That does not sound like a counter-argument; two bad design choices
> compensating each other's shortcomings, perhaps ;-)
I was phrasing it worse than I meant to. I should have pointed out the
positive aspect of having first all clones done and then the
local post processing in the downstream pipe afterwards.
>
>> Having a file though would allow us to continue after human
>> interaction fixed one problem.
>
> Yes. That does sound like a better design.
I don't think the proposed patches make it worse than it already is.
Before we have the "submodule--helper list" which tells downpipe to
do all the things. Now we just take out the cloning and make it an
upfront action, eventually faster due to parallelism.
Also I think we should not promote "git submodule" and specially
its update sub-command to be the best command available. Ideally
we want to rather implement implicit submodule handling in the
other commands such as clone, pull, fetch, checkout, merge, rebase.
and by that I mean better defaults than just "don't touch the submodules,
as that's the safest thing to do now".
>
> This obviously depends on the impact to the other part of what
> cmd_update() does, but your earlier idea to investigate the
> feasibility and usefulness of updating "clone --recurse-submodules"
> does sound like a good thing to do, too. That's an excellent point.
I investigated and I think it's a bad idea now :)
Because of the --recursive switch we would need to do more than just
submodules_init()
run_parallel(.. clone_and_checkout...);
but each cloned submodule would need to be inspected for recursive
submodules again and then we would need to add that to the list of
submodules to process.
I estimate this is about as much work as improving "git submodule update"
to do uncontroversial checkouts in the first parallel phase.
--
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
gitweb not running as expected
Dear All, I am trying to install GIT on web. Running "git instaweb" gives me this? $ git instaweb --httpd=lighttpd Instance already running. Restarting... No known browser available. http://127.0.0.1:1234 Any idea? What I need to do on my GIT server? Thanks -Gaurav -- View this message in context: http://git.661346.n2.nabble.com/gitweb-not-running-as-expected-tp7641718.html Sent from the git mailing list archive at Nabble.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
Re: [PATCH 8/8] git submodule update: Have a dedicated helper for cloning
Stefan Beller writes:
> I'd like to counter your argument with quoting code from update_clone
> method:
>
> run_processes_parallel(1, get_next_task, start_failure,
> task_finished, &pp);
>
> if (pp.print_unmatched) {
> printf("#unmatched\n");
> return 1;
> }
>
> for_each_string_list_item(item, &pp.projectlines) {
> utf8_fprintf(stdout, "%s", item->string);
> }
>
> So we do already all the cloning first, and then once we did all of that
> we just put out all accumulated lines of text. (It was harder to come up with
> a sufficient file name than just storing it in memory. I don't think
> memory is an
> issue here, only a few bytes per submodule. So even 1000 submodules would
> consume maybe 100kB)
That does not sound like a counter-argument; two bad design choices
compensating each other's shortcomings, perhaps ;-)
> Having a file though would allow us to continue after human
> interaction fixed one problem.
Yes. That does sound like a better design.
This obviously depends on the impact to the other part of what
cmd_update() does, but your earlier idea to investigate the
feasibility and usefulness of updating "clone --recurse-submodules"
does sound like a good thing to do, too. That's an excellent point.
--
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 24/34] mailinfo: move content/content_top to struct mailinfo
On Wed, Oct 21, 2015 at 2:04 PM, Junio C Hamano wrote: > > Looking at their final resting place, I do not think so. Right, I comment along the way without looking ahead, so this was a bad comment. -- 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/8] run-command: Call get_next_task with a clean child process.
On Wed, Oct 21, 2015 at 1:30 PM, Junio C Hamano wrote: > Junio C Hamano writes: > >> And of course we already have these array-clear calls in >> finish_command(). >> >> So I agree that deinit helper should exist, but >> >> * it should be file-scope static; >> >> * it should be called by finish_command(); and >> >> * if you are calling it from collect_finished(), then existing >>calls to array-clear should go. >> >> Other than that, this looks good. > > I'll queue this instead (the above squashed in and description > corrected). > Thanks, Stefan -- 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 8/8] git submodule update: Have a dedicated helper for cloning
On Wed, Oct 21, 2015 at 1:47 PM, Junio C Hamano wrote:
> Stefan Beller writes:
>
>> This introduces a new helper function in git submodule--helper
>> which takes care of cloning all submodules, which we want to
>> parallelize eventually.
>>
>> Some tests (such as empty URL, update_mode==none) are required in the
>> helper to make the decision for cloning. These checks have been moved
>> into the C function as well. (No need to repeat them in the shell
>> script)
>>
>> As we can only access the stderr channel from within the parallel
>> processing engine, so we need to reroute the error message for
>> specified but initialized submodules to stderr. As it is an error
>> message, this should have gone to stderr in the first place, so a
>> bug fix along the way.
>
> The last paragraph is hard to parse; perhaps it is slightly
> ungrammatical.
I seem to have started a habit starting my sentences with "so..."
even in spoken English. If left out, this may be easier to read:
As we can only access the stderr channel from within the parallel
processing engine, we need to reroute the error message for
"specified but initialized submodules" to stderr. As it is an error
message, this should have gone to stderr in the first place.
It's a bug fix along the way.
>
> It would be a really good idea to split the small bit to redirect
> the output that should have gone to the standard error to where it
> should as a preparatory step before showing this patch.
ok.
>
> I sense that this one is still a WIP/RFC, so I'll only skim it in
> this round (but I may come back and read it again later with finer
> toothed comb).
>
>> +static int get_next_task(void **pp_task_cb,
>> + struct child_process *cp,
>> + struct strbuf *err,
>> + void *pp_cb)
>
> Will you have only one caller of the parallel run-command API in
> this file, or will you be adding more to allow various different
> operations run in parallel as more things are rewritten? I am
> guessing that it would be the latter, but if that is the case,
> perhaps the function wants to be named a bit more specificly for
> this first user, no? Same for start_failure and task_finished.
Ok, will rename.
Although I am not sure if I need to rewrite more in C for "git submodule".
I only rewrite git submodule update because git clone --recurse is just
blindly calling out to git submodule update. So instead of parallelizing
"submodule update" I could have put a parallel submodule clone into
the clone command. (That looks strangely appealing now, because it
may be even faster as there is no downstream pipe with sequential
checkouts, so you could have one parallel pool with chained clone
and checkout commands).
>
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index 8b0eb9a..ea883b9 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -655,17 +655,18 @@ cmd_update()
>> cmd_init "--" "$@" || return
>> fi
>>
>> - cloned_modules=
>> - git submodule--helper list --prefix "$wt_prefix" "$@" | {
>> + git submodule--helper update-clone ${GIT_QUIET:+--quiet} \
>> + ${wt_prefix:+--prefix "$wt_prefix"} \
>> + ${prefix:+--recursive_prefix "$prefix"} \
>> + ${update:+--update "$update"} \
>> + ${reference:+--reference "$reference"} \
>> + ${depth:+--depth "$depth"} \
>> + "$@" | {
>> err=
>> - while read mode sha1 stage sm_path
>> + while read mode sha1 stage just_cloned sm_path
>> do
>
> I wonder if you really want this to be upstream of a pipe. When the
> downstream loop needs to abort, what happens to the remainder of the
> "clone" part of the processing that is still ongoing in the upstream
> of the pipe? I would imagine that the "update-clone" network
> accessing phase is the more human-time consuming part, so I suspect
> that it would be much better to let the cloning part go and finish
> first (during which time the human-user can spend time for other
> things, like getting cup of coffee or filling expense reports) and
> before moving to the loop that can stop and ask the human-user for
> help.
>
> The fix for the above could be trivial (do not pipe, just take the
> output to a temporary file, and then feed the "while read" loop from
> that temporary file), and I suspect it would make a big difference
> for usability.
I'd like to counter your argument with quoting code from update_clone
method:
run_processes_parallel(1, get_next_task, start_failure,
task_finished, &pp);
if (pp.print_unmatched) {
printf("#unmatched\n");
return 1;
}
for_each_string_list_item(item, &pp.projectlines) {
utf8_fprintf(stdout, "%s", item->string);
}
So we do already all the cloning first, and then once we did all of that
we just put out all accumulated lines of text. (It was harder to come up with
a sufficient file name than just s
Re: [PATCH v3 24/34] mailinfo: move content/content_top to struct mailinfo
Stefan Beller writes:
> On Mon, Oct 19, 2015 at 12:28 AM, Junio C Hamano wrote:
>> Signed-off-by: Junio C Hamano
>> ---
>> builtin/mailinfo.c | 45 ++---
>> 1 file changed, 26 insertions(+), 19 deletions(-)
>>
>> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
>> index 2c194da..ec65805 100644
>> --- a/builtin/mailinfo.c
>> +++ b/builtin/mailinfo.c
>> @@ -7,6 +7,8 @@
>> #include "utf8.h"
>> #include "strbuf.h"
>>
>> +#define MAX_BOUNDARIES 5
>> +
>> struct mailinfo {
>> FILE *input;
>> FILE *output;
>> @@ -22,6 +24,8 @@ struct mailinfo {
>> int use_inbody_headers; /* defaults to 1 */
>> const char *metainfo_charset;
>>
>> + struct strbuf *content[MAX_BOUNDARIES];
>> + struct strbuf **content_top;
>> struct strbuf charset;
>> char *message_id;
>> enum {
>> @@ -35,7 +39,6 @@ struct mailinfo {
>> };
>>
>> #define MAX_HDR_PARSED 10
>> -#define MAX_BOUNDARIES 5
>
> Would it make sense to also move MAX_HDR_PARSED, such that we have all
> defines in one place?
Looking at their final resting place, I do not think so.
--
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 17/34] mailinfo: move use_scissors and use_inbody_headers to struct mailinfo
Stefan Beller writes:
> On Mon, Oct 19, 2015 at 12:28 AM, Junio C Hamano wrote:
>> Signed-off-by: Junio C Hamano
>> ---
>> builtin/mailinfo.c | 23 +--
>> 1 file changed, 13 insertions(+), 10 deletions(-)
>>
>> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
>> index c0522f2..2c8f249 100644
>> --- a/builtin/mailinfo.c
>> +++ b/builtin/mailinfo.c
>> @@ -20,6 +20,8 @@ struct mailinfo {
>> int keep_subject;
>> int keep_non_patch_brackets_in_subject;
>> int add_message_id;
>> + int use_scissors;
>> + int use_inbody_headers; /* defaults to 1 */
>
> IMHO there is no need for the comment here, stating its default.
> That can be looked up in the init function, which is as convenient as
> reading globals in a file?
It was crucial to ensure correctness during conversion, that is, a
conversion done in
(1) Remove "static int use_inbody_headers = 1"
(2) Add "int use_inbody_headers;"
(3) Update setup_mailinfo()
sequence would have lost a clue as to what the right value to assign
in step (3). Tweaking (2) to leave "/* this starts at 1 */" helped
avoid such a mistake.
When reading the end result, the above three appear to have been
done simultaneously, so I agree that the comment does not help.
--
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 8/8] git submodule update: Have a dedicated helper for cloning
Stefan Beller writes:
> This introduces a new helper function in git submodule--helper
> which takes care of cloning all submodules, which we want to
> parallelize eventually.
>
> Some tests (such as empty URL, update_mode==none) are required in the
> helper to make the decision for cloning. These checks have been moved
> into the C function as well. (No need to repeat them in the shell
> script)
>
> As we can only access the stderr channel from within the parallel
> processing engine, so we need to reroute the error message for
> specified but initialized submodules to stderr. As it is an error
> message, this should have gone to stderr in the first place, so a
> bug fix along the way.
The last paragraph is hard to parse; perhaps it is slightly
ungrammatical.
It would be a really good idea to split the small bit to redirect
the output that should have gone to the standard error to where it
should as a preparatory step before showing this patch.
I sense that this one is still a WIP/RFC, so I'll only skim it in
this round (but I may come back and read it again later with finer
toothed comb).
> +static int get_next_task(void **pp_task_cb,
> + struct child_process *cp,
> + struct strbuf *err,
> + void *pp_cb)
Will you have only one caller of the parallel run-command API in
this file, or will you be adding more to allow various different
operations run in parallel as more things are rewritten? I am
guessing that it would be the latter, but if that is the case,
perhaps the function wants to be named a bit more specificly for
this first user, no? Same for start_failure and task_finished.
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 8b0eb9a..ea883b9 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -655,17 +655,18 @@ cmd_update()
> cmd_init "--" "$@" || return
> fi
>
> - cloned_modules=
> - git submodule--helper list --prefix "$wt_prefix" "$@" | {
> + git submodule--helper update-clone ${GIT_QUIET:+--quiet} \
> + ${wt_prefix:+--prefix "$wt_prefix"} \
> + ${prefix:+--recursive_prefix "$prefix"} \
> + ${update:+--update "$update"} \
> + ${reference:+--reference "$reference"} \
> + ${depth:+--depth "$depth"} \
> + "$@" | {
> err=
> - while read mode sha1 stage sm_path
> + while read mode sha1 stage just_cloned sm_path
> do
I wonder if you really want this to be upstream of a pipe. When the
downstream loop needs to abort, what happens to the remainder of the
"clone" part of the processing that is still ongoing in the upstream
of the pipe? I would imagine that the "update-clone" network
accessing phase is the more human-time consuming part, so I suspect
that it would be much better to let the cloning part go and finish
first (during which time the human-user can spend time for other
things, like getting cup of coffee or filling expense reports) and
before moving to the loop that can stop and ask the human-user for
help.
The fix for the above could be trivial (do not pipe, just take the
output to a temporary file, and then feed the "while read" loop from
that temporary file), and I suspect it would make a big difference
for usability.
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
Condominium Owners List
Hi, Hope this email finds you well! Would you be interested in reaching out to "Condominium Owners List” for across USA with opt-in verified contact information? We also have the list of Home Owners, Pool Owners, Homes with Swimming Pool, Time-share Owners, Spa & Resort Visitors, HNI List and many more... Consumers Data fields on each record contains: Contact Name (First and Last name), Mailing Address, IP Address, Source, List Type and Opt-In email address. All the contacts are opt-in verified, 100% permission based and can be used for unlimited multi-channel marketing. If you can let me know target criteria,I can get back to you with proper response. Look forward to your prompt response. Best Regards, Scott Sisk Reply “Leaveout” if you do not wish to receive emails from us. --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus -- 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 24/34] mailinfo: move content/content_top to struct mailinfo
On Mon, Oct 19, 2015 at 12:28 AM, Junio C Hamano wrote:
> Signed-off-by: Junio C Hamano
> ---
> builtin/mailinfo.c | 45 ++---
> 1 file changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
> index 2c194da..ec65805 100644
> --- a/builtin/mailinfo.c
> +++ b/builtin/mailinfo.c
> @@ -7,6 +7,8 @@
> #include "utf8.h"
> #include "strbuf.h"
>
> +#define MAX_BOUNDARIES 5
> +
> struct mailinfo {
> FILE *input;
> FILE *output;
> @@ -22,6 +24,8 @@ struct mailinfo {
> int use_inbody_headers; /* defaults to 1 */
> const char *metainfo_charset;
>
> + struct strbuf *content[MAX_BOUNDARIES];
> + struct strbuf **content_top;
> struct strbuf charset;
> char *message_id;
> enum {
> @@ -35,7 +39,6 @@ struct mailinfo {
> };
>
> #define MAX_HDR_PARSED 10
> -#define MAX_BOUNDARIES 5
Would it make sense to also move MAX_HDR_PARSED, such that we have all
defines in one place? The previous patch moved HDR related stuff around, so
either there or here?
>
> static void cleanup_space(struct strbuf *sb);
>
> @@ -180,10 +183,6 @@ static int slurp_attr(const char *line, const char
> *name, struct strbuf *attr)
> return 1;
> }
>
> -static struct strbuf *content[MAX_BOUNDARIES];
> -
> -static struct strbuf **content_top = content;
> -
> static void handle_content_type(struct mailinfo *mi, struct strbuf *line)
> {
> struct strbuf *boundary = xmalloc(sizeof(struct strbuf));
> @@ -191,11 +190,11 @@ static void handle_content_type(struct mailinfo *mi,
> struct strbuf *line)
>
> if (slurp_attr(line->buf, "boundary=", boundary)) {
> strbuf_insert(boundary, 0, "--", 2);
> - if (++content_top >= &content[MAX_BOUNDARIES]) {
> + if (++mi->content_top >= &mi->content[MAX_BOUNDARIES]) {
> fprintf(stderr, "Too many boundaries to handle\n");
> exit(1);
> }
> - *content_top = boundary;
> + *(mi->content_top) = boundary;
> boundary = NULL;
> }
> slurp_attr(line->buf, "charset=", &mi->charset);
> @@ -223,10 +222,12 @@ static void handle_content_transfer_encoding(struct
> mailinfo *mi,
> mi->transfer_encoding = TE_DONTCARE;
> }
>
> -static int is_multipart_boundary(const struct strbuf *line)
> +static int is_multipart_boundary(struct mailinfo *mi, const struct strbuf
> *line)
> {
> - return (((*content_top)->len <= line->len) &&
> - !memcmp(line->buf, (*content_top)->buf, (*content_top)->len));
> + struct strbuf *content_top = *(mi->content_top);
> +
> + return ((content_top->len <= line->len) &&
> + !memcmp(line->buf, content_top->buf, content_top->len));
> }
>
> static void cleanup_subject(struct mailinfo *mi, struct strbuf *subject)
> @@ -799,7 +800,7 @@ static void handle_filter(struct mailinfo *mi, struct
> strbuf *line)
> static int find_boundary(struct mailinfo *mi, struct strbuf *line)
> {
> while (!strbuf_getline(line, mi->input, '\n')) {
> - if (*content_top && is_multipart_boundary(line))
> + if (*(mi->content_top) && is_multipart_boundary(mi, line))
> return 1;
> }
> return 0;
> @@ -811,18 +812,18 @@ static int handle_boundary(struct mailinfo *mi, struct
> strbuf *line)
>
> strbuf_addch(&newline, '\n');
> again:
> - if (line->len >= (*content_top)->len + 2 &&
> - !memcmp(line->buf + (*content_top)->len, "--", 2)) {
> + if (line->len >= (*(mi->content_top))->len + 2 &&
> + !memcmp(line->buf + (*(mi->content_top))->len, "--", 2)) {
> /* we hit an end boundary */
> /* pop the current boundary off the stack */
> - strbuf_release(*content_top);
> - free(*content_top);
> - *content_top = NULL;
> + strbuf_release(*(mi->content_top));
> + free(*(mi->content_top));
> + *(mi->content_top) = NULL;
>
> /* technically won't happen as is_multipart_boundary()
>will fail first. But just in case..
> */
> - if (--content_top < content) {
> + if (--mi->content_top < mi->content) {
> fprintf(stderr, "Detected mismatched boundaries, "
> "can't recover\n");
> exit(1);
> @@ -857,14 +858,14 @@ static void handle_body(struct mailinfo *mi, struct
> strbuf *line)
> struct strbuf prev = STRBUF_INIT;
>
> /* Skip up to the first boundary */
> - if (*content_top) {
> + if (*(mi->content_top)) {
> if (!find_boundary(mi, line))
> goto handle_body_out;
> }
>
> do {
>
Re: [PATCH v3 23/34] mailinfo: move [ps]_hdr_data to struct mailinfo
On Mon, Oct 19, 2015 at 12:28 AM, Junio C Hamano wrote:
> @@ -733,7 +733,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct
> strbuf *line)
> }
>
> if (mi->use_inbody_headers && mi->header_stage) {
> - mi->header_stage = check_header(mi, line, s_hdr_data, 0);
> + mi->header_stage = check_header(mi, line, mi->s_hdr_data, 0);
Would it make sense to get rid of the third argument for check_header instead?
We already pass in mi, so check_header can access s_hdr_data?
> if (mi->header_stage)
--
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/8] run-command: Call get_next_task with a clean child process.
Junio C Hamano writes:
> And of course we already have these array-clear calls in
> finish_command().
>
> So I agree that deinit helper should exist, but
>
> * it should be file-scope static;
>
> * it should be called by finish_command(); and
>
> * if you are calling it from collect_finished(), then existing
>calls to array-clear should go.
>
> Other than that, this looks good.
I'll queue this instead (the above squashed in and description
corrected).
-- >8 --
From: Stefan Beller
Date: Tue, 20 Oct 2015 15:43:44 -0700
Subject: [PATCH] run-command: clear leftover state from child_process structure
pp_start_one() function finds an unused child_process structure
passes it to start_command(), but the structure may have already
been used earlier. finish_command() has code to clear leftover
states in the structure so that it can be reused, but the parallel
execution machinery does not (and cannot) use it, and instead has
its own pp_collect_finished(). This function, after culling a
process that has just finished, forgot to clear the child_process
structure before marking it ready for reuse.
Introduce child_process_deinit() helper function that clears two
instances of argv_array (one for arg, the other for env) in the
structure, make the existing codepaths that clear them call the
helper instead (which in turn will make sure that we will not leak
resources later even when we add new fields to the structure), and
also add a call to it in pp_collect_finished() before the function
marks the structure read for reuse.
Signed-off-by: Stefan Beller
Signed-off-by: Junio C Hamano
---
run-command.c | 25 +++--
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/run-command.c b/run-command.c
index b9363da..684ccee 100644
--- a/run-command.c
+++ b/run-command.c
@@ -13,6 +13,12 @@ void child_process_init(struct child_process *child)
argv_array_init(&child->env_array);
}
+static void child_process_deinit(struct child_process *child)
+{
+ argv_array_clear(&child->args);
+ argv_array_clear(&child->env_array);
+}
+
struct child_to_clean {
pid_t pid;
struct child_to_clean *next;
@@ -338,8 +344,7 @@ int start_command(struct child_process *cmd)
fail_pipe:
error("cannot create %s pipe for %s: %s",
str, cmd->argv[0], strerror(failed_errno));
- argv_array_clear(&cmd->args);
- argv_array_clear(&cmd->env_array);
+ child_process_deinit(cmd);
errno = failed_errno;
return -1;
}
@@ -525,8 +530,7 @@ fail_pipe:
close_pair(fderr);
else if (cmd->err)
close(cmd->err);
- argv_array_clear(&cmd->args);
- argv_array_clear(&cmd->env_array);
+ child_process_deinit(cmd);
errno = failed_errno;
return -1;
}
@@ -552,8 +556,7 @@ fail_pipe:
int finish_command(struct child_process *cmd)
{
int ret = wait_or_whine(cmd->pid, cmd->argv[0]);
- argv_array_clear(&cmd->args);
- argv_array_clear(&cmd->env_array);
+ child_process_deinit(cmd);
return ret;
}
@@ -962,6 +965,7 @@ static struct parallel_processes *pp_init(int n,
for (i = 0; i < n; i++) {
strbuf_init(&pp->children[i].err, 0);
+ child_process_init(&pp->children[i].process);
pp->pfd[i].events = POLLIN;
pp->pfd[i].fd = -1;
}
@@ -973,8 +977,10 @@ static void pp_cleanup(struct parallel_processes *pp)
{
int i;
- for (i = 0; i < pp->max_processes; i++)
+ for (i = 0; i < pp->max_processes; i++) {
strbuf_release(&pp->children[i].err);
+ child_process_deinit(&pp->children[i].process);
+ }
free(pp->children);
free(pp->pfd);
@@ -1128,12 +1134,11 @@ static int pp_collect_finished(struct
parallel_processes *pp)
&pp->children[i].data))
result = 1;
- argv_array_clear(&pp->children[i].process.args);
- argv_array_clear(&pp->children[i].process.env_array);
-
pp->nr_processes--;
pp->children[i].in_use = 0;
pp->pfd[i].fd = -1;
+ child_process_deinit(&pp->children[i].process);
+ child_process_init(&pp->children[i].process);
if (i != pp->output_owner) {
strbuf_addbuf(&pp->buffered_output,
&pp->children[i].err);
--
2.6.2-394-gc0a4d5b
--
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] fix flaky untracked-cache test
On Mon, Oct 19, 2015 at 9:48 PM, David Turner wrote: > Dirty the test worktree's root directory, as the test expects. > > When testing the untracked-cache, we previously assumed that checking > out master would be sufficient to mark the mtime of the worktree's > root directory as racily-dirty. But sometimes, the checkout would > happen at 12345.999 seconds and the status at 12346.001 seconds, > meaning that the worktree's root directory would not be racily-dirty. > And since it was not truly dirty, occasionally the test would fail. > By making the root truly dirty, the test will always succeed. I'm sorry for my deadly silence lately. I hope it will end in a few weeks. From a quick glance, the description and the change look ok (but don't weigh too much on my opinion). Back to my silence.. -- 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
Re: [PATCH v3 17/34] mailinfo: move use_scissors and use_inbody_headers to struct mailinfo
On Mon, Oct 19, 2015 at 12:28 AM, Junio C Hamano wrote:
> Signed-off-by: Junio C Hamano
> ---
> builtin/mailinfo.c | 23 +--
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
> index c0522f2..2c8f249 100644
> --- a/builtin/mailinfo.c
> +++ b/builtin/mailinfo.c
> @@ -20,6 +20,8 @@ struct mailinfo {
> int keep_subject;
> int keep_non_patch_brackets_in_subject;
> int add_message_id;
> + int use_scissors;
> + int use_inbody_headers; /* defaults to 1 */
IMHO there is no need for the comment here, stating its default.
That can be looked up in the init function, which is as convenient as
reading globals in a file?
>
> char *message_id;
> int patch_lines;
> @@ -34,8 +36,6 @@ static enum {
> static struct strbuf charset = STRBUF_INIT;
>
> static struct strbuf **p_hdr_data, **s_hdr_data;
> -static int use_scissors;
> -static int use_inbody_headers = 1;
>
> #define MAX_HDR_PARSED 10
> #define MAX_BOUNDARIES 5
> @@ -734,7 +734,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct
> strbuf *line)
> return 0;
> }
>
> - if (use_inbody_headers && mi->header_stage) {
> + if (mi->use_inbody_headers && mi->header_stage) {
> mi->header_stage = check_header(mi, line, s_hdr_data, 0);
> if (mi->header_stage)
> return 0;
> @@ -748,7 +748,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct
> strbuf *line)
> if (metainfo_charset)
> convert_to_utf8(line, charset.buf);
>
> - if (use_scissors && is_scissors_line(line)) {
> + if (mi->use_scissors && is_scissors_line(line)) {
> int i;
> if (fseek(cmitmsg, 0L, SEEK_SET))
> die_errno("Could not rewind output message file");
> @@ -1009,12 +1009,14 @@ static int mailinfo(struct mailinfo *mi, const char
> *msg, const char *patch)
> return 0;
> }
>
> -static int git_mailinfo_config(const char *var, const char *value, void
> *unused)
> +static int git_mailinfo_config(const char *var, const char *value, void *mi_)
> {
> + struct mailinfo *mi = mi_;
> +
> if (!starts_with(var, "mailinfo."))
> - return git_default_config(var, value, unused);
> + return git_default_config(var, value, NULL);
> if (!strcmp(var, "mailinfo.scissors")) {
> - use_scissors = git_config_bool(var, value);
> + mi->use_scissors = git_config_bool(var, value);
> return 0;
> }
> /* perhaps others here */
> @@ -1027,6 +1029,7 @@ static void setup_mailinfo(struct mailinfo *mi)
> strbuf_init(&mi->name, 0);
> strbuf_init(&mi->email, 0);
> mi->header_stage = 1;
> + mi->use_inbody_headers = 1;
> git_config(git_mailinfo_config, &mi);
> }
>
> @@ -1068,11 +1071,11 @@ int cmd_mailinfo(int argc, const char **argv, const
> char *prefix)
> else if (starts_with(argv[1], "--encoding="))
> metainfo_charset = argv[1] + 11;
> else if (!strcmp(argv[1], "--scissors"))
> - use_scissors = 1;
> + mi.use_scissors = 1;
> else if (!strcmp(argv[1], "--no-scissors"))
> - use_scissors = 0;
> + mi.use_scissors = 0;
> else if (!strcmp(argv[1], "--no-inbody-headers"))
> - use_inbody_headers = 0;
> + mi.use_inbody_headers = 0;
> else
> usage(mailinfo_usage);
> argc--; argv++;
> --
> 2.6.2-383-g144b2e6
>
> --
> 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
--
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 14/34] mailinfo: move filter/header stage to struct mailinfo
On Mon, Oct 19, 2015 at 12:28 AM, Junio C Hamano wrote:
> @@ -28,6 +31,7 @@ static enum {
>
> static struct strbuf charset = STRBUF_INIT;
> static int patch_lines;
> +
nitpicking here. Why is this patch the best in series to introduce a
new line at this position? ;)
> static struct strbuf **p_hdr_data, **s_hdr_data;
> static int use_scissors;
> static int add_message_id;
> @@ -718,25 +722,25 @@ static int is_scissors_line(const struct strbuf *line)
> gap * 2 < perforation);
> }
--
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 00/34] libify mailinfo and call it directly from am
Johannes Sixt writes: > Am 21.10.2015 um 17:51 schrieb Ramsay Jones: >> On 20/10/15 22:24, Junio C Hamano wrote: >>> >>> time git am mbox >/dev/null >>> >>> are >>> >>>(master) (with the series) >>> real0m0.648sreal0m0.537s >>> user0m0.358suser0m0.338s >>> sys 0m0.172ssys 0m0.154s >>> >> >> The corresponding times for me were: >> >> (master) (with the series) >>real 0m9.760s real 0m5.744s >>user 0m0.531s user 0m0.656s >>sys 0m5.726s sys 0m3.520s >> >> So, yes, a noticeable improvement! :) > > Same here, on Windows built with the old msysgit environment: > > (master) (with the series) > real0m3.147s real0m1.947s > user0m0.016s user0m0.000s > sys 0m0.015s sys 0m0.031s > > Although I tested 'git am patches/*' where the patches/* are the > result of 'git-format-patch v2.6.1..github/jc/mailinfo-lib'. Thanks, both. -- 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 00/34] libify mailinfo and call it directly from am
Am 21.10.2015 um 17:51 schrieb Ramsay Jones: On 20/10/15 22:24, Junio C Hamano wrote: Junio C Hamano writes: some numbers on my desktop (Dell T3500 2.66GHz Xeon X5650 with 12GB, running Ubuntu), I suspect that I haven't tested exactly the same version as you, but I had a quick look at testing this on Cygwin today. I have included a complete transcript (below), so you can see what I did wrong! :-P Between 'master' and the version with this series (on 'jch'), applying this 34-patch series itself on top of 'master' using "git am", best of 5 numbers for running: time git am mbox >/dev/null are (master) (with the series) real0m0.648sreal0m0.537s user0m0.358suser0m0.338s sys 0m0.172ssys 0m0.154s The corresponding times for me were: (master) (with the series) real 0m9.760s real 0m5.744s user 0m0.531s user 0m0.656s sys 0m5.726s sys 0m3.520s So, yes, a noticeable improvement! :) Same here, on Windows built with the old msysgit environment: (master) (with the series) real0m3.147s real0m1.947s user0m0.016s user0m0.000s sys 0m0.015s sys 0m0.031s Although I tested 'git am patches/*' where the patches/* are the result of 'git-format-patch v2.6.1..github/jc/mailinfo-lib'. -- 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: [PATCH] git-p4: import the ctypes module
Hi Etienne, thanks for reporting this! Junio is right, I messed that up on my Windows testing box! :-( Sorry! If you have any questions around submitting patches I am happy to help as I just recently went through the learning process myself! @Dennis: Thanks for the quick patch! Thanks, Lars On 21 Oct 2015, at 10:23, Etienne Girard wrote: > Hello, > > I couldn't work further on this yesterday (but I read > Documentation/SubmittingPatches, which is a good start I guess). The > diff proposed by Dennis works on my machine, I'll try to figure out > why the original script worked with 2.7.10. > > Thanks > > 2015-10-21 1:00 GMT+02:00 Luke Diamand : >> On 20/10/15 20:36, Junio C Hamano wrote: >>> >>> Dennis Kaarsemaker writes: >>> > I do not follow Python development, but does the above mean that > with recent 2.x you can say ctypes without first saying "import > ctypes"? It feels somewhat non-pythonesque that identifiers like > this is given to you without you asking with an explicit 'import', > so I am puzzled. No, you cannot do that. The reason others may not have noticed this bug is that in git-p4.py, ctypes is only used on windows. 111 if platform.system() == 'Windows': 112 free_bytes = ctypes.c_ulonglong(0) 113 ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()), None, None, ctypes.pointer(free_bytes)) The fact that it works for the OP with 2.7.10 is puzzling (assuming that it's on the same system). >>> >>> >>> Exactly. That is where my "I am puzzled" comes from. >>> >>> The patch looks obviously the right thing to do. Luke? Lars? >> >> >> It looks sensible to me, and works fine on Linux, thanks. ack. >> >> I can't test on Windows today but I can't see why it wouldn't work. >> >> Luke >> >> -- 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 26/26] introduce "extensions" form of core.repositoryformatversion
David Turner writes: > From: Jeff King I just made sure this is bit-for-bit identical with the first of the two patches I received from Peff and I locally have kept. Re-reading the two patches again, I do not see a reason why we should reject them. I'll queue this and the other "precious object" one separately. Thanks. > > Normally we try to avoid bumps of the whole-repository > core.repositoryformatversion field. However, it is > unavoidable if we want to safely change certain aspects of > git in a backwards-incompatible way (e.g., modifying the set > of ref tips that we must traverse to generate a list of > unreachable, safe-to-prune objects). > > If we were to bump the repository version for every such > change, then any implementation understanding version `X` > would also have to understand `X-1`, `X-2`, and so forth, > even though the incompatibilities may be in orthogonal parts > of the system, and there is otherwise no reason we cannot > implement one without the other (or more importantly, that > the user cannot choose to use one feature without the other, > weighing the tradeoff in compatibility only for that > particular feature). > > This patch documents the existing repositoryformatversion > strategy and introduces a new format, "1", which lets a > repository specify that it must run with an arbitrary set of > extensions. This can be used, for example: > > - to inform git that the objects should not be pruned based >only on the reachability of the ref tips (e.g, because it >has "clone --shared" children) > > - that the refs are stored in a format besides the usual >"refs" and "packed-refs" directories > > Because we bump to format "1", and because format "1" > requires that a running git knows about any extensions > mentioned, we know that older versions of the code will not > do something dangerous when confronted with these new > formats. > > For example, if the user chooses to use database storage for > refs, they may set the "extensions.refbackend" config to > "db". Older versions of git will not understand format "1" > and bail. Versions of git which understand "1" but do not > know about "refbackend", or which know about "refbackend" > but not about the "db" backend, will refuse to run. This is > annoying, of course, but much better than the alternative of > claiming that there are no refs in the repository, or > writing to a location that other implementations will not > read. > > Note that we are only defining the rules for format 1 here. > We do not ever write format 1 ourselves; it is a tool that > is meant to be used by users and future extensions to > provide safety with older implementations. > > Signed-off-by: Jeff King > --- > Documentation/technical/repository-version.txt | 81 > ++ > cache.h| 6 ++ > setup.c| 37 +++- > t/t1302-repo-version.sh| 38 > 4 files changed, 159 insertions(+), 3 deletions(-) > create mode 100644 Documentation/technical/repository-version.txt > > diff --git a/Documentation/technical/repository-version.txt > b/Documentation/technical/repository-version.txt > new file mode 100644 > index 000..3d7106d > --- /dev/null > +++ b/Documentation/technical/repository-version.txt > @@ -0,0 +1,81 @@ > +Git Repository Format Versions > +== > + > +Every git repository is marked with a numeric version in the > +`core.repositoryformatversion` key of its `config` file. This version > +specifies the rules for operating on the on-disk repository data. An > +implementation of git which does not understand a particular version > +advertised by an on-disk repository MUST NOT operate on that repository; > +doing so risks not only producing wrong results, but actually losing > +data. > + > +Because of this rule, version bumps should be kept to an absolute > +minimum. Instead, we generally prefer these strategies: > + > + - bumping format version numbers of individual data files (e.g., > +index, packfiles, etc). This restricts the incompatibilities only to > +those files. > + > + - introducing new data that gracefully degrades when used by older > +clients (e.g., pack bitmap files are ignored by older clients, which > +simply do not take advantage of the optimization they provide). > + > +A whole-repository format version bump should only be part of a change > +that cannot be independently versioned. For instance, if one were to > +change the reachability rules for objects, or the rules for locking > +refs, that would require a bump of the repository format version. > + > +Note that this applies only to accessing the repository's disk contents > +directly. An older client which understands only format `0` may still > +connect via `git://` to a repository using format `1`, as long as the > +server process understands format `1`. > + > +The preferred strate
Re: [PATCH v4 23/26] initdb: move safe_create_dir into common code
David Turner writes:
> In a moment, we'll create initdb functions for ref backends, and code
> from initdb that calls this function needs to move into the files
> backend. So this function needs to be public.
OK, but unlike the static function, being in public interface part
can invite mistakes of using this for things outside $GIT_DIR [*1*].
Let's have "Never use this for working tree directories" somewhere
in its docstring.
Other than that, this one, 24/26 and 25/26 looked fine to me.
Thanks.
[Footnote]
*1* Anything created by this function and everything underneath are
repository metadata and this function must not be used to do with
anything with the working tree, as it is clear with the use of
adjust_shared_perm().
> diff --git a/cache.h b/cache.h
> index 9a905a8..1d8a051 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1737,4 +1737,9 @@ void stat_validity_update(struct stat_validity *sv, int
> fd);
> int versioncmp(const char *s1, const char *s2);
> void sleep_millisec(int millisec);
>
> +/*
> + * Create a directory and (if share is nonzero) adjust its permissions
> + * according to the shared_repository setting.
> + */
> +void safe_create_dir(const char *dir, int share);
> #endif /* CACHE_H */
> diff --git a/path.c b/path.c
> index 212695a..9e0283c 100644
> --- a/path.c
> +++ b/path.c
> @@ -723,6 +723,18 @@ int adjust_shared_perm(const char *path)
> return 0;
> }
>
> +void safe_create_dir(const char *dir, int share)
> +{
> + if (mkdir(dir, 0777) < 0) {
> + if (errno != EEXIST) {
> + perror(dir);
> + exit(1);
> + }
> + }
> + else if (share && adjust_shared_perm(dir))
> + die(_("Could not make %s writable by group"), dir);
> +}
> +
> static int have_same_root(const char *path1, const char *path2)
> {
> int is_abs1, is_abs2;
--
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: t7063-status-untracked-cache.sh test failure on next
On 21/10/15 18:50, David Turner wrote: > On Wed, 2015-10-21 at 18:05 +0200, Torsten Bögershausen wrote: >> On 21.10.15 16:37, Ramsay Jones wrote: >>> Hi Junio, >>> >>> While testing the next branch today, I had a test failure, viz: >>> >>> $ tail ntest-out-fail >>> Test Summary Report >>> --- >>> t7063-status-untracked-cache.sh (Wstat: 256 Tests: 32 >>> Failed: 22) >> >> Does this patch help ? >> (Recently send & tested by David. I just copy & paste the diff) > > My patch fixes one of the tests, but Ramsay has a zillion failures > (presumably because test 1 fails and most everything else follows from > that). > > That test could fail if your clock were running fast and you had > 1-second resolution timetamps on your filesystem and you were somewhat > unlucky. yep. I've just stopped running the test in a loop after about an hour. In that time it has executed the test about 130+ times (yeah, I forgot the count), with no failures. I'm going to give up now and declare that I was simply unlucky! :-D 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 v4] name-hash: don't reuse cache_entry in dir_entry
David Turner writes: > Stop reusing cache_entry in dir_entry; doing so causes a > use-after-free bug. > > During merges, we free entries that we no longer need in the > destination index. But those entries might have also been stored in > the dir_entry cache, and when a later call to add_to_index found them, > they would be used after being freed. > > To prevent this, change dir_entry to store a copy of the name instead > of a pointer to a cache_entry. This entails some refactoring of code > that expects the cache_entry. > > Keith McGuigan diagnosed this bug and wrote > the initial patch, but this version does not use any of Keith's code. > > Helped-by: Keith McGuigan > Helped-by: Junio C Hamano > Signed-off-by: David Turner > --- The patch looks good to me. Will replace the ce-refcnt one with this. Thanks for following it through. -- 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 23/26] initdb: move safe_create_dir into common code
On Wed, 2015-10-21 at 12:38 -0700, Junio C Hamano wrote: > David Turner writes: > > > In a moment, we'll create initdb functions for ref backends, and code > > from initdb that calls this function needs to move into the files > > backend. So this function needs to be public. > > OK, but unlike the static function, being in public interface part > can invite mistakes of using this for things outside $GIT_DIR [*1*]. > Let's have "Never use this for working tree directories" somewhere > in its docstring. Will fix in the re-roll, 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 v4 1/1] Makefile: make curl-config path configurable
Remi Pommarel wrote: > Signed-off-by: Remi Pommarel > Reviewed-by: Jonathan Nieder Yep. ;) 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 v4 1/1] Makefile: link libcurl before zlib
Remi Pommarel wrote: > Signed-off-by: Remi Pommarel Reviewed-by: Jonathan Nieder 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 v4 06/26] refs.c: move delete_pseudoref and delete_ref to the common code
On Thu, 2015-10-15 at 15:46 -0400, David Turner wrote: > --- a/refs.c > +++ b/refs.c > @@ -117,3 +117,60 @@ int update_ref(const char *msg, const char *refname, > ref_transaction_free(t); > return 0; > } > + > + > +static int delete_pseudoref(const char *pseudoref, const unsigned char > *old_sha1) extra newline (will fix on reroll) -- 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 05/26] refs.c: move update_ref to refs.c
On Thu, 2015-10-15 at 15:46 -0400, David Turner wrote: > From: Ronnie Sahlberg > > Move update_ref() to the refs.c file since this function does not > contain any backend specific code. Move the ref classifier functions and write_pseudoref (will fix on reroll). -- 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: Commit 5841520b makes it impossible to connect to github from behind my company's firewall.
From: Junio C Hamano [mailto:jch2...@gmail.com] On Behalf Of Junio C Hamano [administrivia: please do not cull people out of the Cc: list] Johan Laenen writes: > Enrique Tobis twosigma.com> writes: > >> There is something I don't understand, though. Johan must be >> configuring his proxy either a) through git config files; or b) >> through environment variables. Johan says his proxy uses NTLM >> authentication. If he is doing a), then my change should not have had >> any impact. We were already setting CURLOPT_PROXYAUTH to CURLAUTH_ANY >> in that case. If it's b), then his proxy couldn't have been using >> NTLM authentication. In the old code path, only _BASIC was available >> as an authentication mechanism. That default is what prompted me to >> make the change in the first place. > > Interesting! > > I tried both git versions, the one with the revert of commit 5841520b > and the one without and both gave me the fatal error "Unknown SSL > protocol error in connection to github.com:443" when using the > ~/.gitconfig [https] and [http] proxy settings instead of using the > https_proxy environment variable. > OK, so the conclusion I draw here is that your NTLM setting is not working at > all, you have been using Basic auth happily before that commit, and you have > to either (1) get NTLM auth working, or (2) find a way to tell Git that your > proxy appears to support NTLM but it is unusable and you need to use Basic. > Even though you may be capable to do (1), other people in the same situation > might not be, in which case we would also need a way to do (2). > Am I reading the above correctly? I draw almost the same conclusions. I agree that he seems to have been using Basic all along. Based on the verbose output Johan posted, I think libcurl is trying to do NEGOTIATE instead of NTLM, so that's what Johan would have to get working. I also agree that other people may need to do (2). -- 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: Commit 5841520b makes it impossible to connect to github from behind my company's firewall.
[administrivia: please do not cull people out of the Cc: list] Johan Laenen writes: > Enrique Tobis twosigma.com> writes: > >> There is something I don't understand, though. Johan must be >> configuring his proxy either a) through git config files; or b) >> through environment variables. Johan says his proxy uses NTLM >> authentication. If he is doing a), then my change should not have >> had any impact. We were already setting CURLOPT_PROXYAUTH to >> CURLAUTH_ANY in that case. If it's b), then his proxy couldn't >> have been using NTLM authentication. In the old code path, only >> _BASIC was available as an authentication mechanism. That >> default is what prompted me to make the change in the first >> place. > > Interesting! > > I tried both git versions, the one with the revert of commit 5841520b and > the one without and both gave me the fatal error "Unknown SSL protocol error > in connection to github.com:443" when using the ~/.gitconfig [https] and > [http] proxy settings instead of using the https_proxy environment variable. OK, so the conclusion I draw here is that your NTLM setting is not working at all, you have been using Basic auth happily before that commit, and you have to either (1) get NTLM auth working, or (2) find a way to tell Git that your proxy appears to support NTLM but it is unusable and you need to use Basic. Even though you may be capable to do (1), other people in the same situation might not be, in which case we would also need a way to do (2). Am I reading the above correctly? -- 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: Poor git write performance to NFS
Daniel Steinborn writes: > currently we are experiencing poor write performance when a repository > is pushed to a nfs volume. Interestingly, this seems to be a problem > in newer git versions: > > v1.7.12.4: Very good performance > > v2.1.4: Bad performance, up to 6 times slower > > Are there any changed default settings or new features that can be the > reason for that problem? > > Please ask for specific details if they are neccessary. Between 1.7.12.x series and v2.1.4, there are more than two years' worth of changes, so it is unreasonable for anybody to expect that such a question can be answered in a meaningful way. Have you tried more recent versions yet? 2.1.x series is over a year old, and I am reasonably sure there have been tons of "earlier we did X for correctness, which unfortunately made things slower, and this ensures the same correctness in a different way that is much more performant" fixes since then. -- 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 v4] name-hash: don't reuse cache_entry in dir_entry
Stop reusing cache_entry in dir_entry; doing so causes a
use-after-free bug.
During merges, we free entries that we no longer need in the
destination index. But those entries might have also been stored in
the dir_entry cache, and when a later call to add_to_index found them,
they would be used after being freed.
To prevent this, change dir_entry to store a copy of the name instead
of a pointer to a cache_entry. This entails some refactoring of code
that expects the cache_entry.
Keith McGuigan diagnosed this bug and wrote
the initial patch, but this version does not use any of Keith's code.
Helped-by: Keith McGuigan
Helped-by: Junio C Hamano
Signed-off-by: David Turner
---
cache.h | 3 ++-
dir.c| 22 --
name-hash.c | 54 --
read-cache.c | 16 +---
4 files changed, 35 insertions(+), 60 deletions(-)
diff --git a/cache.h b/cache.h
index 752031e..ccc329a 100644
--- a/cache.h
+++ b/cache.h
@@ -520,7 +520,8 @@ extern int write_locked_index(struct index_state *, struct
lock_file *lock, unsi
extern int discard_index(struct index_state *);
extern int unmerged_index(const struct index_state *);
extern int verify_path(const char *path);
-extern struct cache_entry *index_dir_exists(struct index_state *istate, const
char *name, int namelen);
+extern int index_dir_exists(struct index_state *istate, const char *name, int
namelen);
+extern void adjust_dirname_case(struct index_state *istate, char *name);
extern struct cache_entry *index_file_exists(struct index_state *istate, const
char *name, int namelen, int igncase);
extern int index_name_pos(const struct index_state *, const char *name, int
namelen);
#define ADD_CACHE_OK_TO_ADD 1 /* Ok to add */
diff --git a/dir.c b/dir.c
index b90484a..c982aac 100644
--- a/dir.c
+++ b/dir.c
@@ -1279,29 +1279,15 @@ enum exist_status {
*/
static enum exist_status directory_exists_in_index_icase(const char *dirname,
int len)
{
- const struct cache_entry *ce = cache_dir_exists(dirname, len);
- unsigned char endchar;
+ struct cache_entry *ce;
- if (!ce)
- return index_nonexistent;
- endchar = ce->name[len];
-
- /*
-* The cache_entry structure returned will contain this dirname
-* and possibly additional path components.
-*/
- if (endchar == '/')
+ if (cache_dir_exists(dirname, len))
return index_directory;
- /*
-* If there are no additional path components, then this cache_entry
-* represents a submodule. Submodules, despite being directories,
-* are stored in the cache without a closing slash.
-*/
- if (!endchar && S_ISGITLINK(ce->ce_mode))
+ ce = cache_file_exists(dirname, len, ignore_case);
+ if (ce && S_ISGITLINK(ce->ce_mode))
return index_gitdir;
- /* This should never be hit, but it exists just in case. */
return index_nonexistent;
}
diff --git a/name-hash.c b/name-hash.c
index 702cd05..332ba95 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -11,16 +11,16 @@
struct dir_entry {
struct hashmap_entry ent;
struct dir_entry *parent;
- struct cache_entry *ce;
int nr;
unsigned int namelen;
+ char name[FLEX_ARRAY];
};
static int dir_entry_cmp(const struct dir_entry *e1,
const struct dir_entry *e2, const char *name)
{
- return e1->namelen != e2->namelen || strncasecmp(e1->ce->name,
- name ? name : e2->ce->name, e1->namelen);
+ return e1->namelen != e2->namelen || strncasecmp(e1->name,
+ name ? name : e2->name, e1->namelen);
}
static struct dir_entry *find_dir_entry(struct index_state *istate,
@@ -41,14 +41,6 @@ static struct dir_entry *hash_dir_entry(struct index_state
*istate,
* closing slash. Despite submodules being a directory, they never
* reach this point, because they are stored
* in index_state.name_hash (as ordinary cache_entries).
-*
-* Note that the cache_entry stored with the dir_entry merely
-* supplies the name of the directory (up to dir_entry.namelen). We
-* track the number of 'active' files in a directory in dir_entry.nr,
-* so we can tell if the directory is still relevant, e.g. for git
-* status. However, if cache_entries are removed, we cannot pinpoint
-* an exact cache_entry that's still active. It is very possible that
-* multiple dir_entries point to the same cache_entry.
*/
struct dir_entry *dir;
@@ -63,10 +55,10 @@ static struct dir_entry *hash_dir_entry(struct index_state
*istate,
dir = find_dir_entry(istate, ce->name, namelen);
if (!dir) {
/* not found, create it and add to hash table */
- dir = xcalloc(1, sizeof(struct dir_entry));
+ dir = xcal
Re: [PATCH] Allow "clone --dissociate" to dissociate from alternates
Alexander Riesen writes:
> Reminder. Is this (or rather the one I'm replying to) patch a better option?
I suspect that the reason why you didn't get any quick response was
because it was unclear from either one of these proposed log
messages why any change is needed in the first place. At least that
is what prevented me from commenting on either.
The "clone --dissociate" was designed to be used with "--reference".
The mindset of those who saw the need for the feature being that
"clone --reference" is the only way to make the resulting
repository's objects incomplete, needing to borrow objects from some
other place, which necessitates the "--dissociate" option.
The readers of this change need to be enlightened with a log message
to remind them that "--reference" is not the only way. Namely, if
you start from a repository with $GIT_DIR/objects/info/alternates,
i.e. the original already borrows from somewhere, and bypass the
normal "Git aware" transport mechanism, i.e. "git clone --local",
then the resulting repository would also become dependent of the
object store that the original depended on before the clone. In
order to make it free-standing, you would need "--dissociate", but
there is no "--reference" involved in that use case.
And once that is clarified, it becomes very clear why it is wrong to
blindly require "--reference" to be there on the command line when
"--dissociate" is given.
As to the patch, I think this one is much simpler and preferrable.
It would hurt those who make a clone without bypassing the normal
"Git aware" transport mechanism and pass "--dissociate" without
"--reference". They will end up making a clone that does not need
repacking to dissociate, but with this patch they would spend extra
cycles to run an unnecessary repack. To avoid that, I think you can
throw in an check at the beginning of dissociate_from_references()
to see if git_path("objects/info/alternates") is there and make the
function a no-op if there isn't.
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: t7063-status-untracked-cache.sh test failure on next
On Wed, 2015-10-21 at 18:05 +0200, Torsten Bögershausen wrote: > On 21.10.15 16:37, Ramsay Jones wrote: > > Hi Junio, > > > > While testing the next branch today, I had a test failure, viz: > > > > $ tail ntest-out-fail > > Test Summary Report > > --- > > t7063-status-untracked-cache.sh (Wstat: 256 Tests: 32 > > Failed: 22) > > Does this patch help ? > (Recently send & tested by David. I just copy & paste the diff) My patch fixes one of the tests, but Ramsay has a zillion failures (presumably because test 1 fails and most everything else follows from that). That test could fail if your clock were running fast and you had 1-second resolution timetamps on your filesystem and you were somewhat unlucky. -- 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 v4 1/1] Makefile: make curl-config path configurable
There are situations, e.g. during cross compilation, where curl-config program is not present in the PATH. Make the makefile use a configurable curl-config program passed through CURL_CONFIG variable which can be set through config.mak. Also make this variable tunable through use of autoconf/configure. Configure will set CURL_CONFIG variable in config.mak.autogen to whatever value has been passed to ac_cv_prog_CURL_CONFIG. Signed-off-by: Remi Pommarel Reviewed-by: Jonathan Nieder --- Changes to v3: - Add Jonathan Nieder's modifications Makefile | 8 ++-- configure.ac | 13 + 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 04c2231..7574c26 100644 --- a/Makefile +++ b/Makefile @@ -39,6 +39,9 @@ all:: # Define CURLDIR=/foo/bar if your curl header and library files are in # /foo/bar/include and /foo/bar/lib directories. # +# Define CURL_CONFIG to curl's configuration program that prints information +# about the library (e.g., its version number). The default is 'curl-config'. +# # Define NO_EXPAT if you do not have expat installed. git-http-push is # not built, and you cannot push using http:// and https:// transports (dumb). # @@ -428,6 +431,7 @@ TCL_PATH = tclsh TCLTK_PATH = wish XGETTEXT = xgettext MSGFMT = msgfmt +CURL_CONFIG = curl-config PTHREAD_LIBS = -lpthread PTHREAD_CFLAGS = GCOV = gcov @@ -1066,13 +1070,13 @@ else REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES) PROGRAM_OBJS += http-fetch.o PROGRAMS += $(REMOTE_CURL_NAMES) - curl_check := $(shell (echo 070908; curl-config --vernum | sed -e '/^70[BC]/s/^/0/') 2>/dev/null | sort -r | sed -ne 2p) + curl_check := $(shell (echo 070908; $(CURL_CONFIG) --vernum | sed -e '/^70[BC]/s/^/0/') 2>/dev/null | sort -r | sed -ne 2p) ifeq "$(curl_check)" "070908" ifndef NO_EXPAT PROGRAM_OBJS += http-push.o endif endif - curl_check := $(shell (echo 072200; curl-config --vernum | sed -e '/^70[BC]/s/^/0/') 2>/dev/null | sort -r | sed -ne 2p) + curl_check := $(shell (echo 072200; $(CURL_CONFIG) --vernum | sed -e '/^70[BC]/s/^/0/') 2>/dev/null | sort -r | sed -ne 2p) ifeq "$(curl_check)" "072200" USE_CURL_FOR_IMAP_SEND = YesPlease endif diff --git a/configure.ac b/configure.ac index 14012fa..01b07ad 100644 --- a/configure.ac +++ b/configure.ac @@ -525,6 +525,19 @@ GIT_UNSTASH_FLAGS($CURLDIR) GIT_CONF_SUBST([NO_CURL]) +if test -z "$NO_CURL"; then + +AC_CHECK_PROG([CURL_CONFIG], [curl-config], +[curl-config], +[no]) + +if test $CURL_CONFIG != no; then +GIT_CONF_SUBST([CURL_CONFIG]) +fi + +fi + + # # Define NO_EXPAT if you do not have expat installed. git-http-push is # not built, and you cannot push using http:// and https:// transports. -- 2.0.1 -- 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 v4 1/1] Makefile: link libcurl before zlib
For static linking especially library order while linking is important. For example, libcurl wants symbols from zlib when building http-push, http-fetch and remote-curl. So for these programs libcurl has to be linked before zlib. Signed-off-by: Remi Pommarel --- Changes to v3: - Initialize IMAP_SEND_LDFLAGS so that no environment leak could occur - Rephrase the description to use zlib example instead of libcurl Makefile | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 04c2231..8f1fa6c 100644 --- a/Makefile +++ b/Makefile @@ -1036,7 +1036,7 @@ ifdef HAVE_ALLOCA_H endif IMAP_SEND_BUILDDEPS = -IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO) +IMAP_SEND_LDFLAGS = ifdef NO_CURL BASIC_CFLAGS += -DNO_CURL @@ -1093,6 +1093,7 @@ else endif endif endif +IMAP_SEND_LDFLAGS += $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO) ifdef ZLIB_PATH BASIC_CFLAGS += -I$(ZLIB_PATH)/include @@ -1978,10 +1979,10 @@ git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS) git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ - $(LIBS) $(CURL_LIBCURL) + $(CURL_LIBCURL) $(LIBS) git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ - $(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) + $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS) git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS $(GITLIBS) $(VCSSVN_LIB) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) \ @@ -1995,7 +1996,7 @@ $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ - $(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) + $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS) $(LIB_FILE): $(LIB_OBJS) $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ -- 2.0.1 -- 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 1/1] Makefile: link libcurl before libssl
Remi Pommarel writes: > On Tue, Oct 20, 2015 at 01:20:18PM -0700, Junio C Hamano wrote: >> >> So, what's the status of this patch and other two patches (I >> consider them as a three-patch series)? > > So I have to fix the non initialized variable and to rephrase a litle > bit the description for this patch. Taking libssl as an example is > misleading, zlib is more appropriate. I'll resend another version shortly. > > For patch "[PATCH v3 1/1] Makefile: make curl-config path configurable" > it has been reviewed by Jonathan Nieder with a litle modification to be > squashed in. I can resend a squashed in version if it is easier for you. Yes, please. > Sorry for the delay. No need to say sorry; people do their part at their own pace and that is perfectly fine. I just wanted to make sure that some sign of this topic not being forgotten would stay as I expunge older messages from my mailbox ;-) 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 v3 1/1] Makefile: link libcurl before libssl
On Tue, Oct 20, 2015 at 01:20:18PM -0700, Junio C Hamano wrote: > Remi Pommarel writes: > > > On Mon, Oct 05, 2015 at 12:41:34PM -0700, Jonathan Nieder wrote: > > ... > >> To protect against a value that might leak in from the environment, this > >> should say > >> > >>IMAP_SEND_LDFLAGS = > >> > >> [...] > > > > Oups my bad. > > ... > > So, what's the status of this patch and other two patches (I > consider them as a three-patch series)? So I have to fix the non initialized variable and to rephrase a litle bit the description for this patch. Taking libssl as an example is misleading, zlib is more appropriate. I'll resend another version shortly. For patch "[PATCH v3 1/1] Makefile: make curl-config path configurable" it has been reviewed by Jonathan Nieder with a litle modification to be squashed in. I can resend a squashed in version if it is easier for you. Sorry for the delay. 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] fix flaky untracked-cache test
On Wed, 2015-10-21 at 07:42 +0200, Torsten Bögershausen wrote: > On 19.10.15 21:48, David Turner wrote: > > > + echo test >base && #we need to ensure that the root dir is touched > > + rm base > > ' > Thanks for working on this, (I can run the test as soon as I have access to a > Mac with SSD) I don't think this depends on a Mac (I can repro on my Thinkpad running Ubuntu). > Minor remark, the echo test can be removed (and may be the comment ?) All the other lines in this test have echo, so I would rather be consistent. > > + >base && > > + rm base > > ' -- 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: t7063-status-untracked-cache.sh test failure on next
On 21/10/15 17:05, Torsten Bögershausen wrote: > On 21.10.15 16:37, Ramsay Jones wrote: >> Hi Junio, >> >> While testing the next branch today, I had a test failure, viz: >> >> $ tail ntest-out-fail >> Test Summary Report >> --- >> t7063-status-untracked-cache.sh (Wstat: 256 Tests: 32 >> Failed: 22) > > Does this patch help ? > (Recently send & tested by David. I just copy & paste the diff) > [] > No, that patch is already in next and was part of the build that failed: $ git log -1 --oneline 9b680fbd3 9b680fb t7063: fix flaky untracked-cache test $ git branch --contains 9b680fbd3 * next pu $ Again, I haven't been able to reproduce the failure ... [I should have said the this is today's next branch @ 3b31934 Sync with master. This is on Linux Mint 17.2] Thanks. ATB, Ramsay Jones > Signed-off-by: David Turner > --- > t/t7063-status-untracked-cache.sh | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/t/t7063-status-untracked-cache.sh > b/t/t7063-status-untracked-cache.sh > index 37a24c1..0e8d0d4 100755 > --- a/t/t7063-status-untracked-cache.sh > +++ b/t/t7063-status-untracked-cache.sh > @@ -412,7 +412,9 @@ test_expect_success 'create/modify files, some of which > are gitignored' ' > echo two bis >done/two && > echo three >done/three && # three is gitignored > echo four >done/four && # four is gitignored at a higher level > - echo five >done/five # five is not gitignored > + echo five >done/five && # five is not gitignored > + echo test >base && #we need to ensure that the root dir is touched > + rm base > ' > > -- 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: t7063-status-untracked-cache.sh test failure on next
On 21.10.15 16:37, Ramsay Jones wrote: > Hi Junio, > > While testing the next branch today, I had a test failure, viz: > > $ tail ntest-out-fail > Test Summary Report > --- > t7063-status-untracked-cache.sh (Wstat: 256 Tests: 32 > Failed: 22) Does this patch help ? (Recently send & tested by David. I just copy & paste the diff) [] Signed-off-by: David Turner --- t/t7063-status-untracked-cache.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh index 37a24c1..0e8d0d4 100755 --- a/t/t7063-status-untracked-cache.sh +++ b/t/t7063-status-untracked-cache.sh @@ -412,7 +412,9 @@ test_expect_success 'create/modify files, some of which are gitignored' ' echo two bis >done/two && echo three >done/three && # three is gitignored echo four >done/four && # four is gitignored at a higher level - echo five >done/five # five is not gitignored + echo five >done/five && # five is not gitignored + echo test >base && #we need to ensure that the root dir is touched + rm base ' -- 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 00/34] libify mailinfo and call it directly from am
On 20/10/15 22:24, Junio C Hamano wrote: > Junio C Hamano writes: > >> During the discussion on the recent "git am" regression, I noticed >> that the command reimplemented in C spawns one "mailsplit" and then >> spawns "mailinfo" followed by "apply --index" to commit the changes >> described in each message. As there are platforms where spawning >> subprocess via run_command() interface is heavy-weight, something >> that is conceptually very simple like "mailinfo" is better called >> directly inside the process---something that is lightweight and >> frequently used is where the overhead of run_command() would be felt >> most. > > Although I still haven't seen any offer to help from those who work > on the platforms that may benefit from this series the most, I have > some numbers on my desktop (Dell T3500 2.66GHz Xeon X5650 with 12GB, > running Ubuntu), where the cost of spawning is not as costly as > elsewhere, making this series less pressing. I suspect that I haven't tested exactly the same version as you, but I had a quick look at testing this on Cygwin today. I have included a complete transcript (below), so you can see what I did wrong! :-P > > Between 'master' and the version with this series (on 'jch'), > applying this 34-patch series itself on top of 'master' using "git > am", best of 5 numbers for running: > > time git am mbox >/dev/null > > are > > (master) (with the series) > real0m0.648sreal0m0.537s > user0m0.358suser0m0.338s > sys 0m0.172ssys 0m0.154s > The corresponding times for me were: (master) (with the series) real 0m9.760s real 0m5.744s user 0m0.531s user 0m0.656s sys 0m5.726s sys 0m3.520s So, yes, a noticeable improvement! :) HTH ATB, Ramsay Jones $ uname -a CYGWIN_NT-6.3 satellite 2.2.1(0.289/5/3) 2015-08-20 11:42 x86_64 Cygwin $ pwd /home/ramsay/git $ git log --decorate --oneline -1 74301d6 (HEAD -> master, origin/master, origin/HEAD) Sync with maint $ ./git version git version 2.6.2.280.g74301d6 $ git format-patch --stdout 2a5ce7c^..896df93 >mailinfo.mbox $ git format-patch --stdout a4106a8^..559e247 >>mailinfo.mbox $ git checkout -b master-mailinfo master Switched to a new branch 'master-mailinfo' $ time ./git am mailinfo.mbox Applying: mailinfo: remove a no-op call convert_to_utf8(it, "") Applying: mailinfo: fold decode_header_bq() into decode_header() Applying: mailinfo: fix an off-by-one error in the boundary stack Applying: mailinfo: explicitly close file handle to the patch output Applying: mailinfo: move handle_boundary() lower Applying: mailinfo: get rid of function-local static states Applying: mailinfo: do not let handle_body() touch global "line" directly Applying: mailinfo: do not let handle_boundary() touch global "line" directly Applying: mailinfo: do not let find_boundary() touch global "line" directly Applying: mailinfo: move global "line" into mailinfo() function Applying: mailinfo: introduce "struct mailinfo" to hold globals Applying: mailinfo: move keep_subject & keep_non_patch_bracket to struct mailinfo Applying: mailinfo: move global "FILE *fin, *fout" to struct mailinfo Applying: mailinfo: move filter/header stage to struct mailinfo Applying: mailinfo: move patch_lines to struct mailinfo Applying: mailinfo: move add_message_id and message_id to struct mailinfo Applying: mailinfo: move use_scissors and use_inbody_headers to struct mailinfo Applying: mailinfo: move metainfo_charset to struct mailinfo Applying: mailinfo: move check for metainfo_charset to convert_to_utf8() Applying: mailinfo: move transfer_encoding to struct mailinfo Applying: mailinfo: move charset to struct mailinfo Applying: mailinfo: move cmitmsg and patchfile to struct mailinfo Applying: mailinfo: move [ps]_hdr_data to struct mailinfo Applying: mailinfo: move content/content_top to struct mailinfo Applying: mailinfo: handle_commit_msg() shouldn't be called after finding patchbreak Applying: mailinfo: keep the parsed log message in a strbuf Applying: mailinfo: move read_one_header_line() closer to its callers Applying: mailinfo: move check_header() after the helpers it uses Applying: mailinfo: move cleanup_space() before its users Applying: mailinfo: move definition of MAX_HDR_PARSED closer to its use Applying: mailinfo: libify Applying: mailinfo: handle charset conversion errors in the caller Applying: mailinfo: remove calls to exit() and die() deep in the callchain Applying: am: make direct call to mailinfo Applying: mailinfo: plug strbuf leak during continuation line handling real 0m9.760s user 0m0.531s sys 0m5.726s $ $ make clean >/dev/null 2>&1 $ make >out.mi 2>&1 $ ./git version git version 2.6.2.315.g1e9f6ff $ git describe v2.6.2-315-g1e9f6ff $ git checkout -b new-mailinfo master Switc
Re: [PATCH] fr.po: Fix "uptream" typo
Le mercredi 21 octobre 2015, 15:25:59 Thomas Schneider a écrit : > Signed-off-by: Thomas Schneider > --- > po/fr.po | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/po/fr.po b/po/fr.po > index 581167f..71c4b54 100644 > --- a/po/fr.po > +++ b/po/fr.po > @@ -8862,7 +8862,7 @@ msgstr "" > "Si vous souhaitez indiquer l'information de suivi distant pour cette " > "branche, vous pouvez le faire avec :\n" > "\n" > -"git branch --set-uptream-to=%s/ %s\n" > +"git branch --set-upstream-to=%s/ %s\n" > > #: builtin/pull.c:476 > #, c-format Thanks, It will be included in the next l10n update. Jean-Noël -- 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
t7063-status-untracked-cache.sh test failure on next
Hi Junio, While testing the next branch today, I had a test failure, viz: $ tail ntest-out-fail Test Summary Report --- t7063-status-untracked-cache.sh (Wstat: 256 Tests: 32 Failed: 22) Failed tests: 1-18, 20-21, 25, 29 Non-zero exit status: 1 Files=726, Tests=13035, 394 wallclock secs ( 3.61 usr 0.51 sys + 82.93 cusr 256.21 csys = 343.26 CPU) Result: FAIL make[1]: *** [prove] Error 1 make[1]: Leaving directory `/home/ramsay/git/t' make: *** [test] Error 2 $ However, I have not been able to reproduce the failure, so it seems to be an intermittent fault. (Unfortunately, I trashed the 'trash' directory before thinking to save it - although I almost always find it useless for debugging if you haven't run the test with '-i -v' anyway.) So, this is just a 'heads up', since I can't debug it. :( 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
[PATCH] fr.po: Fix "uptream" typo
Signed-off-by: Thomas Schneider --- po/fr.po | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/po/fr.po b/po/fr.po index 581167f..71c4b54 100644 --- a/po/fr.po +++ b/po/fr.po @@ -8862,7 +8862,7 @@ msgstr "" "Si vous souhaitez indiquer l'information de suivi distant pour cette " "branche, vous pouvez le faire avec :\n" "\n" -"git branch --set-uptream-to=%s/ %s\n" +"git branch --set-upstream-to=%s/ %s\n" #: builtin/pull.c:476 #, c-format -- 2.4.3 -- 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
Poor git write performance to NFS
Hi, currently we are experiencing poor write performance when a repository is pushed to a nfs volume. Interestingly, this seems to be a problem in newer git versions: v1.7.12.4: Very good performance v2.1.4: Bad performance, up to 6 times slower Are there any changed default settings or new features that can be the reason for that problem? The tests are done on a Debian 8.2 VM. Please ask for specific details if they are neccessary. Thanks for your help! Best regards, Daniel Steinborn -- Daniel Steinborn Leibniz-Rechenzentrum Boltzmannstraße 1 85748 Garching bei München -- 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] git-p4: import the ctypes module
I was wrong, the script doesn't work on my machine if ctypes is not imported regardless of python version. I guess I was confused by using a version of git-p4 before ctypes was introduced, the failing version and the patched version, as well as several python versions. Sorry for this misleading claim, and thanks for the quick fix. 2015-10-21 10:23 GMT+02:00 Etienne Girard : > Hello, > > I couldn't work further on this yesterday (but I read > Documentation/SubmittingPatches, which is a good start I guess). The > diff proposed by Dennis works on my machine, I'll try to figure out > why the original script worked with 2.7.10. > > Thanks > > 2015-10-21 1:00 GMT+02:00 Luke Diamand : >> On 20/10/15 20:36, Junio C Hamano wrote: >>> >>> Dennis Kaarsemaker writes: >>> > I do not follow Python development, but does the above mean that > with recent 2.x you can say ctypes without first saying "import > ctypes"? It feels somewhat non-pythonesque that identifiers like > this is given to you without you asking with an explicit 'import', > so I am puzzled. No, you cannot do that. The reason others may not have noticed this bug is that in git-p4.py, ctypes is only used on windows. 111 if platform.system() == 'Windows': 112 free_bytes = ctypes.c_ulonglong(0) 113 ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()), None, None, ctypes.pointer(free_bytes)) The fact that it works for the OP with 2.7.10 is puzzling (assuming that it's on the same system). >>> >>> >>> Exactly. That is where my "I am puzzled" comes from. >>> >>> The patch looks obviously the right thing to do. Luke? Lars? >> >> >> It looks sensible to me, and works fine on Linux, thanks. ack. >> >> I can't test on Windows today but I can't see why it wouldn't work. >> >> Luke >> >> -- 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] git-p4: import the ctypes module
Hello, I couldn't work further on this yesterday (but I read Documentation/SubmittingPatches, which is a good start I guess). The diff proposed by Dennis works on my machine, I'll try to figure out why the original script worked with 2.7.10. Thanks 2015-10-21 1:00 GMT+02:00 Luke Diamand : > On 20/10/15 20:36, Junio C Hamano wrote: >> >> Dennis Kaarsemaker writes: >> I do not follow Python development, but does the above mean that with recent 2.x you can say ctypes without first saying "import ctypes"? It feels somewhat non-pythonesque that identifiers like this is given to you without you asking with an explicit 'import', so I am puzzled. >>> >>> >>> No, you cannot do that. The reason others may not have noticed this bug >>> is that >>> in git-p4.py, ctypes is only used on windows. >>> >>> 111 if platform.system() == 'Windows': >>> 112 free_bytes = ctypes.c_ulonglong(0) >>> 113 >>> ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(os.getcwd()), >>> None, None, ctypes.pointer(free_bytes)) >>> >>> The fact that it works for the OP with 2.7.10 is puzzling (assuming that >>> it's >>> on the same system). >> >> >> Exactly. That is where my "I am puzzled" comes from. >> >> The patch looks obviously the right thing to do. Luke? Lars? > > > It looks sensible to me, and works fine on Linux, thanks. ack. > > I can't test on Windows today but I can't see why it wouldn't work. > > Luke > > -- 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] Allow "clone --dissociate" to dissociate from alternates
Reminder. Is this (or rather the one I'm replying to) patch a better option? Regards, Alex On 10/15/2015 04:38 PM, Alexander Riesen wrote: The option requiring the explicit reference repositories is a bit of overkill: the alternates in the original repository *are* reference repositories and would be dissociated from should one pass any reference repository (even an unrelated one). Signed-off-by: Alex Riesen --- On 10/15/2015 04:11 PM, Johannes Schindelin wrote: On Thu, 15 Oct 2015, Alexander Riesen wrote: > >> The "--dissociate" option required reference repositories, which sometimes >> demanded a look into the objects/info/alternates by the user. As this >> is something which can be figured out automatically, do it in the >> clone unless there is no other reference repositories. > > Would it not make sense to reuse the copy_alternates() function to simply > copy the alternates and let `--dissociate` run its course with the copied > .objects/info/alternate file? That would make for less new code... IIUC, I should validate the alternates in the source repository... But, the only thing the user looses if it is not validated, is the nice warning regarding no reference repositories to dissociate from, right? So maybe we can just remove the reset of option_dissociate and be done with it? I would actually suggest removing the warning as well: the alternates are something to dissociate from. And I see no harm otherwise. How about this instead? -- 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] difftool: gracefully handle symlinks to directories
difftool's dir-diff feature was blindly feeding worktree paths
to hash-object without checking whether the path was indeed a
file, causing the feature to fail when repositories contain
symlinks to directories.
Ensure that only files are ever given to hash-object.
Add a test to demonstrate the breakage.
Reported-by: Ismail Badawi
Signed-off-by: David Aguilar
---
git-difftool.perl | 4 +---
t/t7800-difftool.sh | 19 +++
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 7df7c8a..1abe647 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -70,9 +70,7 @@ sub use_wt_file
my ($repo, $workdir, $file, $sha1) = @_;
my $null_sha1 = '0' x 40;
- if (! -e "$workdir/$file") {
- # If the file doesn't exist in the working tree, we cannot
- # use it.
+ if (! -f "$workdir/$file") {
return (0, $null_sha1);
}
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 48c6e2b..ec8bc8c 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -504,4 +504,23 @@ test_expect_success PERL 'difftool properly honors gitlink
and core.worktree' '
)
'
+test_expect_success PERL,SYMLINKS 'difftool --dir-diff symlinked directories' '
+ git init dirlinks &&
+ (
+ cd dirlinks &&
+ git config diff.tool checktrees &&
+ git config difftool.checktrees.cmd "echo good" &&
+ mkdir foo &&
+ : >foo/bar &&
+ git add foo/bar &&
+ test_commit symlink-one &&
+ ln -s foo link &&
+ git add link &&
+ test_commit symlink-two &&
+ echo good >expect &&
+ git difftool --tool=checktrees --dir-diff HEAD~ >actual &&
+ test_cmp expect actual
+ )
+'
+
test_done
--
2.6.2.281.gac28444
--
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: Commit 5841520b makes it impossible to connect to github from behind my company's firewall.
Enrique Tobis twosigma.com> writes: > There is something I don't understand, though. Johan must be configuring his proxy either a) through git > config files; or b) through environment variables. Johan says his proxy uses NTLM authentication. If he > is doing a), then my change should not have had any impact. We were already setting CURLOPT_PROXYAUTH to > CURLAUTH_ANY in that case. If it's b), then his proxy couldn't have been using NTLM authentication. In the > old code path, only _BASIC was available as an authentication mechanism. That default is what prompted me > to make the change in the first place. Interesting! I tried both git versions, the one with the revert of commit 5841520b and the one without and both gave me the fatal error "Unknown SSL protocol error in connection to github.com:443" when using the ~/.gitconfig [https] and [http] proxy settings instead of using the https_proxy environment variable. Greetings, Johan -- 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
