Re: [PATCH 4/8] strbuf: add strbuf_read_once to read without blocking

[Stefan Beller]

On Mon, Dec 14, 2015 at 3:16 PM, Eric Sunshine  wrote:
> On Mon, Dec 14, 2015 at 2:37 PM, Stefan Beller  wrote:
>> The new call will read from a file descriptor into a strbuf once. The
>> underlying call xread_nonblock is meant to execute without blocking if
>> the file descriptor is set to O_NONBLOCK. It is a bug to call
>> strbuf_read_once on a file descriptor which would block.
>>
>> Signed-off-by: Stefan Beller 
>> Signed-off-by: Junio C Hamano 
>> ---
>> diff --git a/strbuf.h b/strbuf.h
>> @@ -367,6 +367,14 @@ extern size_t strbuf_fread(struct strbuf *, size_t, 
>> FILE *);
>>  extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);
>>
>>  /**
>> + * Returns the number of new bytes appended to the sb.
>> + * Negative return value signals there was an error returned from
>> + * underlying read(2), in which case the caller should check errno.
>> + * e.g. errno == EAGAIN when the read may have blocked.
>> + */
>> +extern ssize_t strbuf_read_once(struct strbuf *, int fd, size_t hint);
>
> strbuf_read_once() is a rather opaque name; without reading the
> documentation, it's difficult to figure out what it means. I wonder if
> strbuf_read_nonblock() or something would be clearer?

Well the underlying read call can block if the fd is not marked for nonblocking,
so I would not name it _nonblock.
I just realize this same argument would make the naming moot for the previous
patch though. (xread_nonblock may block if not marked unblocking)

Currently we really want is a read once and do not try to grab as much
as possible,
but just return quickly. We do not do the non blocking setup, as we deemed that
unneeded because of the preceding poll in a higher layer to signal we have data
there to read.

Junio suggested in the discussion [4/8] maybe we can just use xread
and strbuf_read
in this series, so I am tempted to drop patches {3,4}/8 as that makes
sense to me.
For non blocking stuff we can later re introduce those helper functions though.
--
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 v1 4/4] git gui: allow for a long recentrepo list

[Philip Oakley]

From: "Eric Sunshine" 
On Mon, Dec 14, 2015 at 10:09 AM, Philip Oakley  
wrote:

The gui.recentrepo list may be longer than the maxrecent setting.
Allow extra space to show any extra entries.

In an ideal world, the git gui would limit the number of entries
to the maxrecent setting, however the recentrepo config list may
have been extended outwith the gui, or the maxrecent setting changed


s/outwith/without/


Ah, it's a Scottish english word, which better 'translates' as as outside, 
rather than being a commutation ;-)


I'll see if there are any other comments before a re-roll.




to a reduced value. Further, when testing the gui's recentrepo
logic it is useful to show these extra, but valid, entries.

Signed-off-by: Philip Oakley 
---
diff --git a/git-gui/lib/choose_repository.tcl 
b/git-gui/lib/choose_repository.tcl

index ad7a888..a08ed4f 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -153,7 +153,7 @@ constructor pick {} {
-background [get_bg_color $w_body.recentlabel] \
-wrap none \
-width 50 \
-   -height $maxrecent
+   -height [expr {$maxrecent + 5}]
$w_recentlist tag conf link \
-foreground blue \
-underline 1
--
2.5.2.windows.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



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

Re: [PATCH 3/8] xread_nonblock: add functionality to read from fds without blocking

[Stefan Beller]

On Mon, Dec 14, 2015 at 4:16 PM, Jeff King  wrote:
> On Mon, Dec 14, 2015 at 04:09:01PM -0800, Stefan Beller wrote:
>
>> > Are we trying to protect ourselves against somebody _else_ giving us a
>> > non-blocking descriptor? In that case we'll quietly spin and waste CPU.
>> > Which isn't great, but perhaps better than returning an error.
>>
>> Yes.
>> This sounds like a good reasoning for 2/8 (add in the poll, so we are
>> more polite), though.
>>
>> This patch is a prerequisite for 4/8, which explicitly doesn't want to loop
>> but a quick return. Maybe we could even drop this patch and just use
>> `read` as is in 4/8. Looking from a higher level perspective, we don't care
>> about strbuf_read_nonblocking to return after a signal without retry.
>
> I was actually thinking about simply teaching xread() not to worry about
> EAGAIN, but that would probably be a regression in the "whoops, somebody
> gave us a non-blocking stdin!" case.
>
> But yeah, I think simply using xread() as-is in strbuf_read_once (or
> whatever it ends up being called) is OK.

I was actually thinking about using {without-x}read, just the plain system call.
Do we have any issues with that for wrapping purposes for Windows?
There is no technical reason to prefer xread over read in strbuf_read_once as
* we are not nonblocking (so the EAGAIN|| EWOULDBLOCK doesn't apply)
* we don't care about EINTR and retrying upon that signal
* we would not care about MAX_IO_SIZE most likely (that's actually one
of the reasons I could technically think of to prefer xread)


> I think all of the
> _intentionally_ non-blocking descriptors are gone in this iteration,
> right?

I think we don't even have unintentional non blocking fds here now as we
create all the fds ourselves and never set the NOBLOCK flag.

> So the caller of strbuf_read_once expects to have to call poll()
> or to block. And that's what xread() does.

ok, I'll drop this patch and use xread there.

>
> -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 v2 0/3] format-patch: introduce option to suppress commit hashes

[brian m. carlson]

On Mon, Dec 14, 2015 at 04:32:39PM -0500, Jeff King wrote:
> The intent here makes sense to me, and with the exception of the
> test_line_count thing that Torsten mentioned, the code looks good.
> 
> I briefly wondered if the option should simply be "--diffable" or
> something like that, and trigger this new behavior as well as implying
> --no-signature. Along with any other relevant options (if any; I don't
> recall if --stat-width is terminal-dependent for format-patch, for
> example).
> 
> But that is probably overkill. People can flip those switches
> individually if they want to (and even if somebody did want
> "--diffable", it may make sense to build it on top, so they can flip the
> zero-commit thing individually if they want).

That does sound like a potentially worthwhile thing to build on top at
some point.

I'll reroll with the other suggested changes and a slight tweak to make
the tests less dependent on the history in both cases.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: PGP signature

Re: [PATCH 3/8] xread_nonblock: add functionality to read from fds without blocking

[Jeff King]

On Mon, Dec 14, 2015 at 03:15:29PM -0800, Junio C Hamano wrote:

> -- >8 --
> From: Stefan Beller 
> Date: Mon, 14 Dec 2015 11:37:13 -0800
> Subject: [PATCH] xread_nonblock: add functionality to read from fds without 
> blocking
> 
> Provide a wrapper to read(), similar to xread(), that restarts on
> EINTR but not EAGAIN (or EWOULDBLOCK). This enables the caller to
> handle polling itself, possibly polling multiple sockets or performing
> some other action.

This makes me wonder why we restart xread() on EAGAIN in the first
place.

On EINTR, sure; signals can come and we want to keep going. But if do
not have non-blocking descriptors, it should never happen, right?

Are we trying to protect ourselves against somebody _else_ giving us a
non-blocking descriptor? In that case we'll quietly spin and waste CPU.
Which isn't great, but perhaps better than returning an error.

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

[PATCH v3 2/3] format-patch: add an option to suppress commit hash

[brian m. carlson]

Oftentimes, patches created by git format-patch will be stored in
version control or compared with diff.  In these cases, two otherwise
identical patches can have different commit hashes, leading to diff
noise.  Teach git format-patch a --zero-commit option that instead
produces an all-zero hash to avoid this diff noise.

Signed-off-by: brian m. carlson 
---
 Documentation/git-format-patch.txt | 4 
 builtin/log.c  | 5 +
 log-tree.c | 3 ++-
 revision.h | 1 +
 t/t4014-format-patch.sh| 7 +++
 5 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-format-patch.txt 
b/Documentation/git-format-patch.txt
index 40356491..e3cdaeb9 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -256,6 +256,10 @@ you can use `--suffix=-patch` to get 
`0001-description-of-my-change-patch`.
using this option cannot be applied properly, but they are
still useful for code review.
 
+--zero-commit::
+  Output an all-zero hash in each patch's From header instead
+  of the hash of the commit.
+
 --root::
Treat the revision argument as a , even if it
is just a single commit (that would normally be treated as a
diff --git a/builtin/log.c b/builtin/log.c
index 069bd3a9..e00cea75 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1196,6 +1196,7 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
int cover_letter = -1;
int boundary_count = 0;
int no_binary_diff = 0;
+   int zero_commit = 0;
struct commit *origin = NULL;
const char *in_reply_to = NULL;
struct patch_ids ids;
@@ -1236,6 +1237,8 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
PARSE_OPT_NOARG | PARSE_OPT_NONEG, keep_callback },
OPT_BOOL(0, "no-binary", _binary_diff,
 N_("don't output binary diffs")),
+   OPT_BOOL(0, "zero-commit", _commit,
+N_("output all-zero hash in From header")),
OPT_BOOL(0, "ignore-if-in-upstream", _if_in_upstream,
 N_("don't include a patch matching a commit 
upstream")),
{ OPTION_SET_INT, 'p', "no-stat", _patch_format, NULL,
@@ -1380,6 +1383,8 @@ int cmd_format_patch(int argc, const char **argv, const 
char *prefix)
/* Always generate a patch */
rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
 
+   rev.zero_commit = zero_commit;
+
if (!DIFF_OPT_TST(, TEXT) && !no_binary_diff)
DIFF_OPT_SET(, BINARY);
 
diff --git a/log-tree.c b/log-tree.c
index 35e78017..f70a30e1 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -342,7 +342,8 @@ void log_write_email_headers(struct rev_info *opt, struct 
commit *commit,
 {
const char *subject = NULL;
const char *extra_headers = opt->extra_headers;
-   const char *name = oid_to_hex(>object.oid);
+   const char *name = oid_to_hex(opt->zero_commit ?
+ _oid : >object.oid);
 
*need_8bit_cte_p = 0; /* unknown */
if (opt->total > 0) {
diff --git a/revision.h b/revision.h
index 5bc96868..23857c0e 100644
--- a/revision.h
+++ b/revision.h
@@ -135,6 +135,7 @@ struct rev_info {
pretty_given:1,
abbrev_commit:1,
abbrev_commit_given:1,
+   zero_commit:1,
use_terminator:1,
missing_newline:1,
date_mode_explicit:1,
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 890db117..2737ca63 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1431,4 +1431,11 @@ test_expect_success 'cover letter auto user override' '
test_line_count = 2 list
 '
 
+test_expect_success 'format-patch --zero-commit' '
+   git format-patch --zero-commit --stdout v2..v1 >patch2 &&
+   grep "^From " patch2 | sort | uniq >actual &&
+   echo "From $_z40 Mon Sep 17 00:00:00 2001" >expect &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.7.0.rc0.194.g1187e4e.dirty

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

[PATCH v3 0/3] format-patch: introduce option to suppress commit hashes

[brian m. carlson]

git format-patch is often used to create patches that are then stored in
version control or displayed with diff.  Having the commit hash in the
"From " line usually just creates diff noise in these cases, so this
series introduces --zero-commit to set that to all zeros.

Changes from v2:
* Improve the tests to be more idiomatic and avoid hard-coding line
  counts.

brian m. carlson (3):
  Introduce a null_oid constant.
  format-patch: add an option to suppress commit hash
  format-patch: check that header line has expected format

 Documentation/git-format-patch.txt |  4 
 builtin/log.c  |  5 +
 cache.h|  1 +
 log-tree.c |  3 ++-
 revision.h |  1 +
 sha1_file.c|  1 +
 t/t4014-format-patch.sh| 14 ++
 7 files changed, 28 insertions(+), 1 deletion(-)

-- 
2.7.0.rc0.194.g1187e4e.dirty

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

[PATCH v3 1/3] Introduce a null_oid constant.

[brian m. carlson]

null_oid is the struct object_id equivalent to null_sha1.

Signed-off-by: brian m. carlson 
---
 cache.h | 1 +
 sha1_file.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/cache.h b/cache.h
index 5ab6cb50..c63fcc11 100644
--- a/cache.h
+++ b/cache.h
@@ -831,6 +831,7 @@ extern const char *find_unique_abbrev(const unsigned char 
*sha1, int len);
 extern int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len);
 
 extern const unsigned char null_sha1[GIT_SHA1_RAWSZ];
+extern const struct object_id null_oid;
 
 static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)
 {
diff --git a/sha1_file.c b/sha1_file.c
index 27ce7b70..a54deb05 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -36,6 +36,7 @@
 static inline uintmax_t sz_fmt(size_t s) { return s; }
 
 const unsigned char null_sha1[20];
+const struct object_id null_oid;
 
 /*
  * This is meant to hold a *small* number of objects that you would
-- 
2.7.0.rc0.194.g1187e4e.dirty

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

Re: [PATCH 3/8] xread_nonblock: add functionality to read from fds without blocking

[Jeff King]

On Mon, Dec 14, 2015 at 04:09:01PM -0800, Stefan Beller wrote:

> > Are we trying to protect ourselves against somebody _else_ giving us a
> > non-blocking descriptor? In that case we'll quietly spin and waste CPU.
> > Which isn't great, but perhaps better than returning an error.
> 
> Yes.
> This sounds like a good reasoning for 2/8 (add in the poll, so we are
> more polite), though.
> 
> This patch is a prerequisite for 4/8, which explicitly doesn't want to loop
> but a quick return. Maybe we could even drop this patch and just use
> `read` as is in 4/8. Looking from a higher level perspective, we don't care
> about strbuf_read_nonblocking to return after a signal without retry.

I was actually thinking about simply teaching xread() not to worry about
EAGAIN, but that would probably be a regression in the "whoops, somebody
gave us a non-blocking stdin!" case.

But yeah, I think simply using xread() as-is in strbuf_read_once (or
whatever it ends up being called) is OK.  I think all of the
_intentionally_ non-blocking descriptors are gone in this iteration,
right? So the caller of strbuf_read_once expects to have to call poll()
or to block. And that's what xread() does.

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

Re: [PATCH 3/8] xread_nonblock: add functionality to read from fds without blocking

[Jeff King]

On Mon, Dec 14, 2015 at 04:25:18PM -0800, Stefan Beller wrote:

> > But yeah, I think simply using xread() as-is in strbuf_read_once (or
> > whatever it ends up being called) is OK.
> 
> I was actually thinking about using {without-x}read, just the plain system 
> call.
> Do we have any issues with that for wrapping purposes for Windows?
> There is no technical reason to prefer xread over read in strbuf_read_once as
> * we are not nonblocking (so the EAGAIN|| EWOULDBLOCK doesn't apply)
> * we don't care about EINTR and retrying upon that signal
> * we would not care about MAX_IO_SIZE most likely (that's actually one
> of the reasons I could technically think of to prefer xread)

I think you do still need to care about EINTR, or at least not barfing
if read() returns -1. If I understand correctly, you want to do
something like:

  while (1) {
poll(some_fds);
for (i = 0; i < nr_fds; i++) {
if (some_fds[i].revents & POLLIN) {
int r = strbuf_read_once(buf[i], some_fds[i]);
/* ??? what do we do with r? */
}
}
  }

If we get EINTR from that read, it's OK for us to loop back to the
poll() and go again.

But if we get a true error in "r", we'd want to know, right? That means
we must distinguish between EINTR and "real" errors (like EIO or
something). We can do that here, but I think it's just as easy to do it
inside of strbuf_read_once (by calling xread() there). It's OK not to
jump back to the poll(), because we know the data that triggered the
POLLIN is still waiting for us to read it.

And we are fine with EAGAIN, too. We don't expect the sockets to be
non-blocking in the first place, but even if they were, we know we just
got POLLIN, so there should be data waiting.

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

Re: [PATCH 3/8] xread_nonblock: add functionality to read from fds without blocking

[Stefan Beller]

On Mon, Dec 14, 2015 at 3:57 PM, Jeff King  wrote:
> On Mon, Dec 14, 2015 at 03:15:29PM -0800, Junio C Hamano wrote:
>
>> -- >8 --
>> From: Stefan Beller 
>> Date: Mon, 14 Dec 2015 11:37:13 -0800
>> Subject: [PATCH] xread_nonblock: add functionality to read from fds without 
>> blocking
>>
>> Provide a wrapper to read(), similar to xread(), that restarts on
>> EINTR but not EAGAIN (or EWOULDBLOCK). This enables the caller to
>> handle polling itself, possibly polling multiple sockets or performing
>> some other action.
>
> This makes me wonder why we restart xread() on EAGAIN in the first
> place.
>
> On EINTR, sure; signals can come and we want to keep going. But if do
> not have non-blocking descriptors, it should never happen, right?

right.

>
> Are we trying to protect ourselves against somebody _else_ giving us a
> non-blocking descriptor? In that case we'll quietly spin and waste CPU.
> Which isn't great, but perhaps better than returning an error.

Yes.
This sounds like a good reasoning for 2/8 (add in the poll, so we are
more polite), though.

This patch is a prerequisite for 4/8, which explicitly doesn't want to loop
but a quick return. Maybe we could even drop this patch and just use
`read` as is in 4/8. Looking from a higher level perspective, we don't care
about strbuf_read_nonblocking to return after a signal without retry.

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

Re: [PATCH 3/8] xread_nonblock: add functionality to read from fds without blocking

[Junio C Hamano]

Jeff King  writes:

> Are we trying to protect ourselves against somebody _else_ giving us a
> non-blocking descriptor? In that case we'll quietly spin and waste CPU.
> Which isn't great, but perhaps better than returning an error.

I think I said it earlier in a message upthread.

> Ahh, there is a difference if the file descriptor the caller feeds
> strbuf_read_once() happens to be marked as nonblock.  read_once()
> wants to return without doing the poll() in such a case.  Even
> though this series would not introduce any use of a nonblocking file
> descriptor, as a general API function, [4/8] must be prepared for
> such a future caller, hence [3/8] is needed.

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

[PATCH v3 3/3] format-patch: check that header line has expected format

[brian m. carlson]

The format of the "From " header line is very specific to allow
utilities to detect Git-style patches.  Add a test that the patches
created are in the expected format.

Signed-off-by: brian m. carlson 
---
 t/t4014-format-patch.sh | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 2737ca63..646c4750 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1438,4 +1438,11 @@ test_expect_success 'format-patch --zero-commit' '
test_cmp expect actual
 '
 
+test_expect_success 'From line has expected format' '
+   git format-patch --stdout v2..v1 >patch2 &&
+   grep "^From " patch2 >from &&
+   grep "^From $_x40 Mon Sep 17 00:00:00 2001$" patch2 >filtered &&
+   test_cmp from filtered
+'
+
 test_done
-- 
2.7.0.rc0.194.g1187e4e.dirty

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

Questions about GIT

[Jack McLear]

Hi

I’ve recently been made aware of GIT and had a few questions.
I’m currently working on creating a middleware between FORAN (a CAD system) and 
Teamcenter.

Do you know if GIT would work between the two?

We’re currently using a Centralised version control system.

So to check my understanding, using GIT to create a distributed version control 
would have the following benefits
The CAD designer who has design a pump assembly for example could create an 
additional branch and “plays around” with the pump to make improvements without 
editing the main pump branch?
Could more than one user create independent branches?
If both users wanted to merge their independent branch with the main branch, 
what would happen? Would one take priority?

Is that the main benefit in terms of a CAD system, the branching ability?

Thanks

Jack

Re: [PATCH 7/8] config: add core.untrackedCache

[Christian Couder]

On Tue, Dec 8, 2015 at 11:43 PM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> Christian Couder  writes:
>>
>>> When we know that mtime is fully supported by the environment, we
>>> might want the untracked cache to be always used by default without
>>> any mtime test or kernel version check being performed.
>>>
>>> Also when we know that mtime is not supported by the environment,
>>> for example because the repo is shared over a network file system,
>>> then we might want 'git update-index --untracked-cache' to fail
>>> immediately instead of preforming tests (because it might work on
>>> some systems using the repo over the network file system but not
>>> others).
>>> ...
>> The logic in this paragraph is fuzzy to me.  Shouldn't the config
>> give a sensible default, that is overriden by command line options?

The problem is that "git update-index --[no-|force-]untracked-cache"
is not just changing things for the duration of the current command.
It is in fact changing the configuration of the repository, because it
is adding the UC (untracked cache) in the index and saving the index,
which will persist the use of the UC for the following commands.

So it is very different from something like "git status
--no-untracked-cache" that would perform a "git status" without using
the UC.

In fact "git update-index --[no-|force-]untracked-cache" is very bad
because it means that two repositories can be configured differently
even if they have the same config files.

>> I agree that it is insane to do a runtime check when the user says
>> "update-index --untracked-cache" to enable it, as the user _knows_
>> that enabling it would help (or the user _knows_ that she wants to
>> play with it).  Similarly, shouldn't the config be ignored when the
>> user says "update-index --no-untracked-cache" (hence removing the
>> untracked cache from the resulting index no matter the config is set
>> to)?  ...
>
> As I think about this more, it really seems to me that we shouldn't
> need to make this configuration variable that special.  Because I
> think it is a *BUG* in the current implementation to do the runtime
> check even when the user explicitly tells us to use untracked-cache,
> I'd imagine something along the lines of the following would be a
> lot simpler, easier to understand and a generally more useful
> bugfix:
>
>  1 Add one boolean configuration variable, core.untrackedCache, that
>defaults to 'false'.
>
>  2 When creating an index file in an empty repository, enable the
>untracked cache in the index (even without the user runninng
>"update-index --untracked-cache") iff the configuration is set to
>'true'.  No runtime check necessary.

I guess this means that when cloning a repo, it would not use the UC
unless either "git -c core.untrackedCache=true clone ..." is used, or
core.untrackedCache is set in the global config.

>  3 When working on an existing index file, unless the operation is
>"update-index --[no-]untracked-cache", keep the current setting,
>regardless of this configuration variable.  No runtime check
>necessary.
>
>  4 "update-index --[no-]untracked-cache" should enable or disable
>the untracked cache as the user tells us, regardless of the
>configuration variable.  No runtime check necessary.

If you want only some repos to use the UC, you will set
core.untrackedCache in the repo config. Then after cloning such a
repo, you will copy the config file, and this will not be enough to
enable the UC. The original repo will use the UC but the cloned one
will not, and you might wonder why is "git status" slower in the
cloned repo despite the machines and the config being the same for
both repos?

And if you have set core.untrackedCache in the global config when you
clone, UC is enabled, but if you have just set it in the repo config
after the clone, it is not enabled.

This is quite bad in my opinion.

Also think about system administrators who have a lot of machines with
lots of repos everywhere on these machines and just upgrade Git from
let's say v2.4.0 to v2.8.0. They find out that using UC git status
will be faster and want to provide that to the machine's users knowing
that they only use recent Linux machines where mtime works well.
Shouldn't it be nice if they could just enable core.untrackedCache in
the global config files without having to also cd into every repo and
use "git update-index --untracked-cache" there?

If we consider that "git update-index --[no-|force-]untracked-cache"
should not have been created in the first place, and that the good
mechanism for this is a config variable, and that maybe "git
update-index --[no-|force-]untracked-cache" should just be deprecated,
then the important thing is to make the core.untrackedCache config
variable work in the best possible way.

> It is OK to then add an "auto-detect" on top of the above, that
> would only affect the second bullet 

[PATCH v1 3/4] git gui: de-dup selected repo from recentrepo history

[Philip Oakley]

When the gui/user selects a repo for display, that repo is brought to
the end of the recentrepo config list. The logic can fail if there are
duplicate old entries for the repo (you cannot unset a single config
entry when duplicates are present).

Similarly, the maxrecentrepo logic could fail if older duplicate entries
are present.

The first commit of this series ({this}~2) fixed the config unsetting
issue. Rather than manipulating a local copy of the $recent list (one
cannot know how many entries were removed), simply re-read it.

Signed-off-by: Philip Oakley 
---
 git-gui/lib/choose_repository.tcl | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/git-gui/lib/choose_repository.tcl 
b/git-gui/lib/choose_repository.tcl
index aa87bcc..ad7a888 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -262,12 +262,11 @@ proc _append_recentrepos {path} {
set i [lsearch $recent $path]
if {$i >= 0} {
_unset_recentrepo $path
-   set recent [lreplace $recent $i $i]
}
 
-   lappend recent $path
git config --global --add gui.recentrepo $path
load_config 1
+   set recent [get_config gui.recentrepo]
 
if {[set maxrecent [get_config gui.maxrecentrepo]] eq {}} {
set maxrecent 10
@@ -275,7 +274,7 @@ proc _append_recentrepos {path} {
 
while {[llength $recent] > $maxrecent} {
_unset_recentrepo [lindex $recent 0]
-   set recent [lrange $recent 1 end]
+   set recent [get_config gui.recentrepo]
}
 }
 
-- 
2.5.2.windows.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 v1 1/4] git-gui: remove duplicate entries from .gitconfig's gui.recentrepo

[Philip Oakley]

The git gui's recent repo list may become contaminated with duplicate
entries. The git gui would barf when attempting to remove one entry.
Remove them all - there is no option within 'git config' to selectively
remove one of the entries.

This issue was reported on the 'Git User' list
(https://groups.google.com/forum/#!topic/git-users/msev4KsQGFc,
Warning: gui.recentrepo has multiply values while executing).

On startup the gui checks that entries in the recentrepo list are still
valid repos and deletes thoses that are not. If duplicate entries are
present the 'git config --unset' will barf and this prevents the gui
from starting.

Subsequent patches fix other parts of recentrepo logic used for syncing
internal lists with the external .gitconfig.

Reported-by: Alexey Astakhov 
Signed-off-by: Philip Oakley 
---
 git-gui/lib/choose_repository.tcl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-gui/lib/choose_repository.tcl 
b/git-gui/lib/choose_repository.tcl
index 75d1da8..133ca0a 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -247,7 +247,7 @@ proc _get_recentrepos {} {
 
 proc _unset_recentrepo {p} {
regsub -all -- {([()\[\]{}\.^$+*?\\])} $p {\\\1} p
-   git config --global --unset gui.recentrepo "^$p\$"
+   git config --global --unset-all gui.recentrepo "^$p\$"
load_config 1
 }
 
-- 
2.5.2.windows.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 v1 0/4] Fix git-gui when recentrepo list has duplicates

[Philip Oakley]

This is the patch series which follows from a user report of not being
able to start the git-gui when it contained duplicate entries.

The git gui design assumes that there will never be duplicate entries
in the recent repo list, and attempts to keep it that way.

For reasons unknown (other applications or tcl bugs?) there are cases
where the global .gitconfig does contain duplicate entries in the
gui.recentrepo config var. contrary to expectation.

The patch series fixes the root of the issue first, then patches the
logic for the two functional usages of _unset_recentrepo.

Finally, the displayable recentrepos list region is expanded to allow
extended recentrepo lists to at least be shown, before the git-gui
wrangles then back down to the allowed maxrecent size.

Philip Oakley (4):
  git-gui: remove duplicate entries from .gitconfig's gui.recentrepo
  git gui: cope with duplicates in _get_recentrepo
  git gui: de-dup selected repo from recentrepo history
  git gui: allow for a long recentrepo list

 git-gui/lib/choose_repository.tcl | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

-- 
2.5.2.windows.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 v1 4/4] git gui: allow for a long recentrepo list

[Philip Oakley]

The gui.recentrepo list may be longer than the maxrecent setting.
Allow extra space to show any extra entries.

In an ideal world, the git gui would limit the number of entries
to the maxrecent setting, however the recentrepo config list may
have been extended outwith the gui, or the maxrecent setting changed
to a reduced value. Further, when testing the gui's recentrepo
logic it is useful to show these extra, but valid, entries.

Signed-off-by: Philip Oakley 
---
 git-gui/lib/choose_repository.tcl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-gui/lib/choose_repository.tcl 
b/git-gui/lib/choose_repository.tcl
index ad7a888..a08ed4f 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -153,7 +153,7 @@ constructor pick {} {
-background [get_bg_color $w_body.recentlabel] \
-wrap none \
-width 50 \
-   -height $maxrecent
+   -height [expr {$maxrecent + 5}]
$w_recentlist tag conf link \
-foreground blue \
-underline 1
-- 
2.5.2.windows.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 v1 2/4] git gui: cope with duplicates in _get_recentrepo

[Philip Oakley]

_get_recentrepo will fail if duplicate invalid entries are present
in the recentrepo config list. The previous commit fixed the
'git config' limitations in _unset_recentrepo by unsetting all config
entries, however this code would fail on the second attempt to unset it.

Refactor the code to pre-sort and de-duplicate the recentrepo list to
avoid a potential second unset attempt.

Signed-off-by: Philip Oakley 
---
 git-gui/lib/choose_repository.tcl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-gui/lib/choose_repository.tcl 
b/git-gui/lib/choose_repository.tcl
index 133ca0a..aa87bcc 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -235,14 +235,14 @@ method _invoke_next {} {
 
 proc _get_recentrepos {} {
set recent [list]
-   foreach p [get_config gui.recentrepo] {
+   foreach p [lsort -unique [get_config gui.recentrepo]] {
if {[_is_git [file join $p .git]]} {
lappend recent $p
} else {
_unset_recentrepo $p
}
}
-   return [lsort $recent]
+   return $recent
 }
 
 proc _unset_recentrepo {p} {
-- 
2.5.2.windows.2

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

Re: [PATCH 3/3] ident: loosen getpwuid error in non-strict mode

[Jeff King]

On Thu, Dec 10, 2015 at 04:41:29PM -0500, Jeff King wrote:

> -static struct passwd *xgetpwuid_self(void)
> +static struct passwd *xgetpwuid_self(int *is_bogus)
>  {
>   struct passwd *pw;
>  
>   errno = 0;
>   pw = getpwuid(getuid());
> - if (!pw)
> - die(_("unable to look up current user in the passwd file: %s"),
> - errno ? strerror(errno) : _("no such user"));
> + if (!pw) {
> + struct passwd fallback;
> + fallback.pw_name = "unknown";
> +#ifndef NO_GECOS_IN_PWENT
> + fallback.pw_gecos = "Unknown";
> +#endif
> + pw = 
> + if (is_bogus)
> + *is_bogus = 1;
> + }
>   return pw;

Ugh. The fallback struct should be static, of course, as we are
returning its address from the function.

Anybody have a brown paper bag I can borrow?

diff --git a/ident.c b/ident.c
index 74de079..831072c 100644
--- a/ident.c
+++ b/ident.c
@@ -32,7 +32,7 @@ static struct passwd *xgetpwuid_self(int *is_bogus)
errno = 0;
pw = getpwuid(getuid());
if (!pw) {
-   struct passwd fallback;
+   static struct passwd fallback;
fallback.pw_name = "unknown";
 #ifndef NO_GECOS_IN_PWENT
fallback.pw_gecos = "Unknown";

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

[PATCH 1/2] push: add '--delete' flag to synopsis

[Patrick Steinhardt]

The delete flag is not mentioned in the synopsis of `git-push`.
Add the flag to make it more discoverable.

Signed-off-by: Patrick Steinhardt 
---
 Documentation/git-push.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 85a4d7d..e830c08 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic] [-n | 
--dry-run] [--receive-pack=]
-  [--repo=] [-f | --force] [--prune] [-v | --verbose]
+  [--repo=] [-f | --force] [--delete] [--prune] [-v | 
--verbose]
   [-u | --set-upstream]
   [--[no-]signed|--sign=(true|false|if-asked)]
   [--force-with-lease[=[:]]]
-- 
2.6.4

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

[PATCH 2/2] push: add '-d' as shorthand for '--delete'

[Patrick Steinhardt]

It is only possible to delete branches on remotes by specifying
the long '--delete' flag. The `git-branch` command, which can be
used to delete local branches with the same '--delete' flag, also
accepts the shorthand '-d'. This may cause confusion for users
which are frequently using the shorthand form of deleting local
branches and subsequently try to use the same shorthand for
`git-push`, which will fail.

Fix this usability issue by adding the '-d' shorthand to
`git-push` and document it.

Signed-off-by: Patrick Steinhardt 
---
 Documentation/git-push.txt | 2 +-
 builtin/push.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index e830c08..6f5d98c 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic] [-n | 
--dry-run] [--receive-pack=]
-  [--repo=] [-f | --force] [--delete] [--prune] [-v | 
--verbose]
+  [--repo=] [-f | --force] [-d | --delete] [--prune] [-v | 
--verbose]
   [-u | --set-upstream]
   [--[no-]signed|--sign=(true|false|if-asked)]
   [--force-with-lease[=[:]]]
diff --git a/builtin/push.c b/builtin/push.c
index 3bda430..093011d 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -540,7 +540,7 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
OPT_BIT( 0 , "all", , N_("push all refs"), 
TRANSPORT_PUSH_ALL),
OPT_BIT( 0 , "mirror", , N_("mirror all refs"),
(TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE)),
-   OPT_BOOL( 0, "delete", , N_("delete refs")),
+   OPT_BOOL('d', "delete", , N_("delete refs")),
OPT_BOOL( 0 , "tags", , N_("push tags (can't be used with 
--all or --mirror)")),
OPT_BIT('n' , "dry-run", , N_("dry run"), 
TRANSPORT_PUSH_DRY_RUN),
OPT_BIT( 0,  "porcelain", , N_("machine-readable 
output"), TRANSPORT_PUSH_PORCELAIN),
-- 
2.6.4

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

Where does http.sslcainfo get set in Windows (2.6.3)?

[Titus Barik]

Hi all,

I'm in Windows using git version: git version 2.6.3.windows.1. Git is
installed to /c/Users/tbarik/AppData/Local/Programs/Git/cmd/git.

However, when I look for the config name http.sslcainfo, it returns:

$ git config --get-all http.sslcainfo
C:/Program Files/Git/mingw64/ssl/certs/ca-bundle.crt

Although I can override the name, I'm trying to figure out where this is
being set, since the correct location should be (in this case)
C:/Users/tbarik/AppData/Local/Programs/Git/mingw64/ssl/certs/ca-bundle.crt.

I don't see C:/Program Files/Git/mingw64/ssl/certs/ca-bundle.crt in
either --global, --system, or --local, hence my confusion as to where
this path is coming from.

Thanks,

Titus

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

Re: [PATCH 2/2] rebase: fix preserving commits with --keep-empty

[Patrick Steinhardt]

On Thu, Dec 10, 2015 at 02:58:06PM -0800, Michael Blume wrote:
> This test does not seem to pass on my mac.
> 
> I've placed the verbose output here:
> https://gist.github.com/MichaelBlume/db7ba222be001d502e57
> 
> On Fri, Nov 20, 2015 at 4:04 AM, Patrick Steinhardt  wrote:
> > When rebasing commits where one or several commits are redundant
> > to commits on the branch that is being rebased upon we error out.
> > This is due to the usage of `--allow-empty` for the invoked
> > cherry-pick command, which will only cause _empty_ commits to be
> > picked instead of also allowing redundant commits. As
> > git-rebase(1) mentions, though, we also want to keep commits that
> > do not change anything from its parents, that is also redundant
> > commits.
> >
> > Fix this by invoking `git cherry-pick --keep-redundant-commits`
> > instead, which will cause redundant commits to be rebased
> > correctly.
> >
> > Signed-off-by: Patrick Steinhardt 
> > ---
> >  git-rebase--am.sh | 2 +-
> >  t/t3400-rebase.sh | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/git-rebase--am.sh b/git-rebase--am.sh
> > index 9ae898b..ea7b897 100644
> > --- a/git-rebase--am.sh
> > +++ b/git-rebase--am.sh
> > @@ -44,7 +44,7 @@ then
> > # empty commits and even if it didn't the format doesn't really lend
> > # itself well to recording empty patches.  fortunately, cherry-pick
> > # makes this easy
> > -   git cherry-pick ${gpg_sign_opt:+"$gpg_sign_opt"} --allow-empty \
> > +   git cherry-pick ${gpg_sign_opt:+"$gpg_sign_opt"} 
> > --keep-redundant-commits \
> > --right-only "$revisions" \
> > ${restrict_revision+^$restrict_revision}
> > ret=$?
> > diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
> > index 6cca319..f43b202 100755
> > --- a/t/t3400-rebase.sh
> > +++ b/t/t3400-rebase.sh
> > @@ -255,7 +255,7 @@ test_expect_success 'rebase commit with an ancient 
> > timestamp' '
> > grep "author .* 34567 +0600$" actual
> >  '
> >
> > -test_expect_failure 'rebase duplicated commit with --keep-empty' '
> > +test_expect_success 'rebase duplicated commit with --keep-empty' '
> > git reset --hard &&
> > git checkout master &&
> >
> > --
> > 2.6.3

Thanks for letting me know. I'll keep this in mind when there is
some consensus reached wether this option actually makes any
sense. Until then I'll just leave it as is.

Patrick


signature.asc
Description: Digital signature

Re: Questions about GIT

[Stefan Beller]

On Mon, Dec 14, 2015 at 2:48 AM, Jack McLear  wrote:
> Hi
>
> I’ve recently been made aware of GIT and had a few questions.
> I’m currently working on creating a middleware between FORAN (a CAD system) 
> and Teamcenter.
>
> Do you know if GIT would work between the two?

Git is designed to track any kind of contents, so yes, it will work
with FORAN and Teamcenter.

However! Git is optimized for files which are text. Tracking binary
files is not as pleasant
as text files. (E.g.: There is no merge driver available, as any
binary file has a different format.
Also file sizes may be a concern)

>
> We’re currently using a Centralised version control system.

You may or may not want to change the workflow. Git as a distributed version
control system allows for more freedom w.r.t. workflows, but a
centralized workflow
is supported just as well. See [1] for examples (Just a quick search,
there are many more
articles out there covering workflows)

[1] http://blog.endpoint.com/2014/05/git-workflows-that-work.html

>
> So to check my understanding, using GIT to create a distributed version 
> control would have the following benefits
> The CAD designer who has design a pump assembly for example could create an 
> additional branch and “plays around” with the pump to make improvements 
> without editing the main pump branch?

Branches are cheap to create. Specially in Git. But other version
control systems also allow for
creation of branches. The big issue is the merging back to mainline.
And in case you have binary
files, which have been edited in both branches to merge, you're out of
luck unless you
write a merge driver yourself.

> Could more than one user create independent branches?

Branches are local. So everybody who has a copy of the repository can create
branches. Depending on the configuration of the server, people may or may not
push new branches (or are only allowed to update existing ones).

> If both users wanted to merge their independent branch with the main branch, 
> what would happen? Would one take priority?

In case of text files, they just merge if they were edited at
different positions of the file (plus some margin for error). If they
were edited at the same position, you need to resolve the merge
conflict manually.
Binary files are not merged automatically as Git has no idea of binary
file formats. You need to provide your own merge driver[2].

[2] https://www.kernel.org/pub/software/scm/git/docs/gitattributes.html
see "Defining a custom merge driver"

>
> Is that the main benefit in terms of a CAD system, the branching ability?
>
> Thanks
>
> Jack
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] xread_nonblock: add functionality to read from fds without blocking

[Johannes Sixt]

Am 15.12.2015 um 01:25 schrieb Stefan Beller:

I was actually thinking about using {without-x}read, just the plain system call.
Do we have any issues with that for wrapping purposes for Windows?


xread() limits the size being read to MAX_IO_SIZE, which is needed on 
some systems (I think that some Windows configurations did fall into 
this category).


-- Hannes

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

RE: Why does send-pack call pack-objects for all remote refs?

[Daniel Koverman]

> You might also try repacking with "git repack -adb", which will
> build reachability bitmaps. Pack-objects can use them to compute
> the set of required objects much faster.

Running "git repack -adb" caused my push time to incease by about 5x.
I made some fresh clones and tried other options with repack, and
consistently anything I tried with -b caused the push time to
increase about 5x.

I don't know much about reachability bitmaps, but perhaps it is
important to note that I timed the pushes after repacking on Git for
Windows. My earlier timings were done on both Linux and Windows and I
did not see a significant difference.

> It's definitely a lot, but it's not unheard of. The git project has
> over 500 tags. That's not 2000, but you're within an order of
> magnitude.
>
> I have seen repositories with 20,000+ tags. I consider that a bit
> more ridiculous, but it does work in practice.

Thanks for the extra context. I'll keep that in mind while I decide
how to approach this.

Daniel

[no subject]

[Ros Sothen]

Sent from my iPhone
--
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 bisect with temporary commits

[Florian Bruhin]

Hi!

Today I bisected a bug which required cherry-picking an (unrelated)
compile fix later in the history so I could test the commits.

After testing a commit, I didn't reset to the commit before the
cherry-picked one, which seemed to work well, but doesn't in my minimal
example:

$ git init
$ echo 'good and does not compile' > file
$ git add file && git commit -m 'good and does not compile'
$ echo again >> file && git commit -am 'still good and does not compile'
$ echo 'bad and compiles' > file && git commit -am 'bad and compiles'
$ git log --oneline --decorate
97f9214 (HEAD, master) bad and compiles
981e109 still good and does not compile
753ed25 good and does not compile

Now I start bisecting and cherry-pick the compile fix in master:

$ git bisect start
$ git bisect bad master
$ git bisect good master~2  # 753ed25
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[981e1093dae24b37189bcba2dd848b0c3388080c] still good and does not compile
florian@ws042:~/tmp/bisect1$ git cherry-pick master
[detached HEAD b49872b] bad and compiles
 Date: Mon Dec 14 17:26:51 2015 +0100
  1 file changed, 1 insertion(+), 2 deletions(-)

Now when trying to say it's good (and forgetting to remove the
temporary commits), I get this:

$ git bisect good
Bisecting: a merge base must be tested
[981e1093dae24b37189bcba2dd848b0c3388080c] still good and does not compile

Is this intended behaviour? Shouldn't git either do a reset to the
commit we're currently bisecting, or warn the user as it was probably
unintended to add new commits?

Currently it seems bisect just treats the current HEAD as good, and
then the bisect algorithm tries to (I think) select a commit between
the currently bisected one and the (temporary) HEAD?

When I used it today, it actually seemed to work well until I hit an
*actual* merge base, and then it started to bisect something
unexpected, which got me a bit confused ;)

Florian

-- 
http://www.the-compiler.org | m...@the-compiler.org (Mail/XMPP)
   GPG: 916E B0C8 FD55 A072 | http://the-compiler.org/pubkey.asc
 I love long mails! | http://email.is-not-s.ms/


signature.asc
Description: Digital signature

Re: What's "wrong" with this fast-import?

[com.git-scm]

On 2015-12-13T01:53:39 +0100
SZEDER Gábor  wrote:

> All changes
> compared to the first parent (i.e. the addition of that new readme file
> on the side branch) have to be listed explicitly.
> 

Apologies for the delay: Thanks for this!

It seems that this issue was actually unintentionally fixed in newer
versions of Fossil, but the schema of repositories created with older
versions had to be updated in order to actually get the benefits of the
fix.

M
--
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: Where does http.sslcainfo get set in Windows (2.6.3)?

[Lars Schneider]

Hi Titus,

try to look here:
C:\Users\All Users\Git\config

(that's where I found it... maybe different on your end).

Cheers,
Lars

> On 14 Dec 2015, at 16:45, Titus Barik  wrote:
> 
> Hi all,
> 
> I'm in Windows using git version: git version 2.6.3.windows.1. Git is
> installed to /c/Users/tbarik/AppData/Local/Programs/Git/cmd/git.
> 
> However, when I look for the config name http.sslcainfo, it returns:
> 
> $ git config --get-all http.sslcainfo
> C:/Program Files/Git/mingw64/ssl/certs/ca-bundle.crt
> 
> Although I can override the name, I'm trying to figure out where this is
> being set, since the correct location should be (in this case)
> C:/Users/tbarik/AppData/Local/Programs/Git/mingw64/ssl/certs/ca-bundle.crt.
> 
> I don't see C:/Program Files/Git/mingw64/ssl/certs/ca-bundle.crt in
> either --global, --system, or --local, hence my confusion as to where
> this path is coming from.
> 
> Thanks,
> 
> Titus
> 
> -- 
> Titus Barik, PE 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Corruption of branch?

[Thomas Nyberg]

Hello,

I have a repository (which I unfortunately cannot provide access to) 
which is having some odd things happening with one (and only one) of its 
branches. This workflow repeats the issue (here `bad_branch` is one of 
the remotes branches; i.e. `origin/bad_branch`):


(1) clone the repository
(2) git checkout `bad_branch`

Basically nothing happens. Nothing is printed and I stay on the master 
branch. I also checked $? and there is no error code that is set. If I 
choose any of other branches, it correctly creates a local branch, sets 
it to track the remote and then switches to the local branch.


It seems like there could be some sort of weird bug in the checkout or 
possibly somehow some corruption in the actual object tree. From my 
vantage point, however, the data appears totally fine. For example, in 
`.git/packed-refs` everything appears normal and if I explicitly 
checkout the commit IDs directly (i.e. just copy the commit 
corresponding to refs/remotes/origin/bad_branch and checkout $commit) it 
checks out fine. If I do this with the bad_branch I get a detached HEAD 
as expected and git log lists the commits that it should.


This seems a bit odd to me. There's certainly some sort of error 
somewhere, but it's passing silently. I'm not really sure how to debug 
this and it's too bad I can't actually link the actual repository. I 
presume if I have the time I could try compiling git from source and 
seeing if it still shows up. I tested it on the following two versions 
of git get the same error:


* 1.9.1 (installed as a package from Linux Mint 17.2 Rafaela)
* 2.1.4 (installed as a package from Debian Jessie 8.2)

Also I should note that the original repository is hosted on Github.

Thanks for any help. Hopefully the fact that I can't provide enough 
information for others to reproduce the issue isn't too large a bother...


Cheers,
Thomas Nyberg
--
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 bisect with temporary commits

[Jeff King]

On Mon, Dec 14, 2015 at 01:17:03PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> >> You should instead tell git that HEAD^ is good, since that is what git
> >> asked you to test.
> >
> > Another alternative is to use "git cherry-pick -n" to create a working
> > tree state that you can test, but leave HEAD at the original commit.
> > Then "git bisect good" does the right thing.
> 
> I was about to say the same, and "bisect good" at that point does
> mark the correct commit, but does it always do the right thing?  I
> think the procedure must be
> 
>   git cherry-pick -n $the_fixup
> test
> git reset --hard
> git bisect good (or bad)

Hmm, you're right. I assumed "git bisect good" would do the equivalent
of "git checkout -f", but it doesn't. I guess it has been long enough
since I have had to cherry-pick a fix that I completely forgot that bit.

It might be convenient if bisect did pass "-f" to checkout, but I guess
it would also be destructive if you had hand-tweaks you forgot to save.

-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 7/8] config: add core.untrackedCache

[Junio C Hamano]

Junio C Hamano  writes:

> If you stop thinking that "update-index --untracked-cache" is
> somehow a "configuration", things will get clearer to you.
> ...
>> "git update-index --[no-|force-]untracked-cache" is a bad way, so
>> let's make it easy for people to not use it at all.
>
> As I disagree with that basic premise, I have to disagree with the
> conclusion as well.

Having said all that, I do not think an option to "update-index"
must be the _only_ interface to tell the index to use (or ignore)
the untracked cache.  Two obvious places that can also have the same
command line option would be "git init" and "git clone".  If either
the per-user configuration (or the per-site one the administrator
sets) gave the default for these two commands, that would make it
unnecessary to use "update-index", unless you are experimenting or
working around bugs in the implementation.

The primary reason why I do not like your "configuration decides" is
it will be a huge source of confusions and bugs.  Imagine what
should happen in this sequence, and when should a stale cached
information be discarded?

 - the configuration is set to 'yes'.
 - the index is updated and written by various commands.
 - more work is done in the working tree without updating the index.
 - the configuration is set to 'no'.
 - more work is done in the working tree without updating the index.
 - the configuration is set to 'yes'.
 - more work is done in the working tree without updating the index.
 - somebody asks "what untracked paths are there?"

In the "configuration decides" world, I am not sure how a sane
implementation efficiently invalidates the cache as needed, without
the config subsystem having intimate knowledge with the untracked
cache (so far, the config subsystem is merely a key-value store and
does not care _what_ it stores; you would want to invalidate the
cache in the index when somebody sets the variable to 'no', which
means the config subsystem needs to know that this variable is
special).

--
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 0/3] format-patch: introduce option to suppress commit hashes

[Jeff King]

On Sun, Dec 13, 2015 at 05:27:15PM +, brian m. carlson wrote:

> git format-patch is often used to create patches that are then stored in
> version control or displayed with diff.  Having the commit hash in the
> "From " line usually just creates diff noise in these cases, so this
> series introduces --zero-commit to set that to all zeros.
> 
> Changes from v1:
> * Rename the option --zero-commit.
> * Improve the tests to look for a 40-hex hash value in "From " header.
> 
> brian m. carlson (3):
>   Introduce a null_oid constant.
>   format-patch: add an option to suppress commit hash
>   format-patch: check that header line has expected format

The intent here makes sense to me, and with the exception of the
test_line_count thing that Torsten mentioned, the code looks good.

I briefly wondered if the option should simply be "--diffable" or
something like that, and trigger this new behavior as well as implying
--no-signature. Along with any other relevant options (if any; I don't
recall if --stat-width is terminal-dependent for format-patch, for
example).

But that is probably overkill. People can flip those switches
individually if they want to (and even if somebody did want
"--diffable", it may make sense to build it on top, so they can flip the
zero-commit thing individually if they want).

-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 6/8] run-command: add an asynchronous parallel child processor

[Stefan Beller]

On Mon, Dec 14, 2015 at 12:39 PM, Johannes Sixt  wrote:
>
> I can't quite parse the first sentence in this paragraph. Perhaps something
> like this:
>
> To detect when a child has finished executing, we check interleaved
> with other actions (such as checking the liveliness of children or
> starting new processes) whether the stderr pipe still exists. Once a
> child closed its stderr stream, we assume it is terminating very soon,
> and use finish_command() from the single external process execution
> interface to collect the exit status.

Sounds much better than my words. If a resend is necessary, I'll have your
reworded version of the paragraph instead.

>
>
>>
>> By maintaining the strong assumption of stderr being open until the
>> very end of a child process, we can avoid other hassle such as an
>> implementation using `waitpid(-1)`, which is not implemented in Windows.
>>
>> Signed-off-by: Stefan Beller 
>
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] format-patch: add an option to suppress commit hash

[Junio C Hamano]

"brian m. carlson"  writes:

> +test_expect_success 'format-patch --zero-commit' '
> + git format-patch --zero-commit --stdout v2..v1 >patch2 &&
> + cnt=$(egrep "^From 0{40} Mon Sep 17 00:00:00 2001" patch2 | wc -l) &&
> + test $cnt = 3
> +'

This test is not wrong per-se, but it makes the test as a whole
somewhat brittle.  People cannot add new commits in the history
being tested, which would add to the number of patches, without
adjusting this test, even though all this test cares about is that
all the mbox "From " lines record the zeroed commit object name.

git format-patch --zero-commit --stdout v2..v1 |
grep "^From " | sort | uniq >actual &&
echo "From $_z40 Mon Sep 17 00:00:00 2001" >expect &&
test_cmp expect actual

or something like that instead?

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

Re: [PATCH v2 3/3] format-patch: check that header line has expected format

[Junio C Hamano]

"brian m. carlson"  writes:

> +test_expect_success 'From line has expected format' '
> + git format-patch --stdout v2..v1 >patch2 &&
> + cnt=$(egrep "^From [0-9a-f]{40} Mon Sep 17 00:00:00 2001" patch2 | wc 
> -l) &&

Also, with $_x40, you do not need egrep.

> + test $cnt = 3
> +'
> +
>  test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Corruption of branch?

[Thomas Nyberg]

In the .git/config there is no [branch "frus"] section. At first this is 
expected (i.e. cleaning cloning), but nothing changes when I execute 
`git checkout frus`. When I execute `git checkout frus_body_cleaning` 
that gets added to .git/config as expected.


.git/refs/heads contains two files "master" and "frus_body_cleaning" 
pointing to their respective commits, but there is nothing else there. 
here's the other command


$ grep frus packed-refs
3a1dbe48299f6eda1cc4b69cab35284c0f0355eb refs/remotes/origin/frus
1750cba5a94b3fe6041aaf49de430a558a3b9bc8 
refs/remotes/origin/frus_body_cleaning


So here frus actually is showing up. The find command isn't working 
either. I changed it to add the * but only shows the frus_body_cleaning 
branch:


$ find .git -name 'frus*'
.git/logs/refs/heads/frus_body_cleaning
.git/refs/heads/frus_body_cleaning

So yeah this is pretty weird. I'm guessing you're looking for name 
collisions of some kind? I had the idea that the problem might that too 
and used git show-index to look for all objects, but none of them have 
frus in them (I thought that maybe if the sha of one of them started 
with "frus" that would explain it but no dice). Since the command `git 
checkout -b frus origin/frus` works, it seems to me like somehow git is 
getting confused going from the `git checkout frus` command to that 
expanded one.


It is pretty baffling.

On 12/14/2015 02:20 PM, David Turner wrote:

On Mon, 2015-12-14 at 13:08 -0500, Thomas Nyberg wrote:

Hi Stefan thanks for much for the response! So I compiled release
version 2.6.4 as well as the current master branch on the git git
repository (2.7.0.rc0.20.g4b9ab0e) and the problem persists on both.

To answer your questions, there are no weird characters. The name of the
bad_branch is "frus". There is another branch called
"frus_body_cleaning" that is totally fine.


What's in .git/config under [branch "frus"] (if anything)?

What does "grep refs/heads/frus .git/packed-refs" say?  What about
find .git -name frus ?

Sorry for the possibly-stupid questions, but I'm really baffled by this
one.


--
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 bisect with temporary commits

[Andreas Schwab]

Florian Bruhin  writes:

> I see - but wouldn't it make more sense for a "git bisect good" (or
> bad, respectively) without arguments to assume I mean the commit
> bisect checked out for me, not HEAD?

The problem is that there is nothing that marks the originally checked
out commit except by being pointed to by HEAD.  Also, testing a
different commit as the one suggested by git can be useful when skipping
over commits that are known to fail for unrelated reasons (see "Avoiding
testing a commit" in git-bisect(1)).

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
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: Corruption of branch?

[Dennis Kaarsemaker]

On ma, 2015-12-14 at 14:59 -0500, Thomas Nyberg wrote:
> I'm guessing you're looking for namecollisions of some kind?

I was thinking the same. Can you share the (sanitised) output of 

git for-each-ref?

-- 
Dennis Kaarsemaker
www.kaarsemaker.net


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

compile error in Git v2.7.0-rc0

[johan defries]

Probably because I have NO_IPV6 defined.

ident.c: In function ‘canonical_name’:
ident.c:89:37: error: ‘buf’ undeclared (first use in this function)
  struct hostent *he = gethostbyname(buf);
 ^
ident.c:89:37: note: each undeclared identifier is reported only once
for each function it appears in
make: *** [ident.o] Fout 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: Corruption of branch?

[Thomas Nyberg]

What exactly are you looking for? Here's the results of the following 
command:


$ git for-each-ref | grep frus
1750cba5a94b3fe6041aaf49de430a558a3b9bc8 commit 
refs/heads/frus_body_cleaning

3a1dbe48299f6eda1cc4b69cab35284c0f0355eb commit refs/remotes/origin/frus
1750cba5a94b3fe6041aaf49de430a558a3b9bc8 commit 
refs/remotes/origin/frus_body_cleaning


Sorry if this isn't what you're looking for. I'm actually not very 
familiar with these different internal git commands...


Regardless this looks to me exactly like what I'd expect given the 
current situation...it's as if I never checked out the "frus" branch at 
all (which I suppose is true since this is a fresh copy and "git 
checkout frus" didn't do anything).


Btw after checking out explicitly with `git checkout -b frus 
origin/frus`, things look as I'd expect.


$ git for-each-ref | grep frus
3a1dbe48299f6eda1cc4b69cab35284c0f0355eb commit refs/heads/frus
1750cba5a94b3fe6041aaf49de430a558a3b9bc8 commit 
refs/heads/frus_body_cleaning

3a1dbe48299f6eda1cc4b69cab35284c0f0355eb commit refs/remotes/origin/frus
1750cba5a94b3fe6041aaf49de430a558a3b9bc8 commit 
refs/remotes/origin/frus_body_cleaning


Btw just to test things a little more I deleted both the frus and 
frus_body_cleaning branches and tried to recheck them out, but the 
problem still persists.


Cheers,
Thomas

On 12/14/2015 03:18 PM, Dennis Kaarsemaker wrote:

On ma, 2015-12-14 at 14:59 -0500, Thomas Nyberg wrote:

I'm guessing you're looking for namecollisions of some kind?


I was thinking the same. Can you share the (sanitised) output of

git for-each-ref?


--
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: compile error in Git v2.7.0-rc0

[Junio C Hamano]

johan defries  writes:

> Probably because I have NO_IPV6 defined.
>
> ident.c: In function ‘canonical_name’:
> ident.c:89:37: error: ‘buf’ undeclared (first use in this function)
>   struct hostent *he = gethostbyname(buf);
>  ^
> ident.c:89:37: note: each undeclared identifier is reported only once
> for each function it appears in
> make: *** [ident.o] Fout 1

Thanks.  This should perhaps do?

 ident.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ident.c b/ident.c
index 4e7f99d..2900879 100644
--- a/ident.c
+++ b/ident.c
@@ -86,6 +86,7 @@ static int canonical_name(const char *host, struct strbuf 
*out)
freeaddrinfo(ai);
}
 #else
+   char buf[1024];
struct hostent *he = gethostbyname(buf);
if (he && strchr(he->h_name, '.')) {
strbuf_addstr(out, he->h_name);
--
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: Corruption of branch?

[Stefan Beller]

On Mon, Dec 14, 2015 at 9:40 AM, Thomas Nyberg  wrote:
> Hello,
>
> I have a repository (which I unfortunately cannot provide access to) which
> is having some odd things happening with one (and only one) of its branches.
> This workflow repeats the issue (here `bad_branch` is one of the remotes
> branches; i.e. `origin/bad_branch`):
>
> (1) clone the repository
> (2) git checkout `bad_branch`
>
> Basically nothing happens. Nothing is printed and I stay on the master
> branch. I also checked $? and there is no error code that is set. If I
> choose any of other branches, it correctly creates a local branch, sets it
> to track the remote and then switches to the local branch.

Does that branch have a funny name? (i.e. is it ASCII only? Or is it
also utf8 characters?
Does it have some special characters in it like points, colons,
question marks etc)

Does it happen only with a single sha1 or can you apply commits on top
of that branch
and the error condition persists?

>
> It seems like there could be some sort of weird bug in the checkout or
> possibly somehow some corruption in the actual object tree. From my vantage
> point, however, the data appears totally fine. For example, in
> `.git/packed-refs` everything appears normal and if I explicitly checkout
> the commit IDs directly (i.e. just copy the commit corresponding to
> refs/remotes/origin/bad_branch and checkout $commit) it checks out fine. If
> I do this with the bad_branch I get a detached HEAD as expected and git log
> lists the commits that it should.
>
> This seems a bit odd to me. There's certainly some sort of error somewhere,
> but it's passing silently. I'm not really sure how to debug this and it's
> too bad I can't actually link the actual repository. I presume if I have the
> time I could try compiling git from source and seeing if it still shows up.
> I tested it on the following two versions of git get the same error:
>
> * 1.9.1 (installed as a package from Linux Mint 17.2 Rafaela)
> * 2.1.4 (installed as a package from Debian Jessie 8.2)

The refs handling code is in flux at the moment. (starting mid of last
year actually)
I cc'd people who did work recently on the file refs.c

So I think trying with a new version of Git would be a valuable datapoint!

>
> Also I should note that the original repository is hosted on Github.
>
> Thanks for any help. Hopefully the fact that I can't provide enough
> information for others to reproduce the issue isn't too large a bother...
>
> Cheers,
> Thomas Nyberg
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git bisect with temporary commits

[Andreas Schwab]

Florian Bruhin  writes:

> Now when trying to say it's good (and forgetting to remove the
> temporary commits), I get this:
>
> $ git bisect good
> Bisecting: a merge base must be tested
> [981e1093dae24b37189bcba2dd848b0c3388080c] still good and does not compile
>
> Is this intended behaviour? Shouldn't git either do a reset to the
> commit we're currently bisecting, or warn the user as it was probably
> unintended to add new commits?

You should instead tell git that HEAD^ is good, since that is what git
asked you to test.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
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/RFC 00/10] ref-filter: use parsing functions

[Junio C Hamano]

Eric Sunshine  writes:

> On Fri, Dec 11, 2015 at 5:49 PM, Junio C Hamano  wrote:
>> Karthik Nayak  writes:
>>>   ref-filter: introduce a parsing function for each atom in valid_atom
>>>   ref-filter: introduce struct used_atom
>>>   ref-fitler: bump match_atom() name to the top
>>>   ref-filter: skip deref specifier in match_atom_name()
>>>   ref-filter: introduce color_atom_parser()
>>>   strbuf: introduce strbuf_split_str_without_term()
>>>   ref-filter: introduce align_atom_parser()
>>>   ref-filter: introduce remote_ref_atom_parser()
>>>   ref-filter: introduce contents_atom_parser()
>>>   ref-filter: introduce objectname_atom_parser()
>>
>> It seems that this series had seen quite a good inputs, mostly from
>> Eric.  I finished reading it over and I didn't find anything more to
>> add.  The patches are mostly good and would be ready once these
>> points raised during the review are addressed, I think
>
> I'm still a bit fuzzy about what this series is trying to achieve. It
> feels like it wants to avoid doing repeated processing of unchanging
> bits of %(foo:bar) atoms for each ref processed, but it only partly
> achieves that goal.

That's very true.

It seems you two already have hashed it out in the downthread, and I
think that is in line with an earlier suggestion by Matthieu to
fully pre-parse in the earlier thread, which was made in response to
(and is much better than) my "let's start with a half-way solution"
in $gmane/279254.

Thanks.

> strcmp()s and starts_with()s in that inner loop, and even the
> unchanging %(color:) argument gets re-evaulated repeatedly, which is
> probably quite expensive.
>
> If the intention is to rid that inner loop of much of the expensive
> processing, then wouldn't we want to introduce an enum of valid atoms
> which is to be a member of 'struct used_atom', and have
> populate_value() switch on the enum value rather than doing all the
> expensive strcmp()s and starts_with()?
>
> enum atom_type {
> AT_REFNAME,
> AT_OBJECTTYPE,
> ...
> AT_COLOR,
> AT_ALIGN
> };
>
> static struct used_atom {
> enum atom_type atom;
> cmp_type cmp;
> union {
> char *color; /* parsed color */
> struct align align;
> enum { ... } remote_ref;
> struct {
> enum { ... } portion;
> unsigned int nlines;
> } contents;
> int short_objname;
> } u;
> } *used_atom;
>
> In fact, the 'cmp_type cmp' field can be dropped altogether since it
> can just as easily be looked up when needed by keeping 'enum
> atom_type' and valid_atoms[] in-sync and indexing into valid_atoms[]
> by atom_type:
>
> struct used_atom *a = ...;
> cmp_type cmp = valid_atoms[a->atom].cmp_type;
>
> As a bonus, an 'enum atom_type' would resolve the problem of how
> starts_with() is abused in populate_value() for certain cases
> (assuming I'm understanding the logic), such as how matching of
> "color" could incorrectly match some yet-to-be-added atom named
> "colorize".
--
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 bisect with temporary commits

[Junio C Hamano]

Florian Bruhin  writes:

> * Andreas Schwab  [2015-12-14 19:08:48 +0100]:
>> Florian Bruhin  writes:
>> 
>> > Now when trying to say it's good (and forgetting to remove the
>> > temporary commits), I get this:
>> >
>> > $ git bisect good
>> > Bisecting: a merge base must be tested
>> > [981e1093dae24b37189bcba2dd848b0c3388080c] still good and does not 
>> > compile
>> >
>> > Is this intended behaviour? Shouldn't git either do a reset to the
>> > commit we're currently bisecting, or warn the user as it was probably
>> > unintended to add new commits?
>> 
>> You should instead tell git that HEAD^ is good, since that is what git
>> asked you to test.
>
> I see - but wouldn't it make more sense for a "git bisect good" (or
> bad, respectively) without arguments to assume I mean the commit
> bisect checked out for me, not HEAD?
>
> I don't see any scenario where the current behaviour would make sense,
> but I might be missing something.

When the commit "bisect" checked out is untestable, the user can
freely go to another commit, e.g. "git reset --hard HEAD^" to go
back one step, and then test it instead.  "git bisect good" has
to mark the then-current HEAD, not the commit that was checked out,
for this to work.

--
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: sb/submodule-parallel-fetch,

[Stefan Beller]

On Fri, Dec 11, 2015 at 3:07 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> On Fri, Dec 11, 2015 at 1:37 PM, Johannes Sixt  wrote:
>>>
>>> Generally, I'm already quite satisfied with the state of the
>>> infrastructure at the tip of the branch.
>>
>> I was about the rework the patch series.
>
> OK.
>
> I think the very early part of the series, up to 8fc3f2ee (sigchain:
> add command to pop all common signals, 2015-09-30), may be fine
> as-is (i.e. just rebase on top of updated 'master').

I did that.

>
> The step after that, i.e. asynch processor, is the most interesting
> and important one.  Since it was written, I think the improvements
> that we want to be rolled into it from the beginning are:
>
>  - do not rely on waitpid(-1);

and documented in the commit message why we abandoned that
implementation.

>
>  - no need for set_nonglocking(), squashing a4433fd4a and
>6f963a895a9 in;

6f963a895a9 (strbuf: update documentation for strbuf_read_once())
squashed into a commit before your anticipated "very early part
which  may be fine as is", but no worries.

>  - correct the early-shutdown bug 79f38577 and again in 63ce47e1;
>
>  - child_process_clear() in 1c53754a, which probably will become
>unnecessary if the series is rebuilt on top of updated 'master';

it did.

>
>  - follow-up fixes and enhancements to the tests in c3a5d11 and
>74cc04d;
>
>  - debugging support in 7eb93e91069 (from the other series that
>builds on top).

not just cherry picking that one, but also adapt the tests in this series to
use it.

>
> That would slim down not just the total number of patches, but also
> the amount of the code that needs to be looked at (as we would not
> add code only to later remove or fixup).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/8] submodule.c: write "Fetching submodule " to stderr

[Stefan Beller]

From: Jonathan Nieder 

The "Pushing submodule " progress output correctly goes to
stderr, but "Fetching submodule " is going to stdout by
mistake.  Fix it to write to stderr.

Noticed while trying to implement a parallel submodule fetch.  When
this particular output line went to a different file descriptor, it
was buffered separately, resulting in wrongly interleaved output if
we copied it to the terminal naively.

Signed-off-by: Jonathan Nieder 
Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 submodule.c |  2 +-
 t/t5526-fetch-submodules.sh | 51 +++--
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/submodule.c b/submodule.c
index 14e7624..8386477 100644
--- a/submodule.c
+++ b/submodule.c
@@ -689,7 +689,7 @@ int fetch_populated_submodules(const struct argv_array 
*options,
git_dir = submodule_git_dir.buf;
if (is_directory(git_dir)) {
if (!quiet)
-   printf("Fetching submodule %s%s\n", prefix, 
ce->name);
+   fprintf(stderr, "Fetching submodule %s%s\n", 
prefix, ce->name);
cp.dir = submodule_path.buf;
argv_array_push(, default_argv);
argv_array_push(, "--submodule-prefix");
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index a4532b0..17759b14 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -16,7 +16,8 @@ add_upstream_commit() {
git add subfile &&
git commit -m new subfile &&
head2=$(git rev-parse --short HEAD) &&
-   echo "From $pwd/submodule" > ../expect.err &&
+   echo "Fetching submodule submodule" > ../expect.err &&
+   echo "From $pwd/submodule" >> ../expect.err &&
echo "   $head1..$head2  master -> origin/master" >> 
../expect.err
) &&
(
@@ -27,6 +28,7 @@ add_upstream_commit() {
git add deepsubfile &&
git commit -m new deepsubfile &&
head2=$(git rev-parse --short HEAD) &&
+   echo "Fetching submodule submodule/subdir/deepsubmodule" >> 
../expect.err
echo "From $pwd/deepsubmodule" >> ../expect.err &&
echo "   $head1..$head2  master -> origin/master" >> 
../expect.err
)
@@ -56,9 +58,7 @@ test_expect_success setup '
(
cd downstream &&
git submodule update --init --recursive
-   ) &&
-   echo "Fetching submodule submodule" > expect.out &&
-   echo "Fetching submodule submodule/subdir/deepsubmodule" >> expect.out
+   )
 '
 
 test_expect_success "fetch --recurse-submodules recurses into submodules" '
@@ -67,7 +67,7 @@ test_expect_success "fetch --recurse-submodules recurses into 
submodules" '
cd downstream &&
git fetch --recurse-submodules >../actual.out 2>../actual.err
) &&
-   test_i18ncmp expect.out actual.out &&
+   test_must_be_empty actual.out &&
test_i18ncmp expect.err actual.err
 '
 
@@ -96,7 +96,7 @@ test_expect_success "using fetchRecurseSubmodules=true in 
.gitmodules recurses i
git config -f .gitmodules 
submodule.submodule.fetchRecurseSubmodules true &&
git fetch >../actual.out 2>../actual.err
) &&
-   test_i18ncmp expect.out actual.out &&
+   test_must_be_empty actual.out &&
test_i18ncmp expect.err actual.err
 '
 
@@ -127,7 +127,7 @@ test_expect_success "--recurse-submodules overrides 
fetchRecurseSubmodules setti
git config --unset -f .gitmodules 
submodule.submodule.fetchRecurseSubmodules &&
git config --unset submodule.submodule.fetchRecurseSubmodules
) &&
-   test_i18ncmp expect.out actual.out &&
+   test_must_be_empty actual.out &&
test_i18ncmp expect.err actual.err
 '
 
@@ -146,7 +146,7 @@ test_expect_success "--dry-run propagates to submodules" '
cd downstream &&
git fetch --recurse-submodules --dry-run >../actual.out 
2>../actual.err
) &&
-   test_i18ncmp expect.out actual.out &&
+   test_must_be_empty actual.out &&
test_i18ncmp expect.err actual.err
 '
 
@@ -155,7 +155,7 @@ test_expect_success "Without --dry-run propagates to 
submodules" '
cd downstream &&
git fetch --recurse-submodules >../actual.out 2>../actual.err
) &&
-   test_i18ncmp expect.out actual.out &&
+   test_must_be_empty actual.out &&
test_i18ncmp expect.err actual.err
 '
 
@@ -166,7 +166,7 @@ test_expect_success "recurseSubmodules=true propagates into 
submodules" '
git config fetch.recurseSubmodules true
git fetch 

[PATCH 0/8] Rerolling sb/submodule-parallel-fetch for the time after 2.7

[Stefan Beller]

I am sending out a new version for replacing sb/submodule-parallel-fetch for
the time after the 2.7 release.

The content are 
 * all patches as in the branch sb/submodule-parallel-fetch
 * inlcuding the fixups as suggested by Hannes, 
 * write a message to the debug log for better testing and debugging purposes
  (a patch cherry picked from the series which is supposed to build on top of 
this)

The patches themselves were rebased such that there are no fixup commits
any more, but we get things right the first time.

The commit message of "run-command: add an asynchronous parallel child 
processor"
has slightly been updated to mention the fact that we don't want to use 
waitpid(-1)
but rather use the assumption of child's stderr living as long as the child 
itself.

Thanks,
Stefan


Jonathan Nieder (1):
  submodule.c: write "Fetching submodule " to stderr

Stefan Beller (7):
  xread: poll on non blocking fds
  xread_nonblock: add functionality to read from fds without blocking
  strbuf: add strbuf_read_once to read without blocking
  sigchain: add command to pop all common signals
  run-command: add an asynchronous parallel child processor
  fetch_populated_submodules: use new parallel job processing
  submodules: allow parallel fetching, add tests and documentation

 Documentation/fetch-options.txt |   7 +
 builtin/fetch.c |   6 +-
 builtin/pull.c  |   6 +
 git-compat-util.h   |   1 +
 run-command.c   | 335 
 run-command.h   |  80 ++
 sigchain.c  |   9 ++
 sigchain.h  |   1 +
 strbuf.c|  11 ++
 strbuf.h|   8 +
 submodule.c | 141 +++--
 submodule.h |   2 +-
 t/t0061-run-command.sh  |  53 +++
 t/t5526-fetch-submodules.sh |  71 ++---
 test-run-command.c  |  55 ++-
 wrapper.c   |  35 -
 16 files changed, 747 insertions(+), 74 deletions(-)

-- 
2.6.4.443.ge094245.dirty

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

[PATCH 4/8] strbuf: add strbuf_read_once to read without blocking

[Stefan Beller]

The new call will read from a file descriptor into a strbuf once. The
underlying call xread_nonblock is meant to execute without blocking if
the file descriptor is set to O_NONBLOCK. It is a bug to call
strbuf_read_once on a file descriptor which would block.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 strbuf.c | 11 +++
 strbuf.h |  8 
 2 files changed, 19 insertions(+)

diff --git a/strbuf.c b/strbuf.c
index d76f0ae..b552a13 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -384,6 +384,17 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)
return sb->len - oldlen;
 }
 
+ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)
+{
+   ssize_t cnt;
+
+   strbuf_grow(sb, hint ? hint : 8192);
+   cnt = xread_nonblock(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
+   if (cnt > 0)
+   strbuf_setlen(sb, sb->len + cnt);
+   return cnt;
+}
+
 #define STRBUF_MAXLINK (2*PATH_MAX)
 
 int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
diff --git a/strbuf.h b/strbuf.h
index 7123fca..c3e5980 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -367,6 +367,14 @@ extern size_t strbuf_fread(struct strbuf *, size_t, FILE 
*);
 extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);
 
 /**
+ * Returns the number of new bytes appended to the sb.
+ * Negative return value signals there was an error returned from
+ * underlying read(2), in which case the caller should check errno.
+ * e.g. errno == EAGAIN when the read may have blocked.
+ */
+extern ssize_t strbuf_read_once(struct strbuf *, int fd, size_t hint);
+
+/**
  * Read the contents of a file, specified by its path. The third argument
  * can be used to give a hint about the file size, to avoid reallocs.
  */
-- 
2.6.4.443.ge094245.dirty

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

Re: [PATCH 7/8] config: add core.untrackedCache

[Junio C Hamano]

Christian Couder  writes:

> In fact "git update-index --[no-|force-]untracked-cache" is very bad
> because it means that two repositories can be configured differently
> even if they have the same config files.

If you stop thinking that "update-index --untracked-cache" is
somehow a "configuration", things will get clearer to you.

Imagine there is no configuration whatsoever.  The index primarily
keeps track of what will be in the next "write-tree", but optionally
it can keep track of other kinds of information, such as the last
unmerged states for paths whose conflicts have been resolved.  Duy's
work adds another kind of information to the mix--cached stat bits
for untracked paths to speed up the next "git status", and the option
is to start keeping track of that information in the index.

Because it is a "cache", once you start keeping track of it, you
need to maintain it--otherwise you will be fooled by stale
information still in the cache.  So of course the effect has to
persist.  There is nothing _wrong_ with that.  If you want to stop
maintaining it, you can tell the index "don't use untracked-cache".

So I think the above "is very bad because" is total nonsense.

> If you want only some repos to use the UC, you will set
> core.untrackedCache in the repo config. Then after cloning such a
> repo, you will copy the config file, and this will not be enough to
> enable the UC.

Surely.  "Does this index file keeps track of the untracked files'
states?" is a property of the index.  Cloning does not propagate the
configuration and copying or not copying is irrelevant.  If you want
to enable, running "update-index --untracked-cache" is a way to do
so.  I cannot see what's so hard about it.

> And if you have set core.untrackedCache in the global config when you
> clone, UC is enabled, but if you have just set it in the repo config
> after the clone, it is not enabled.

That's fine.  In your patch series, if you set it in the global, you
will get the cache in the new one.  With the cleaned-up semantics I
suggested, the same thing will happen.

And with the cleaned-up semantics, the configuration is *ONLY* used
to give the *DEFAULT* before other things happen, i.e. creation of
the index file for the first time.  Because the configuration is
only the default, an explicit "update-index --[no-]untracked-cache"
will defeat it, just like any other config/option interaction.

The biggest issue I had with your patch series, IIRC, is that
configuration will defeat the command line option.

> Shouldn't it be nice if they could just enable core.untrackedCache in
> the global config files without having to also cd into every repo and
> use "git update-index --untracked-cache" there?

NO.  It is bad to change the behaviour behind users' back.

Set that once, _future_ repositories their users will create will
use that by default, and tell their users what to do to their
existing repositories.  Problem solved.

> "git update-index --[no-|force-]untracked-cache" is a bad way, so
> let's make it easy for people to not use it at all.

As I disagree with that basic premise, I have to disagree with the
conclusion as well.
--
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 3/3] format-patch: check that header line has expected format

[Junio C Hamano]

"brian m. carlson"  writes:

> The format of the "From " header line is very specific to allow
> utilities to detect Git-style patches.  Add a test that the patches
> created are in the expected format.
>
> Signed-off-by: brian m. carlson 
> ---
>  t/t4014-format-patch.sh | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index b740e3da..362bc228 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1437,4 +1437,10 @@ test_expect_success 'format-patch --zero-commit' '
>   test $cnt = 3
>  '
>  
> +test_expect_success 'From line has expected format' '
> + git format-patch --stdout v2..v1 >patch2 &&
> + cnt=$(egrep "^From [0-9a-f]{40} Mon Sep 17 00:00:00 2001" patch2 | wc 
> -l) &&

Don't you want to anchor the pattern to the right as well?

> + test $cnt = 3
> +'
> +
>  test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] git-p4: fix for handling of multiple depot paths

[Junio C Hamano]

Luke Diamand  writes:

> Having just fixed this, I've now just spotted that Sam Hocevar's fix
> to reduce the number of P4 transactions also fixes it:
>
> https://www.mail-archive.com/git%40vger.kernel.org/msg81880.html
>
> That seems like a cleaner fix.

Hmm, do you mean I should ignore this series and take the other one,
take only 1/2 from this for tests and then both patches in the other
one, or something else?

Thanks.

>
> Luke
>
>
> On 13 December 2015 at 20:07, Luke Diamand  wrote:
>> James Farwell reported a bug I introduced into git-p4 with
>> handling of multiple depot paths:
>>
>> http://article.gmane.org/gmane.comp.version-control.git/282297
>>
>> This patch series adds a failing test case, and a fix for this
>> problem.
>>
>> Luke
>>
>> Luke Diamand (2):
>>   git-p4: failing test case for skipping changes with multiple depots
>>   git-p4: fix handling of multiple depot paths
>>
>>  git-p4.py   |  8 +---
>>  t/t9818-git-p4-block.sh | 28 +++-
>>  2 files changed, 32 insertions(+), 4 deletions(-)
>>
>> --
>> 2.6.2.474.g3eb3291
>>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] push: add '-d' as shorthand for '--delete'

[Junio C Hamano]

Patrick Steinhardt  writes:

> It is only possible to delete branches on remotes by specifying
> the long '--delete' flag.

Not really.  "git push origin :unnecessary-branch" should just work
with out "--delete" or "-d".
--
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: Corruption of branch?

[David Turner]

On Mon, 2015-12-14 at 13:08 -0500, Thomas Nyberg wrote:
> Hi Stefan thanks for much for the response! So I compiled release 
> version 2.6.4 as well as the current master branch on the git git 
> repository (2.7.0.rc0.20.g4b9ab0e) and the problem persists on both.
> 
> To answer your questions, there are no weird characters. The name of the 
> bad_branch is "frus". There is another branch called 
> "frus_body_cleaning" that is totally fine.

What's in .git/config under [branch "frus"] (if anything)?

What does "grep refs/heads/frus .git/packed-refs" say?  What about
find .git -name frus ?

Sorry for the possibly-stupid questions, but I'm really baffled by this
one.

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

[PATCH 2/8] xread: poll on non blocking fds

[Stefan Beller]

>From the man page:
EAGAIN The file descriptor fd refers to a file other than a socket
   and has been marked nonblocking (O_NONBLOCK), and the read
   would block.

EAGAIN or EWOULDBLOCK
   The file descriptor fd refers to a socket and has been marked
   nonblocking (O_NONBLOCK), and the read would block.  POSIX.1-2001
   allows either error to be returned for this case, and does not
   require these constants to have the same value, so a portable
   application should check for both possibilities.

If we get an EAGAIN or EWOULDBLOCK the fd must have set O_NONBLOCK.
As the intent of xread is to read as much as possible either until the
fd is EOF or an actual error occurs, we can ease the feeder of the fd
by not spinning the whole time, but rather wait for it politely by not
busy waiting.

We should not care if the call to poll failed, as we're in an infinite
loop and can only get out with the correct read().

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 wrapper.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/wrapper.c b/wrapper.c
index 6fcaa4d..4f720fe 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -236,8 +236,17 @@ ssize_t xread(int fd, void *buf, size_t len)
len = MAX_IO_SIZE;
while (1) {
nr = read(fd, buf, len);
-   if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
-   continue;
+   if (nr < 0) {
+   if (errno == EINTR)
+   continue;
+   if (errno == EAGAIN || errno == EWOULDBLOCK) {
+   struct pollfd pfd;
+   pfd.events = POLLIN;
+   pfd.fd = fd;
+   /* We deliberately ignore the return value */
+   poll(, 1, -1);
+   }
+   }
return nr;
}
 }
-- 
2.6.4.443.ge094245.dirty

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

[PATCH 5/8] sigchain: add command to pop all common signals

[Stefan Beller]

The new method removes all common signal handlers that were installed
by sigchain_push.

CC: Jeff King 
Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 sigchain.c | 9 +
 sigchain.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/sigchain.c b/sigchain.c
index faa375d..2ac43bb 100644
--- a/sigchain.c
+++ b/sigchain.c
@@ -50,3 +50,12 @@ void sigchain_push_common(sigchain_fun f)
sigchain_push(SIGQUIT, f);
sigchain_push(SIGPIPE, f);
 }
+
+void sigchain_pop_common(void)
+{
+   sigchain_pop(SIGPIPE);
+   sigchain_pop(SIGQUIT);
+   sigchain_pop(SIGTERM);
+   sigchain_pop(SIGHUP);
+   sigchain_pop(SIGINT);
+}
diff --git a/sigchain.h b/sigchain.h
index 618083b..138b20f 100644
--- a/sigchain.h
+++ b/sigchain.h
@@ -7,5 +7,6 @@ int sigchain_push(int sig, sigchain_fun f);
 int sigchain_pop(int sig);
 
 void sigchain_push_common(sigchain_fun f);
+void sigchain_pop_common(void);
 
 #endif /* SIGCHAIN_H */
-- 
2.6.4.443.ge094245.dirty

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

[PATCH 8/8] submodules: allow parallel fetching, add tests and documentation

[Stefan Beller]

This enables the work of the previous patches.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 Documentation/fetch-options.txt |  7 +++
 builtin/fetch.c |  6 +-
 builtin/pull.c  |  6 ++
 submodule.c |  3 +--
 submodule.h |  2 +-
 t/t5526-fetch-submodules.sh | 20 
 6 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 45583d8..6b109f6 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -100,6 +100,13 @@ ifndef::git-pull[]
reference to a commit that isn't already in the local submodule
clone.
 
+-j::
+--jobs=::
+   Number of parallel children to be used for fetching submodules.
+   Each will fetch from different submodules, such that fetching many
+   submodules will be faster. By default submodules will be fetched
+   one at a time.
+
 --no-recurse-submodules::
Disable recursive fetching of submodules (this has the same effect as
using the '--recurse-submodules=no' option).
diff --git a/builtin/fetch.c b/builtin/fetch.c
index c85f347..586840d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -37,6 +37,7 @@ static int prune = -1; /* unspecified */
 static int all, append, dry_run, force, keep, multiple, update_head_ok, 
verbosity;
 static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static int tags = TAGS_DEFAULT, unshallow, update_shallow;
+static int max_children = 1;
 static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
@@ -99,6 +100,8 @@ static struct option builtin_fetch_options[] = {
N_("fetch all tags and associated objects"), TAGS_SET),
OPT_SET_INT('n', NULL, ,
N_("do not fetch all tags (--no-tags)"), TAGS_UNSET),
+   OPT_INTEGER('j', "jobs", _children,
+   N_("number of submodules fetched in parallel")),
OPT_BOOL('p', "prune", ,
 N_("prune remote-tracking branches no longer on remote")),
{ OPTION_CALLBACK, 0, "recurse-submodules", NULL, N_("on-demand"),
@@ -1213,7 +1216,8 @@ int cmd_fetch(int argc, const char **argv, const char 
*prefix)
result = fetch_populated_submodules(,
submodule_prefix,
recurse_submodules,
-   verbosity < 0);
+   verbosity < 0,
+   max_children);
argv_array_clear();
}
 
diff --git a/builtin/pull.c b/builtin/pull.c
index 5145fc6..9e3c738 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -95,6 +95,7 @@ static int opt_force;
 static char *opt_tags;
 static char *opt_prune;
 static char *opt_recurse_submodules;
+static char *max_children;
 static int opt_dry_run;
 static char *opt_keep;
 static char *opt_depth;
@@ -178,6 +179,9 @@ static struct option pull_options[] = {
N_("on-demand"),
N_("control recursive fetching of submodules"),
PARSE_OPT_OPTARG),
+   OPT_PASSTHRU('j', "jobs", _children, N_("n"),
+   N_("number of submodules pulled in parallel"),
+   PARSE_OPT_OPTARG),
OPT_BOOL(0, "dry-run", _dry_run,
N_("dry run")),
OPT_PASSTHRU('k', "keep", _keep, NULL,
@@ -525,6 +529,8 @@ static int run_fetch(const char *repo, const char 
**refspecs)
argv_array_push(, opt_prune);
if (opt_recurse_submodules)
argv_array_push(, opt_recurse_submodules);
+   if (max_children)
+   argv_array_push(, max_children);
if (opt_dry_run)
argv_array_push(, "--dry-run");
if (opt_keep)
diff --git a/submodule.c b/submodule.c
index 6a2d786..0b48734 100644
--- a/submodule.c
+++ b/submodule.c
@@ -729,10 +729,9 @@ static int fetch_finish(int retvalue, struct child_process 
*cp,
 
 int fetch_populated_submodules(const struct argv_array *options,
   const char *prefix, int command_line_option,
-  int quiet)
+  int quiet, int max_parallel_jobs)
 {
int i;
-   int max_parallel_jobs = 1;
struct submodule_parallel_fetch spf = SPF_INIT;
 
spf.work_tree = get_git_work_tree();
diff --git a/submodule.h b/submodule.h
index 5507c3d..cbc0003 100644
--- a/submodule.h
+++ b/submodule.h
@@ -31,7 +31,7 @@ void set_config_fetch_recurse_submodules(int value);
 void check_for_new_submodule_commits(unsigned char new_sha1[20]);
 int fetch_populated_submodules(const struct argv_array *options,
   const char *prefix, 

[PATCH 7/8] fetch_populated_submodules: use new parallel job processing

[Stefan Beller]

In a later patch we enable parallel processing of submodules, this
only adds the possibility for it. So this change should not change
any user facing behavior.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 submodule.c | 142 +---
 1 file changed, 98 insertions(+), 44 deletions(-)

diff --git a/submodule.c b/submodule.c
index 8386477..6a2d786 100644
--- a/submodule.c
+++ b/submodule.c
@@ -12,6 +12,7 @@
 #include "sha1-array.h"
 #include "argv-array.h"
 #include "blob.h"
+#include "thread-utils.h"
 
 static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
 static struct string_list changed_submodule_paths;
@@ -610,37 +611,28 @@ static void calculate_changed_submodule_paths(void)
initialized_fetch_ref_tips = 0;
 }
 
-int fetch_populated_submodules(const struct argv_array *options,
-  const char *prefix, int command_line_option,
-  int quiet)
+struct submodule_parallel_fetch {
+   int count;
+   struct argv_array args;
+   const char *work_tree;
+   const char *prefix;
+   int command_line_option;
+   int quiet;
+   int result;
+};
+#define SPF_INIT {0, ARGV_ARRAY_INIT, NULL, NULL, 0, 0, 0}
+
+static int get_next_submodule(struct child_process *cp,
+ struct strbuf *err, void *data, void **task_cb)
 {
-   int i, result = 0;
-   struct child_process cp = CHILD_PROCESS_INIT;
-   struct argv_array argv = ARGV_ARRAY_INIT;
-   const char *work_tree = get_git_work_tree();
-   if (!work_tree)
-   goto out;
-
-   if (read_cache() < 0)
-   die("index file corrupt");
-
-   argv_array_push(, "fetch");
-   for (i = 0; i < options->argc; i++)
-   argv_array_push(, options->argv[i]);
-   argv_array_push(, "--recurse-submodules-default");
-   /* default value, "--submodule-prefix" and its value are added later */
-
-   cp.env = local_repo_env;
-   cp.git_cmd = 1;
-   cp.no_stdin = 1;
-
-   calculate_changed_submodule_paths();
+   int ret = 0;
+   struct submodule_parallel_fetch *spf = data;
 
-   for (i = 0; i < active_nr; i++) {
+   for ( ; spf->count < active_nr; spf->count++) {
struct strbuf submodule_path = STRBUF_INIT;
struct strbuf submodule_git_dir = STRBUF_INIT;
struct strbuf submodule_prefix = STRBUF_INIT;
-   const struct cache_entry *ce = active_cache[i];
+   const struct cache_entry *ce = active_cache[spf->count];
const char *git_dir, *default_argv;
const struct submodule *submodule;
 
@@ -652,7 +644,7 @@ int fetch_populated_submodules(const struct argv_array 
*options,
submodule = submodule_from_name(null_sha1, ce->name);
 
default_argv = "yes";
-   if (command_line_option == RECURSE_SUBMODULES_DEFAULT) {
+   if (spf->command_line_option == RECURSE_SUBMODULES_DEFAULT) {
if (submodule &&
submodule->fetch_recurse !=
RECURSE_SUBMODULES_NONE) {
@@ -675,40 +667,102 @@ int fetch_populated_submodules(const struct argv_array 
*options,
default_argv = "on-demand";
}
}
-   } else if (command_line_option == RECURSE_SUBMODULES_ON_DEMAND) 
{
+   } else if (spf->command_line_option == 
RECURSE_SUBMODULES_ON_DEMAND) {
if 
(!unsorted_string_list_lookup(_submodule_paths, ce->name))
continue;
default_argv = "on-demand";
}
 
-   strbuf_addf(_path, "%s/%s", work_tree, ce->name);
+   strbuf_addf(_path, "%s/%s", spf->work_tree, ce->name);
strbuf_addf(_git_dir, "%s/.git", submodule_path.buf);
-   strbuf_addf(_prefix, "%s%s/", prefix, ce->name);
+   strbuf_addf(_prefix, "%s%s/", spf->prefix, ce->name);
git_dir = read_gitfile(submodule_git_dir.buf);
if (!git_dir)
git_dir = submodule_git_dir.buf;
if (is_directory(git_dir)) {
-   if (!quiet)
-   fprintf(stderr, "Fetching submodule %s%s\n", 
prefix, ce->name);
-   cp.dir = submodule_path.buf;
-   argv_array_push(, default_argv);
-   argv_array_push(, "--submodule-prefix");
-   argv_array_push(, submodule_prefix.buf);
-   cp.argv = argv.argv;
-   if (run_command())
-   result = 1;
-   argv_array_pop();
- 

[PATCH 3/8] xread_nonblock: add functionality to read from fds without blocking

[Stefan Beller]

Provide a wrapper to read(), similar to xread(), that restarts on
EINTR but not EAGAIN (or EWOULDBLOCK). This enables the caller to
handle polling itself, possibly polling multiple sockets or performing
some other action.

Helped-by: Jacob Keller 
Helped-by: Jeff King ,
Helped-by: Junio C Hamano 
Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 git-compat-util.h |  1 +
 wrapper.c | 22 ++
 2 files changed, 23 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index 8e39867..87456a3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -723,6 +723,7 @@ extern void *xmmap(void *start, size_t length, int prot, 
int flags, int fd, off_
 extern void *xmmap_gently(void *start, size_t length, int prot, int flags, int 
fd, off_t offset);
 extern int xopen(const char *path, int flags, ...);
 extern ssize_t xread(int fd, void *buf, size_t len);
+extern ssize_t xread_nonblock(int fd, void *buf, size_t len);
 extern ssize_t xwrite(int fd, const void *buf, size_t len);
 extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
 extern int xdup(int fd);
diff --git a/wrapper.c b/wrapper.c
index 4f720fe..f71237c 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -252,6 +252,28 @@ ssize_t xread(int fd, void *buf, size_t len)
 }
 
 /*
+ * xread_nonblock() is the same a read(), but it automatically restarts read()
+ * interrupted operations (EINTR). xread_nonblock() DOES NOT GUARANTEE that
+ * "len" bytes is read. EWOULDBLOCK is turned into EAGAIN.
+ */
+ssize_t xread_nonblock(int fd, void *buf, size_t len)
+{
+   ssize_t nr;
+   if (len > MAX_IO_SIZE)
+   len = MAX_IO_SIZE;
+   while (1) {
+   nr = read(fd, buf, len);
+   if (nr < 0) {
+   if (errno == EINTR)
+   continue;
+   if (errno == EWOULDBLOCK)
+   errno = EAGAIN;
+   }
+   return nr;
+   }
+}
+
+/*
  * xwrite() is the same a write(), but it automatically restarts write()
  * operations with a recoverable error (EAGAIN and EINTR). xwrite() DOES NOT
  * GUARANTEE that "len" bytes is written even if the operation is successful.
-- 
2.6.4.443.ge094245.dirty

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

[PATCH 6/8] run-command: add an asynchronous parallel child processor

[Stefan Beller]

This allows to run external commands in parallel with ordered output
on stderr.

If we run external commands in parallel we cannot pipe the output directly
to the our stdout/err as it would mix up. So each process's output will
flow through a pipe, which we buffer. One subprocess can be directly
piped to out stdout/err for a low latency feedback to the user.

Example:
Let's assume we have 5 submodules A,B,C,D,E and each fetch takes a
different amount of time as the different submodules vary in size, then
the output of fetches in sequential order might look like this:

 time -->
 output: |---A---| |-B-| |---C---| |-D-| |-E-|

When we schedule these submodules into maximal two parallel processes,
a schedule and sample output over time may look like this:

process 1: |---A---| |-D-| |-E-|

process 2: |-B-| |---C---|

output:|---A---|B|---C---|DE

So A will be perceived as it would run normally in the single child
version. As B has finished by the time A is done, we can dump its whole
progress buffer on stderr, such that it looks like it finished in no
time. Once that is done, C is determined to be the visible child and
its progress will be reported in real time.

So this way of output is really good for human consumption, as it only
changes the timing, not the actual output.

For machine consumption the output needs to be prepared in the tasks,
by either having a prefix per line or per block to indicate whose tasks
output is displayed, because the output order may not follow the
original sequential ordering:

 |A| |--B--| |-C-|

will be scheduled to be all parallel:

process 1: |A|
process 2: |--B--|
process 3: |-C-|
output:|A|CB

This happens because C finished before B did, so it will be queued for
output before B.

The detection when a child has finished executing is done the same way as
two fold. First we check regularly if the stderr pipe still exists in an
interleaved manner with other actions such as checking other children
for their liveliness or starting new children. Once a child closed their
stderr stream, we assume it is stopping very soon, such that we can use
the `finish_command` code borrowed from the single external process
execution interface.

By maintaining the strong assumption of stderr being open until the
very end of a child process, we can avoid other hassle such as an
implementation using `waitpid(-1)`, which is not implemented in Windows.

Signed-off-by: Stefan Beller 
---
 run-command.c  | 335 +
 run-command.h  |  80 
 t/t0061-run-command.sh |  53 
 test-run-command.c |  55 +++-
 4 files changed, 522 insertions(+), 1 deletion(-)

diff --git a/run-command.c b/run-command.c
index 13fa452..51fd72c 100644
--- a/run-command.c
+++ b/run-command.c
@@ -3,6 +3,8 @@
 #include "exec_cmd.h"
 #include "sigchain.h"
 #include "argv-array.h"
+#include "thread-utils.h"
+#include "strbuf.h"
 
 void child_process_init(struct child_process *child)
 {
@@ -865,3 +867,336 @@ int capture_command(struct child_process *cmd, struct 
strbuf *buf, size_t hint)
close(cmd->out);
return finish_command(cmd);
 }
+
+enum child_state {
+   GIT_CP_FREE,
+   GIT_CP_WORKING,
+   GIT_CP_WAIT_CLEANUP,
+};
+
+struct parallel_processes {
+   void *data;
+
+   int max_processes;
+   int nr_processes;
+
+   get_next_task_fn get_next_task;
+   start_failure_fn start_failure;
+   task_finished_fn task_finished;
+
+   struct {
+   enum child_state state;
+   struct child_process process;
+   struct strbuf err;
+   void *data;
+   } *children;
+   /*
+* The struct pollfd is logically part of *children,
+* but the system call expects it as its own array.
+*/
+   struct pollfd *pfd;
+
+   unsigned shutdown : 1;
+
+   int output_owner;
+   struct strbuf buffered_output; /* of finished children */
+};
+
+static int default_start_failure(struct child_process *cp,
+struct strbuf *err,
+void *pp_cb,
+void *pp_task_cb)
+{
+   int i;
+
+   strbuf_addstr(err, "Starting a child failed:");
+   for (i = 0; cp->argv[i]; i++)
+   strbuf_addf(err, " %s", cp->argv[i]);
+
+   return 0;
+}
+
+static int default_task_finished(int result,
+struct child_process *cp,
+struct strbuf *err,
+void *pp_cb,
+void *pp_task_cb)
+{
+   int i;
+
+   if (!result)
+   return 0;
+
+   strbuf_addf(err, "A child failed with return code %d:", result);
+   for (i = 0; cp->argv[i]; i++)
+   strbuf_addf(err, " %s", cp->argv[i]);
+
+   return 0;
+}
+
+static void 

Re: git bisect with temporary commits

[Florian Bruhin]

* Junio C Hamano  [2015-12-14 11:21:06 -0800]:
> Florian Bruhin  writes:
> 
> > * Andreas Schwab  [2015-12-14 19:08:48 +0100]:
> >> Florian Bruhin  writes:
> >> 
> >> > Now when trying to say it's good (and forgetting to remove the
> >> > temporary commits), I get this:
> >> >
> >> > $ git bisect good
> >> > Bisecting: a merge base must be tested
> >> > [981e1093dae24b37189bcba2dd848b0c3388080c] still good and does not 
> >> > compile
> >> >
> >> > Is this intended behaviour? Shouldn't git either do a reset to the
> >> > commit we're currently bisecting, or warn the user as it was probably
> >> > unintended to add new commits?
> >> 
> >> You should instead tell git that HEAD^ is good, since that is what git
> >> asked you to test.
> >
> > I see - but wouldn't it make more sense for a "git bisect good" (or
> > bad, respectively) without arguments to assume I mean the commit
> > bisect checked out for me, not HEAD?
> >
> > I don't see any scenario where the current behaviour would make sense,
> > but I might be missing something.
> 
> When the commit "bisect" checked out is untestable, the user can
> freely go to another commit, e.g. "git reset --hard HEAD^" to go
> back one step, and then test it instead.  "git bisect good" has
> to mark the then-current HEAD, not the commit that was checked out,
> for this to work.

That makes sense - thanks for the explanation!

Florian

-- 
http://www.the-compiler.org | m...@the-compiler.org (Mail/XMPP)
   GPG: 916E B0C8 FD55 A072 | http://the-compiler.org/pubkey.asc
 I love long mails! | http://email.is-not-s.ms/


signature.asc
Description: Digital signature

Re: Corruption of branch?

[Thomas Nyberg]

Hi Stefan thanks for much for the response! So I compiled release 
version 2.6.4 as well as the current master branch on the git git 
repository (2.7.0.rc0.20.g4b9ab0e) and the problem persists on both.


To answer your questions, there are no weird characters. The name of the 
bad_branch is "frus". There is another branch called 
"frus_body_cleaning" that is totally fine.


As to whether the error continues after commits, the answer is no which 
is good. I.e. if I run `git checkout -b frus origin/frus` that works 
fine. I then decided to checkout a new branch (so as to not mess with 
the original branch and possibly turning this into a Heisenbug) and add 
a test commit which I pushed upstream. I then recloned the repository 
and was able to checkout this new branch just fine, but I still couldn't 
check out the original frus branch using the simplified command.


So of course on the practical side, I have a fix which is to just use 
`git checkout -b frus origin/frus` (apparently only this one time) and 
then be on my merry way (in fact I had only just broken myself of the 
habit typing it this older way after many versions of the newer simpler 
syntax), but it seems like it could be good to sort out what's going on 
here...


Thanks again for the response!

Cheers,
Thomas

On 12/14/2015 12:51 PM, Stefan Beller wrote:

On Mon, Dec 14, 2015 at 9:40 AM, Thomas Nyberg  wrote:

Hello,

I have a repository (which I unfortunately cannot provide access to) which
is having some odd things happening with one (and only one) of its branches.
This workflow repeats the issue (here `bad_branch` is one of the remotes
branches; i.e. `origin/bad_branch`):

(1) clone the repository
(2) git checkout `bad_branch`

Basically nothing happens. Nothing is printed and I stay on the master
branch. I also checked $? and there is no error code that is set. If I
choose any of other branches, it correctly creates a local branch, sets it
to track the remote and then switches to the local branch.


Does that branch have a funny name? (i.e. is it ASCII only? Or is it
also utf8 characters?
Does it have some special characters in it like points, colons,
question marks etc)

Does it happen only with a single sha1 or can you apply commits on top
of that branch
and the error condition persists?



It seems like there could be some sort of weird bug in the checkout or
possibly somehow some corruption in the actual object tree. From my vantage
point, however, the data appears totally fine. For example, in
`.git/packed-refs` everything appears normal and if I explicitly checkout
the commit IDs directly (i.e. just copy the commit corresponding to
refs/remotes/origin/bad_branch and checkout $commit) it checks out fine. If
I do this with the bad_branch I get a detached HEAD as expected and git log
lists the commits that it should.

This seems a bit odd to me. There's certainly some sort of error somewhere,
but it's passing silently. I'm not really sure how to debug this and it's
too bad I can't actually link the actual repository. I presume if I have the
time I could try compiling git from source and seeing if it still shows up.
I tested it on the following two versions of git get the same error:

* 1.9.1 (installed as a package from Linux Mint 17.2 Rafaela)
* 2.1.4 (installed as a package from Debian Jessie 8.2)


The refs handling code is in flux at the moment. (starting mid of last
year actually)
I cc'd people who did work recently on the file refs.c

So I think trying with a new version of Git would be a valuable datapoint!



Also I should note that the original repository is hosted on Github.

Thanks for any help. Hopefully the fact that I can't provide enough
information for others to reproduce the issue isn't too large a bother...

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

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

Re: git bisect with temporary commits

[Florian Bruhin]

* Andreas Schwab  [2015-12-14 19:08:48 +0100]:
> Florian Bruhin  writes:
> 
> > Now when trying to say it's good (and forgetting to remove the
> > temporary commits), I get this:
> >
> > $ git bisect good
> > Bisecting: a merge base must be tested
> > [981e1093dae24b37189bcba2dd848b0c3388080c] still good and does not 
> > compile
> >
> > Is this intended behaviour? Shouldn't git either do a reset to the
> > commit we're currently bisecting, or warn the user as it was probably
> > unintended to add new commits?
> 
> You should instead tell git that HEAD^ is good, since that is what git
> asked you to test.

I see - but wouldn't it make more sense for a "git bisect good" (or
bad, respectively) without arguments to assume I mean the commit
bisect checked out for me, not HEAD?

I don't see any scenario where the current behaviour would make sense,
but I might be missing something.

Florian

-- 
http://www.the-compiler.org | m...@the-compiler.org (Mail/XMPP)
   GPG: 916E B0C8 FD55 A072 | http://the-compiler.org/pubkey.asc
 I love long mails! | http://email.is-not-s.ms/


signature.asc
Description: Digital signature

Re: [PATCH] refs: mark some symbols static

[David Turner]

Will do.


On Sat, 2015-12-12 at 14:33 +, Ramsay Jones wrote:
> Signed-off-by: Ramsay Jones 
> ---
> 
> Hi David,
> 
> If you need to re-roll your 'dt/refs-backend-lmdb' branch, could
> you please squash the relevant parts of this patch into yours.
> 
> [yes, I didn't reference the movement of the external declaration
> in the commit message! :-D ]
> 
> Thanks!
> 
> ATB,
> Ramsay Jones
> 
>  refs.c   | 7 +++
>  refs/files-backend.c | 2 +-
>  refs/refs-internal.h | 2 ++
>  3 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/refs.c b/refs.c
> index 0be7065..c0faa97 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -9,7 +9,7 @@
>  #include "object.h"
>  #include "tag.h"
>  
> -const char split_transaction_fail_warning[] =
> +static const char split_transaction_fail_warning[] =
>   "A ref transaction was split across two refs backends.  Part of the "
>   "transaction succeeded, but then the update to the per-worktree refs "
>   "failed.  Your repository may be in an inconsistent state.";
> @@ -17,12 +17,11 @@ const char split_transaction_fail_warning[] =
>  /*
>   * We always have a files backend and it is the default.
>   */
> -extern struct ref_be refs_be_files;
> -struct ref_be *the_refs_backend = _be_files;
> +static struct ref_be *the_refs_backend = _be_files;
>  /*
>   * List of all available backends
>   */
> -struct ref_be *refs_backends = _be_files;
> +static struct ref_be *refs_backends = _be_files;
>  
>  const char *refs_backend_type;
>  
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 0efc507..e8181ae 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -3280,7 +3280,7 @@ static int ref_present(const char *refname,
>   return string_list_has_string(affected_refnames, refname);
>  }
>  
> -void files_init_backend(void *data)
> +static void files_init_backend(void *data)
>  {
>   /* do nothing */
>  }
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index 9c17fdf..8fb360d 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -313,4 +313,6 @@ struct ref_be {
>   for_each_replace_ref_fn *for_each_replace_ref;
>  };
>  
> +extern struct ref_be refs_be_files;
> +
>  #endif /* REFS_REFS_INTERNAL_H */


--
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 jk/prune-mtime] prune: close directory earlier during loose-object directory traversal

[Junio C Hamano]

Johannes Schindelin  writes:

>> > Sorry for reviving this old thread, but I noticed that we do not
>> > have this patch in our tree yet.  I'll queue to 'pu' for now lest I
>> > forget.  If I missed a good argument or concensus against the change
>> > please let me know, otherwise I'll fast track the change to 2.7 final
>> 
>> Ah, thanks for doing that. I noticed it when picking through "git branch
>> --no-merged pu" of your workspace a few weeks ago, but forgot to follow
>> up. I certainly have no objections.
>
> Git for Windows carries this patch since Git for Windows v2.5.0. So: no
> objection from my side, either.

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

git-svn does not honor preserve-empty-dirs

[Andreas Dröscher]

Hi

I think git-svn 2.6.4 does not behave as intended.

According to the documentation preserve-empty-dirs should ensure that empty-
directories are kept in all cases: "Create a placeholder file in the local Git
repository for each empty directory fetched from Subversion. This includes
directories that become empty by removing all entries in the Subversion
repository (but not the directory itself)."

I've attached an svn repo to demonstrate the issue. Everything goes fine
during the first 3 commits. In commit r4 a file is removed from the demo
folder, but not the directory. This works in svn but not in git.

Steps to Reproduce:
1. Extract tar to a directory of your choice e.g. /tmp/svn/
2. git svn clone --stdlayout --preserve-empty-dirs file:///tmp/svn/
3. Now trunk is empty. However the directory demo should have been preserved.

I'm not a member of the mailing-list. Please CC me.

Best Wishes
Andreas

SVN Commit Log:

r4 | andreas | 2015-12-14 22:52:49 +0100 (Mo, 14. Dez 2015) | 1 Zeile

empty dir

r3 | andreas | 2015-12-14 22:52:22 +0100 (Mo, 14. Dez 2015) | 1 Zeile

fill dir

r2 | andreas | 2015-12-14 22:51:39 +0100 (Mo, 14. Dez 2015) | 1 Zeile

add empty dir demo

r1 | andreas | 2015-12-14 22:50:46 +0100 (Mo, 14. Dez 2015) | 1 Zeile

initial import




svn.tar.bz2
Description: BZip2 compressed data

Re: Why does send-pack call pack-objects for all remote refs?

[Jonathan Nieder]

Jeff King wrote:

> Hmm. I guess that makes sense. The bitmap we want is the set difference
> between the objects we are sending, and the tips the other side has. If
> we have a bitmap at each ref tip, that's very fast. But if you have a
> very large number of refs, we don't make one for each ref, and it has to
> fallback to walking to the nearest one (and it ends up worse than a
> regular walk, because it's filling in the bitmap for each tree, rather
> than just doing the "good enough" commit walk that we usually do).
>
> I suspect there's room for improvement in the way we select commits to
> store bitmaps for (so that the average walk is smaller). But it's rather
> tricky; there's not a single constant to change to make it work better.

Git gc and JGit GC differ here.  JGit partitions the commits being
packed by branch and then runs a selection algorithm on each part.
Git runs a selection once on a list of all commits.

Some effects:
- JGit selects more bitmaps, so the gc takes longer and the resulting
  bitmap file is larger (bad)
- JGit is more likely to have bitmaps for the commits involved in
  pushes and fetches (good)

The commit selection code, for reference:

https://eclipse.googlesource.com/jgit/jgit/+/86af34e1/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapPreparer.java#151
https://kernel.googlesource.com/pub/scm/git/git/+/ed1c9977/pack-bitmap-write.c#383

Thoughts?
Jonathan
--
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 v1 4/4] git gui: allow for a long recentrepo list

[Eric Sunshine]

On Mon, Dec 14, 2015 at 10:09 AM, Philip Oakley  wrote:
> The gui.recentrepo list may be longer than the maxrecent setting.
> Allow extra space to show any extra entries.
>
> In an ideal world, the git gui would limit the number of entries
> to the maxrecent setting, however the recentrepo config list may
> have been extended outwith the gui, or the maxrecent setting changed

s/outwith/without/

> to a reduced value. Further, when testing the gui's recentrepo
> logic it is useful to show these extra, but valid, entries.
>
> Signed-off-by: Philip Oakley 
> ---
> diff --git a/git-gui/lib/choose_repository.tcl 
> b/git-gui/lib/choose_repository.tcl
> index ad7a888..a08ed4f 100644
> --- a/git-gui/lib/choose_repository.tcl
> +++ b/git-gui/lib/choose_repository.tcl
> @@ -153,7 +153,7 @@ constructor pick {} {
> -background [get_bg_color $w_body.recentlabel] \
> -wrap none \
> -width 50 \
> -   -height $maxrecent
> +   -height [expr {$maxrecent + 5}]
> $w_recentlist tag conf link \
> -foreground blue \
> -underline 1
> --
> 2.5.2.windows.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Why does send-pack call pack-objects for all remote refs?

[Jeff King]

On Mon, Dec 14, 2015 at 02:31:55PM -0800, Jonathan Nieder wrote:

> > I suspect there's room for improvement in the way we select commits to
> > store bitmaps for (so that the average walk is smaller). But it's rather
> > tricky; there's not a single constant to change to make it work better.
> 
> Git gc and JGit GC differ here.  JGit partitions the commits being
> packed by branch and then runs a selection algorithm on each part.
> Git runs a selection once on a list of all commits.
> 
> Some effects:
> - JGit selects more bitmaps, so the gc takes longer and the resulting
>   bitmap file is larger (bad)
> - JGit is more likely to have bitmaps for the commits involved in
>   pushes and fetches (good)
> 
> The commit selection code, for reference:
> 
> https://eclipse.googlesource.com/jgit/jgit/+/86af34e1/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/pack/PackWriterBitmapPreparer.java#151
> https://kernel.googlesource.com/pub/scm/git/git/+/ed1c9977/pack-bitmap-write.c#383
> 
> Thoughts?

My thought is it would be great if somebody wanted to work on this. :)

My understanding is that JGit's approach has some problems, too. Terry's
message doesn't seem to have made it to the list, but you can see in the
quoted bits he mentions some OOM problems during the bitmap write:

  http://article.gmane.org/gmane.comp.version-control.git/281476

That may not be a big deal to work around. I really just haven't looked
at it at all. Vicent did the original bitmap selection code for C Git,
and I don't think it has been touched since then.

-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 0/2] git-p4: fix for handling of multiple depot paths

[Junio C Hamano]

Luke Diamand  writes:

> On 14 December 2015 at 19:16, Junio C Hamano  wrote:
>> Luke Diamand  writes:
>>
>>> Having just fixed this, I've now just spotted that Sam Hocevar's fix
>>> to reduce the number of P4 transactions also fixes it:
>>>
>>> https://www.mail-archive.com/git%40vger.kernel.org/msg81880.html
>>>
>>> That seems like a cleaner fix.
>>
>> Hmm, do you mean I should ignore this series and take the other one,
>> take only 1/2 from this for tests and then both patches in the other
>> one, or something else?
>
> The second of those (take only 1/2 from this for tests, and then both
> from the other) seems like the way to go.

OK.  Should I consider the two patches from Sam "Reviewed-by" you?
--
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

Publish Research Papers

[Cynthia Ken (editor)]

To Research Authors (Professor/Doctor/Lecturer/Student), Please be informed 
that your are invited to submit your articles for publication in our esteem 
journal. Your articles will undergo language copy-editing, typesetting, and 
reference validation in order to provide the highest publication quality 
possible. Please do not hesitate to contact me if you have any questions about 
the journal.Regards, Editor of SJP Journals. **TO STOP RECEIVING OUR 
NEWSLETTER, SIMPLY REPLY THIS MAIL WITH "STOP" AS THE SUBJECT LINE.**
--
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 v1 1/4] git-gui: remove duplicate entries from .gitconfig's gui.recentrepo

[Eric Sunshine]

On Monday, December 14, 2015, Philip Oakley  wrote:
> The git gui's recent repo list may become contaminated with duplicate
> entries. The git gui would barf when attempting to remove one entry.
> Remove them all - there is no option within 'git config' to selectively
> remove one of the entries.
>
> This issue was reported on the 'Git User' list
> (https://groups.google.com/forum/#!topic/git-users/msev4KsQGFc,
> Warning: gui.recentrepo has multiply values while executing).

s/multiply/multiple/

> On startup the gui checks that entries in the recentrepo list are still
> valid repos and deletes thoses that are not. If duplicate entries are

s/thoses/those/

> present the 'git config --unset' will barf and this prevents the gui

s/present the/present, then/

> from starting.
>
> Subsequent patches fix other parts of recentrepo logic used for syncing
> internal lists with the external .gitconfig.
>
> Reported-by: Alexey Astakhov 
> Signed-off-by: Philip Oakley 
> ---
> diff --git a/git-gui/lib/choose_repository.tcl 
> b/git-gui/lib/choose_repository.tcl
> index 75d1da8..133ca0a 100644
> --- a/git-gui/lib/choose_repository.tcl
> +++ b/git-gui/lib/choose_repository.tcl
> @@ -247,7 +247,7 @@ proc _get_recentrepos {} {
>
>  proc _unset_recentrepo {p} {
> regsub -all -- {([()\[\]{}\.^$+*?\\])} $p {\\\1} p
> -   git config --global --unset gui.recentrepo "^$p\$"
> +   git config --global --unset-all gui.recentrepo "^$p\$"
> load_config 1
>  }
>
> --
> 2.5.2.windows.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8] Rerolling sb/submodule-parallel-fetch for the time after 2.7

[Johannes Sixt]

Am 14.12.2015 um 20:37 schrieb Stefan Beller:

I am sending out a new version for replacing sb/submodule-parallel-fetch for
the time after the 2.7 release.

The content are
  * all patches as in the branch sb/submodule-parallel-fetch
  * inlcuding the fixups as suggested by Hannes,
  * write a message to the debug log for better testing and debugging purposes
   (a patch cherry picked from the series which is supposed to build on top of 
this)

The patches themselves were rebased such that there are no fixup commits
any more, but we get things right the first time.

The commit message of "run-command: add an asynchronous parallel child 
processor"
has slightly been updated to mention the fact that we don't want to use 
waitpid(-1)
but rather use the assumption of child's stderr living as long as the child 
itself.


Thanks! I rebased a version of sb/submodule-parallel-fetch that includes 
my suggested improvements, and the result is identical to this series 
except for the trace output mentioned in the last bullet point.


With or without addressing my note about the commit message in 6/8:

Acked-by: Johannes Sixt 


Thanks,
Stefan


Jonathan Nieder (1):
   submodule.c: write "Fetching submodule " to stderr

Stefan Beller (7):
   xread: poll on non blocking fds
   xread_nonblock: add functionality to read from fds without blocking
   strbuf: add strbuf_read_once to read without blocking
   sigchain: add command to pop all common signals
   run-command: add an asynchronous parallel child processor
   fetch_populated_submodules: use new parallel job processing
   submodules: allow parallel fetching, add tests and documentation

  Documentation/fetch-options.txt |   7 +
  builtin/fetch.c |   6 +-
  builtin/pull.c  |   6 +
  git-compat-util.h   |   1 +
  run-command.c   | 335 
  run-command.h   |  80 ++
  sigchain.c  |   9 ++
  sigchain.h  |   1 +
  strbuf.c|  11 ++
  strbuf.h|   8 +
  submodule.c | 141 +++--
  submodule.h |   2 +-
  t/t0061-run-command.sh  |  53 +++
  t/t5526-fetch-submodules.sh |  71 ++---
  test-run-command.c  |  55 ++-
  wrapper.c   |  35 -
  16 files changed, 747 insertions(+), 74 deletions(-)



--
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: Corruption of branch?

[Thomas Nyberg]

wow right on the button. yeah i have the "frus" folder in the root of my 
repository. i never knew that git checkout also searches the root of the 
repository like that. it appears i'm a fool who doesn't read 
documentation...


i learned something knew and can move this from the "bizarre index 
corruption" category to the "user error" category. thanks so much everyone!


On 12/14/2015 03:40 PM, Dennis Kaarsemaker wrote:

On ma, 2015-12-14 at 15:33 -0500, Thomas Nyberg wrote:

What exactly are you looking for? Here's the results of the following
command:

$ git for-each-ref | grep frus
1750cba5a94b3fe6041aaf49de430a558a3b9bc8 commit
refs/heads/frus_body_cleaning
3a1dbe48299f6eda1cc4b69cab35284c0f0355eb commit refs/remotes/o
rigin/frus
1750cba5a94b3fe6041aaf49de430a558a3b9bc8 commit
refs/remotes/origin/frus_body_cleaning

Sorry if this isn't what you're looking for. I'm actually not very
familiar with these different internal git commands...


This is what I was looking for. Unfortunately it doesn't show any of
the smoking guns I had hoped for.

That leaves only one option: you also have a file or directory named
'frus' in the root of your repository. In this case 'git checkout frus'
does the same as 'git checkout -- frus' instead of DWIM'ing 'git
checkout frus' to 'git checkout -b frus origin/frus'


--
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: compile error in Git v2.7.0-rc0

[Jeff King]

On Mon, Dec 14, 2015 at 03:46:25PM -0500, Jeff King wrote:

> I don't think that fix is right, though. We should be passing "host" to
> gethostbyname.

Here it is in patch form. It can go on top of ep/ident-with-getaddrinfo.

-- >8 --
Subject: [PATCH] ident: fix undefined variable when NO_IPV6 is set

Commit 00bce77 (ident.c: add support for IPv6, 2015-11-27)
moved the "gethostbyname" call out of "add_domainname" and
into the helper function "canonical_name". But when moving
the code, it forgot that the "buf" variable is passed as
"host" in the helper.

Reported-by: johan defries 
Signed-off-by: Jeff King 
---
 ident.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ident.c b/ident.c
index 4e7f99d..00a62e0 100644
--- a/ident.c
+++ b/ident.c
@@ -86,7 +86,7 @@ static int canonical_name(const char *host, struct strbuf 
*out)
freeaddrinfo(ai);
}
 #else
-   struct hostent *he = gethostbyname(buf);
+   struct hostent *he = gethostbyname(host);
if (he && strchr(he->h_name, '.')) {
strbuf_addstr(out, he->h_name);
status = 0;
-- 
2.7.0.rc0.348.g8e7037f

--
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 0/2] git-p4: fix for handling of multiple depot paths

[Luke Diamand]

On 14 December 2015 at 19:16, Junio C Hamano  wrote:
> Luke Diamand  writes:
>
>> Having just fixed this, I've now just spotted that Sam Hocevar's fix
>> to reduce the number of P4 transactions also fixes it:
>>
>> https://www.mail-archive.com/git%40vger.kernel.org/msg81880.html
>>
>> That seems like a cleaner fix.
>
> Hmm, do you mean I should ignore this series and take the other one,
> take only 1/2 from this for tests and then both patches in the other
> one, or something else?

The second of those (take only 1/2 from this for tests, and then both
from the other) seems like the way to go.

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

Re: [PATCH 0/8] Rerolling sb/submodule-parallel-fetch for the time after 2.7

[Junio C Hamano]

Johannes Sixt  writes:

> Am 14.12.2015 um 20:37 schrieb Stefan Beller:
>> I am sending out a new version for replacing sb/submodule-parallel-fetch for
>> the time after the 2.7 release.
>>
>> The content are
>>   * all patches as in the branch sb/submodule-parallel-fetch
>>   * inlcuding the fixups as suggested by Hannes,
>>   * write a message to the debug log for better testing and debugging 
>> purposes
>>(a patch cherry picked from the series which is supposed to build on top 
>> of this)
>>
>> The patches themselves were rebased such that there are no fixup commits
>> any more, but we get things right the first time.
>>
>> The commit message of "run-command: add an asynchronous parallel child 
>> processor"
>> has slightly been updated to mention the fact that we don't want to use 
>> waitpid(-1)
>> but rather use the assumption of child's stderr living as long as the child 
>> itself.
>
> Thanks! I rebased a version of sb/submodule-parallel-fetch that
> includes my suggested improvements, and the result is identical to
> this series except for the trace output mentioned in the last bullet
> point.
>
> With or without addressing my note about the commit message in 6/8:
>
> Acked-by: Johannes Sixt 

Thanks, both.  This round does look very clean to build on top.
--
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: query regarding git merge

[Jeff King]

On Sun, Dec 13, 2015 at 07:23:17PM +, brian m. carlson wrote:

> On Mon, Dec 14, 2015 at 12:03:18AM +0530, Rohit Gupta wrote:
> > Thanks brian. I understood my mistake in understanding the working of git
> > merge.
> > But isn't it wrong? As after merging, branch's logic can't work. How to get
> > that right then ?
> 
> If you know that the merge didn't go the way you wanted, you can either
> add a follow-up commit, or you can do "git commit --amend" on the merge
> after making the necessary changes.  In such a case, it may be useful to
> add a note to the commit message stating that you modified it from the
> original merge.

And a fundamental takeaway here is that git-merge can only find
_textual_ conflicts. It is up to the user to determine that the merge
didn't introduce any _semantic_ conflicts. For example, by building and
testing the result, which is out of git's scope.

-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: Why does send-pack call pack-objects for all remote refs?

[Jeff King]

On Mon, Dec 14, 2015 at 01:47:39PM +, Daniel Koverman wrote:

> > You might also try repacking with "git repack -adb", which will
> > build reachability bitmaps. Pack-objects can use them to compute
> > the set of required objects much faster.
> 
> Running "git repack -adb" caused my push time to incease by about 5x.
> I made some fresh clones and tried other options with repack, and
> consistently anything I tried with -b caused the push time to
> increase about 5x.
> 
> I don't know much about reachability bitmaps, but perhaps it is
> important to note that I timed the pushes after repacking on Git for
> Windows. My earlier timings were done on both Linux and Windows and I
> did not see a significant difference.

Hmm. I guess that makes sense. The bitmap we want is the set difference
between the objects we are sending, and the tips the other side has. If
we have a bitmap at each ref tip, that's very fast. But if you have a
very large number of refs, we don't make one for each ref, and it has to
fallback to walking to the nearest one (and it ends up worse than a
regular walk, because it's filling in the bitmap for each tree, rather
than just doing the "good enough" commit walk that we usually do).

I suspect there's room for improvement in the way we select commits to
store bitmaps for (so that the average walk is smaller). But it's rather
tricky; there's not a single constant to change to make it work better.

Thanks for trying out my suggestion.

-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 2/2] push: add '-d' as shorthand for '--delete'

[Jeff King]

On Mon, Dec 14, 2015 at 04:23:04PM +0100, Patrick Steinhardt wrote:

> It is only possible to delete branches on remotes by specifying
> the long '--delete' flag. The `git-branch` command, which can be
> used to delete local branches with the same '--delete' flag, also
> accepts the shorthand '-d'. This may cause confusion for users
> which are frequently using the shorthand form of deleting local
> branches and subsequently try to use the same shorthand for
> `git-push`, which will fail.
> 
> Fix this usability issue by adding the '-d' shorthand to
> `git-push` and document it.

I think we didn't give it "-d" originally, because we usually avoid
allocating short-options (which are a limited resource) until an option
has proven itself.

At this point, it seems that "--delete" is useful, and nothing else has
been proposed for "-d" in the intervening years. It seems like a
reasonable use of the flag to me.

I have been bitten by this myself. I know about "git push origin
:ref-to-delete", of course, but my brain would much rather type "-d"
(and it's also easier when piping to xargs).

-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 6/8] run-command: add an asynchronous parallel child processor

[Johannes Sixt]

Am 14.12.2015 um 20:37 schrieb Stefan Beller:

This allows to run external commands in parallel with ordered output
on stderr.

If we run external commands in parallel we cannot pipe the output directly
to the our stdout/err as it would mix up. So each process's output will
flow through a pipe, which we buffer. One subprocess can be directly
piped to out stdout/err for a low latency feedback to the user.

Example:
Let's assume we have 5 submodules A,B,C,D,E and each fetch takes a
different amount of time as the different submodules vary in size, then
the output of fetches in sequential order might look like this:

  time -->
  output: |---A---| |-B-| |---C---| |-D-| |-E-|

When we schedule these submodules into maximal two parallel processes,
a schedule and sample output over time may look like this:

process 1: |---A---| |-D-| |-E-|

process 2: |-B-| |---C---|

output:|---A---|B|---C---|DE

So A will be perceived as it would run normally in the single child
version. As B has finished by the time A is done, we can dump its whole
progress buffer on stderr, such that it looks like it finished in no
time. Once that is done, C is determined to be the visible child and
its progress will be reported in real time.

So this way of output is really good for human consumption, as it only
changes the timing, not the actual output.

For machine consumption the output needs to be prepared in the tasks,
by either having a prefix per line or per block to indicate whose tasks
output is displayed, because the output order may not follow the
original sequential ordering:

  |A| |--B--| |-C-|

will be scheduled to be all parallel:

process 1: |A|
process 2: |--B--|
process 3: |-C-|
output:|A|CB

This happens because C finished before B did, so it will be queued for
output before B.

The detection when a child has finished executing is done the same way as
two fold. First we check regularly if the stderr pipe still exists in an
interleaved manner with other actions such as checking other children
for their liveliness or starting new children. Once a child closed their
stderr stream, we assume it is stopping very soon, such that we can use
the `finish_command` code borrowed from the single external process
execution interface.


I can't quite parse the first sentence in this paragraph. Perhaps 
something like this:


To detect when a child has finished executing, we check interleaved
with other actions (such as checking the liveliness of children or
starting new processes) whether the stderr pipe still exists. Once a
child closed its stderr stream, we assume it is terminating very soon,
and use finish_command() from the single external process execution
interface to collect the exit status.



By maintaining the strong assumption of stderr being open until the
very end of a child process, we can avoid other hassle such as an
implementation using `waitpid(-1)`, which is not implemented in Windows.

Signed-off-by: Stefan Beller 


--
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: Corruption of branch?

[Dennis Kaarsemaker]

On ma, 2015-12-14 at 15:33 -0500, Thomas Nyberg wrote:
> What exactly are you looking for? Here's the results of the following
> command:
> 
> $ git for-each-ref | grep frus
> 1750cba5a94b3fe6041aaf49de430a558a3b9bc8 commit 
> refs/heads/frus_body_cleaning
> 3a1dbe48299f6eda1cc4b69cab35284c0f0355eb commit   refs/remotes/o
> rigin/frus
> 1750cba5a94b3fe6041aaf49de430a558a3b9bc8 commit 
> refs/remotes/origin/frus_body_cleaning
> 
> Sorry if this isn't what you're looking for. I'm actually not very 
> familiar with these different internal git commands...

This is what I was looking for. Unfortunately it doesn't show any of
the smoking guns I had hoped for.

That leaves only one option: you also have a file or directory named
'frus' in the root of your repository. In this case 'git checkout frus'
does the same as 'git checkout -- frus' instead of DWIM'ing 'git
checkout frus' to 'git checkout -b frus origin/frus'

-- 
Dennis Kaarsemaker
www.kaarsemaker.net


--
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: Corruption of branch?

[Junio C Hamano]

Dennis Kaarsemaker  writes:

> That leaves only one option: you also have a file or directory named
> 'frus' in the root of your repository. In this case 'git checkout frus'
> does the same as 'git checkout -- frus' instead of DWIM'ing 'git
> checkout frus' to 'git checkout -b frus origin/frus'

;-)  That would be my guess.
--
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: compile error in Git v2.7.0-rc0

[Jeff King]

On Mon, Dec 14, 2015 at 12:35:25PM -0800, Junio C Hamano wrote:

> johan defries  writes:
> 
> > Probably because I have NO_IPV6 defined.
> >
> > ident.c: In function ‘canonical_name’:
> > ident.c:89:37: error: ‘buf’ undeclared (first use in this function)
> >   struct hostent *he = gethostbyname(buf);
> >  ^
> > ident.c:89:37: note: each undeclared identifier is reported only once
> > for each function it appears in
> > make: *** [ident.o] Fout 1
> 
> Thanks.  This should perhaps do?
> 
>  ident.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/ident.c b/ident.c
> index 4e7f99d..2900879 100644
> --- a/ident.c
> +++ b/ident.c
> @@ -86,6 +86,7 @@ static int canonical_name(const char *host, struct strbuf 
> *out)
>   freeaddrinfo(ai);
>   }
>  #else
> + char buf[1024];
>   struct hostent *he = gethostbyname(buf);
>   if (he && strchr(he->h_name, '.')) {
>   strbuf_addstr(out, he->h_name);

Whoops. Looks like we didn't test the NO_IPV6 code path.

I don't think that fix is right, though. We should be passing "host" to
gethostbyname.

-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

update index mtime etc metadata

[Joey Hess]

Is there any available plumbing that can change the mtime etc metadata
that is recorded in the index for a file, to user-provided values? Or,
to force the current file stat metadata to be updated in the index?

I know, git update-index --refresh, but I have a case where that's too
expensive. I'm using smudge filters; I know that the cleaned version of
the file will be unchanged from what's in the index now and only the
stat metadata will change, and so I want to avoid
git update-index --refresh running the clean filter, which can
be quite expensive for a large file.

At the moment I don't see a way to do it other than using eg libgit2 to
update the appropriate fields in the index structure.

-- 
see shy jo


signature.asc
Description: PGP signature

Re: [PATCH 3/8] xread_nonblock: add functionality to read from fds without blocking

[Junio C Hamano]

Stefan Beller  writes:

> Provide a wrapper to read(), similar to xread(), that restarts on
> EINTR but not EAGAIN (or EWOULDBLOCK). This enables the caller to
> handle polling itself, possibly polling multiple sockets or performing
> some other action.

Do you still need this (and use of this in 4/8)?  The description of
a4433fd4 (run-command: remove set_nonblocking(), 2015-11-05) from
the previous iteration justifies the removal of set_nonblocking()
this way:

run-command: remove set_nonblocking()

strbuf_read_once can also operate on blocking file descriptors if we
are sure they are ready.  And the poll(2) we call before calling
this ensures that this is the case.

Updated run-command in this reroll lacks set_nonblocking(), and does
have the poll(2) before it calls strbuf_read_once().  Which means
that xread_nonblock() introduced here and used by [4/8] would read a
file descriptor that is not marked as nonblock, the read(2) in there
would never error-return with EWOULDBLOCK, and would be identical to 
xread() updated by [2/8] in this reroll.

So it appears to me that you can lose this and call xread() in [4/8]?

Ahh, there is a difference if the file descriptor the caller feeds
strbuf_read_once() happens to be marked as nonblock.  read_once()
wants to return without doing the poll() in such a case.  Even
though this series would not introduce any use of a nonblocking file
descriptor, as a general API function, [4/8] must be prepared for
such a future caller, hence [3/8] is needed.

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

Re: git bisect with temporary commits

[Jeff King]

On Mon, Dec 14, 2015 at 07:08:48PM +0100, Andreas Schwab wrote:

> Florian Bruhin  writes:
> 
> > Now when trying to say it's good (and forgetting to remove the
> > temporary commits), I get this:
> >
> > $ git bisect good
> > Bisecting: a merge base must be tested
> > [981e1093dae24b37189bcba2dd848b0c3388080c] still good and does not 
> > compile
> >
> > Is this intended behaviour? Shouldn't git either do a reset to the
> > commit we're currently bisecting, or warn the user as it was probably
> > unintended to add new commits?
> 
> You should instead tell git that HEAD^ is good, since that is what git
> asked you to test.

Another alternative is to use "git cherry-pick -n" to create a working
tree state that you can test, but leave HEAD at the original commit.
Then "git bisect good" does the right thing.

It's the same principle and I don't think there is a reason to prefer
one over the other. I just find it harder to screw up, as I usually
script the build/test, so I can stick the cherry-pick there and not have
to remember it on each "good/bad" report.

-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: compile error in Git v2.7.0-rc0

[Junio C Hamano]

On Mon, Dec 14, 2015 at 12:52 PM, Jeff King  wrote:
> On Mon, Dec 14, 2015 at 03:46:25PM -0500, Jeff King wrote:
>
>> I don't think that fix is right, though. We should be passing "host" to
>> gethostbyname.
>
> Here it is in patch form. It can go on top of ep/ident-with-getaddrinfo.


Thanks.

I recall you were looking for a brown-paper-bag earlier.
When you are done with it, could you pass it to me ;-)?
--
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

Downloading for mac.

[Rob Cifre]

Hello it seems that the download for mac isn’t working on your website.
Any other location I can download it from?

https://git-scm.com/download/mac--
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 bisect with temporary commits

[Junio C Hamano]

Jeff King  writes:

>> You should instead tell git that HEAD^ is good, since that is what git
>> asked you to test.
>
> Another alternative is to use "git cherry-pick -n" to create a working
> tree state that you can test, but leave HEAD at the original commit.
> Then "git bisect good" does the right thing.

I was about to say the same, and "bisect good" at that point does
mark the correct commit, but does it always do the right thing?  I
think the procedure must be

git cherry-pick -n $the_fixup
test
git reset --hard
git bisect good (or bad)

for it to always work, which is not all that different from

git cherry-pick $the_fixup
test
git reset --hard HEAD^
git bisect good (or bad)

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

[PATCHv6 0/8] Expose submodule parallelism to the user

[Stefan Beller]

This is a resend of sb/submodule-parallel-update and is available at github[1] 
as well.

What does it do?
---
This series should finish the on going efforts of parallelizing
submodule network traffic. The patches contain tests for clone,
fetch and submodule update to use the actual parallelism both via
command line as well as a configured option. I decided to go with
"submodule.fetchJobs" for all three for now.

What changed to v5?
---
No major changes, I just made it compile again as thr order of parameters
to the parallel processing engine changed.

Thanks,
Stefan


[1] https://github.com/stefanbeller/git/tree/submodule-parallel-update-2 which 
is on
top of https://github.com/stefanbeller/git/tree/submodule-parallel-fetch-2 
which I sent
to the list about 3 hours ago.

Stefan Beller (8):
  submodule-config: keep update strategy around
  submodule-config: drop check against NULL
  submodule-config: remove name_and_item_from_var
  submodule-config: introduce parse_generic_submodule_config
  fetching submodules: respect `submodule.fetchJobs` config option
  git submodule update: have a dedicated helper for cloning
  submodule update: expose parallelism to the user
  clone: allow an explicit argument for parallel submodule clones

 Documentation/config.txt|   7 ++
 Documentation/git-clone.txt |   6 +-
 Documentation/git-submodule.txt |   7 +-
 builtin/clone.c |  19 +++-
 builtin/fetch.c |   2 +-
 builtin/submodule--helper.c | 239 
 git-submodule.sh|  54 -
 submodule-config.c  | 109 +++---
 submodule-config.h  |   3 +
 submodule.c |   5 +
 t/t5526-fetch-submodules.sh |  14 +++
 t/t7400-submodule-basic.sh  |   4 +-
 t/t7406-submodule-update.sh |  27 +
 13 files changed, 413 insertions(+), 83 deletions(-)

-- 
2.6.4.443.ge094245.dirty

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

[PATCH 1/8] submodule-config: keep update strategy around

[Stefan Beller]

We need the submodule update strategies in a later patch.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 submodule-config.c | 11 +++
 submodule-config.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/submodule-config.c b/submodule-config.c
index afe0ea8..4239b0e 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -194,6 +194,7 @@ static struct submodule *lookup_or_create_by_name(struct 
submodule_cache *cache,
 
submodule->path = NULL;
submodule->url = NULL;
+   submodule->update = NULL;
submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
submodule->ignore = NULL;
 
@@ -311,6 +312,16 @@ static int parse_config(const char *var, const char 
*value, void *data)
free((void *) submodule->url);
submodule->url = xstrdup(value);
}
+   } else if (!strcmp(item.buf, "update")) {
+   if (!value)
+   ret = config_error_nonbool(var);
+   else if (!me->overwrite && submodule->update != NULL)
+   warn_multiple_config(me->commit_sha1, submodule->name,
+"update");
+   else {
+   free((void *) submodule->update);
+   submodule->update = xstrdup(value);
+   }
}
 
strbuf_release();
diff --git a/submodule-config.h b/submodule-config.h
index 9061e4e..f9e2a29 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -14,6 +14,7 @@ struct submodule {
const char *url;
int fetch_recurse;
const char *ignore;
+   const char *update;
/* the sha1 blob id of the responsible .gitmodules file */
unsigned char gitmodules_sha1[20];
 };
-- 
2.6.4.443.ge094245.dirty

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

[PATCH 3/8] submodule-config: remove name_and_item_from_var

[Stefan Beller]

`name_and_item_from_var` does not provide the proper abstraction
we need here in a later patch.

Signed-off-by: Stefan Beller 
---
 submodule-config.c | 48 
 1 file changed, 16 insertions(+), 32 deletions(-)

diff --git a/submodule-config.c b/submodule-config.c
index 6d01941..b826841 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -161,31 +161,17 @@ static struct submodule *cache_lookup_name(struct 
submodule_cache *cache,
return NULL;
 }
 
-static int name_and_item_from_var(const char *var, struct strbuf *name,
- struct strbuf *item)
-{
-   const char *subsection, *key;
-   int subsection_len, parse;
-   parse = parse_config_key(var, "submodule", ,
-   _len, );
-   if (parse < 0 || !subsection)
-   return 0;
-
-   strbuf_add(name, subsection, subsection_len);
-   strbuf_addstr(item, key);
-
-   return 1;
-}
-
 static struct submodule *lookup_or_create_by_name(struct submodule_cache 
*cache,
-   const unsigned char *gitmodules_sha1, const char *name)
+ const unsigned char 
*gitmodules_sha1,
+ const char *name_ptr, int 
name_len)
 {
struct submodule *submodule;
struct strbuf name_buf = STRBUF_INIT;
+   char *name = xmemdupz(name_ptr, name_len);
 
submodule = cache_lookup_name(cache, gitmodules_sha1, name);
if (submodule)
-   return submodule;
+   goto out;
 
submodule = xmalloc(sizeof(*submodule));
 
@@ -201,7 +187,8 @@ static struct submodule *lookup_or_create_by_name(struct 
submodule_cache *cache,
hashcpy(submodule->gitmodules_sha1, gitmodules_sha1);
 
cache_add(cache, submodule);
-
+out:
+   free(name);
return submodule;
 }
 
@@ -251,18 +238,18 @@ static int parse_config(const char *var, const char 
*value, void *data)
 {
struct parse_config_parameter *me = data;
struct submodule *submodule;
-   struct strbuf name = STRBUF_INIT, item = STRBUF_INIT;
-   int ret = 0;
+   int subsection_len, ret = 0;
+   const char *subsection, *key;
 
-   /* this also ensures that we only parse submodule entries */
-   if (!name_and_item_from_var(var, , ))
+   if (parse_config_key(var, "submodule", ,
+_len, ) < 0 || !subsection_len)
return 0;
 
submodule = lookup_or_create_by_name(me->cache,
 me->gitmodules_sha1,
-name.buf);
+subsection, subsection_len);
 
-   if (!strcmp(item.buf, "path")) {
+   if (!strcmp(key, "path")) {
if (!value)
ret = config_error_nonbool(var);
else if (!me->overwrite && submodule->path)
@@ -275,7 +262,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
submodule->path = xstrdup(value);
cache_put_path(me->cache, submodule);
}
-   } else if (!strcmp(item.buf, "fetchrecursesubmodules")) {
+   } else if (!strcmp(key, "fetchrecursesubmodules")) {
/* when parsing worktree configurations we can die early */
int die_on_error = is_null_sha1(me->gitmodules_sha1);
if (!me->overwrite &&
@@ -286,7 +273,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
submodule->fetch_recurse = parse_fetch_recurse(
var, value,
die_on_error);
-   } else if (!strcmp(item.buf, "ignore")) {
+   } else if (!strcmp(key, "ignore")) {
if (!value)
ret = config_error_nonbool(var);
else if (!me->overwrite && submodule->ignore)
@@ -302,7 +289,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
free((void *) submodule->ignore);
submodule->ignore = xstrdup(value);
}
-   } else if (!strcmp(item.buf, "url")) {
+   } else if (!strcmp(key, "url")) {
if (!value) {
ret = config_error_nonbool(var);
} else if (!me->overwrite && submodule->url) {
@@ -312,7 +299,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
free((void *) submodule->url);
submodule->url = xstrdup(value);
}
-   } else if (!strcmp(item.buf, "update")) {
+   } else if (!strcmp(key, "update")) {
if (!value)
ret = config_error_nonbool(var);
   

[PATCH 5/8] fetching submodules: respect `submodule.fetchJobs` config option

[Stefan Beller]

This allows to configure fetching and updating in parallel
without having the command line option.

This moved the responsibility to determine how many parallel processes
to start from builtin/fetch to submodule.c as we need a way to communicate
"The user did not specify the number of parallel processes in the command
line options" in the builtin fetch. The submodule code takes care of
the precedence (CLI > config > default)

Signed-off-by: Stefan Beller 
---
 Documentation/config.txt|  7 +++
 builtin/fetch.c |  2 +-
 submodule-config.c  | 15 +++
 submodule-config.h  |  2 ++
 submodule.c |  5 +
 t/t5526-fetch-submodules.sh | 14 ++
 6 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2d06b11..eda3276 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2646,6 +2646,13 @@ submodule..ignore::
"--ignore-submodules" option. The 'git submodule' commands are not
affected by this setting.
 
+submodule.fetchJobs::
+   This is used to determine how many submodules will be
+   fetched/cloned at the same time. Specifying a positive integer
+   allows up to that number of submodules being fetched in parallel.
+   This is used in fetch and clone operations only. A value of 0 will
+   give some reasonable configuration. It defaults to 1.
+
 tag.sort::
This variable controls the sort ordering of tags when displayed by
linkgit:git-tag[1]. Without the "--sort=" option provided, the
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 586840d..5aa1c2d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -37,7 +37,7 @@ static int prune = -1; /* unspecified */
 static int all, append, dry_run, force, keep, multiple, update_head_ok, 
verbosity;
 static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static int tags = TAGS_DEFAULT, unshallow, update_shallow;
-static int max_children = 1;
+static int max_children = -1;
 static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
diff --git a/submodule-config.c b/submodule-config.c
index 29e21b2..a32259e 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -32,6 +32,7 @@ enum lookup_type {
 
 static struct submodule_cache cache;
 static int is_cache_init;
+static int parallel_jobs = -1;
 
 static int config_path_cmp(const struct submodule_entry *a,
   const struct submodule_entry *b,
@@ -239,6 +240,15 @@ static int parse_generic_submodule_config(const char *key,
  const char *value,
  struct parse_config_parameter *me)
 {
+   if (!strcmp(key, "fetchjobs")) {
+   parallel_jobs = strtol(value, NULL, 10);
+   if (parallel_jobs < 0) {
+   warning("submodule.fetchJobs not allowed to be 
negative.");
+   parallel_jobs = 1;
+   return 1;
+   }
+   }
+
return 0;
 }
 
@@ -482,3 +492,8 @@ void submodule_free(void)
cache_free();
is_cache_init = 0;
 }
+
+int config_parallel_submodules(void)
+{
+   return parallel_jobs;
+}
diff --git a/submodule-config.h b/submodule-config.h
index f9e2a29..d9bbf9a 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -27,4 +27,6 @@ const struct submodule *submodule_from_path(const unsigned 
char *commit_sha1,
const char *path);
 void submodule_free(void);
 
+int config_parallel_submodules(void);
+
 #endif /* SUBMODULE_CONFIG_H */
diff --git a/submodule.c b/submodule.c
index 0b48734..d97c8f5 100644
--- a/submodule.c
+++ b/submodule.c
@@ -751,6 +751,11 @@ int fetch_populated_submodules(const struct argv_array 
*options,
argv_array_push(, "--recurse-submodules-default");
/* default value, "--submodule-prefix" and its value are added later */
 
+   if (max_parallel_jobs < 0)
+   max_parallel_jobs = config_parallel_submodules();
+   if (max_parallel_jobs < 0)
+   max_parallel_jobs = 1;
+
calculate_changed_submodule_paths();
run_processes_parallel(max_parallel_jobs,
   get_next_submodule,
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 1241146..954d0e4 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -471,4 +471,18 @@ test_expect_success "don't fetch submodule when newly 
recorded commits are alrea
test_i18ncmp expect.err actual.err
 '
 
+test_expect_success 'fetching submodules respects parallel settings' '
+   git config fetch.recurseSubmodules true &&
+   (
+   cd downstream &&
+   GIT_TRACE=$(pwd)/trace.out git fetch --jobs 7 &&
+   grep "7 tasks" trace.out &&
+   git config 

[PATCH 2/8] submodule-config: drop check against NULL

[Stefan Beller]

Adhere to the common coding style of Git and not check explicitly
for NULL throughout the file. There are still other occurrences in the
code base but that is usually inside of conditions with side effects.

Signed-off-by: Stefan Beller 
---
 submodule-config.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/submodule-config.c b/submodule-config.c
index 4239b0e..6d01941 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -265,7 +265,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
if (!strcmp(item.buf, "path")) {
if (!value)
ret = config_error_nonbool(var);
-   else if (!me->overwrite && submodule->path != NULL)
+   else if (!me->overwrite && submodule->path)
warn_multiple_config(me->commit_sha1, submodule->name,
"path");
else {
@@ -289,7 +289,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
} else if (!strcmp(item.buf, "ignore")) {
if (!value)
ret = config_error_nonbool(var);
-   else if (!me->overwrite && submodule->ignore != NULL)
+   else if (!me->overwrite && submodule->ignore)
warn_multiple_config(me->commit_sha1, submodule->name,
"ignore");
else if (strcmp(value, "untracked") &&
@@ -305,7 +305,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
} else if (!strcmp(item.buf, "url")) {
if (!value) {
ret = config_error_nonbool(var);
-   } else if (!me->overwrite && submodule->url != NULL) {
+   } else if (!me->overwrite && submodule->url) {
warn_multiple_config(me->commit_sha1, submodule->name,
"url");
} else {
@@ -315,7 +315,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
} else if (!strcmp(item.buf, "update")) {
if (!value)
ret = config_error_nonbool(var);
-   else if (!me->overwrite && submodule->update != NULL)
+   else if (!me->overwrite && submodule->update)
warn_multiple_config(me->commit_sha1, submodule->name,
 "update");
else {
-- 
2.6.4.443.ge094245.dirty

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

[PATCH 8/8] clone: allow an explicit argument for parallel submodule clones

[Stefan Beller]

Just pass it along to "git submodule update", which may pick reasonable
defaults if you don't specify an explicit number.

Signed-off-by: Stefan Beller 
---
 Documentation/git-clone.txt |  6 +-
 builtin/clone.c | 19 +--
 t/t7406-submodule-update.sh | 15 +++
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 6bf000d..6db7b6d 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -14,7 +14,7 @@ SYNOPSIS
  [-o ] [-b ] [-u ] [--reference ]
  [--dissociate] [--separate-git-dir ]
  [--depth ] [--[no-]single-branch]
- [--recursive | --recurse-submodules] [--] 
+ [--recursive | --recurse-submodules] [--jobs ] [--] 
  []
 
 DESCRIPTION
@@ -221,6 +221,10 @@ objects from the source repository into a pack in the 
cloned repository.
The result is Git repository can be separated from working
tree.
 
+-j ::
+--jobs ::
+   The number of submodules fetched at the same time.
+   Defaults to the `submodule.fetchJobs` option.
 
 ::
The (possibly remote) repository to clone from.  See the
diff --git a/builtin/clone.c b/builtin/clone.c
index a0b3cd9..b004fb4 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -50,6 +50,7 @@ static int option_progress = -1;
 static struct string_list option_config;
 static struct string_list option_reference;
 static int option_dissociate;
+static int max_jobs = -1;
 
 static struct option builtin_clone_options[] = {
OPT__VERBOSITY(_verbosity),
@@ -72,6 +73,8 @@ static struct option builtin_clone_options[] = {
N_("initialize submodules in the clone")),
OPT_BOOL(0, "recurse-submodules", _recursive,
N_("initialize submodules in the clone")),
+   OPT_INTEGER('j', "jobs", _jobs,
+   N_("number of submodules cloned in parallel")),
OPT_STRING(0, "template", _template, N_("template-directory"),
   N_("directory from which templates will be used")),
OPT_STRING_LIST(0, "reference", _reference, N_("repo"),
@@ -95,10 +98,6 @@ static struct option builtin_clone_options[] = {
OPT_END()
 };
 
-static const char *argv_submodule[] = {
-   "submodule", "update", "--init", "--recursive", NULL
-};
-
 static const char *get_repo_path_1(struct strbuf *path, int *is_bundle)
 {
static char *suffix[] = { "/.git", "", ".git/.git", ".git" };
@@ -724,8 +723,16 @@ static int checkout(void)
err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
   sha1_to_hex(sha1), "1", NULL);
 
-   if (!err && option_recursive)
-   err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
+   if (!err && option_recursive) {
+   struct argv_array args = ARGV_ARRAY_INIT;
+   argv_array_pushl(, "submodule", "update", "--init", 
"--recursive", NULL);
+
+   if (max_jobs != -1)
+   argv_array_pushf(, "--jobs=%d", max_jobs);
+
+   err = run_command_v_opt(args.argv, RUN_GIT_CMD);
+   argv_array_clear();
+   }
 
return err;
 }
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 7fd5142..090891e 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -786,4 +786,19 @@ test_expect_success 'submodule update can be run in 
parallel' '
 grep "9 tasks" trace.out
)
 '
+
+test_expect_success 'git clone passes the parallel jobs config on to 
submodules' '
+   test_when_finished "rm -rf super4" &&
+   GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 7 . 
super4 &&
+   grep "7 tasks" trace.out &&
+   rm -rf super4 &&
+   git config --global submodule.fetchJobs 8 &&
+   GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules . super4 &&
+   grep "8 tasks" trace.out &&
+   rm -rf super4 &&
+   GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 9 . 
super4 &&
+   grep "9 tasks" trace.out &&
+   rm -rf super4
+'
+
 test_done
-- 
2.6.4.443.ge094245.dirty

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