Re: [PATCH v6 25/25] refs: break out ref conflict checks

[Junio C Hamano]

Junio C Hamano  writes:

> Michael Haggerty  writes:
>
>> Junio, if there are no more comments, would you mind
>>
>> s/verify_no_descendants/find_descendant_ref/
>>
>> in the log message of this commit? And then, if you are also OK with the
>> new subdirectory introduced in this patch series, David and I seem to be
>> in agreement that it is ready to go. It would be great if this patch
>> series could be merged in a timely manner, as it will conflict with
>> nearly any other changes that people might want to undertake in the refs
>> code.
>
> Thanks for working well together.  Let me see what I can do today...

What I'll push out later today merges this to the tip of 'pu'.
The resolution is the same for 'jch' or 'next' (I checked).

I have to say that the merge of this topioc is not pretty.  A topic
that is already in flight has changed ref_is_hidden() in refs.c; you
move this block of code first to refs/refs-backend.c and then to
refs/refs.c, and the recursive merge ends up saying "The trunk side
changed this block of code in refs/refs-backend.c while the side
branch removed that block".

The resolution has to become an evil merge that brings in a new file
refs/refs.c from the tip of your topic to the trunk while replaying
that change in the lost block to that new file.  Because an
in-flight topic like this one needs to be merged over and over until
it gets merged to 'master' I'd prepare an evil merge-fix to be
squashed into the result of an auto-merge to help this process for
the interim maintainer, but this topic is placing more burden than
it otherwise would to the entire process.

Incidentally, that was why I originally asked you if you want to be
an interim maintainer for this cycle.  Whoever is doing a large code
movement with a large patch series must be the one who would know
the best how its interaction with other topics is best managed.

I wonder if refs.c -> refs/refs.c rename is a good idea.  I do agree
that refs/ subdirectory that collects the backend implementation
details is a very sensible thing to do, but if the public and
generic API were left in refs.c, this particular conflict might have
been less severe and easier to handle.  Whatever.

For those who are listening in from sideline, in case when they need
to deal with a similar situation "the code our side changed gets
moved to elsewhere by a side branch", here is what I did:

 * let "git merge --conflict=diff3" attempt and fail.

 * a conflicted file will have something like this:

 ours
... the code with changes made by our side (trunk) ...
 base
... the code before our side (trunk) made the above changes ...

 theirs

 * create two new files, OURS and BASE.  Save the part in that
   conflicted file between  and  to OURS, and between 
   and  to BASE.

 * look at "diff -u BASE OURS", find in the (failed) automerge
   result where the original went (a sample of it is at the end of
   this message), and apply that change manually.

The above is only to resolve this conflict *once*.

Automating future merges of this branch into a slightly updated
codebase needs help from rerere and also merge-fix/ machinery (this
is not in core-git proper, but the tool is in the 'todo' branch and
its use is described in howto/maintain-git.txt).

Essentially the procedure were:

 * "git checkout pu^0"

 * let "git merge --conflict=diff3" attempt and fail.

 * accept removal of the conflicted block in refs/files-backend.c in
   the editor, do "git rerere" to record it.  commit the result.

 * apply the above diff between BASE and OURS, commit the result.

 * git update-ref refs/merge-fix/dt/refs-backend-pre-vtable HEAD

With this, the Reintegrate script (on 'todo', checked out in "Meta/"
subdirectory) will be able to reproduce the evil merge, e.g.

 $ git checkout pu
 $ echo dt/refs-backend-pre-vtable | Meta/Reintegrate

would first let "git rerere" replay the removal of conflicted block
in refs/files-backend.c and then amend the result by squashing the
change in merge-fix/dt/refs-backend-pre-vtable.


--- V_BASE  2015-11-06 14:51:10.150197900 -0800
+++ V_OURS  2015-11-06 14:51:05.638059250 -0800
@@ -117,7 +117,7 @@
return 0;
 }
 
-int ref_is_hidden(const char *refname)
+int ref_is_hidden(const char *refname, const char *refname_full)
 {
int i;
 
@@ -125,6 +125,7 @@
return 0;
for (i = hide_refs->nr - 1; i >= 0; i--) {
const char *match = hide_refs->items[i].string;
+   const char *subject;
int neg = 0;
int len;
 
@@ -133,10 +134,18 @@
match++;
}
 
-   if (!starts_with(refname, match))
+   if (*match == '^') {
+   subject = refname_full;
+   match++;
+   } else {
+   subject = refname;
+   }
+
+   

What's cooking in git.git (Nov 2015, #02; Fri, 6)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

I'll be offline for a few weeks, and Jeff King graciously agreed to
help shepherd the project forward in the meantime as an interim
maintainer.  Please be gentle.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[New Topics]

* dt/refs-backend-pre-vtable (2015-11-03) 26 commits
 - refs: break out ref conflict checks
 - refs: create a shared version of verify_refname_available
 - files_log_ref_write: new function
 - initdb: make safe_create_dir public
 - refs: move should_autocreate_reflog to common code
 - refs: move peel_object to the common code
 - refs: move copy_msg to the common code
 - refs: move refname_is_safe to the common code
 - refs: move transaction functions to the common code
 - refs: move head_ref_namespaced to the common code
 - refs: move ref iterators to the common code
 - refs: move prettify_refname to the common code
 - refs: move is_branch to the common code
 - refs: move check_refname_format to the common code
 - refs: move resolve_refdup to common
 - refs: move read_ref, read_ref_full and ref_exists to the common code
 - refs: move warn_if_dangling_symref* to the common code
 - refs: move dwim and friend functions to the common refs code
 - refs: move the hidden refs functions to the common code
 - refs: move read_ref_at to the common refs file
 - refs: move delete_pseudoref and delete_ref to the common code
 - refs: move update_ref to refs/refs.c
 - refs: add a new file, refs/refs.c, to hold common refs code
 - refs/files-backend.c: new file, renamed from refs.c
 - refs: make is_branch public
 - Merge branch 'jk/war-on-sprintf' into HEAD

 This is a verbatim copy of refs-backend-pre-vtable branch taken
 from https://github.com/mhagger/git; not even signed-off by me, and
 "thanks for noticing; Junio can you amend?" obviously has not been
 done.  We would want to rebase the series (after discarding the
 bottom "Merge into HEAD") on top of 228905f in order to do the
 amending and then re-merge, but I ran out of time (and patience).


* rc/configure-use-libs-when-checking-a-lib (2015-11-06) 1 commit
 - configure.ac: use $LIBS not $CFLAGS when testing -lpthread

 The "configure" script did not test for -lpthread correctly, which
 upset some linkers.

 Will merge to 'next'.

--
[Graduated to "master"]

* dt/http-range (2015-11-02) 2 commits
  (merged to 'next' on 2015-11-03 at 7c3cc60)
 + http: use off_t to store partial file size
 + http.c: use CURLOPT_RANGE for range requests

 A Range: request can be responded with a full response and when
 asked properly libcurl knows how to strip the result down to the
 requested range.  However, we were hand-crafting a range request
 and it did not kick in.


* ea/checkout-progress (2015-11-01) 1 commit
  (merged to 'next' on 2015-11-03 at 08a8d1f)
 + checkout: add --progress option

 "git checkout" did not follow the usual "--[no-]progress"
 convention and implemented only "--quiet" that is essentially
 a superset of "--no-progress".  Extend the command to support the
 usual "--[no-]progress".

--
[Stalled]

* nd/ita-cleanup (2015-09-06) 6 commits
 - grep: make it clear i-t-a entries are ignored
 - checkout(-index): do not checkout i-t-a entries
 - apply: make sure check_preimage() does not leave empty file on error
 - apply: fix adding new files on i-t-a entries
 - add and use a convenience macro ce_intent_to_add()
 - blame: remove obsolete comment

 Paths that have been told the index about with "add -N" are not yet
 in the index, but various commands behaved as if they already are.

 Some commits need better explanation.

 Waiting for a reroll.


* ld/p4-detached-head (2015-09-09) 2 commits
 - git-p4: work with a detached head
 - git-p4: add failing test for submit from detached head

 Will be rerolled.
 ($gmane/277574)


* mg/httpd-tests-update-for-apache-2.4 (2015-04-08) 2 commits
 - t/lib-git-svn: check same httpd module dirs as lib-httpd
 - t/lib-httpd: load mod_unixd

 This is the first two commits in a three-patch series $gmane/266962

 Becoming tired of waiting for a reroll.
 with updated log message ($gmane/268061).


* wp/sha1-name-negative-match (2015-06-08) 2 commits
 - sha1_name.c: introduce '^{/!-}' notation
 - test for '!' handling in rev-parse's named commits

 Introduce "branch^{/!-}" notation to name a commit
 reachable from branch that does not match the given pattern.

 Becoming tired of waiting for a reroll.
 ($gmane/271213).


* ak/format-patch-odir-config (2015-06-19) 1 commit
 - format-patch: introduce format.outputDirectory configuration

 Reroll exists but didn't pick it up as it seemed to be still
 

[PATCH] receive-pack: avoid sending duplicate "have" lines

[Lukas Fleischer]

Alternates and refs outside the current namespace are advertised as
"have" lines. To this end, the object identifiers of alternates are
collected in an array and repeated hashes are omitted before
transmission. In contrast, refs outside the used namespace are currently
converted into "have" lines and transmitted immediately, without
checking for duplicate lines. This means that exactly the same "have"
line might be transmitted several times.

Optimize this by using a single pool to collect all object identifiers
to be converted into "have" lines (including both alternates and refs
outside the namespace) first and transmit them later, omitting any
duplicates.

Suggested-by: Junio C Hamano 
Signed-off-by: Lukas Fleischer 
---
This is based on pu. I am not sure whether we should also change the
name of show_one_alternate_sha1() in this patch since it is now used
to transmit refs outside the current namespace as well...

 builtin/receive-pack.c | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index f06f70a..548d4ce 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -217,23 +217,24 @@ static void show_ref(const char *path, const unsigned 
char *sha1)
 }
 
 static int show_ref_cb(const char *path_full, const struct object_id *oid,
-  int flag, void *unused)
+  int flag, void *data)
 {
const char *path = strip_namespace(path_full);
 
if (ref_is_hidden(path, path_full))
return 0;
 
-   /*
-* Advertise refs outside our current namespace as ".have"
-* refs, so that the client can use them to minimize data
-* transfer but will otherwise ignore them. This happens to
-* cover ".have" that are thrown in by add_one_alternate_ref()
-* to mark histories that are complete in our alternates as
-* well.
-*/
-   if (!path)
-   path = ".have";
+   if (!path) {
+   /*
+* Advertise refs outside our current namespace as ".have"
+* refs, so that the client can use them to minimize data
+* transfer but will otherwise ignore them.
+*/
+   struct sha1_array *sa = data;
+   sha1_array_append(sa, oid->hash);
+   return 0;
+   }
+
show_ref(path, oid->hash);
return 0;
 }
@@ -254,9 +255,9 @@ static void write_head_info(void)
struct sha1_array sa = SHA1_ARRAY_INIT;
 
for_each_alternate_ref(collect_one_alternate_ref, );
+   for_each_ref(show_ref_cb, );
sha1_array_for_each_unique(, show_one_alternate_sha1, NULL);
sha1_array_clear();
-   for_each_ref(show_ref_cb, NULL);
if (!sent_capabilities)
show_ref("capabilities^{}", null_sha1);
 
-- 
2.6.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

[PATCH v2 1/3] git-p4: add failing test for submit from detached head

[Luke Diamand]

git-p4 can't submit from a detached head. This test case
demonstrates the problem.

Signed-off-by: Luke Diamand 
---
 t/t9800-git-p4-basic.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 90d41ed..114b19f 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -241,6 +241,22 @@ test_expect_success 'unresolvable host in P4PORT should 
display error' '
)
 '
 
+test_expect_failure 'submit from detached head' '
+   test_when_finished cleanup_git &&
+   git p4 clone --dest="$git" //depot &&
+   (
+   cd "$git" &&
+   git checkout p4/master &&
+   >detached_head_test &&
+   git add detached_head_test &&
+   git commit -m "add detached_head" &&
+   git config git-p4.skipSubmitEdit true &&
+   git p4 submit &&
+   git p4 rebase &&
+   git log p4/master | grep detached_head
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
2.6.0.rc3.238.gc07a1e8

--
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] wt-status: use strncmp() for length-limited string comparison

[René Scharfe]

When a branch name is longer than four characters, memcmp() can read
past the end of the string literal "HEAD".  Use strncmp() instead, which
stops at the end of a string.  This fixes the following test failures
with AddressSanitizer:

t3203-branch-output.sh   (Wstat: 256 Tests: 18 Failed: 
4)
  Failed tests:  12, 15-17
  Non-zero exit status: 1
t3412-rebase-root.sh (Wstat: 256 Tests: 31 Failed: 
3)
  Failed tests:  28-29, 31
  Non-zero exit status: 1
t3507-cherry-pick-conflict.sh(Wstat: 256 Tests: 31 Failed: 
4)
  Failed tests:  14, 29-31
  Non-zero exit status: 1
t3510-cherry-pick-sequence.sh(Wstat: 256 Tests: 39 Failed: 
14)
  Failed tests:  17, 22-26, 28-30, 34-35, 37-39
  Non-zero exit status: 1
t3420-rebase-autostash.sh(Wstat: 256 Tests: 28 Failed: 
4)
  Failed tests:  24-27
  Non-zero exit status: 1
t3404-rebase-interactive.sh  (Wstat: 256 Tests: 91 Failed: 
57)
  Failed tests:  17, 19, 21-42, 44, 46-74, 77, 81-82
  Non-zero exit status: 1
t3900-i18n-commit.sh (Wstat: 256 Tests: 34 Failed: 
1)
  Failed test:  34
  Non-zero exit status: 1
t5407-post-rewrite-hook.sh   (Wstat: 256 Tests: 14 Failed: 
6)
  Failed tests:  9-14
  Non-zero exit status: 1
t7001-mv.sh  (Wstat: 256 Tests: 46 Failed: 
5)
  Failed tests:  39-43
  Non-zero exit status: 1
t7509-commit.sh  (Wstat: 256 Tests: 12 Failed: 
2)
  Failed tests:  11-12
  Non-zero exit status: 1
t7512-status-help.sh (Wstat: 256 Tests: 39 Failed: 
35)
  Failed tests:  5-39
  Non-zero exit status: 1
t6030-bisect-porcelain.sh(Wstat: 256 Tests: 70 Failed: 
1)
  Failed test:  13
  Non-zero exit status: 1

Signed-off-by: Rene Scharfe 
---
 wt-status.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/wt-status.c b/wt-status.c
index 435fc28..8dc281b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1319,7 +1319,7 @@ static int grab_1st_switch(unsigned char *osha1, unsigned 
char *nsha1,
hashcpy(cb->nsha1, nsha1);
for (end = target; *end && *end != '\n'; end++)
;
-   if (!memcmp(target, "HEAD", end - target)) {
+   if (!strncmp(target, "HEAD", end - target)) {
/* HEAD is relative. Resolve it to the right reflog entry. */
strbuf_addstr(>buf,
  find_unique_abbrev(nsha1, DEFAULT_ABBREV));
-- 
2.6.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

[PATCH v2 3/3] git-p4: work with a detached head

[Luke Diamand]

When submitting, git-p4 finds the current branch in
order to know if it is allowed to submit (configuration
"git-p4.allowSubmit").

On a detached head, detecting the branch would fail, and
git-p4 would report a cryptic error.

This change teaches git-p4 to recognise a detached head and
submit successfully.

Signed-off-by: Luke Diamand 
---
 git-p4.py   | 23 ---
 t/t9800-git-p4-basic.sh |  2 +-
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 9d55f9c..0cfc866 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -544,7 +544,12 @@ def p4Where(depotPath):
 return clientPath
 
 def currentGitBranch():
-return read_pipe("git name-rev HEAD").split(" ")[1].strip()
+retcode = system(["git", "symbolic-ref", "-q", "HEAD"], ignore_error=True)
+if retcode != 0:
+# on a detached head
+return None
+else:
+return read_pipe(["git", "name-rev", "HEAD"]).split(" ")[1].strip()
 
 def isValidGitDir(path):
 if (os.path.exists(path + "/HEAD")
@@ -1653,8 +1658,6 @@ class P4Submit(Command, P4UserMap):
 def run(self, args):
 if len(args) == 0:
 self.master = currentGitBranch()
-if len(self.master) == 0 or not gitBranchExists("refs/heads/%s" % 
self.master):
-die("Detecting current git branch failed!")
 elif len(args) == 1:
 self.master = args[0]
 if not branchExists(self.master):
@@ -1662,9 +1665,10 @@ class P4Submit(Command, P4UserMap):
 else:
 return False
 
-allowSubmit = gitConfig("git-p4.allowSubmit")
-if len(allowSubmit) > 0 and not self.master in allowSubmit.split(","):
-die("%s is not in git-p4.allowSubmit" % self.master)
+if self.master:
+allowSubmit = gitConfig("git-p4.allowSubmit")
+if len(allowSubmit) > 0 and not self.master in 
allowSubmit.split(","):
+die("%s is not in git-p4.allowSubmit" % self.master)
 
 [upstream, settings] = findUpstreamBranchPoint()
 self.depotPath = settings['depot-paths'][0]
@@ -1732,7 +1736,12 @@ class P4Submit(Command, P4UserMap):
 self.check()
 
 commits = []
-for line in read_pipe_lines(["git", "rev-list", "--no-merges", 
"%s..%s" % (self.origin, self.master)]):
+if self.master:
+commitish = self.master
+else:
+commitish = 'HEAD'
+
+for line in read_pipe_lines(["git", "rev-list", "--no-merges", 
"%s..%s" % (self.origin, commitish)]):
 commits.append(line.strip())
 commits.reverse()
 
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 114b19f..0730f18 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -241,7 +241,7 @@ test_expect_success 'unresolvable host in P4PORT should 
display error' '
)
 '
 
-test_expect_failure 'submit from detached head' '
+test_expect_success 'submit from detached head' '
test_when_finished cleanup_git &&
git p4 clone --dest="$git" //depot &&
(
-- 
2.6.0.rc3.238.gc07a1e8

--
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/3] git-p4: add option to system() to return subshell status

[Luke Diamand]

Add an optional parameter ignore_error to the git-p4 system()
function. If used, it will return the subshell exit status
rather than throwing an exception.

Signed-off-by: Luke Diamand 
---
 git-p4.py | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 0093fa3..9d55f9c 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -192,14 +192,16 @@ def p4_has_move_command():
 # assume it failed because @... was invalid changelist
 return True
 
-def system(cmd):
+def system(cmd, ignore_error=False):
 expand = isinstance(cmd,basestring)
 if verbose:
 sys.stderr.write("executing %s\n" % str(cmd))
 retcode = subprocess.call(cmd, shell=expand)
-if retcode:
+if retcode and not ignore_error:
 raise CalledProcessError(retcode, cmd)
 
+return retcode
+
 def p4_system(cmd):
 """Specifically invoke p4 as the system command. """
 real_cmd = p4_build_cmd(cmd)
-- 
2.6.0.rc3.238.gc07a1e8

--
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] git-p4: allow submission from detached head

[Luke Diamand]

This is a reroll of my earlier patch to teach git-p4
about detached heads.

It uses Junio's suggestion of calling "git symbolic-ref"
to determine if we're on a detached head, rather than
parsing text strings.

Luke Diamand (3):
  git-p4: add failing test for submit from detached head
  git-p4: add option to system() to return subshell status
  git-p4: work with a detached head

 git-p4.py   | 29 -
 t/t9800-git-p4-basic.sh | 16 
 2 files changed, 36 insertions(+), 9 deletions(-)

-- 
2.6.0.rc3.238.gc07a1e8

--
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: What's cooking in git.git (Nov 2015, #02; Fri, 6)

[Edmundo Carmona Antoranz]

On Fri, Nov 6, 2015 at 5:41 PM, Junio C Hamano  wrote:
> I'll be offline for a few weeks, and Jeff King graciously agreed to
> help shepherd the project forward in the meantime as an interim
> maintainer.  Please be gentle.

Be gentle? To Jeff??? But he's an ass

Just kidding, man! Way to go, Peff! Not gonna offer my kind help cause
there's not much help I can offer at the time given my little
knowledge of the... 'business'. And Junio, if it's vacations, enjoy!
Pura vida, mae! (That'd be the tico version of 'Hakuna Matata!'
'tico' being 'Costa Rican' 'Costa Rica' being)

Cheers!
--
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] receive-pack: avoid sending duplicate "have" lines

[Junio C Hamano]

Lukas Fleischer  writes:

> @@ -254,9 +255,9 @@ static void write_head_info(void)
>   struct sha1_array sa = SHA1_ARRAY_INIT;
>  
>   for_each_alternate_ref(collect_one_alternate_ref, );
> + for_each_ref(show_ref_cb, );
>   sha1_array_for_each_unique(, show_one_alternate_sha1, NULL);

Heh, I didn't realize that we already have half a support for this
deduping.  Good find.

We used to show ".have" from alternates first and then our own, but
now we show the refs that matter and then ".have"s from alternates
and ".have"s for our repository outside the current namespace.  That
shouldn't cause problems and the result would probably make more
sense from aesthetics point of view ;-)

I suspect that many of these that turn into ".have"s point the same
object as those sit at the tip of our refs (e.g. tags in alternates
we borrow from, which are the folks of the same project that copy
the same tags from the same upstream).  I wonder if it easy to filter
them out?  After all, if we say object X sits at refs/tags/v1.0
there is no point showing ".have" for that same object X.

>   sha1_array_clear();
> - for_each_ref(show_ref_cb, NULL);
>   if (!sent_capabilities)
>   show_ref("capabilities^{}", null_sha1);
--
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] run-command: detect finished children by closed pipe rather than waitpid

[Stefan Beller]

Detect if a child stopped working by checking if their stderr pipe
was closed instead of checking their state with waitpid.
As waitpid is not fully working in Windows, this is an approach which
allows for better cross platform operation. (It's less code, too)

Previously we did not close the read pipe of finished children, which we
do now.

The old way missed some messages on an early abort. We just killed the
children and did not bother to look what was left over. With this approach
we'd send a signal to the children and wait for them to close the pipe to
have all the messages (including possible "killed by signal 15" messages).

To have the test suite passing as before, we allow for real graceful
abortion now. In case the user wishes to abort parallel execution
the user needs to provide either the signal used to kill all children
or the children are let run until they finish normally.

Signed-off-by: Stefan Beller 
---
 Hi,
 
 this applis on top of origin/sb/submodule-parallel-fetch,
 making Windows folks possibly even more happy. It makes the code easier
 to read and has less races on cleaning up a terminated child.
 
 It follows the idea of Johannes patch, instead of encoding information in .err
 I removed the in_use flag and added a state, currently having 3 states.
 
 Thanks,
 Stefan

 Johannes schrieb:
 > First let me say that I find it very questionable that the callbacks
 > receive a struct child_process.
 
 I tried to get rid of the child_process struct in the callbacks, but that's
 not as easy as one may think. The submodule fetch code looks like this:
 
 get_next_submodule(..) {
...
child_process_init(cp);
cp->dir = strbuf_detach(_path, NULL);
cp->env = local_repo_env;
cp->git_cmd = 1;
if (!spf->quiet)
strbuf_addf(err, "Fetching submodule %s%s\n",
spf->prefix, ce->name);
argv_array_init(>args);
argv_array_pushv(>args, spf->args.argv);
argv_array_push(>args, default_argv);
argv_array_push(>args, "--submodule-prefix");
argv_array_push(>args, submodule_prefix.buf);
 }
 
 So we need to have access to the args, and git_cmd, environment variables
 and the directory where to run the command in. (So quite a lot of the
 internals already)
 

 run-command.c  | 140 +++--
 run-command.h  |  12 +++--
 submodule.c|   3 --
 test-run-command.c |   3 --
 4 files changed, 69 insertions(+), 89 deletions(-)

diff --git a/run-command.c b/run-command.c
index 1fbd286..cf17baf 100644
--- a/run-command.c
+++ b/run-command.c
@@ -858,6 +858,12 @@ int capture_command(struct child_process *cmd, struct 
strbuf *buf, size_t hint)
return finish_command(cmd);
 }
 
+enum child_state {
+   FREE,
+   WORKING,
+   WAIT_CLEANUP,
+};
+
 static struct parallel_processes {
void *data;
 
@@ -869,7 +875,7 @@ static struct parallel_processes {
task_finished_fn task_finished;
 
struct {
-   unsigned in_use : 1;
+   enum child_state state;
struct child_process process;
struct strbuf err;
void *data;
@@ -923,7 +929,7 @@ static void kill_children(struct parallel_processes *pp, 
int signo)
int i, n = pp->max_processes;
 
for (i = 0; i < n; i++)
-   if (pp->children[i].in_use)
+   if (pp->children[i].state == WORKING)
kill(pp->children[i].process.pid, signo);
 }
 
@@ -967,7 +973,7 @@ static struct parallel_processes *pp_init(int n,
for (i = 0; i < n; i++) {
strbuf_init(>children[i].err, 0);
child_process_init(>children[i].process);
-   pp->pfd[i].events = POLLIN;
+   pp->pfd[i].events = POLLIN | POLLHUP;
pp->pfd[i].fd = -1;
}
sigchain_push_common(handle_children_on_signal);
@@ -1011,41 +1017,48 @@ static void set_nonblocking(int fd)
  *  0 if a new task was started.
  *  1 if no new jobs was started (get_next_task ran out of work, non critical
  *problem with starting a new command)
- * -1 no new job was started, user wishes to shutdown early.
+ * <0 no new job was started, user wishes to shutdown early. Use negative code
+ *to signal the children.
  */
 static int pp_start_one(struct parallel_processes *pp)
 {
-   int i;
+   int i, code;
 
for (i = 0; i < pp->max_processes; i++)
-   if (!pp->children[i].in_use)
+   if (pp->children[i].state == FREE)
break;
if (i == pp->max_processes)
die("BUG: bookkeeping is hard");
 
-   if (!pp->get_next_task(>children[i].data,
-  >children[i].process,
-  >children[i].err,
-  pp->data)) {
+   code = pp->get_next_task(>children[i].data,
+   

Re: [PATCH] run-command: detect finished children by closed pipe rather than waitpid

[Torsten Bögershausen]

On 07.11.15 00:48, Stefan Beller wrote:
>  
> +enum child_state {
> + FREE,
> + WORKING,
> + WAIT_CLEANUP,
> +};

This kind of  "generic names" feels are begging for 
name clash, some day, may be in the future.
How about something like this:
> + GIT_CP_FREE,
> + GIT_CP_WORKING,
> + GIT_CP_WAIT_CLEANUP,

--
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] In configure.ac, try -lpthread in $LIBS instead of $CFLAGS to make picky linkers happy

[Matthieu Moy]

"Rainer M. Canavan"  writes:

> Subject: Re: [PATCH] In configure.ac, try -lpthread in $LIBS instead of 
> $CFLAGS to make picky linkers happy

The patch looks good, but the subject should be improved. We normally
try to stick to 50 characters (to let "git log --oneline" fit in a 80
columns terminal). 50 is a bit strict and not always followed, but yours
is definitely too long.

Also, the convention is "subsystem: what you did". I'd suggest this:

configure.ac: try -lpthread in $LIBS instead of $CFLAGS

The "to make picky linkers happy" is important, but redundant with the
body of the message.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 4/4] Add Travis CI support

[Eric Sunshine]

On Fri, Nov 6, 2015 at 3:58 AM,   wrote:
> The tests are currently executed on "Ubuntu 12.04 LTS Server Edition
> 64 bit" and on "OS X Mavericks" using gcc and clang.
>
> Perforce and Git-LFS are installed and therefore available for the
> respective tests.
>
> Signed-off-by: Lars Schneider 
> ---
> diff --git a/.travis.yml b/.travis.yml
> @@ -0,0 +1,131 @@
> +  matrix:
> +-
> +  # NO_ICONV=YesPlease
> +- >
> +  NO_CURL=YesPlease
> +  NO_D_INO_IN_DIRENT=YesPlease
> +  NO_DEFLATE_BOUND=YesPlease
> +  NO_EXPAT=YesPlease
> +  NO_GECOS_IN_PWENT=YesPlease
> +  NO_GETTEXT=YesPlease
> +  NO_HMAC_CTX_CLEANUP=YesPlease
> +  NO_HSTRERROR=YesPlease
> +  NO_INET_NTOP=YesPlease
> +  NO_INET_PTON=YesPlease
> +  NO_INITGROUPS=YesPlease
> +  NO_INTTYPES_H=YesPlease
> +  NO_IPV6=YesPlease
> +  NO_IPV6=YesPlease
> +  NO_LIBGEN_H=YesPlease
> +  NO_MEMMEM=YesPlease
> +  NO_MKDTEMP=YesPlease
> +  NO_MKSTEMPS=YesPlease
> +  NO_MMAP=YesPlease
> +  NO_NSEC=YesPlease
> +  NO_OPENSSL=YesPlease
> +  NO_PERL=YesPlease
> +  NO_PTHREADS=YesPlease
> +  NO_REGEX=YesPlease
> +  NO_SETENV=YesPlease
> +  NO_SETITIMER=YesPlease
> +  NO_SOCKADDR_STORAGE=YesPlease
> +  NO_STRCASESTR=YesPlease
> +  NO_STRLCPY=YesPlease
> +  NO_STRTOUMAX=YesPlease
> +  NO_STRUCT_ITIMERVAL=YesPlease
> +  NO_SYMLINK_HEAD=YesPlease
> +  NO_SYS_POLL_H=YesPlease
> +  NO_SYS_SELECT_H=YesPlease
> +  NO_UINTMAX_T=YesPlease
> +  NO_UNSETENV=YesPlease

Hmm, aren't you losing test coverage by disabling some of these? Is
that desirable for continuous integration testing? Am I missing
something?
--
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 0/4] Add Travis CI support

[larsxschneider]

From: Lars Schneider 

diff to v3:
* Removed git-p4 OS X test fixes and submitted them in a separate roll as
  request by Junio [1].
* Fix brew formula typo.
  (thanks Eric)
* Move "make configure && ./configure" from "install" to "before_script" phase.
  (thanks Sebastian)
* Run a build configuration with all kinds of "NO_*" flags. I don't understand
  all of them. Please let me know if some of them don't make sense. I also
  noticed that "NO_ICONV" will cause various tests to fail [2]. Is this a bug
  that should be fixed as part of this roll? If this is no bug, would it make
  sense to remove this flag?
  (thanks Junio, Matthieu)
* Run make with multiple jobs. I found that -j2 is the fastest for building
  git on Travis. -j6 is the fastest to run the unit tests. I limited that to
  -j3 because I had the impression that I got more test failures with higher
  utilization.
  (thanks Jean-Noel, Matthieu)
* Run Linux builds on new Travis CI container infrastructure .
  (thanks Matthieu)
* Print Perforce/Git-LFS binary versions
* Run tests with test extension flags enabled. Unfortunately "GIT_TEST_LONG"
  caused "t0021" to fail on Linux [3]. I wasn't able to reproduce this on my
  local machine, yet. Ideas anyone? Furthermore "GETTEXT_POISON" causes a bunch
  of failed tests on Linux and OS X [4]. Is this intentional or a bug?
* Print verbose output of failed tests in the "after_failure" phase. Please
  note the little triangle on the left in the TravisCI log to expand the failed
  test output.
* Update LFS to 1.0.2
* Accept SIGPIPE output in some tests (similar to [5] by Junio)
* Make git-p4 tests more robust with a general timeout and retry logic for
  cleanup commands (I now the "rm" and "mkdir" retry looks stupid but I was
  able to reproduce this error on TravisCI consistently - does anyone have
  a possible explanation why this could be necessary? Open file handles?)
* Disable email notifications (at least initially). I found and fixed a few
  issues that caused flaky tests. However, I expect there are more and we can
  only find them over time. If you add this patch then TravisCI can run
  silently in the background. If people want to look at the TravisCI result
  then they can. All other people are not bothered by emails. If we find in
  six months from now that TravisCI has too many false failures then we could
  remove the .travis.yml, again, and move on.

I had to apply this patch on master to get David Turner's fix for flaky
t7063 [6]/[7].

You can find the TravisCI run for this roll here:
https://travis-ci.org/larsxschneider/git/builds/89598194

Cheers,
Lars

[1] http://thread.gmane.org/gmane.comp.version-control.git/279348/focus=279362
[2] https://travis-ci.org/larsxschneider/git/builds/89555114
[3] https://travis-ci.org/larsxschneider/git/builds/89554983
[4] https://travis-ci.org/larsxschneider/git/builds/89554899
[5] http://thread.gmane.org/gmane.comp.version-control.git/280120/focus=280543
[6] http://thread.gmane.org/gmane.comp.version-control.git/279647
[7] http://thread.gmane.org/gmane.comp.version-control.git/279889

Lars Schneider (4):
  add function test_must_fail_or_sigpipe and use it to fix flaky tests
  git-p4: add p4d timeout in tests
  git-p4: retry kill/cleanup operations in tests with timeout
  Add Travis CI support

 .travis.yml | 131 
 t/lib-git-p4.sh |  51 +---
 t/t5504-fetch-receive-strict.sh |   3 +-
 t/t5516-fetch-push.sh   |   8 +--
 t/test-lib-functions.sh |  23 +++
 5 files changed, 201 insertions(+), 15 deletions(-)
 create mode 100644 .travis.yml

--
2.5.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 v4 2/4] git-p4: add p4d timeout in tests

[Eric Sunshine]

On Fri, Nov 6, 2015 at 3:58 AM,   wrote:
> In rare cases p4d seems to hang. This watchdog will kill the p4d
> process after 300s in any case. That means each individual git p4 test
> needs to finish before 300s or it will fail.
>
> Signed-off-by: Lars Schneider 
> ---
> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
> @@ -81,6 +85,19 @@ start_p4d() {
> # will be caught with the "kill -0" check below.
> i=${P4D_START_PATIENCE:-300}
> pid=$(cat "$pidfile")
> +
> +   timeout=$(($(date +%s) + $P4D_TIMEOUT))
> +while true

The 'while' line is incorrectly indented with spaces rather than tabs.

> +   do
> +   if test $(date +%s) -gt $timeout

I don't know how portable you intend this to be, but 'date +%s' is not
universally supported (it's missing on Solaris, for instance, and
perhaps AIX). For 6a9d16a (filter-branch: add passed/remaining seconds
on progress, 2015-09-07), we ultimately settled upon detecting +%s
support dynamically:

if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'
# it's supported
fi

Perhaps you'd want to detect this via a lazy prerequisite and skip
this if not supported?

> +   then
> +   kill -9 $pid
> +   exit 1
> +   fi
> +   sleep 1
> +   done &
> +   watchdog_pid=$!
> +
> ready=
> while test $i -gt 0
> do
> @@ -131,7 +148,8 @@ kill_p4d() {
> done &&
> # complain if it would not die
> test_must_fail kill $pid >/dev/null 2>&1 &&
> -   rm -rf "$db" "$cli" "$pidfile"
> +   rm -rf "$db" "$cli" "$pidfile" &&
> +   retry_until_fail kill -9 $watchdog_pid
>  }
>
>  cleanup_git() {
> --
> 2.5.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 v4 3/4] git-p4: retry kill/cleanup operations in tests with timeout

[Lars Schneider]

> On 06 Nov 2015, at 10:28, Eric Sunshine  wrote:
> 
> On Fri, Nov 6, 2015 at 3:58 AM,   wrote:
>> In rare cases kill/cleanup operations in tests fail. Retry these
>> operations with a timeout to make the test less flaky.
>> 
>> Signed-off-by: Lars Schneider 
>> ---
>> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
>> +retry_until_success() {
>> +timeout=$(($(date +%s) + $RETRY_TIMEOUT))
>> +until "$@" 2>/dev/null || test $(date +%s) -gt $timeout
>> +do :
>> +done
>> +}
>> +
>> +retry_until_fail() {
>> +timeout=$(($(date +%s) + $RETRY_TIMEOUT))
>> +until ! "$@" 2>/dev/null || test $(date +%s) -gt $timeout
>> +do :
>> +done
>> +}
> 
> I'm confused by this. Patch 2/4 was already calling
> retry_until_fail(), but it's introduction seems to be here in patch
> 3/4. Am I missing something obvious?

No, my fault. I reordered the commits and forgot about this. I will fix the 
order in the next roll.

Thanks,
Lars--
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/4] add function test_must_fail_or_sigpipe and use it to fix flaky tests

[larsxschneider]

From: Lars Schneider 

t5516 "75 - deny fetch unreachable SHA1, allowtipsha1inwant=true" is
flaky in the following case:
1. remote upload-pack finds out "not our ref"
2. remote sends a response and closes the pipe
3. fetch-pack still tries to write commands to the remote upload-pack
4. write call in wrapper.c dies with SIGPIPE

t5504 "9 - push with transfer.fsckobjects" is flaky, too, and returns
SIGPIPE once in a while. I had to remove the final "To dst..." output
check because there is no output if the process dies with SIGPIPE.

This patch accepts the SIGPIPE exit as legitimate test exit.

Signed-off-by: Lars Schneider 
---
 t/t5504-fetch-receive-strict.sh |  3 +--
 t/t5516-fetch-push.sh   |  8 
 t/test-lib-functions.sh | 23 +++
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
index 44f3d5f..129efa8 100755
--- a/t/t5504-fetch-receive-strict.sh
+++ b/t/t5504-fetch-receive-strict.sh
@@ -111,8 +111,7 @@ test_expect_success 'push with transfer.fsckobjects' '
cd dst &&
git config transfer.fsckobjects true
) &&
-   test_must_fail git push --porcelain dst master:refs/heads/test >act &&
-   test_cmp exp act
+   test_must_fail_or_sigpipe git push --porcelain dst 
master:refs/heads/test >act
 '

 cat >bogus-commit <<\EOF
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index ec22c98..22a941b 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1162,15 +1162,15 @@ do
mk_empty shallow &&
(
cd shallow &&
-   test_must_fail git fetch ../testrepo/.git $SHA1_3 &&
-   test_must_fail git fetch ../testrepo/.git $SHA1_1 &&
+   test_must_fail_or_sigpipe git fetch ../testrepo/.git 
$SHA1_3 &&
+   test_must_fail_or_sigpipe git fetch ../testrepo/.git 
$SHA1_1 &&
git --git-dir=../testrepo/.git config 
uploadpack.allowreachablesha1inwant true &&
git fetch ../testrepo/.git $SHA1_1 &&
git cat-file commit $SHA1_1 &&
-   test_must_fail git cat-file commit $SHA1_2 &&
+   test_must_fail_or_sigpipe git cat-file commit $SHA1_2 &&
git fetch ../testrepo/.git $SHA1_2 &&
git cat-file commit $SHA1_2 &&
-   test_must_fail git fetch ../testrepo/.git $SHA1_3
+   test_must_fail_or_sigpipe git fetch ../testrepo/.git 
$SHA1_3
)
'
 done
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 73e37a1..19a598e 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -600,6 +600,29 @@ test_must_fail () {
return 0
 }

+# Similar to test_must_fail, but tolerates sigpipe signal, too.
+
+test_must_fail_or_sigpipe () {
+   "$@"
+   exit_code=$?
+   if test $exit_code = 0; then
+   echo >&2 "test_must_fail: command succeeded: $*"
+   return 1
+   elif test $exit_code -ne 141 && \
+test $exit_code -gt 129 && \
+test $exit_code -le 192; then
+   echo >&2 "test_must_fail: died by signal: $*"
+   return 1
+   elif test $exit_code = 127; then
+   echo >&2 "test_must_fail: command not found: $*"
+   return 1
+   elif test $exit_code = 126; then
+   echo >&2 "test_must_fail: valgrind error: $*"
+   return 1
+   fi
+   return 0
+}
+
 # Similar to test_must_fail, but tolerates success, too.  This is
 # meant to be used in contexts like:
 #
--
2.5.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 3/4] git-p4: retry kill/cleanup operations in tests with timeout

[larsxschneider]

From: Lars Schneider 

In rare cases kill/cleanup operations in tests fail. Retry these
operations with a timeout to make the test less flaky.

Signed-off-by: Lars Schneider 
---
 t/lib-git-p4.sh | 31 +++
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index b1660f6..9043b51 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -10,6 +10,10 @@ TEST_NO_CREATE_REPO=NoThanks
 # the defined timeout in seconds.
 P4D_TIMEOUT=300
 
+# Some operations require multiple attempts to be successful. Define
+# here the maximal retry timeout in seconds.
+RETRY_TIMEOUT=60
+
 . ./test-lib.sh
 
 if ! test_have_prereq PYTHON
@@ -138,14 +142,24 @@ p4_add_user() {
EOF
 }
 
+retry_until_success() {
+timeout=$(($(date +%s) + $RETRY_TIMEOUT))
+until "$@" 2>/dev/null || test $(date +%s) -gt $timeout
+do :
+done
+}
+
+retry_until_fail() {
+timeout=$(($(date +%s) + $RETRY_TIMEOUT))
+until ! "$@" 2>/dev/null || test $(date +%s) -gt $timeout
+do :
+done
+}
+
 kill_p4d() {
pid=$(cat "$pidfile")
-   # it had better exist for the first kill
-   kill $pid &&
-   for i in 1 2 3 4 5 ; do
-   kill $pid >/dev/null 2>&1 || break
-   sleep 1
-   done &&
+   retry_until_fail kill $pid
+   retry_until_fail kill -9 $pid
# complain if it would not die
test_must_fail kill $pid >/dev/null 2>&1 &&
rm -rf "$db" "$cli" "$pidfile" &&
@@ -153,8 +167,9 @@ kill_p4d() {
 }
 
 cleanup_git() {
-   rm -rf "$git" &&
-   mkdir "$git"
+   retry_until_success rm -r "$git"
+   test_must_fail test -d "$git" &&
+   retry_until_success mkdir "$git"
 }
 
 marshal_dump() {
-- 
2.5.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 2/4] git-p4: add p4d timeout in tests

[larsxschneider]

From: Lars Schneider 

In rare cases p4d seems to hang. This watchdog will kill the p4d
process after 300s in any case. That means each individual git p4 test
needs to finish before 300s or it will fail.

Signed-off-by: Lars Schneider 
---
 t/lib-git-p4.sh | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 7548225..b1660f6 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -6,6 +6,10 @@
 # a subdirectory called "$git"
 TEST_NO_CREATE_REPO=NoThanks
 
+# Sometimes p4d seems to hang. Terminate the p4d process automatically after
+# the defined timeout in seconds.
+P4D_TIMEOUT=300
+
 . ./test-lib.sh
 
 if ! test_have_prereq PYTHON
@@ -81,6 +85,19 @@ start_p4d() {
# will be caught with the "kill -0" check below.
i=${P4D_START_PATIENCE:-300}
pid=$(cat "$pidfile")
+
+   timeout=$(($(date +%s) + $P4D_TIMEOUT))
+while true
+   do
+   if test $(date +%s) -gt $timeout
+   then
+   kill -9 $pid
+   exit 1
+   fi
+   sleep 1
+   done &
+   watchdog_pid=$!
+
ready=
while test $i -gt 0
do
@@ -131,7 +148,8 @@ kill_p4d() {
done &&
# complain if it would not die
test_must_fail kill $pid >/dev/null 2>&1 &&
-   rm -rf "$db" "$cli" "$pidfile"
+   rm -rf "$db" "$cli" "$pidfile" &&
+   retry_until_fail kill -9 $watchdog_pid
 }
 
 cleanup_git() {
-- 
2.5.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 4/4] Add Travis CI support

[larsxschneider]

From: Lars Schneider 

The tests are currently executed on "Ubuntu 12.04 LTS Server Edition
64 bit" and on "OS X Mavericks" using gcc and clang.

Perforce and Git-LFS are installed and therefore available for the
respective tests.

Signed-off-by: Lars Schneider 
---
 .travis.yml | 131 
 1 file changed, 131 insertions(+)
 create mode 100644 .travis.yml

diff --git a/.travis.yml b/.travis.yml
new file mode 100644
index 000..61c70fa
--- /dev/null
+++ b/.travis.yml
@@ -0,0 +1,131 @@
+language: c
+
+os:
+  - linux
+  - osx
+
+compiler:
+  - clang
+  - gcc
+
+addons:
+  apt:
+packages:
+- language-pack-is
+
+env:
+  global:
+- P4_VERSION="15.1"
+- GIT_LFS_VERSION="1.0.2"
+- DEFAULT_TEST_TARGET=prove
+- GIT_PROVE_OPTS="--timer --jobs 3"
+- GIT_TEST_OPTS="--verbose --tee"
+- GETTEXT_ISO_LOCALE=YesPlease
+- GETTEXT_LOCALE=YesPlease
+# - GETTEXT_POISON=YesPlease
+- GIT_TEST_CHAIN_LINT=YesPlease
+- GIT_TEST_CLONE_2GB=YesPlease
+# - GIT_TEST_LONG=YesPlease
+  matrix:
+-
+  # NO_ICONV=YesPlease
+- >
+  NO_CURL=YesPlease
+  NO_D_INO_IN_DIRENT=YesPlease
+  NO_DEFLATE_BOUND=YesPlease
+  NO_EXPAT=YesPlease
+  NO_GECOS_IN_PWENT=YesPlease
+  NO_GETTEXT=YesPlease
+  NO_HMAC_CTX_CLEANUP=YesPlease
+  NO_HSTRERROR=YesPlease
+  NO_INET_NTOP=YesPlease
+  NO_INET_PTON=YesPlease
+  NO_INITGROUPS=YesPlease
+  NO_INTTYPES_H=YesPlease
+  NO_IPV6=YesPlease
+  NO_IPV6=YesPlease
+  NO_LIBGEN_H=YesPlease
+  NO_MEMMEM=YesPlease
+  NO_MKDTEMP=YesPlease
+  NO_MKSTEMPS=YesPlease
+  NO_MMAP=YesPlease
+  NO_NSEC=YesPlease
+  NO_OPENSSL=YesPlease
+  NO_PERL=YesPlease
+  NO_PTHREADS=YesPlease
+  NO_REGEX=YesPlease
+  NO_SETENV=YesPlease
+  NO_SETITIMER=YesPlease
+  NO_SOCKADDR_STORAGE=YesPlease
+  NO_STRCASESTR=YesPlease
+  NO_STRLCPY=YesPlease
+  NO_STRTOUMAX=YesPlease
+  NO_STRUCT_ITIMERVAL=YesPlease
+  NO_SYMLINK_HEAD=YesPlease
+  NO_SYS_POLL_H=YesPlease
+  NO_SYS_SELECT_H=YesPlease
+  NO_UINTMAX_T=YesPlease
+  NO_UNSETENV=YesPlease
+
+before_install:
+  - >
+case "${TRAVIS_OS_NAME:-linux}" in
+linux)
+  mkdir --parents custom/p4
+  pushd custom/p4
+wget --quiet 
http://filehost.perforce.com/perforce/r$P4_VERSION/bin.linux26x86_64/p4d
+wget --quiet 
http://filehost.perforce.com/perforce/r$P4_VERSION/bin.linux26x86_64/p4
+chmod u+x p4d
+chmod u+x p4
+export PATH="$(pwd):$PATH"
+  popd
+  mkdir --parents custom/git-lfs
+  pushd custom/git-lfs
+wget --quiet 
https://github.com/github/git-lfs/releases/download/v$GIT_LFS_VERSION/git-lfs-linux-amd64-$GIT_LFS_VERSION.tar.gz
+tar --extract --gunzip --file 
"git-lfs-linux-amd64-$GIT_LFS_VERSION.tar.gz"
+cp git-lfs-$GIT_LFS_VERSION/git-lfs .
+export PATH="$(pwd):$PATH"
+  popd
+  ;;
+osx)
+  brew_force_set_latest_binary_hash () {
+FORMULA=$1
+SHA=$(brew fetch --force $FORMULA 2>&1 | grep ^SHA256: | cut -d ' ' -f 
2)
+sed -E -i.bak "s/sha256 \"[0-9a-f]{64}\"/sha256 \"$SHA\"/g" \
+  /usr/local/Library/Taps/homebrew/homebrew-binary/$FORMULA.rb
+  }
+  brew update --quiet
+  brew tap homebrew/binary --quiet
+  brew_force_set_latest_binary_hash perforce
+  brew_force_set_latest_binary_hash perforce-server
+  brew install git-lfs perforce-server perforce
+  ;;
+esac;
+echo "$(tput setaf 6)Perfoce Server Version$(tput sgr0)";
+p4d -V | grep Rev.;
+echo "$(tput setaf 6)Perfoce Client Version$(tput sgr0)";
+p4 -V | grep Rev.;
+echo "$(tput setaf 6)Git-LFS Version$(tput sgr0)";
+git-lfs version;
+
+before_script: make configure && ./configure && make --jobs=2
+
+script: make --quiet test
+
+after_failure:
+  - >
+: '<-- Click here to see detailed test output! 
   ';
+for TEST_EXIT in t/test-results/*.exit;
+do
+  if [ "$(cat "$TEST_EXIT")" != "0" ];
+  then
+TEST_OUT="${TEST_EXIT%exit}out";
+echo 
"";
+echo "$(tput setaf 1)${TEST_OUT}...$(tput sgr0)";
+echo 
"";
+cat "${TEST_OUT}";
+  fi;
+done;
+
+notifications:
+  email: false
-- 
2.5.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 v4 3/4] git-p4: retry kill/cleanup operations in tests with timeout

[Eric Sunshine]

On Fri, Nov 6, 2015 at 3:58 AM,   wrote:
> In rare cases kill/cleanup operations in tests fail. Retry these
> operations with a timeout to make the test less flaky.
>
> Signed-off-by: Lars Schneider 
> ---
> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
> +retry_until_success() {
> +timeout=$(($(date +%s) + $RETRY_TIMEOUT))
> +until "$@" 2>/dev/null || test $(date +%s) -gt $timeout
> +do :
> +done
> +}
> +
> +retry_until_fail() {
> +timeout=$(($(date +%s) + $RETRY_TIMEOUT))
> +until ! "$@" 2>/dev/null || test $(date +%s) -gt $timeout
> +do :
> +done
> +}

I'm confused by this. Patch 2/4 was already calling
retry_until_fail(), but it's introduction seems to be here in patch
3/4. Am I missing something obvious?

>  kill_p4d() {
> pid=$(cat "$pidfile")
> -   # it had better exist for the first kill
> -   kill $pid &&
> -   for i in 1 2 3 4 5 ; do
> -   kill $pid >/dev/null 2>&1 || break
> -   sleep 1
> -   done &&
> +   retry_until_fail kill $pid
> +   retry_until_fail kill -9 $pid
> # complain if it would not die
> test_must_fail kill $pid >/dev/null 2>&1 &&
> rm -rf "$db" "$cli" "$pidfile" &&
> @@ -153,8 +167,9 @@ kill_p4d() {
>  }
>
>  cleanup_git() {
> -   rm -rf "$git" &&
> -   mkdir "$git"
> +   retry_until_success rm -r "$git"
> +   test_must_fail test -d "$git" &&
> +   retry_until_success mkdir "$git"
>  }
>
>  marshal_dump() {
> --
> 2.5.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 v2] gitk: add -C commandline parameter to change path

[Eric Sunshine]

On Thu, Nov 5, 2015 at 4:19 AM, Juha-Pekka Heikkila
 wrote:
> This patch adds -C (change working directory) parameter to
> gitk. With this parameter, instead of need to cd to directory
> with .git folder, one can point the correct folder from
> commandline.

Thanks, -C makes more sense than --cwd, and is more consistent with
other commands.

You'll want to also address the patch to Paul Mackerras (I've cc:'d
him) since he's the gitk maintainer, and Junio pulls gitk from him.

> v2: Adjusted the parameter as per Eric's suggestion. I think
> it now work in similar manner as in many GNU tools as well
> as git itself.

This sort of explanation of of changes between versions is definitely
welcome, but should be placed below the "---" line just under your
sign-off so that it does not become part of the permanent commit
message.

> Signed-off-by: Juha-Pekka Heikkila 
> ---

You'd place your commentary about changes between patch versions here.
More below...

>  Documentation/gitk.txt |  7 +++
>  gitk-git/gitk  | 26 +-
>  2 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> @@ -12279,20 +12279,14 @@ setui $uicolor
>
>  setoptions
>
> -# check that we can find a .git directory somewhere...
> -if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
> -show_error {} . [mc "Cannot find a git repository here."]
> -exit 1
> -}
> -
>  set selecthead {}
>  set selectheadid {}
>
>  set revtreeargs {}
>  set cmdline_files {}
> -set i 0
>  set revtreeargscmd {}
> -foreach arg $argv {
> +for {set i 0} {$i < [llength $argv]} {incr i} {
> +   set arg [lindex $argv [expr {$i}]]
>  switch -glob -- $arg {
> "" { }
> "--" {
> @@ -12305,11 +12299,25 @@ foreach arg $argv {
> "--argscmd=*" {
> set revtreeargscmd [string range $arg 10 end]
> }
> +   "-C*" {
> +   if {[string length $arg] < 3} {
> +   incr i
> +   cd [lindex $argv [expr {$i}]]

What happens if no arguments follow -C (that is, the entire
command-line is "gitk -C")? Will this simply run "cd" with no argument
or with an empty argument or error out or what? Should there be a
check for this condition?

> +   continue

Why does this 'continue' need to be here? Am I missing something obvious?

> +   } else {
> +   cd [string range $arg 2 end]
> +   }
> +   }
> default {
> lappend revtreeargs $arg
> }
>  }
> -incr i
> +}
> +
> +# check that we can find a .git directory somewhere...
> +if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
> +show_error {} . [mc "Cannot find a git repository here."]
> +exit 1
>  }
>
>  if {$selecthead eq "HEAD"} {
> --
> 1.9.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 v2] gitk: add -C commandline parameter to change path

[Juha-Pekka Heikkila]

On 06.11.2015 11:48, Eric Sunshine wrote:

On Thu, Nov 5, 2015 at 4:19 AM, Juha-Pekka Heikkila
 wrote:

This patch adds -C (change working directory) parameter to
gitk. With this parameter, instead of need to cd to directory
with .git folder, one can point the correct folder from
commandline.


Thanks, -C makes more sense than --cwd, and is more consistent with
other commands.

You'll want to also address the patch to Paul Mackerras (I've cc:'d
him) since he's the gitk maintainer, and Junio pulls gitk from him.


v2: Adjusted the parameter as per Eric's suggestion. I think
 it now work in similar manner as in many GNU tools as well
 as git itself.


This sort of explanation of of changes between versions is definitely
welcome, but should be placed below the "---" line just under your
sign-off so that it does not become part of the permanent commit
message.


Ok, I'll make bit later another version. I didn't realize to check where 
to put this comment as some projects want these in the commit message.





Signed-off-by: Juha-Pekka Heikkila 
---


You'd place your commentary about changes between patch versions here.
More below...


  Documentation/gitk.txt |  7 +++
  gitk-git/gitk  | 26 +-
  2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
@@ -12279,20 +12279,14 @@ setui $uicolor

  setoptions

-# check that we can find a .git directory somewhere...
-if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
-show_error {} . [mc "Cannot find a git repository here."]
-exit 1
-}
-
  set selecthead {}
  set selectheadid {}

  set revtreeargs {}
  set cmdline_files {}
-set i 0
  set revtreeargscmd {}
-foreach arg $argv {
+for {set i 0} {$i < [llength $argv]} {incr i} {
+   set arg [lindex $argv [expr {$i}]]
  switch -glob -- $arg {
 "" { }
 "--" {
@@ -12305,11 +12299,25 @@ foreach arg $argv {
 "--argscmd=*" {
 set revtreeargscmd [string range $arg 10 end]
 }
+   "-C*" {
+   if {[string length $arg] < 3} {
+   incr i
+   cd [lindex $argv [expr {$i}]]


What happens if no arguments follow -C (that is, the entire
command-line is "gitk -C")? Will this simply run "cd" with no argument
or with an empty argument or error out or what? Should there be a
check for this condition?


+   continue


Why does this 'continue' need to be here? Am I missing something obvious?



I'll add the missing try/catch for these cd commands, I slowly start to 
understand how tcl work. Earlier catch was not for some reason needed 
here when the loop was ran with 'foreach', error was reported once 
reached UI but now it seem to advertise "error in startup script.." 
'continue' was part of this skipping but doesn't do its trick anymore.


Anyway, I'll make v3 and resend. :)


+   } else {
+   cd [string range $arg 2 end]
+   }
+   }
 default {
 lappend revtreeargs $arg
 }
  }
-incr i
+}
+
+# check that we can find a .git directory somewhere...
+if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
+show_error {} . [mc "Cannot find a git repository here."]
+exit 1
  }

  if {$selecthead eq "HEAD"} {
--
1.9.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: A note from the maintainer

[Xue Fuqiao]

Hi Junio,

Thanks for writing this note!  It is very helpful.

On Fri, Nov 6, 2015 at 7:14 AM, Junio C Hamano  wrote:
> The list archive is available at a few public sites:
>
> http://news.gmane.org/gmane.comp.version-control.git/
> http://marc.theaimsgroup.com/?l=git
> http://www.spinics.net/lists/git/

The second link is broken.  The following link is the correct version
now:

https://marc.info/?l=git
--
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 v6 25/25] refs: break out ref conflict checks

[Junio C Hamano]

Michael Haggerty  writes:

> Junio, if there are no more comments, would you mind
>
> s/verify_no_descendants/find_descendant_ref/
>
> in the log message of this commit? And then, if you are also OK with the
> new subdirectory introduced in this patch series, David and I seem to be
> in agreement that it is ready to go. It would be great if this patch
> series could be merged in a timely manner, as it will conflict with
> nearly any other changes that people might want to undertake in the refs
> code.

Thanks for working well together.  Let me see what I can do today...
--
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 v6 25/25] refs: break out ref conflict checks

[Michael Haggerty]

On 11/05/2015 05:22 PM, David Turner wrote:
> [...] But while looking at it, I
> noticed that the commit message doesn't look quite right (my fault):
> 
> On Thu, 2015-11-05 at 05:00 +0100, Michael Haggerty wrote:
>> Create new function verify_no_descendants, to hold one of the ref
>> conflict checks used in verify_refname_available. Multiple backends
>> will need this function, so move it to the common code.
> 
> The function is find_descendant_ref not verify_no_descendants.

Thanks for noticing.

Junio, if there are no more comments, would you mind

s/verify_no_descendants/find_descendant_ref/

in the log message of this commit? And then, if you are also OK with the
new subdirectory introduced in this patch series, David and I seem to be
in agreement that it is ready to go. It would be great if this patch
series could be merged in a timely manner, as it will conflict with
nearly any other changes that people might want to undertake in the refs
code.

Thanks,
Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
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/4] add function test_must_fail_or_sigpipe and use it to fix flaky tests

[Junio C Hamano]

Junio C Hamano  writes:

> Adding the third variant in the way this patch does is making things
> worse by inviting more mistakes.
>
> How about doing something like the attached to consolidate the
> existing two into one, and then build this third one on top?

Actually, I think this other variant I came up with is cleaner and
is more easily extensible.  It already has the support for sigpipe,
e.g. you can do

test_must_fail ok=sigpipe git fetch ...

Of course, you can do

test_must_fail_or_sigpipe () {
test_must_fail ok=sigpipe "$@"
}

if it is easier to read.  I do not have a very strong opinion either
way.

 t/test-lib-functions.sh | 28 +---
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index e8d3c0f..b732f87 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -560,11 +560,26 @@ test_line_count () {
 # the failure could be due to a segv.  We want a controlled failure.
 
 test_must_fail () {
+   case "$1" in
+   ok=*)
+   _test_ok=${1#ok=}
+   shift
+   ;;
+   *)
+   _test_ok=
+   ;;
+   esac
"$@"
exit_code=$?
-   if test $exit_code = 0; then
+   if ! case ",$_test_ok," in *,success,*) false;; esac &&
+  test $exit_code = 0
+   then
echo >&2 "test_must_fail: command succeeded: $*"
return 1
+   elif ! case ",$_test_ok," in *,sigpipe,*) false;; esac &&
+  test $exit_code = 141
+   then
+   return 0
elif test $exit_code -gt 129 && test $exit_code -le 192; then
echo >&2 "test_must_fail: died by signal: $*"
return 1
@@ -590,16 +605,7 @@ test_must_fail () {
 # because we want to notice if it fails due to segv.
 
 test_might_fail () {
-   "$@"
-   exit_code=$?
-   if test $exit_code -gt 129 && test $exit_code -le 192; then
-   echo >&2 "test_might_fail: died by signal: $*"
-   return 1
-   elif test $exit_code = 127; then
-   echo >&2 "test_might_fail: command not found: $*"
-   return 1
-   fi
-   return 0
+   test_must_fail ok=success "$@"
 }
 
 # Similar to test_must_fail and test_might_fail, but check that a

--
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: Odd problems trying to build an orphaned branch

[Jeff King]

On Fri, Nov 06, 2015 at 10:32:56AM -0800, a...@clueserver.org wrote:

> > Yes. There is no defined format for merge patches, so git-format-patch
> > cannot show them. What you're trying to do won't work.
> 
> This makes me worry about using git-format-patch. If it cannot handle
> merge commits correctly, then using it to send patches to customers is
> risky at best. (I work for a place that does not want to distribute the
> kernel, just patches on top of the kernel. The case of having a large
> number of merge commits in the tree seems to break that.)

If you do not know if the history contains merges and are blindly using
format-patch, you are right to be worried. It will not work well for
your case.

> > Was it a local clone? Depth is ignored for those (it _should_ print a
> > warning). If so, try --no-local to make it act like a "regular" clone.
> 
> I did not add any options for "local" vs "regular". What defines that?

If the clone is on the local filesystem (i.e., the source is a regular
path, not a URL or an ssh endpoint), git will optimize some of the
transfer. For example, it will hardlink objects, which makes computing a
shallow clone more expensive than simply providing all of the objects.

But it should warn in this case.

For example:

  $ git clone --depth=1 /home/peff/compile/linux clone-of-linux
  Cloning into 'clone-of-linux'...
  warning: --depth is ignored in local clones; use file:// instead.
  done.

You can disable these local optimizations by using a "file://" URL
instead of just a filename, or by using the "--no-local" flag.

> > Of course that leaves only the problem that filter-branch is
> > horrendously slow (for the kernel, most of the time goes to populating
> > the index for each commit; I think filter-branch could probably learn to
> > skip this step if there is no index or tree filter at work).
> 
> I have to only run this once, so I don't care. Running at all would be nice.

It may be sufficiently slow on the kernel that it will not count as
"running it all".  :)

The patch I posted earlier seemed to make it workable.

Yet another option (because you wanted more, right?) is to pipe
git-fast-export into git-fast-import. Something like:

  git fast-export \
--no-data \
--refspec refs/heads/master:refs/heads/filtered \
v3.12-rc1..master |
  git fast-import

I don't know if there are any gotchas there, though.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-stash: Don't GPG sign when stashing changes

[Junio C Hamano]

Cameron Currie  writes:

> This is helpful for folks with commit.gpgsign = true in their .gitconfig.
> ---
>  git-stash.sh | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

I have to wonder if the right fix is to change "git commit-tree" to
ignore that configuration variable?  After all, the plumbing
commands are about stability of the interface, not convenience, and
it feels wrong for them to be affected by end-user configurations.

Patching git-stash.sh does not help other scripts (either in-tree or
third-party) that use commit-tree; they will also be broken by
over-eager users who set commit.gpgsign configuration.

>
> diff --git a/git-stash.sh b/git-stash.sh
> index c7c65e2..fcf01b9 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -85,7 +85,7 @@ create_stash () {
>   # state of the index
>   i_tree=$(git write-tree) &&
>   i_commit=$(printf 'index on %s\n' "$msg" |
> - git commit-tree $i_tree -p $b_commit) ||
> + git commit-tree --no-gpg-sign $i_tree -p $b_commit) ||
>   die "$(gettext "Cannot save the current index state")"
>  
>   if test -n "$untracked"
> @@ -99,7 +99,7 @@ create_stash () {
>   rm -f "$TMPindex" &&
>   git update-index -z --add --remove --stdin &&
>   u_tree=$(git write-tree) &&
> - printf 'untracked files on %s\n' "$msg" | git 
> commit-tree $u_tree  &&
> + printf 'untracked files on %s\n' "$msg" | git 
> commit-tree --no-gpg-sign $u_tree  &&
>   rm -f "$TMPindex"
>   ) ) || die "Cannot save the untracked files"
>  
> @@ -153,7 +153,7 @@ create_stash () {
>   stash_msg=$(printf 'On %s: %s' "$branch" "$stash_msg")
>   fi
>   w_commit=$(printf '%s\n' "$stash_msg" |
> - git commit-tree $w_tree -p $b_commit -p $i_commit 
> $untracked_commit_option) ||
> + git commit-tree --no-gpg-sign $w_tree -p $b_commit -p $i_commit 
> $untracked_commit_option) ||
>   die "$(gettext "Cannot record working tree state")"
>  }
>  
>
> --
> https://github.com/git/git/pull/186
--
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 1/2] run-command: Remove set_nonblocking

[Stefan Beller]

On Thu, Nov 5, 2015 at 12:27 PM, Johannes Sixt  wrote:
> However, I think that the infrastructure can be simplified even further
> to a level that we do not need additional emulation on Windows.
>
> First let me say that I find it very questionable that the callbacks
> receive a struct child_process.

ok, I try to clean that up as well. However I made this choice as we
need to set the working dir as well as the environment variables.
I guess these will go into an extra argument then


> Here is a prototype patch. Feel free to pick it up. It marks a process
> whose EOF we have found by setting .err to -1. It's probably better to
> extend the meaning of the in_use indicator for this purpose.

Thanks for the proposal, I'll take that and make in_use a tristate for now
(an enum consisting of FREE, WORKING, WAIT_CLEANUP)

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] Update japanese translation

[Hiroshi Miura]

On Tue, Nov 03, 2015 at 12:40:45PM -0800, Junio C Hamano wrote:
> Could you redo this change against Paul Mackerras's tree, which is
> our upstream "gitk" project, that is found here:
> 
> git://git.kernel.org/pub/scm/gitk/gitk.git
> 
> and Cc Paul Mackerras   when you post your patch?
 
 Thank you for advise. I tried again.

-- 
Hiroshi Miura

--
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] gitk: Update Japanese translation

[miurahr]

From: Hiroshi Miura 

- Update untranslated terms in gitk in Japanese

Signed-off-by: Hiroshi Miura 
---
 gitk-git/po/ja.po | 119 --
 1 file changed, 53 insertions(+), 66 deletions(-)

diff --git a/gitk-git/po/ja.po b/gitk-git/po/ja.po
index 59e42a8..6d55bca 100644
--- a/gitk-git/po/ja.po
+++ b/gitk-git/po/ja.po
@@ -9,26 +9,27 @@ msgstr ""
 "Project-Id-Version: gitk\n"
 "Report-Msgid-Bugs-To: \n"
 "POT-Creation-Date: 2015-05-17 14:32+1000\n"
-"PO-Revision-Date: 2009-11-06 01:45+0900\n"
-"Last-Translator: Mizar \n"
+"PO-Revision-Date: 2015-11-04 08:47+0900\n"
+"Last-Translator: Hiroshi Miura \n"
 "Language-Team: Japanese\n"
-"Language: \n"
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
 "Plural-Forms: nplurals=1; plural=0;\n"
+"X-Generator: Poedit 1.5.4\n"
+"Language: ja\n"
 
 #: gitk:140
 msgid "Couldn't get list of unmerged files:"
-msgstr "マージされていないファイルのリストを取得できません:"
+msgstr "未マージのファイル一覧を取得できません:"
 
 #: gitk:212 gitk:2381
 msgid "Color words"
-msgstr ""
+msgstr "差分単語に着色"
 
 #: gitk:217 gitk:2381 gitk:8220 gitk:8253
 msgid "Markup words"
-msgstr ""
+msgstr "差分単語をマーク"
 
 #: gitk:324
 msgid "Error parsing revisions:"
@@ -176,11 +177,11 @@ msgstr "コミット"
 #: gitk:2299 gitk:2301 gitk:4687 gitk:4710 gitk:4734 gitk:6755 gitk:6827
 #: gitk:6912
 msgid "containing:"
-msgstr "含む:"
+msgstr "文字を含む:"
 
 #: gitk:2302 gitk:3526 gitk:3531 gitk:4763
 msgid "touching paths:"
-msgstr "パスの一部:"
+msgstr "パスに含む:"
 
 #: gitk:2303 gitk:4777
 msgid "adding/removing string:"
@@ -188,15 +189,15 @@ msgstr "追加/除去する文字列:"
 
 #: gitk:2304 gitk:4779
 msgid "changing lines matching:"
-msgstr ""
+msgstr "変更行が含む："
 
 #: gitk:2313 gitk:2315 gitk:4766
 msgid "Exact"
-msgstr "英字の大小を区別する"
+msgstr "大小文字を区別"
 
 #: gitk:2315 gitk:4854 gitk:6723
 msgid "IgnCase"
-msgstr "英字の大小を区別しない"
+msgstr "大小文字を同一視"
 
 #: gitk:2315 gitk:4736 gitk:4852 gitk:6719
 msgid "Regexp"
@@ -204,7 +205,7 @@ msgstr "正規表現"
 
 #: gitk:2317 gitk:2318 gitk:4874 gitk:4904 gitk:4911 gitk:6848 gitk:6916
 msgid "All fields"
-msgstr "全ての項目"
+msgstr "全項目"
 
 #: gitk:2318 gitk:4871 gitk:4904 gitk:6786
 msgid "Headline"
@@ -249,7 +250,7 @@ msgstr "空白の違いを無視"
 
 #: gitk:2378 gitk:2380 gitk:7959 gitk:8206
 msgid "Line diff"
-msgstr ""
+msgstr "行差分表示"
 
 #: gitk:2445
 msgid "Patch"
@@ -269,7 +270,7 @@ msgstr "選択したコミットとこれのdiffを見る"
 
 #: gitk:2619 gitk:2639
 msgid "Make patch"
-msgstr "パッチ作成"
+msgstr "パッチを作成"
 
 #: gitk:2620 gitk:9254
 msgid "Create tag"
@@ -301,26 +302,23 @@ msgstr "マークを付けた所に戻る"
 
 #: gitk:2627
 msgid "Find descendant of this and mark"
-msgstr "これとマークをつけた所との子孫を見つける"
+msgstr "これとマークを付けた所との子孫を見つける"
 
 #: gitk:2628
 msgid "Compare with marked commit"
 msgstr "マークを付けたコミットと比較する"
 
 #: gitk:2629 gitk:2640
-#, fuzzy
 msgid "Diff this -> marked commit"
-msgstr "これと選択したコミットのdiffを見る"
+msgstr "これ->マークしたコミットのdiff"
 
 #: gitk:2630 gitk:2641
-#, fuzzy
 msgid "Diff marked commit -> this"
-msgstr "選択したコミットとこれのdiffを見る"
+msgstr "マークしたコミット->これのdiff"
 
 #: gitk:2631
-#, fuzzy
 msgid "Revert this commit"
-msgstr "このコミットにマークをつける"
+msgstr "このコミットをリバートする"
 
 #: gitk:2647
 msgid "Check out this branch"
@@ -332,7 +330,7 @@ msgstr "このブランチを除去する"
 
 #: gitk:2649
 msgid "Copy branch name"
-msgstr ""
+msgstr "ブランチ名をコピーする"
 
 #: gitk:2656
 msgid "Highlight this too"
@@ -352,7 +350,7 @@ msgstr "親コミットから blame をかける"
 
 #: gitk:2660
 msgid "Copy path"
-msgstr ""
+msgstr "パスをコピー"
 
 #: gitk:2667
 msgid "Show origin of this line"
@@ -363,7 +361,6 @@ msgid "Run git gui blame on this line"
 msgstr "この行に git gui で blame をかける"
 
 #: gitk:3014
-#, fuzzy
 msgid ""
 "\n"
 "Gitk - a commit viewer for git\n"
@@ -375,7 +372,7 @@ msgstr ""
 "\n"
 "Gitk - gitコミットビューア\n"
 "\n"
-"Copyright \\u00a9 2005-2010 Paul Mackerras\n"
+"Copyright © 2005-2014 Paul Mackerras\n"
 "\n"
 "使用および再配布は GNU General Public License に従ってください"
 
@@ -397,9 +394,9 @@ msgid "<%s-Q>\t\tQuit"
 msgstr "<%s-Q>\t\t終了"
 
 #: gitk:3049
-#, fuzzy, tcl-format
+#, tcl-format
 msgid "<%s-W>\t\tClose window"
-msgstr "<%s-F>\t\t検索"
+msgstr "<%s-W>\t\tウインドウを閉じる"
 
 #: gitk:3050
 msgid "\t\tMove to first commit"
@@ -410,19 +407,16 @@ msgid "\t\tMove to last commit"
 msgstr "\t\t最後のコミットに移動"
 
 #: gitk:3052
-#, fuzzy
 msgid ", p, k\tMove up one commit"
-msgstr ", p, i\t一つ上のコミットに移動"
+msgstr ", p, k\t一つ上のコミットに移動"
 
 #: gitk:3053
-#, fuzzy
 msgid ", n, j\tMove down one commit"
-msgstr ", n, k\t一つ下のコミットに移動"
+msgstr ", n, j\t一つ下のコミットに移動"
 
 #: gitk:3054
-#, fuzzy
 msgid ", z, h\tGo back in history list"
-msgstr ", z, j\t履歴の前に戻る"
+msgstr ", z, h\t履歴の前に戻る"
 
 #: gitk:3055
 msgid ", x, l\tGo forward in history list"
@@ -431,7 +425,7 @@ msgstr ", x, l\t履歴の次へ進む"
 #: gitk:3056
 #, tcl-format
 msgid "<%s-n>\tGo to n-th parent of current commit in history list"
-msgstr ""
+msgstr "<%s-n>\tヒストリの現在からn番目の親コミットへ移動する"
 
 #: gitk:3057
 msgid "\tMove up one page in commit list"
@@ -514,9 +508,8 @@ msgid 

Re: git blame performance

[Michael Haggerty]

On 11/06/2015 02:37 PM, Jan Smets wrote:
> I have recently migrated a fairly large project from CVS to Git.
> One of the issues we're having is the blame/annotate performance.
> [...]
> cvs annotate of the same file (over the network) is ready in 0.8 seconds.
> blame/annotate is a frequently used operation, ranging between 5 to 20
> usages a day per developer.

cvs annotate and git blame both have to follow history back until they
find the commit that introduced the oldest line that is still in the
current version of the file. So for a really old file, a lot of history
has to be walked through.

The reason that cvs annotate is so much faster than git blame is that
CVS stores revisions filewise, with all of the modifications to file
$FILE being stored in a single $FILE,v file. So in the worst case, CVS
only has to read this one file.

Git, on the other hand, stores revisions treewise. It has no way of
knowing, ab initio, which revisions touched a given file. (In fact, this
concept is not even well-defined because the answer depends on things
like whether copy (-C) and move (-M) detection are turned on and what
parameters they were given.) This means that git blame has to traverse
most of history to find the commits that touched $FILE.

Slow git blame is thus a relatively unavoidable consequence of Git's
data model. That's not to say that it can't be sped up somewhat, but it
will never reach CVS speeds.

But it does have some features that can reduce the work:

-L ,, -L : -- Annotate only the given line range.
This option can speed things up (1) if the range of lines does not
include the oldest lines, (2) by limiting which parents of merge commits
have to be followed.

--incremental -- if you are using this command to build tooling, this
option allows partial results to be returned early, to reduce the wait
until the user sees something.

If you are not interested in changes older than a certain date or
revision, you can limit the amount of history that git blame traverses.
See SPECIFYING RANGES in the manpage.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
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 2/4] git-p4: add p4d timeout in tests

[Lars Schneider]

> On 06 Nov 2015, at 10:23, Eric Sunshine  wrote:
> 
> On Fri, Nov 6, 2015 at 3:58 AM,   wrote:
>> In rare cases p4d seems to hang. This watchdog will kill the p4d
>> process after 300s in any case. That means each individual git p4 test
>> needs to finish before 300s or it will fail.
>> 
>> Signed-off-by: Lars Schneider 
>> ---
>> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
>> @@ -81,6 +85,19 @@ start_p4d() {
>># will be caught with the "kill -0" check below.
>>i=${P4D_START_PATIENCE:-300}
>>pid=$(cat "$pidfile")
>> +
>> +   timeout=$(($(date +%s) + $P4D_TIMEOUT))
>> +while true
> 
> The 'while' line is incorrectly indented with spaces rather than tabs.'

Oh, you're right. One more thing that I will add to my pre mailing list submit 
check script :-)

> 
>> +   do
>> +   if test $(date +%s) -gt $timeout
> 
> I don't know how portable you intend this to be, but 'date +%s' is not
> universally supported (it's missing on Solaris, for instance, and
> perhaps AIX). For 6a9d16a (filter-branch: add passed/remaining seconds
> on progress, 2015-09-07), we ultimately settled upon detecting +%s
> support dynamically:
> 
>if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'
># it's supported
>fi
> 
> Perhaps you'd want to detect this via a lazy prerequisite and skip
> this if not supported?

AFAIK Perforce does not run on Solaris and AIX anyways (see supported platforms 
[1]). Therefore these tests should not be executed on these platforms. A lazy 
prerequisite sounds like a good idea to make this explicit!

Thanks,
Lars

[1] https://www.perforce.com/perforce/doc.current/user/relnotes.txt

--
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 4/4] Add Travis CI support

[Sebastian Schuberth]

On Fri, Nov 6, 2015 at 2:11 PM, Lars Schneider  wrote:

> Per platform/compiler (Linux/clang) we run two configurations. One
> normal configuration (see the lonely "-" right under "matrix:") and one
> configuration with all sorts of things are disabled ("NO...").
>
> You can see all 8 configurations ([linux, mac] * [clang, gcc] * [normal,
> NO_...]) here:
> https://travis-ci.org/larsxschneider/git/builds/89598194

Aren't these 8 configurations a bit too much? I see the total running
time is about 2 hours. For my taste, this is way to much to give
developers feedback about the status of their PR. It should be
something < 30 minutes.

IMO, the purpose of the Travis CI configuration mainly is to 1) save
developers work by not requiring to build manually, 2) build on other
platforms than the developer has access to. I doubt that the average
developer manually builds anything close to these 8 configurations
before we had this job, so I wouldn't make it a requirement for Travis
to do much more than a developer could / would to manually.

On the other hand, I see the point in letting a CI system test more
configurations than manually feasible. But that should be done as part
of a different job then. E.g. we could have a "fast" PR validation
job, and a "slow" nightly build job.

-- 
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 4/4] Add Travis CI support

[Sebastian Schuberth]

On Fri, Nov 6, 2015 at 2:28 PM, Lars Schneider  wrote:

> Well, I partly agree. Right now the running time is ~20 min (that means less 
> than your 30min target!). After ~10min you even have all Linux results, Mac 
> takes a bit longer. Travis shows you 2h because that is the time that would 
> be required if all builds where run one after another (we run builds in 
> parallel).

Are you sure about than? I mean, what sense does it make to show how
long it *would* have taken *if* the builds were running serially? I
can see that the longest of the jobs took 21 minutes, which is ok. But
that does not mean that all jobs completed in within 21 minutes. It
could be that not all jobs started at (about) the same time due to a
lack of resources, and that the last job did not compete before the 2
hours were over because it only started to run 1 hours and 40 minutes
befor ethe first job was started.

> That being said, I see your point of to avoiding to burn Travis CI resources 
> meaningless. If I am not mistaken then you can configure Travis in a way that 
> it runs different configurations for different branches. E.g. I would like to 
> run all 8 configurations on maint, master, next and maybe pu. All other 
> branches on peoples own forks should be fine with the default Linux build 
> (~10min).
>
> What do you think?

I think running different configuration per branch makes sense, yes.

-- 
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: A note from the maintainer

[Junio C Hamano]

Thanks.  I've known about the URL moving to marc.info for a long
time, and I am kind of surprised that I had this stale one left
un-updated for so long.

Fixed.

--
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 4/4] Add Travis CI support

[Lars Schneider]

> On 06 Nov 2015, at 14:57, Sebastian Schuberth  wrote:
> 
> On Fri, Nov 6, 2015 at 2:55 PM, Lars Schneider  
> wrote:
> 
>>> I think running different configuration per branch makes sense, yes.
>> 
>> If the list decides to accept this patch. Do you think that would be a 
>> necessary requirement for the first iteration?
> 
> No. I think this could be addressed later as an improvements. To me
> it's more important to finally get *some* sane Travis CI configuration
> in.
True. However, as I stated in my v4 cover letter the Travis CI integration is 
not yet perfect. I am constantly running builds to find flaky tests. Eg. here 
is one of them in git-p4 area that I will tackle next:
https://s3.amazonaws.com/archive.travis-ci.org/jobs/89603763/log.txt

I also see a weird "prove Tests our of sequence" error one in a while:
https://s3.amazonaws.com/archive.travis-ci.org/jobs/89603770/log.txt

Does anyone have an idea what could cause this?

Thanks,
Lars

--
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 4/4] Add Travis CI support

[Lars Schneider]

> On 06 Nov 2015, at 14:20, Sebastian Schuberth  wrote:
> 
> On Fri, Nov 6, 2015 at 2:11 PM, Lars Schneider  
> wrote:
> 
>> Per platform/compiler (Linux/clang) we run two configurations. One
>> normal configuration (see the lonely "-" right under "matrix:") and one
>> configuration with all sorts of things are disabled ("NO...").
>> 
>> You can see all 8 configurations ([linux, mac] * [clang, gcc] * [normal,
>> NO_...]) here:
>> https://travis-ci.org/larsxschneider/git/builds/89598194
> 
> Aren't these 8 configurations a bit too much? I see the total running
> time is about 2 hours. For my taste, this is way to much to give
> developers feedback about the status of their PR. It should be
> something < 30 minutes.
> 
> IMO, the purpose of the Travis CI configuration mainly is to 1) save
> developers work by not requiring to build manually, 2) build on other
> platforms than the developer has access to. I doubt that the average
> developer manually builds anything close to these 8 configurations
> before we had this job, so I wouldn't make it a requirement for Travis
> to do much more than a developer could / would to manually.
> 
> On the other hand, I see the point in letting a CI system test more
> configurations than manually feasible. But that should be done as part
> of a different job then. E.g. we could have a "fast" PR validation
> job, and a "slow" nightly build job.
> 

Well, I partly agree. Right now the running time is ~20 min (that means less 
than your 30min target!). After ~10min you even have all Linux results, Mac 
takes a bit longer. Travis shows you 2h because that is the time that would be 
required if all builds where run one after another (we run builds in parallel).

That being said, I see your point of to avoiding to burn Travis CI resources 
meaningless. If I am not mistaken then you can configure Travis in a way that 
it runs different configurations for different branches. E.g. I would like to 
run all 8 configurations on maint, master, next and maybe pu. All other 
branches on peoples own forks should be fine with the default Linux build 
(~10min).

What do you think?

Thanks,
Lars--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

git blame performance

[Jan Smets]

Hi

I have recently migrated a fairly large project from CVS to Git.
One of the issues we're having is the blame/annotate performance.

The repository contains +650k commits total, of which ~300k are on 
master. (raw size = ~8GB)


Running blame on one of the oldest files takes over 30 seconds.
This is on a fairly beefy (server) machine with lots of ram and the 
repository on a ramdisk. Running git 2.5.2


cvs annotate of the same file (over the network) is ready in 0.8 seconds.
blame/annotate is a frequently used operation, ranging between 5 to 20 
usages a day per developer.


I have two questions

 1) Is there a way to speed this up (in git)? eg: can it run multi 
threaded? build pre-cached blame views?
 2) I was thinking to work around the issue and use gitweb 
blame_incremental and pre-populate the cache.


If you can think of any other (short term) solutions I would really like 
to hear them.


Thank you

- Jan







--
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 4/4] Add Travis CI support

[Lars Schneider]

> On 06 Nov 2015, at 14:36, Sebastian Schuberth  wrote:
> 
> On Fri, Nov 6, 2015 at 2:28 PM, Lars Schneider  
> wrote:
> 
>> Well, I partly agree. Right now the running time is ~20 min (that means less 
>> than your 30min target!). After ~10min you even have all Linux results, Mac 
>> takes a bit longer. Travis shows you 2h because that is the time that would 
>> be required if all builds where run one after another (we run builds in 
>> parallel).
> 
> Are you sure about than? I mean, what sense does it make to show how
> long it *would* have taken *if* the builds were running serially? I
> can see that the longest of the jobs took 21 minutes, which is ok. But
> that does not mean that all jobs completed in within 21 minutes. It
> could be that not all jobs started at (about) the same time due to a
> lack of resources, and that the last job did not compete before the 2
> hours were over because it only started to run 1 hours and 40 minutes
> befor ethe first job was started.
I am fairly certain about this. 

See, here is the first configuration and the first test case of a job:
https://travis-ci.org/larsxschneider/git/jobs/89598195
[08:21:06] t0002-gitfile.sh 

Here is the last configuration and the last test case of the same job:
[08:51:22] t9903-bash-prompt.sh

~30 min for all 8 configurations. You can enable Travis CI for you GitHub 
account and try it easily yourself :-)

> 
>> That being said, I see your point of to avoiding to burn Travis CI resources 
>> meaningless. If I am not mistaken then you can configure Travis in a way 
>> that it runs different configurations for different branches. E.g. I would 
>> like to run all 8 configurations on maint, master, next and maybe pu. All 
>> other branches on peoples own forks should be fine with the default Linux 
>> build (~10min).
>> 
>> What do you think?
> 
> I think running different configuration per branch makes sense, yes.
If the list decides to accept this patch. Do you think that would be a 
necessary requirement for the first iteration?

Thanks,
Lars--
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] git-stash: Don't GPG sign when stashing changes

[Cameron Currie]

This is helpful for folks with commit.gpgsign = true in their .gitconfig.
---
 git-stash.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index c7c65e2..fcf01b9 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -85,7 +85,7 @@ create_stash () {
# state of the index
i_tree=$(git write-tree) &&
i_commit=$(printf 'index on %s\n' "$msg" |
-   git commit-tree $i_tree -p $b_commit) ||
+   git commit-tree --no-gpg-sign $i_tree -p $b_commit) ||
die "$(gettext "Cannot save the current index state")"
 
if test -n "$untracked"
@@ -99,7 +99,7 @@ create_stash () {
rm -f "$TMPindex" &&
git update-index -z --add --remove --stdin &&
u_tree=$(git write-tree) &&
-   printf 'untracked files on %s\n' "$msg" | git 
commit-tree $u_tree  &&
+   printf 'untracked files on %s\n' "$msg" | git 
commit-tree --no-gpg-sign $u_tree  &&
rm -f "$TMPindex"
) ) || die "Cannot save the untracked files"
 
@@ -153,7 +153,7 @@ create_stash () {
stash_msg=$(printf 'On %s: %s' "$branch" "$stash_msg")
fi
w_commit=$(printf '%s\n' "$stash_msg" |
-   git commit-tree $w_tree -p $b_commit -p $i_commit 
$untracked_commit_option) ||
+   git commit-tree --no-gpg-sign $w_tree -p $b_commit -p $i_commit 
$untracked_commit_option) ||
die "$(gettext "Cannot record working tree state")"
 }
 

--
https://github.com/git/git/pull/186
--
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/4] add function test_must_fail_or_sigpipe and use it to fix flaky tests

[Junio C Hamano]

larsxschnei...@gmail.com writes:

> From: Lars Schneider 
>
> t5516 "75 - deny fetch unreachable SHA1, allowtipsha1inwant=true" is
> flaky in the following case:
> 1. remote upload-pack finds out "not our ref"
> 2. remote sends a response and closes the pipe
> 3. fetch-pack still tries to write commands to the remote upload-pack
> 4. write call in wrapper.c dies with SIGPIPE
>
> t5504 "9 - push with transfer.fsckobjects" is flaky, too, and returns
> SIGPIPE once in a while. I had to remove the final "To dst..." output
> check because there is no output if the process dies with SIGPIPE.

Thanks for a clear write-up.

> This patch accepts the SIGPIPE exit as legitimate test exit.

It is not this patch that accepts such a failure as legit ;-).

"Accept such a death-with-sigpipe also as OK when we are expecting a
failure", perhaps?



> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index ec22c98..22a941b 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -1162,15 +1162,15 @@ do
>   mk_empty shallow &&
>   (
>   cd shallow &&
> - test_must_fail git fetch ../testrepo/.git $SHA1_3 &&
> - test_must_fail git fetch ../testrepo/.git $SHA1_1 &&
> + test_must_fail_or_sigpipe git fetch ../testrepo/.git 
> $SHA1_3 &&
> + test_must_fail_or_sigpipe git fetch ../testrepo/.git 
> $SHA1_1 &&
>   git --git-dir=../testrepo/.git config 
> uploadpack.allowreachablesha1inwant true &&

These I understand.

>   git fetch ../testrepo/.git $SHA1_1 &&
>   git cat-file commit $SHA1_1 &&
> - test_must_fail git cat-file commit $SHA1_2 &&
> + test_must_fail_or_sigpipe git cat-file commit $SHA1_2 &&

This I don't.  Under what condition is it sane for this "cat-file
commit" to fail with sigpipe?

>   git fetch ../testrepo/.git $SHA1_2 &&
>   git cat-file commit $SHA1_2 &&
> - test_must_fail git fetch ../testrepo/.git $SHA1_3
> + test_must_fail_or_sigpipe git fetch ../testrepo/.git 
> $SHA1_3

And this I do.

>   )
>   '
>  done
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 73e37a1..19a598e 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -600,6 +600,29 @@ test_must_fail () {
>   return 0
>  }
>
> +# Similar to test_must_fail, but tolerates sigpipe signal, too.
> +
> +test_must_fail_or_sigpipe () {
> + "$@"
> + exit_code=$?
> + if test $exit_code = 0; then
> + echo >&2 "test_must_fail: command succeeded: $*"
> + return 1
> + elif test $exit_code -ne 141 && \
> +  test $exit_code -gt 129 && \
> +  test $exit_code -le 192; then
> + echo >&2 "test_must_fail: died by signal: $*"
> + return 1
> + elif test $exit_code = 127; then
> + echo >&2 "test_must_fail: command not found: $*"
> + return 1
> + elif test $exit_code = 126; then
> + echo >&2 "test_must_fail: valgrind error: $*"
> + return 1
> + fi
> + return 0
> +}
> +

When you are coming up with this patch, you must have checked the
existing code around this area.  Did you notice that 126 is handled
differently between must_fail and might_fail?  Can you explain and
justify the difference?

One explanation I can think of is that the contributor who did
eeb69131 (tests: notice valgrind error in test_must_fail,
2013-03-31) did not notice that we do essentially the same thing in
might_fail and forgot to add it.  And it is not the fault of that
contributor, but the fault of the duplicated and poorly organized
code back then.

Adding the third variant in the way this patch does is making things
worse by inviting more mistakes.

How about doing something like the attached to consolidate the
existing two into one, and then build this third one on top?

 t/test-lib-functions.sh | 41 +
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index e8d3c0f..54497d6 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -560,22 +560,7 @@ test_line_count () {
 # the failure could be due to a segv.  We want a controlled failure.
 
 test_must_fail () {
-   "$@"
-   exit_code=$?
-   if test $exit_code = 0; then
-   echo >&2 "test_must_fail: command succeeded: $*"
-   return 1
-   elif test $exit_code -gt 129 && test $exit_code -le 192; then
-   echo >&2 "test_must_fail: died by signal: $*"
-   return 1
-   elif test $exit_code = 127; then
-   echo >&2 "test_must_fail: command not found: $*"
-   return 1
-   elif test 

Re: Odd problems trying to build an orphaned branch

[alan]

> On Thu, Nov 05, 2015 at 01:16:54PM -0800, a...@clueserver.org wrote:
>
>> I created an orphan branch from 3.12-rc1. I then used git format-patch
>> to
>> generate patches from 3.12-rc1 to HEAD. (Over 7000 patches.) I use git
>> am
>> to apply them to the orphan branch. At patch 237 it fails to apply. (It
>> appears the patch is from a block of code added with a merge commit, but
>> it is somewhere in the middle of the block.)
>>
>> Are merge commits supposed to screw up git-format-patch?
>
> Yes. There is no defined format for merge patches, so git-format-patch
> cannot show them. What you're trying to do won't work.

This makes me worry about using git-format-patch. If it cannot handle
merge commits correctly, then using it to send patches to customers is
risky at best. (I work for a place that does not want to distribute the
kernel, just patches on top of the kernel. The case of having a large
number of merge commits in the tree seems to break that.)

> If your goal is to have the history at HEAD truncated at 3.12-rc1, you
> are probably better off using a graft and having "filter-branch" rewrite
> the history based on that. That will preserve merges and the general
> shape of history.

I tried using that.  The documentation on how to do it correctly is vague.
It seemed to want to take the patches before the graft point, not after.
When filter-branch hit a commit with no author, it died. (It does not
allow a rewrite of a commit that does not have an author.)

>
>> I also tried using clone with depth and --single-branch set.  It ignored
>> the depth setting and gave me the whole branch all the way back to
>> 2.6.x.
>
> Was it a local clone? Depth is ignored for those (it _should_ print a
> warning). If so, try --no-local to make it act like a "regular" clone.

I did not add any options for "local" vs "regular". What defines that?

>> I tried using graft and filter-branch. None of the descriptions are very
>> clear. None of them worked either. Filter-branch died on a commit
>> somewhere in 2.6 land that had no author. (Which is outside of the
>> commits
>> I want to keep.)
>
> I suspect you need to graft more than just the commit at v3.12-rc1. For
> example, consider this history graph:
>
>   --A--B--C--D---G--H
>\/
> E--F
>
> If we imagine that H is the current HEAD, and D is our tag (v3.12-rc1),
> then making a cut between D and C will not have any effect on the side
> branch that contains E and F. Commits A and B are still reachable
> through them.
>
> You can find the complete set of boundary commits like this:
>
>   git log --boundary --format='%m %H' v3.12-rc1..HEAD
>
> and then graft them all like this:
>
>   git log --boundary --format='%m %H' v3.12-rc1..HEAD |
> grep ^- | cut -d' ' -f2 >.git/info/grafts
>
> Then you should be able to run "git filter-branch" to rewrite the
> history based on that.
>
> I think you can probably get the same effect by running:
>
>   git filter-branch v3.12-rc1..HEAD

I will try this and see what happens.

> Of course that leaves only the problem that filter-branch is
> horrendously slow (for the kernel, most of the time goes to populating
> the index for each commit; I think filter-branch could probably learn to
> skip this step if there is no index or tree filter at work).

I have to only run this once, so I don't care. Running at all would be nice.

>> I tried creating an orphan branch and using cherry-pick
>> v3.12-rc1..linux-3.12.y. It blew up on the first merge commit it hit. I
>> tried adding in "-m 1" to try to get it to pick a parent, but then it
>> died
>> on the first commit because it was not a merge.
>
> That won't do what you want. Cherry-pick doesn't preserve merges. When
> you pick a merge and choose a mainline, it is effectively saying "treat
> that as the only interesting parent" and squashes the result down to a
> single non-merge commit.
>
> If you wanted to follow this path (starting at an orphan and moving the
> patches over), I think rebase's "--preserve-merges" would be your best
> bet. It used to have some corner cases, though, and I don't know if
> those were ever fixed. I'd say filter-branch is the most-supported way
> to do what you want.
>
>> All I want to do is take a branch from linux-stable and create a branch
>> that contains just the commits from where it was branched off of master
>> until it hits HEAD. That is it. All the scripts that I have seen that
>> claim to do just what I want break when it hits a merge or a bogus
>> author.
>> (How that got into linux-stable, I have no idea. The commit is 10 year
>> old!)
>
> As an aside, which commit caused the bogus-author problem? Filter-branch
> generally tries to preserve or fix problems rather than barfing, exactly
> because it is often used to rewrite-out crap. I wonder if there is
> something it could be doing better (though again, I think in your case
> you are hitting the commit only because of an incomplete cut with your
> grafts).

I will try and 

Partnership,

[Rayher, Phillip B]

Partnership,
I would like to contact you personally for an important proposal that
that could be of interest to you .I have something very important to discuss 
with you. Contact me for details by: Email: 
wilfredbe...@lawyer.com with your direct 
contacts.

Best Regard,

Advocaat Wilfred Beyer.




--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git blame performance

[Junio C Hamano]

Michael Haggerty  writes:

> The reason that cvs annotate is so much faster than git blame is that
> CVS stores revisions filewise, with all of the modifications to file
> $FILE being stored in a single $FILE,v file. So in the worst case, CVS
> only has to read this one file.
>
> Git, on the other hand, stores revisions treewise. It has no way of
> knowing, ab initio, which revisions touched a given file. (In fact, this
> concept is not even well-defined because the answer depends on things
> like whether copy (-C) and move (-M) detection are turned on and what
> parameters they were given.) This means that git blame has to traverse
> most of history to find the commits that touched $FILE.
>
> Slow git blame is thus a relatively unavoidable consequence of Git's
> data model. That's not to say that it can't be sped up somewhat, but it
> will never reach CVS speeds.

Another thing to consider for a converted repository is that mass
converters tend to either not make a pack at all or make a pack that
is horribly inefficient to access.  Running "git repack -a -d -f"
with a small value of "--depth" may be a thing worth trying, if that
is contributing to the performance.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/4] git-p4: retry kill/cleanup operations in tests with timeout

[Lars Schneider]

> On 06 Nov 2015, at 10:28, Eric Sunshine  wrote:
> 
> On Fri, Nov 6, 2015 at 3:58 AM,   wrote:
>> In rare cases kill/cleanup operations in tests fail. Retry these
>> operations with a timeout to make the test less flaky.
>> 
>> Signed-off-by: Lars Schneider 
>> ---
>> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
>> +retry_until_success() {
>> +timeout=$(($(date +%s) + $RETRY_TIMEOUT))
>> +until "$@" 2>/dev/null || test $(date +%s) -gt $timeout
>> +do :
>> +done
>> +}
>> +
>> +retry_until_fail() {
>> +timeout=$(($(date +%s) + $RETRY_TIMEOUT))
>> +until ! "$@" 2>/dev/null || test $(date +%s) -gt $timeout
>> +do :
>> +done
>> +}
> 
> I'm confused by this. Patch 2/4 was already calling
> retry_until_fail(), but it's introduction seems to be here in patch
> 3/4. Am I missing something obvious?
No, my fault. I reordered the commits and forgot about this. I will fix the 
order in the next roll.

Thanks,
Lars
--
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 4/4] Add Travis CI support

[Lars Schneider]

> On 06 Nov 2015, at 10:56, Eric Sunshine  wrote:
> 
> On Fri, Nov 6, 2015 at 3:58 AM,   wrote:
>> The tests are currently executed on "Ubuntu 12.04 LTS Server Edition
>> 64 bit" and on "OS X Mavericks" using gcc and clang.
>> 
>> Perforce and Git-LFS are installed and therefore available for the
>> respective tests.
>> 
>> Signed-off-by: Lars Schneider 
>> ---
>> diff --git a/.travis.yml b/.travis.yml
>> @@ -0,0 +1,131 @@
>> +  matrix:
>> +-
>> +  # NO_ICONV=YesPlease
>> +- >
>> +  NO_CURL=YesPlease
>> +  NO_D_INO_IN_DIRENT=YesPlease
>> +  NO_DEFLATE_BOUND=YesPlease
>> +  NO_EXPAT=YesPlease
>> +  NO_GECOS_IN_PWENT=YesPlease
>> +  NO_GETTEXT=YesPlease
>> +  NO_HMAC_CTX_CLEANUP=YesPlease
>> +  NO_HSTRERROR=YesPlease
>> +  NO_INET_NTOP=YesPlease
>> +  NO_INET_PTON=YesPlease
>> +  NO_INITGROUPS=YesPlease
>> +  NO_INTTYPES_H=YesPlease
>> +  NO_IPV6=YesPlease
>> +  NO_IPV6=YesPlease
>> +  NO_LIBGEN_H=YesPlease
>> +  NO_MEMMEM=YesPlease
>> +  NO_MKDTEMP=YesPlease
>> +  NO_MKSTEMPS=YesPlease
>> +  NO_MMAP=YesPlease
>> +  NO_NSEC=YesPlease
>> +  NO_OPENSSL=YesPlease
>> +  NO_PERL=YesPlease
>> +  NO_PTHREADS=YesPlease
>> +  NO_REGEX=YesPlease
>> +  NO_SETENV=YesPlease
>> +  NO_SETITIMER=YesPlease
>> +  NO_SOCKADDR_STORAGE=YesPlease
>> +  NO_STRCASESTR=YesPlease
>> +  NO_STRLCPY=YesPlease
>> +  NO_STRTOUMAX=YesPlease
>> +  NO_STRUCT_ITIMERVAL=YesPlease
>> +  NO_SYMLINK_HEAD=YesPlease
>> +  NO_SYS_POLL_H=YesPlease
>> +  NO_SYS_SELECT_H=YesPlease
>> +  NO_UINTMAX_T=YesPlease
>> +  NO_UNSETENV=YesPlease
> 
> Hmm, aren't you losing test coverage by disabling some of these? Is
> that desirable for continuous integration testing? Am I missing
> something?
Per platform/compiler (Linux/clang) we run two configurations. One 
normal configuration (see the lonely "-" right under "matrix:") and one 
configuration with all sorts of things are disabled ("NO...").

You can see all 8 configurations ([linux, mac] * [clang, gcc] * [normal, 
NO_...]) here:
https://travis-ci.org/larsxschneider/git/builds/89598194

Cheers,
Lars 

--
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 4/4] Add Travis CI support

[Sebastian Schuberth]

On Fri, Nov 6, 2015 at 2:55 PM, Lars Schneider  wrote:

>> I think running different configuration per branch makes sense, yes.
>
> If the list decides to accept this patch. Do you think that would be a 
> necessary requirement for the first iteration?

No. I think this could be addressed later as an improvements. To me
it's more important to finally get *some* sane Travis CI configuration
in.

-- 
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re:git在不。(AD)

[drn]

git
欢迎加入爱奇艺vip分享QQ群：5650047 每天发放好几波爱奇艺vip会员，看蜀山战纪，灵魂摆渡2等热门大片不要钱！限时开放！验证码511










































































































































































































































































































































































































































































































































































































































































































































































































































































































































































































使得画面平稳？捆扎？砷角化病；睑裂高？相従倾盖只今年：Compass 
“行贺；动滑轮”大金园：穆释修。垃圾掠集兽”不同剂量！感物我心劳，清真八大碗，小引出线封装，硫酸奥西那林？少校的头冠：辐射烧嘴？研发机构“乙状结肠穿孔修补术，四季亭；多同僻处住：海上发射，先应种柳？王充道送水仙花五十支“全器官大切片(检查与诊断)：羽衣草属！超人风电工程概预算软件2009
 
2009”注意要点；泽天夬”南山假日，N�r��yb�X��ǧv�^�)޺{.n�+ا���ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf

Re: [PATCH 1/2] run-command: Remove set_nonblocking

[Johannes Sixt]

Am 06.11.2015 um 20:00 schrieb Stefan Beller:

On Thu, Nov 5, 2015 at 12:27 PM, Johannes Sixt  wrote:

Here is a prototype patch. Feel free to pick it up. It marks a process
whose EOF we have found by setting .err to -1. It's probably better to
extend the meaning of the in_use indicator for this purpose.


Thanks for the proposal, I'll take that and make in_use a tristate for now
(an enum consisting of FREE, WORKING, WAIT_CLEANUP)


I'd like to report that the prototype patch works on Windows. I tested 
it lightly using test-run-command with commands producing output from 
around 100 bytes to 10MB. So, I'm confident that this is the right approach.


Thank you for keeping the ball rolling!

-- 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