Problem with environment of hook execution when git is run with --work-tree / --git-dir

[Michael Sloan]

Hi!

I noticed a potential bug with the invocation of a pre-commit hook
when running git with --work-tree and --git-dir.  In particular, I was
investigating how hooks can still run git commands properly even when
the work-tree or git-dir is overridden via CLI args.  I put the
following in "/home/mgsloan/.dotfiles-git/hooks/pre-commit":

#!/bin/sh
env

after this, running "git --work-tree=/home/mgsloan
--git-dir=/home/mgsloan/.dotfiles-git commit" has output with a bunch
of variables, here are the important ones:

GIT_WORK_TREE=.
GIT_DIR=.dotfiles-git/
PWD=/home/mgsloan

So what's the problem with this choice of environment variables?
Well, the problem is that if PWD is changed during the execution of
the script, then GIT_WORK_TREE and GIT_DIR will no longer work
properly. For example, if the pre-commit hook is

#!/bin/sh
cd some-dir
git status

This will fail with

Not a git repository: '.dotfiles-git'

There is another detail here, which is that when --git-dir /
--work-tree is not specified, the no GIT_WORK_TREE / GIT_DIR
environment variable is set.  This means that in this case, changing
PWD in the hook will work fine as long as the search for .git will
find the right one.  Note that this also means that changing PWD in a
script can change which git repo the command is being run on, for
example, when the hook is interacting with a submodule.

A half-fix to this would be to have the GIT_WORK_TREE and GIT_DIR set
when running hooks use absolute paths.  However, this would not have
the same behavior as when git is used without --git-dir / --work-tree.
As described in the paragraph above, if PWD is relied upon to instead
target a different git repo, then things break.

Not sure what the total fix for this would be.   I think the
information that needs to be conveyed to the hook's git invocations is
that "the work-tree /home/mgsloan should be associated with the
git-dir /home/mgsloan/.dotfiles-git".  Could have an env var like

GIT_DIR_MAPPINGS="/home/mgsloan!/home/mgsloan/.dotfiles-git"

The idea is that this would be a list of mappings from GIT_WORK_TREE
to GIT_DIR.  If this variable is set, then it will be followed when
git is searching parents of PWD for ".git" directories.  I chose "!"
rather arbitrarily here.  "->" would look nicer, but people might
forget to escape it when programmatically setting this var.

What do y'all think of this idea?

Some of you might be wondering what I'm doing with my work tree being
my home directory.  It is the approach suggested here -
https://developer.atlassian.com/blog/2016/02/best-way-to-store-dotfiles-git-bare-repo/
- for versioning your configuration files directly.

Apologies if this has already been discussed, I could not find a good
way to search the mailinglist archives.

Thanks!
-Michael

Re: submodules and merging (Was: Re: [PATCH 02/30] merge-recursive: Fix logic ordering issue)

[Jacob Keller]

On Sat, Nov 25, 2017 at 2:37 PM, Elijah Newren  wrote:
> On Wed, Nov 15, 2017 at 9:13 AM, Jacob Keller  wrote:
>> On Tue, Nov 14, 2017 at 10:13 AM, Stefan Beller  wrote:
>
>>> But this line of though might be distracting from your original point,
>>> which was that we have so much to keep in mind when doing tree
>>> operations (flags, D/F conflicts, now submodules too). I wonder how
>>> a sensible refactoring would look like to detangle all these aspects,
>>> but still keeping Git fast and not overengineered.
>>
>> I think given how complex a lot of these code paths are, that an
>> attempt to refactor it a bit to detangle some of the mess would be
>> well worth the time. I'd suspect it might make handling the more
>> complex task of actually resolving conflicts to be easier, so the
>> effort to clean up the code here should be worth it.
>
> I think changing from a 4-way merge to a 3-way merge would make things
> much better, as Junio outlined here:
>
> https://public-inbox.org/git/xmqqd147kpdm@gitster.mtv.corp.google.com/
>
> I don't know of any way to detangle the other aspects, yet.

I agree, that is absolutely a (big) step in the right direction.

Thanks,
Jake

Re: [PATCH] Makefile: check that tcl/tk is installed

[Junio C Hamano]

Christian Couder  writes:

> On Mon, Nov 20, 2017 at 6:15 PM, Christian Couder
>  wrote:
>> By default running `make install` in the root directory of the
>> project will set TCLTK_PATH to `wish` and then go into the "git-gui"
>> and "gitk-git" sub-directories to build and install these 2
>> sub-projects.
>
> Has this patch fallen through the cracks or is there an unresolved issue?

I had an impression that the conclusion was that the existing error
message at runtime already does an adequate job and there is no
issue to be addressed by this patch.  Am I mistaken?

Re: [PATCH] submodule--helper.c: i18n: add a missing space in message

[Junio C Hamano]

Thanks.

Re: [PATCH] RelNotes: minor typo fixes in 2.15.1 draft

[Junio C Hamano]

Todd Zullinger  writes:

> It was mildly surprising that the script didn't warn or complain about
> an unknown option.  After a quick look, that seems to be due to the
> Getopt::Long pass_through option which sends unknown options to
> format-patch.

Heh, that is another reason why I recommend against letting
send-email run format-patch from within (another a lot bigger one is
that running them seprately gives you a chance to look and proofread
the output from format-patch one more time before actually sending
them out, which also allows you to update additional comments after
the three-dash lines).

Re: [PATCH v5 1/2] http-backend: respect CONTENT_LENGTH as specified by rfc3875

[Junio C Hamano]

Max Kirillov  writes:

> Author: Florian Manschwetus 

This should read "From: ...";

> Date: Wed, 30 Mar 2016 09:08:56 +
>
> http-backend reads whole input until EOF. However, the RFC 3875 specifies
> that a script must read only as many bytes as specified by CONTENT_LENGTH
> environment variable. Web server may exercise the specification by not closing
> the script's standard input after writing content. In that case http-backend
> would hang waiting for the input. The issue is known to happen with
> IIS/Windows, for example.
>
> Make http-backend read only CONTENT_LENGTH bytes, if it's defined, rather than
> the whole input until EOF. If the variable is not defined, keep older behavior
> of reading until EOF because it is used to support chunked transfer-encoding.
>
> Signed-off-by: Florian Manschwetus 
> [mk: fixed trivial build failures and polished style issues]
>
> Signed-off-by: Max Kirillov 

There shouldn't be a blank line before your sign-off.

The above are only for future reference; I can fix them up before
queuing if there isn't any other issues in this patch.


> +ssize_t git_env_ssize_t(const char *k, ssize_t val)
> +{
> + const char *v = getenv(k);
> + if (v && !git_parse_ssize_t(v, ))
> + die("failed to parse %s", k);
> + return val;
> +}
> +

If this were passing "v" that is a string the caller obtains from
any source (and the callee does not care about where it came from),
that would be a different story, but as a public interface that is
part of the config.c layer, "k" that has to be the name of the
environment variable sticks out like a sore thumb.

If we were to add one more public function to the interface, I
suspect that exposing the existing git_parse_ssize_t() and have the
caller implement the above function for its use would be a much
better way to go.

Re: [PATCH v4 4/4] worktree: make add dwim

[Junio C Hamano]

Thomas Gummerer  writes:

> Of course that assumes that it's used directly, not in scripts, and
> that users will actually read the output of the command when they
> invoke it.  Maybe these are not safe assumptions to make though, and
> we'd rather not have this on by default then.

None of these is a safe assumption to make.  With a transition plan,
we could change the default eventually, but if this feature is already
used by real users, the proposed change without any transition plan
will hurt them and make them hate us, I am afraid.

Re: git status always modifies index?

[Junio C Hamano]

Jeff King  writes:

> What I was trying to get at is that naming it "status --no-lock-index"
> would not be the same thing (even though with the current implementation
> it would behave the same). IOW, can we improve the documentation of
> "status" to point to make it easier to discover this use case.

Yeah, the name is unfortunate. 

What the end user really wants to see, I suspect, is a "--read-only"
option that applies to any filesystem entity and to any command, in
the context of this thread, and also in the original discussion that
led to the introduction of that option.  

While I think the variable losing "index" from its name was a vast
improvement relative to "--no-lock-index", simply because it
expresses what we do a bit closer to "do not just do things without
modifying anything my repository", it did not go far enough.

Re: [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value

[Junio C Hamano]

Junio C Hamano  writes:

>  - 5/6: What we see in [v4 1/6] (except for introduction of the
> helper, which already happened in an earlier step).  Move
> the promise of covering this new output format with a follow
> up series to the proposed log message of this step from [v4
> 6/6].
>
>  - 6/6: Realize the promise made in 5/6.
> ...
> For rererence, here is what I just whipped up that would come in
> between the steps 4/6 and 5/6 in the above outline (i.e. split from
> 6/6 to be placed before 5/6 happens).

Just to be sure, I wrote this on top of [v4 1-6/6], so you'd see
that its preimage already knows about GIT_PRINT_SHA1_ELLIPSIS=yes.
In a real reroll of the series that follow the above reorganized
structure, of course that would not appear as "-" line in the patch
;-)

Thanks.

Re: [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value

[Junio C Hamano]

... and this is a sample of the "remainder" of 6/6 outlined in the
previous message, to become part of 5/6 to show how the new output
would look like in a test form.

 t/t4013-diff-various.sh| 1 +
 t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial | 6 ++
 2 files changed, 7 insertions(+)
 create mode 100644 
t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial

diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 7288b54045..8b5474a087 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -178,6 +178,7 @@ diff-tree --root --abbrev initial
 diff-tree --root -r initial
 diff-tree --root -r --abbrev initial
 diff-tree --root -r --abbrev=4 initial
+:noellipses diff-tree --root -r --abbrev=4 initial
 diff-tree -p initial
 diff-tree --root -p initial
 diff-tree --patch-with-stat initial
diff --git a/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial 
b/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial
new file mode 100644
index 00..26fbfeb177
--- /dev/null
+++ b/t/t4013/diff.noellipses-diff-tree_--root_-r_--abbrev=4_initial
@@ -0,0 +1,6 @@
+$ git diff-tree --root -r --abbrev=4 initial
+444ac553ac7612cc88969031b02b3767fb8a353a
+:00 100644  35d2 A dir/sub
+:00 100644  01e7 A file0
+:00 100644  01e7 A file2
+$
-- 
2.15.0-479-ge11ee266c9

Re: [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value

[Junio C Hamano]

Junio C Hamano  writes:

> Thanks for sticking with this topic---very much appreciated, as we
> saw many newcomers get tired of doing repeated polishing and abandon
> their topic in the past.  I have to go offline now, but will comment
> later when I come back.

Regarding the overall structure of the series, I think 1/6 will
break tests and the breakage will stay until it is fixed at the
last step.  Also, now print_sha1_ellipsis() is prominently not about
"diff", it may want to move out of "diff.[ch]".

So, perhaps this may be a better structure:

 - 1/6: What we see in [v4 4/6] as an independent typofix.

 - 2/6: What we see in [v4 3/6] that is not about the output from
"git checkout" (e.g. the use of ellipsis to shorten the
example input the user gives, as if the user is using full
40-hex and we are not bothering to show everything in the
documentation), with "there is no need to use full 40-hex to
identify the object names like the examples hint at by
omitting the tail part of an object name as if that has to
be spelled out but the example omits them only for
brevity. Give examples using an abbreviated object names
without ellipses just like how people do in real life", or
something like that, as an explanation.

 - 3/6: Introduce a helper print_sha1_ellipsis() that pays attention
to GIT_PRINT_SHA1_ELLIPSIS environment, and prepares the
tests to unconditionally set it for the test pieces that
will be broken once the code stops showing the extra dots by
default.  What we see in [v4 5/6] may also want to be rolled
into this patch.

Nobody calls print_sha1_ellipsis() function yet at this
step.  cache.h and environment.c may be a better place for
the declaration and definition of the helper.

Mention that the removal of these dots is merely a plan at
this step and has not happened yet but soon will be in the
proposed log message.

 - 4/6: What we see in [v4 2/6], optionally with two additional
tests that looks at the output with and without the
environment variable (it is optional because the output is
purely for human consumption sent to the standard error
stream).  Parts of [v4 3/6] about the output from the "git
checkout" command also belongs to this step.

 - 5/6: What we see in [v4 1/6] (except for introduction of the
helper, which already happened in an earlier step).  Move
the promise of covering this new output format with a follow
up series to the proposed log message of this step from [v4
6/6].

 - 6/6: Realize the promise made in 5/6.

Alternatively, just like the step 3/6 above prepares the
tests for "upcoming" feature without exercising that
preparation, a part of this can be split to allow annotating
the "here-document" test vectors with "does this test expect
that ellipsis will be missing in the output?" (i.e. change
the logic in the loop that parses "<8 --
Subject: [PATCH] t4013: prepare for upcoming "diff --raw --abbrev" format change

Most of the t4013 tests go through a list of sample command lines,
and each of them is executed and its output compared with an
expected one stored in t4013/ directory.  Allow these lines to begin
with a colon followed by magic word(s) so that test conditions can
easily be tweaked.

The expected use that will happen in later steps of this is to run
tests expecting the traditional output and run the same test without
the GIT_PRINT_SHA1_ELLIPSIS=yes environment exported for (perhaps
some of) them, which will have to expect different output.  Since
all of the existing tests are meant to run with the environment,
use the magic word "noellipses" to cause the variable not to be set
and exported.

As this step does not add any new test with the magic word, all
tests still run with the environment variable, expecting the
traditional output, but it will change soon.

Signed-off-by: Junio C Hamano 
---
 t/t4013-diff-various.sh | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 9bed64d53e..7288b54045 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -118,20 +118,37 @@ test_expect_success setup '
 EOF
 
 V=$(git version | 

[PATCH] pretty: fix buffer over-read with %> and %

[mwnx]

A buffer over-read of the format string would occur with unterminated
formats of the form '%>(#' and '%<(#', where '#' represents a number.

This error can be witnessed by running git log under valgrind like so:

valgrind git log -n1 --format='%<(42'

This was due to the fact that the "not found" case for strcspn() was
being checked in the same way that one checks the "not found" case for
strchr(), i.e. by checking for a NULL pointer return value. Instead, one
must check for the end of the string since strcspn() points to the
character where it finished its search (which will be a '\0' if
unsuccessful).

Signed-off-by: mwnx 
---
 pretty.c  | 2 +-
 t/t4205-log-pretty-formats.sh | 6 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/pretty.c b/pretty.c
index 2f6b0ae6c..4c70bad45 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1021,7 +1021,7 @@ static size_t parse_padding_placeholder(struct strbuf *sb,
const char *end = start + strcspn(start, ",)");
char *next;
int width;
-   if (!end || end == start)
+   if (!*end || end == start)
return 0;
width = strtol(start, , 10);
if (next == start || width == 0)
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 591f35daa..4d9555962 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -598,4 +598,10 @@ test_expect_success ':only and :unfold work together' '
test_cmp expect actual
 '
 
+test_expect_success 'unterminated alignment formatting' '
+   git log -n1 --format="%<(42" >actual &&
+   echo "%<(42" >expected &&
+   test_cmp expected actual
+'
+
 test_done
-- 
2.15.0.90.g6559daec7

Re: [PATCH] merge-recursive: ignore_case shouldn't reject intentional removals

[Junio C Hamano]

Elijah Newren  writes:

> I had another email I had been composing to try to argue for changing
> merge-recursive.c's design to the above, assuming I could get the time
> to work on it.  Nice to see that I'm not crazy, and that I apparently
> don't need to do much convincing.  :-)

You might even be better off coming up with a *new* merge strategy
backend if you want to do this, without using much from the existing
code in merge-recursive.c at all (I've written off that code as
mostly unsalvageable long time ago, and thanked for whoever had a
clever idea to allow different strategy backend to be made without
disrupting the rest of the system).

After we gain more confidence with the rewrite, we can switch the
internally built-in backend used by different codepaths from
merge-recursive.c::merge_recursive() to the new thing.

Bug in pathspec handling (in conjunction with submodules)

[Johannes Schindelin]

Hi Duy & Brandon,

in 74ed43711fd (grep: enable recurse-submodules to work on  objects,
2016-12-16), the do_match() function in tree-walk.c was changed so that it
can recurse across submodule boundaries.

However, there is a bug, and I *think* there may be two bugs actually. Or
even three.

First of all, here is an MCVE that I distilled from
https://github.com/git-for-windows/git/issues/1371:

git init repo
cd repo

git init submodule
git -C submodule commit -m initial --allow-empty

touch "[bracket]"
git add "[bracket]"
git commit -m bracket
git add submodule
git commit -m submodule

git rev-list HEAD -- "[bracket]"

Nothing fancy, just adding a file with brackets in the name, then a
submodule, then showing the commit history filtered by the funny file
name.

However, the log prints *both* commits. Clearly the submodule commit
should *not* be shown.

Now, how does this all happen?

Since the pathspec contains brackets, parse_pathspec() marks it as
containing wildcards and sets nowildcard_len to 0.

Now, note that [bracket] *is* a wildcard expression: it should only match
a single character that is one of  a, b, c, e, k, r or t.

I think this is the first bug: `git rev-list` should not even match the
commit that adds the file [bracket] because its file name does not match
that expression. From where I sit, it would appear that f1a2ddbbc2d
(tree_entry_interesting(): optimize wildcard matching when base is
matched, 2010-12-15) simply added the fnmatch() code without disabling the
literal match_entry() code when the pathspec contains a pattern.

But it does not stop there: there is *another* bug which causes the
pattern to somehow match the submodule. I *guess* the idea of
https://github.com/git/git/commit/74ed43711#diff-7a08243175f2cae66aedf53f7dce3bdfR1015
was to allow a pattern like *.c to match files in a submodule, but the
pattern [bracket] should not match any file in submodule/. I think that
that code needs to be a little bit more careful to try to match the
submodule's name against the pattern (it seems to interpret nowildcard_len
== 0 to mean that the wildcard is `*`).

However, the commit introducing that code wanted to teach *grep* (not
*rev-list*) a new trick, and it relies on the `recursive` flag of the
pathspec to be set.

And now it gets really interesting. Or confusing, depending on your mental
condition. This recursive flag of the pathspec is set in
ll_diff_tree_paths() (yep, changing the flag in the passed-in opt
structure... which I found a bit... unexpected, given the function name, I
would have been less surprised if that function only diff'ed the trees and
used the options without changing the options). That flag-change was
introduced in
https://github.com/git/git/commit/bc96cc87dbb2#diff-15203e8cd8ee9191113894de9d97a8a6R149
which is another patch that changed the tree diff machinery to accommodate
`git grep` (but maybe not really paying a lot of attention to the fact
that the same machinery is called repeatedly by the revision machinery,
too).

I am really confused by this code mainly due to the fact that the term
"recursive" is pretty ambiguous in that context: does it refer to
directories/tree objects, or to submodules? I guess it is used for both
when there should be two flags so that rev-list can recurse over tree
objects but not submodules (unless told to do so).

The problem, of course, is that `git rev-list HEAD -- '[bracket]'` never
recurses into the submodule. And therefore, the promised "more accurate
matching [...] in the submodule" is never performed. And the commit adding
the submodule is never pruned.

Since I am not really familiar with all that tree diff code (and as a
general rule to protect my mental health, I try my best to stay away from
submodules, too), but you two are, may I ask you gentle people to have a
closer look to fix those bugs?

Thanks,
Dscho

Re: [PATCH] http-backend: respect CONTENT_LENGTH as specified by rfc3875

[Max Kirillov]

On Fri, Nov 24, 2017 at 02:54:50PM +0900, Junio C Hamano wrote:
> Max Kirillov  writes:
>> I hope I marked it correctly in the trailers.
> 
> It is probably more conventional to do it like so:
> 
> From: Florian Manschwetus 
> Date: 
> 
> Signed-off-by: Florian Manschwetus 
> [mk: fixed trivial build failures and stuff]
> Signed-off-by: Max Kirillov 

Thanks. Have done so

[PATCH v4 1/2] http-backend: respect CONTENT_LENGTH as specified by rfc3875

[Max Kirillov]

http-backend reads whole input until EOF. However, the RFC 3875 specifies
that a script must read only as many bytes as specified by CONTENT_LENGTH
environment variable. Web server may exercise the specification by not closing
the script's standard input after writing content. In that case http-backend
would hang waiting for the input. The issue is known to happen with
IIS/Windows, for example.

Make http-backend read only CONTENT_LENGTH bytes, if it's defined, rather than
the whole input until EOF. If the variable is not defined, keep older behavior
of reading until EOF because it is used to support chunked transfer-encoding.

Signed-off-by: Florian Manschwetus 
[mk: fixed trivial build failures and polished style issues]

Signed-off-by: Max Kirillov 
---
 config.c   |  8 
 config.h   |  1 +
 http-backend.c | 39 ++-
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/config.c b/config.c
index 231f9a750b..925bb65dfa 100644
--- a/config.c
+++ b/config.c
@@ -1525,6 +1525,14 @@ unsigned long git_env_ulong(const char *k, unsigned long 
val)
return val;
 }
 
+ssize_t git_env_ssize_t(const char *k, ssize_t val)
+{
+   const char *v = getenv(k);
+   if (v && !git_parse_ssize_t(v, ))
+   die("failed to parse %s", k);
+   return val;
+}
+
 int git_config_system(void)
 {
return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0);
diff --git a/config.h b/config.h
index 0352da117b..947695c304 100644
--- a/config.h
+++ b/config.h
@@ -74,6 +74,7 @@ extern int git_config_rename_section_in_file(const char *, 
const char *, const c
 extern const char *git_etc_gitconfig(void);
 extern int git_env_bool(const char *, int);
 extern unsigned long git_env_ulong(const char *, unsigned long);
+extern ssize_t git_env_ssize_t(const char *, ssize_t);
 extern int git_config_system(void);
 extern int config_error_nonbool(const char *);
 #if defined(__GNUC__)
diff --git a/http-backend.c b/http-backend.c
index 519025d2c3..b4ba83b624 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -280,7 +280,7 @@ static struct rpc_service *select_service(struct strbuf 
*hdr, const char *name)
  * hit max_request_buffer we die (we'd rather reject a
  * maliciously large request than chew up infinite memory).
  */
-static ssize_t read_request(int fd, unsigned char **out)
+static ssize_t read_request_eof(int fd, unsigned char **out)
 {
size_t len = 0, alloc = 8192;
unsigned char *buf = xmalloc(alloc);
@@ -317,6 +317,43 @@ static ssize_t read_request(int fd, unsigned char **out)
}
 }
 
+static ssize_t read_request_fixed_len(int fd, ssize_t req_len, unsigned char 
**out)
+{
+   unsigned char *buf = NULL;
+   ssize_t cnt = 0;
+
+   if (max_request_buffer < req_len) {
+   die("request was larger than our maximum size (%lu): %lu;"
+   " try setting GIT_HTTP_MAX_REQUEST_BUFFER",
+   max_request_buffer,
+   req_len);
+   }
+
+   if (req_len <= 0) {
+   *out = NULL;
+   return 0;
+   }
+
+   buf = xmalloc(req_len);
+   cnt = read_in_full(fd, buf, req_len);
+   if (cnt < 0) {
+   free(buf);
+   return -1;
+   } else {
+   *out = buf;
+   return cnt;
+   }
+}
+
+static ssize_t read_request(int fd, unsigned char **out)
+{
+   ssize_t req_len = git_env_ssize_t("CONTENT_LENGTH", -1);
+   if (req_len < 0)
+   return read_request_eof(fd, out);
+   else
+   return read_request_fixed_len(fd, req_len, out);
+}
+
 static void inflate_request(const char *prog_name, int out, int buffer_input)
 {
git_zstream stream;
-- 
2.11.0.1122.gc3fec58.dirty

[PATCH v5 2/2] t5560-http-backend-noserver.sh: add CONTENT_LENGTH cases

[Max Kirillov]

Add tests for cases:

* CONTENT_LENGTH is set, script's stdin has more data.
  (Failure would make it read GIT_HTTP_MAX_REQUEST_BUFFER bytes from /dev/zero
  and fail. It does not seem to cause any performance issues with the default
  value of GIT_HTTP_MAX_REQUEST_BUFFER.)
* CONTENT_LENGTH is specified to a value which does not fix into ssize_t.

Signed-off-by: Max Kirillov 
---
 Makefile |  1 +
 t/helper/test-print-values.c | 10 ++
 t/t5560-http-backend-noserver.sh | 30 ++
 3 files changed, 41 insertions(+)
 create mode 100644 t/helper/test-print-values.c

diff --git a/Makefile b/Makefile
index 461c845d33..616408b32c 100644
--- a/Makefile
+++ b/Makefile
@@ -653,6 +653,7 @@ TEST_PROGRAMS_NEED_X += test-mktemp
 TEST_PROGRAMS_NEED_X += test-online-cpus
 TEST_PROGRAMS_NEED_X += test-parse-options
 TEST_PROGRAMS_NEED_X += test-path-utils
+TEST_PROGRAMS_NEED_X += test-print-values
 TEST_PROGRAMS_NEED_X += test-prio-queue
 TEST_PROGRAMS_NEED_X += test-read-cache
 TEST_PROGRAMS_NEED_X += test-ref-store
diff --git a/t/helper/test-print-values.c b/t/helper/test-print-values.c
new file mode 100644
index 00..8f7e5af319
--- /dev/null
+++ b/t/helper/test-print-values.c
@@ -0,0 +1,10 @@
+#include 
+#include 
+
+int cmd_main(int argc, const char **argv)
+{
+   if (argc == 2 && strcmp(argv[1], "(size_t)(-20)") == 0)
+   printf("%zu", (ssize_t)(-20));
+
+   return 0;
+}
diff --git a/t/t5560-http-backend-noserver.sh b/t/t5560-http-backend-noserver.sh
index 9fafcf1945..f452090216 100755
--- a/t/t5560-http-backend-noserver.sh
+++ b/t/t5560-http-backend-noserver.sh
@@ -71,4 +71,34 @@ test_expect_success 'http-backend blocks bad PATH_INFO' '
expect_aliased 1 //domain/data.txt
 '
 
+# overrides existing definition for further cases
+run_backend() {
+   CONTENT_LENGTH="${#2}" && export CONTENT_LENGTH &&
+   ( echo "$2" && cat /dev/zero ) |
+   QUERY_STRING="${1#*[?]}" \
+   PATH_TRANSLATED="$HTTPD_DOCUMENT_ROOT_PATH/${1%%[?]*}" \
+   git http-backend >act.out 2>act.err
+}
+
+test_expect_success 'CONTENT_LENGTH set and infinite input' '
+   config http.uploadpack true &&
+   GET info/refs?service=git-upload-pack "200 OK"  &&
+   ! grep "fatal:.*" act.err &&
+   POST git-upload-pack  "200 OK" &&
+   ! grep "fatal:.*" act.err
+'
+
+test_expect_success 'CONTENT_LENGTH overflow ssite_t' '
+   NOT_FIT_IN_SSIZE=`"$GIT_BUILD_DIR/t/helper/test-print-values" 
"(size_t)(-20)"` &&
+   env \
+   CONTENT_TYPE=application/x-git-upload-pack-request \
+   QUERY_STRING=/repo.git/git-upload-pack \
+   PATH_TRANSLATED="$PWD"/.git/git-upload-pack \
+   GIT_HTTP_EXPORT_ALL=TRUE \
+   REQUEST_METHOD=POST \
+   CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \
+   git http-backend /dev/null 2>err &&
+   grep -q "fatal:.*CONTENT_LENGTH" err
+'
+
 test_done
-- 
2.11.0.1122.gc3fec58.dirty

[PATCH v4 0/2] http-backend: respect CONTENT_LENGTH as specified by rfc3875

[Max Kirillov]

* polished style
* add tests
* marked the authorship more correctly

Max Kirillov (2):
  http-backend: respect CONTENT_LENGTH as specified by rfc3875
  t5560-http-backend-noserver.sh: add CONTENT_LENGTH cases

 Makefile |  1 +
 config.c |  8 
 config.h |  1 +
 http-backend.c   | 39 ++-
 t/helper/test-print-values.c | 10 ++
 t/t5560-http-backend-noserver.sh | 30 ++
 6 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 t/helper/test-print-values.c

-- 
2.11.0.1122.gc3fec58.dirty

[PATCH v5 1/2] http-backend: respect CONTENT_LENGTH as specified by rfc3875

[Max Kirillov]

Author: Florian Manschwetus 
Date: Wed, 30 Mar 2016 09:08:56 +

http-backend reads whole input until EOF. However, the RFC 3875 specifies
that a script must read only as many bytes as specified by CONTENT_LENGTH
environment variable. Web server may exercise the specification by not closing
the script's standard input after writing content. In that case http-backend
would hang waiting for the input. The issue is known to happen with
IIS/Windows, for example.

Make http-backend read only CONTENT_LENGTH bytes, if it's defined, rather than
the whole input until EOF. If the variable is not defined, keep older behavior
of reading until EOF because it is used to support chunked transfer-encoding.

Signed-off-by: Florian Manschwetus 
[mk: fixed trivial build failures and polished style issues]

Signed-off-by: Max Kirillov 
---
 config.c   |  8 
 config.h   |  1 +
 http-backend.c | 39 ++-
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/config.c b/config.c
index 231f9a750b..925bb65dfa 100644
--- a/config.c
+++ b/config.c
@@ -1525,6 +1525,14 @@ unsigned long git_env_ulong(const char *k, unsigned long 
val)
return val;
 }
 
+ssize_t git_env_ssize_t(const char *k, ssize_t val)
+{
+   const char *v = getenv(k);
+   if (v && !git_parse_ssize_t(v, ))
+   die("failed to parse %s", k);
+   return val;
+}
+
 int git_config_system(void)
 {
return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0);
diff --git a/config.h b/config.h
index 0352da117b..947695c304 100644
--- a/config.h
+++ b/config.h
@@ -74,6 +74,7 @@ extern int git_config_rename_section_in_file(const char *, 
const char *, const c
 extern const char *git_etc_gitconfig(void);
 extern int git_env_bool(const char *, int);
 extern unsigned long git_env_ulong(const char *, unsigned long);
+extern ssize_t git_env_ssize_t(const char *, ssize_t);
 extern int git_config_system(void);
 extern int config_error_nonbool(const char *);
 #if defined(__GNUC__)
diff --git a/http-backend.c b/http-backend.c
index 519025d2c3..b4ba83b624 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -280,7 +280,7 @@ static struct rpc_service *select_service(struct strbuf 
*hdr, const char *name)
  * hit max_request_buffer we die (we'd rather reject a
  * maliciously large request than chew up infinite memory).
  */
-static ssize_t read_request(int fd, unsigned char **out)
+static ssize_t read_request_eof(int fd, unsigned char **out)
 {
size_t len = 0, alloc = 8192;
unsigned char *buf = xmalloc(alloc);
@@ -317,6 +317,43 @@ static ssize_t read_request(int fd, unsigned char **out)
}
 }
 
+static ssize_t read_request_fixed_len(int fd, ssize_t req_len, unsigned char 
**out)
+{
+   unsigned char *buf = NULL;
+   ssize_t cnt = 0;
+
+   if (max_request_buffer < req_len) {
+   die("request was larger than our maximum size (%lu): %lu;"
+   " try setting GIT_HTTP_MAX_REQUEST_BUFFER",
+   max_request_buffer,
+   req_len);
+   }
+
+   if (req_len <= 0) {
+   *out = NULL;
+   return 0;
+   }
+
+   buf = xmalloc(req_len);
+   cnt = read_in_full(fd, buf, req_len);
+   if (cnt < 0) {
+   free(buf);
+   return -1;
+   } else {
+   *out = buf;
+   return cnt;
+   }
+}
+
+static ssize_t read_request(int fd, unsigned char **out)
+{
+   ssize_t req_len = git_env_ssize_t("CONTENT_LENGTH", -1);
+   if (req_len < 0)
+   return read_request_eof(fd, out);
+   else
+   return read_request_fixed_len(fd, req_len, out);
+}
+
 static void inflate_request(const char *prog_name, int out, int buffer_input)
 {
git_zstream stream;
-- 
2.11.0.1122.gc3fec58.dirty

[PATCH v5 0/2] http-backend: respect CONTENT_LENGTH as specified by rfc3875

[Max Kirillov]

I seem to forgot to put the authorship lines, and also did something with
replies, so sending another sequence.

v5:

* marked the authorship more correctly

v4:

* polished style
* add tests

Max Kirillov (2):
  http-backend: respect CONTENT_LENGTH as specified by rfc3875
  t5560-http-backend-noserver.sh: add CONTENT_LENGTH cases

 Makefile |  1 +
 config.c |  8 
 config.h |  1 +
 http-backend.c   | 39 ++-
 t/helper/test-print-values.c | 10 ++
 t/t5560-http-backend-noserver.sh | 30 ++
 6 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 t/helper/test-print-values.c

-- 
2.11.0.1122.gc3fec58.dirty

[PATCH v4 2/2] t5560-http-backend-noserver.sh: add CONTENT_LENGTH cases

[Max Kirillov]

Add tests for cases:

* CONTENT_LENGTH is set, script's stdin has more data.
  (Failure would make it read GIT_HTTP_MAX_REQUEST_BUFFER bytes from /dev/zero
  and fail. It does not seem to cause any performance issues with the default
  value of GIT_HTTP_MAX_REQUEST_BUFFER.)
* CONTENT_LENGTH is specified to a value which does not fix into ssize_t.

Signed-off-by: Max Kirillov 
---
 Makefile |  1 +
 t/helper/test-print-values.c | 10 ++
 t/t5560-http-backend-noserver.sh | 30 ++
 3 files changed, 41 insertions(+)
 create mode 100644 t/helper/test-print-values.c

diff --git a/Makefile b/Makefile
index 461c845d33..616408b32c 100644
--- a/Makefile
+++ b/Makefile
@@ -653,6 +653,7 @@ TEST_PROGRAMS_NEED_X += test-mktemp
 TEST_PROGRAMS_NEED_X += test-online-cpus
 TEST_PROGRAMS_NEED_X += test-parse-options
 TEST_PROGRAMS_NEED_X += test-path-utils
+TEST_PROGRAMS_NEED_X += test-print-values
 TEST_PROGRAMS_NEED_X += test-prio-queue
 TEST_PROGRAMS_NEED_X += test-read-cache
 TEST_PROGRAMS_NEED_X += test-ref-store
diff --git a/t/helper/test-print-values.c b/t/helper/test-print-values.c
new file mode 100644
index 00..8f7e5af319
--- /dev/null
+++ b/t/helper/test-print-values.c
@@ -0,0 +1,10 @@
+#include 
+#include 
+
+int cmd_main(int argc, const char **argv)
+{
+   if (argc == 2 && strcmp(argv[1], "(size_t)(-20)") == 0)
+   printf("%zu", (ssize_t)(-20));
+
+   return 0;
+}
diff --git a/t/t5560-http-backend-noserver.sh b/t/t5560-http-backend-noserver.sh
index 9fafcf1945..f452090216 100755
--- a/t/t5560-http-backend-noserver.sh
+++ b/t/t5560-http-backend-noserver.sh
@@ -71,4 +71,34 @@ test_expect_success 'http-backend blocks bad PATH_INFO' '
expect_aliased 1 //domain/data.txt
 '
 
+# overrides existing definition for further cases
+run_backend() {
+   CONTENT_LENGTH="${#2}" && export CONTENT_LENGTH &&
+   ( echo "$2" && cat /dev/zero ) |
+   QUERY_STRING="${1#*[?]}" \
+   PATH_TRANSLATED="$HTTPD_DOCUMENT_ROOT_PATH/${1%%[?]*}" \
+   git http-backend >act.out 2>act.err
+}
+
+test_expect_success 'CONTENT_LENGTH set and infinite input' '
+   config http.uploadpack true &&
+   GET info/refs?service=git-upload-pack "200 OK"  &&
+   ! grep "fatal:.*" act.err &&
+   POST git-upload-pack  "200 OK" &&
+   ! grep "fatal:.*" act.err
+'
+
+test_expect_success 'CONTENT_LENGTH overflow ssite_t' '
+   NOT_FIT_IN_SSIZE=`"$GIT_BUILD_DIR/t/helper/test-print-values" 
"(size_t)(-20)"` &&
+   env \
+   CONTENT_TYPE=application/x-git-upload-pack-request \
+   QUERY_STRING=/repo.git/git-upload-pack \
+   PATH_TRANSLATED="$PWD"/.git/git-upload-pack \
+   GIT_HTTP_EXPORT_ALL=TRUE \
+   REQUEST_METHOD=POST \
+   CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \
+   git http-backend /dev/null 2>err &&
+   grep -q "fatal:.*CONTENT_LENGTH" err
+'
+
 test_done
-- 
2.11.0.1122.gc3fec58.dirty

Re: [PATCH v3 21/33] merge-recursive: add get_directory_renames()

[Elijah Newren]

On Sat, Nov 25, 2017 at 4:52 PM, Johannes Schindelin
 wrote:
> On Tue, 21 Nov 2017, Elijah Newren wrote:
>
>> diff --git a/merge-recursive.c b/merge-recursive.c

>> + if (old_len != new_len || strncmp(old_path, new_path, old_len)) {
>> + *old_dir = strndup(old_path, old_len);
>> + *new_dir = strndup(new_path, new_len);
>
> These two callers of strndup() are the only ones in Git's code base now.
> It is also causing a compile error on Windows.
>
> Any reason you did not use xstrndup() here?
>
> Ciao,
> Dscho

Nope, was just unaware.  I'll go ahead and switch them over for my
next roll of the series.  Sorry for the pain.

Re: [PATCH v3 21/33] merge-recursive: add get_directory_renames()

[Johannes Schindelin]

Hi Elijah,

On Tue, 21 Nov 2017, Elijah Newren wrote:

> diff --git a/merge-recursive.c b/merge-recursive.c
> index 2f4f85314a..6a0a6d4366 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1384,6 +1384,132 @@ static struct diff_queue_struct *get_diffpairs(struct 
> merge_options *o,
>   return ret;
>  }
>  
> +static void get_renamed_dir_portion(const char *old_path, const char 
> *new_path,
> + char **old_dir, char **new_dir)
> +{
> + char *end_of_old, *end_of_new;
> + int old_len, new_len;
> +
> + *old_dir = NULL;
> + *new_dir = NULL;
> +
> + /* For
> +  *"a/b/c/d/foo.c" -> "a/b/something-else/d/foo.c"
> +  * the "d/foo.c" part is the same, we just want to know that
> +  *"a/b/c" was renamed to "a/b/something-else"
> +  * so, for this example, this function returns "a/b/c" in
> +  * *old_dir and "a/b/something-else" in *new_dir.
> +  *
> +  * Also, if the basename of the file changed, we don't care.  We
> +  * want to know which portion of the directory, if any, changed.
> +  */
> + end_of_old = strrchr(old_path, '/');
> + end_of_new = strrchr(new_path, '/');
> +
> + if (end_of_old == NULL || end_of_new == NULL)
> + return;
> + while (*--end_of_new == *--end_of_old &&
> +end_of_old != old_path &&
> +end_of_new != new_path)
> + ; /* Do nothing; all in the while loop */
> + /*
> +  * We've found the first non-matching character in the directory
> +  * paths.  That means the current directory we were comparing
> +  * represents the rename.  Move end_of_old and end_of_new back
> +  * to the full directory name.
> +  */
> + if (*end_of_old == '/')
> + end_of_old++;
> + if (*end_of_old != '/')
> + end_of_new++;
> + end_of_old = strchr(end_of_old, '/');
> + end_of_new = strchr(end_of_new, '/');
> +
> + /*
> +  * It may have been the case that old_path and new_path were the same
> +  * directory all along.  Don't claim a rename if they're the same.
> +  */
> + old_len = end_of_old - old_path;
> + new_len = end_of_new - new_path;
> +
> + if (old_len != new_len || strncmp(old_path, new_path, old_len)) {
> + *old_dir = strndup(old_path, old_len);
> + *new_dir = strndup(new_path, new_len);

These two callers of strndup() are the only ones in Git's code base now.
It is also causing a compile error on Windows.

Any reason you did not use xstrndup() here?

Ciao,
Dscho

Re: [PATCH] http-backend: respect CONTENT_LENGTH as specified by rfc3875

[Max Kirillov]

On Sat, Nov 25, 2017 at 07:38:33PM -0500, Eric Sunshine wrote:
> On Sat, Nov 25, 2017 at 4:47 PM, Max Kirillov  wrote:
>> On Thu, Nov 23, 2017 at 08:30:39PM -0500, Eric Sunshine wrote:
 +static ssize_t read_request_fix_len(int fd, size_t req_len, unsigned char 
 **out)
>>>
>>> Wrong data type: s/size_t req_len/ssize_t req_len/
>>
>> Passing negative value to the function makes no sense. I
>> could add explicit type cast to make it clear. It should be
>> safe as site_t's range is bigger, and overflown
>> CONTENT_LENGTH results in die() at parsing (I have a test
>> which verifies it)
> 
> A concern with requesting size_t bytes is that, if it does read all
> bytes, that value can't necessarily be represented by the ssize_t
> returned from the function. Where would the cast be placed that you
> suggest? How do other git functions deal with this sort of situation?

Right... ok, let it be ssize_t

Re: [PATCH] http-backend: respect CONTENT_LENGTH as specified by rfc3875

[Eric Sunshine]

On Sat, Nov 25, 2017 at 4:47 PM, Max Kirillov  wrote:
> Thanks for the review. I saw only reaction of the Jeff in
> the original thread and though that it is ok otherwise. I'm
> fixing the things you mentioned.

The commentary (in which you talked about restoring the patch and
squashing) seemed to imply that this had been posted somewhere before,
but it wasn't marked as "v2" (or whatever attempt) and lacked a URL
pointing at the previous attempt, so it was difficult to judge.

> On Thu, Nov 23, 2017 at 08:30:39PM -0500, Eric Sunshine wrote:
>>> +static ssize_t read_request_fix_len(int fd, size_t req_len, unsigned char 
>>> **out)
>>
>> Wrong data type: s/size_t req_len/ssize_t req_len/
>
> Passing negative value to the function makes no sense. I
> could add explicit type cast to make it clear. It should be
> safe as site_t's range is bigger, and overflown
> CONTENT_LENGTH results in die() at parsing (I have a test
> which verifies it)

A concern with requesting size_t bytes is that, if it does read all
bytes, that value can't necessarily be represented by the ssize_t
returned from the function. Where would the cast be placed that you
suggest? How do other git functions deal with this sort of situation?

Re: [PATCH] submodule--helper.c: i18n: add a missing space in message

[Eric Sunshine]

On Sat, Nov 25, 2017 at 2:55 PM, Jean-Noel Avila  wrote:
> The message spans over 2 lines but the C conconcatenation does not add

s/conconcatenation/concatenation/

> the needed space between the two lines.
>
> Signed-off-by: Jean-Noel Avila 
> ---
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 2086f0eb0..a5c4a8a69 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -623,7 +623,7 @@ static void status_submodule(const char *path, const 
> struct object_id *ce_oid,
>
> if (refs_head_ref(get_submodule_ref_store(path),
>   handle_submodule_head_ref, ))
> -   die(_("could not resolve HEAD ref inside the"
> +   die(_("could not resolve HEAD ref inside the "
>   "submodule '%s'"), path);
>
> print_status(flags, '+', path, , displaypath);
> --
> 2.15.0

Re: [PATCH v4 4/4] worktree: make add dwim

[Paul Smith]

On Sat, 2017-11-25 at 20:06 +, Thomas Gummerer wrote:
> This part is getting done in 3/4, and is definitely going to work
> without an additional flag, so this is (hopefully) soon going to work
> just as you want :)

Yay!  Thanks!

Re: Clone repository computer A to remote B doenst work

[Johannes Schindelin]

Hi Roberto,

On Sat, 25 Nov 2017, Roberto Garcia wrote:

> I'm trying clone in windows a git repository to other remote machine
> (NAS Linux based).
> I have installed git for windows but i didn't installed nothing in the
> other remote machine (NAS Linux based).

You need a Git on the remote side. Otherwise Git will not be able to clone
from there.

Ciao,
Johannes

Re: submodules and merging (Was: Re: [PATCH 02/30] merge-recursive: Fix logic ordering issue)

[Elijah Newren]

On Wed, Nov 15, 2017 at 9:13 AM, Jacob Keller  wrote:
> On Tue, Nov 14, 2017 at 10:13 AM, Stefan Beller  wrote:

>> But this line of though might be distracting from your original point,
>> which was that we have so much to keep in mind when doing tree
>> operations (flags, D/F conflicts, now submodules too). I wonder how
>> a sensible refactoring would look like to detangle all these aspects,
>> but still keeping Git fast and not overengineered.
>
> I think given how complex a lot of these code paths are, that an
> attempt to refactor it a bit to detangle some of the mess would be
> well worth the time. I'd suspect it might make handling the more
> complex task of actually resolving conflicts to be easier, so the
> effort to clean up the code here should be worth it.

I think changing from a 4-way merge to a 3-way merge would make things
much better, as Junio outlined here:

https://public-inbox.org/git/xmqqd147kpdm@gitster.mtv.corp.google.com/

I don't know of any way to detangle the other aspects, yet.

Re: [PATCH] merge-recursive: ignore_case shouldn't reject intentional removals

[Elijah Newren]

On Fri, Nov 24, 2017 at 7:29 PM, Junio C Hamano  wrote:
> Elijah Newren  writes:
>
>> But what it really is forced to do is more of a 4-way merge; a good
>> chunk of its annoying complexity is based around this (undocumented
>> and unfortunate) reality.  It derives from what I consider a simple
>> design flaw.
>
> Yes, and it does not help that it wants to write into the filesystem
> while it performs the outermost merges.
>
> In the ideal world, we should
>
>  - ask unpack_trees() to do "read-tree -m" without "-u";
>
>  - do all the merge-recursive computations in-core and prepare the
>resulting index, while keeping the current index intact;
>
>  - compare the current in-core index and the resulting in-core
>index, and notice the paths that need to be added, updated or
>removed in the working tree, and ensure that there is no loss of
>information when the change is reflected to the working tree,
>e.g. the result wants to create a file where the working tree
>currently has a directory with non-expendable contents in it, the
>result wants to remove a file where the working tree file has
>local modification, etc.; and then finally
>
>  - carry out the working tree update to make it match what the
>resulting in-core index says it should look like.

I had another email I had been composing to try to argue for changing
merge-recursive.c's design to the above, assuming I could get the time
to work on it.  Nice to see that I'm not crazy, and that I apparently
don't need to do much convincing.  :-)

Re: Problem installing Git

[Johannes Schindelin]

Hi Igor,

On Thu, 23 Nov 2017, Igor Djordjevic wrote:

> [ +Cc:  Git for Windows mailing list ]

I have no idea why it claimed that that group does not exist, the email
address looks correct to me.

> On 23/11/2017 19:51, Phil Martel wrote:
> > I'm trying to install Git-2.15.0-64-bit.exe onto my Windows 10 
> > machine.  When I run this installer program no matter what options I 
> > try or whether I run as administrator it ends up with an error box 
> > saying "The drive or UNC share you selected does not exist or is not 
> > accessible. Please select another".  I do not see any way of 
> > selecting a drive.  Any suggestions?
> 
> From what I could Google around, this seems to be (Inno Setup?) 
> installation related issue...?

Indeed.

> Do you already have "Git for Windows" installed? If so, does it work 
> if you try uninstalling it first?

That is a workaround, correct.

> p.s. Note the existence of "Git for Windows"[1] specific mailing list 
> as well, where this issue might belong better.
> 
> [1] git-for-wind...@googlegroups.com

I think a much better place is the Git for Windows bug tracker (if you
ever wonder where the bug tracker is, or the home page, or the repository
or the FAQ, there are links in the upper left of the release notes --
which probably nobody reads, even if I really try to make them worth the
while -- and which you can find in C:\Program Files\Git\ReleaseNotes.html
if you closed the tab after installing Git for Windows).

And indeed, there is already a ticket for this issue:
https://github.com/git-for-windows/git/issues/1074

The original reporter did not respond to any questions, maybe you can do
better, Phil?

Ciao,
Johannes

Re: git status always modifies index?

[Johannes Schindelin]

Hi Peff,

On Wed, 22 Nov 2017, Jeff King wrote:

> On Wed, Nov 22, 2017 at 01:56:35PM -0800, Jonathan Nieder wrote:
> 
> > Jeff King wrote:
> > > On Wed, Nov 22, 2017 at 12:27:20PM -0800, Jonathan Nieder wrote:
> > 
> > >> That said, I wonder if this use case is an illustration that a name
> > >> like --no-lock-index (as was used in Git for Windows when this feature
> > >> first appeared) or --no-refresh-on-disk-index (sorry, I am terrible at
> > >> coming up with option names) would make the feature easier to
> > >> discover.
> > [...]
> > > Or maybe just living with the minor philosophical rough edges,
> > > since it seems OK in practice.
> > 
> > To be clear, my concern is not philosophical but practical: I'm saying
> > if it's a "git status" option (or at least shows up in the "git
> > status" manpage) and it is memorably about $GIT_DIR/index (at least
> > mentions that in its description) then it is more likely to help
> > people.
> 
> Right, I went a little off track of your original point.
> 
> What I was trying to get at is that naming it "status --no-lock-index"
> would not be the same thing (even though with the current implementation
> it would behave the same). IOW, can we improve the documentation of
> "status" to point to make it easier to discover this use case.

I had the hunch that renaming the option (and moving it away from `git
status`, even if it is currently only affecting `git status` and even if
it will most likely be desirable to have the option to really only prevent
`git status` from writing .lock files) was an unfortunate decision (and
made my life as Git for Windows quite a bit harder than really necessary,
it cost me over one workday of a bug hunt, mainly due to a false flag
indicating `git rebase` to be the culprit). And I hinted at it, too.

Maybe I should trust my instincts and act on them more. It is not like it
was the first time that I had doubts that turned out to have merit, see
e.g. the regression introduced into the formerly rock-solid
set_hidden_flag() patches due to changes I made reluctantly during
upstreaming, or the regression introduced during v1->v2 in my regex-buf
patches that caused problems with mulibc and AIX.

I really never understood why --no-optional-locks had to be introduced
when it did exactly the same as --no-lock-index, and when the latter has a
right to exist in the first place, even in the purely hypothetical case
that we teach --no-optional-locks to handle more cases than just `git
status`' writing of the index (and in essence, it looks like premature
optimization): it is a very concrete use case that a user may want `git
status` to refrain from even trying to write any file, as this thread
shows very eloquently.

Maybe it is time to reintroduce --no-lock-index, and make
--no-optional-locks' functionality a true superset of --no-lock-index'.

At least that is what my gut feeling tells me should be done.

Ciao,
Dscho

Re: [PATCH] http-backend: respect CONTENT_LENGTH as specified by rfc3875

[Max Kirillov]

Thanks for the review. I saw only reaction of the Jeff in
the original thread and though that it is ok otherwise. I'm
fixing the things you mentioned.

On Thu, Nov 23, 2017 at 08:30:39PM -0500, Eric Sunshine wrote:
>> +static ssize_t read_request_fix_len(int fd, size_t req_len, unsigned char 
>> **out)
> 
> Wrong data type: s/size_t req_len/ssize_t req_len/

Passing negative value to the function makes no sense. I
could add explicit type cast to make it clear. It should be
safe as site_t's range is bigger, and overflown
CONTENT_LENGTH results in die() at parsing (I have a test
which verifies it)

> Rather than writing an entirely new "read" function, how about just
> modifying the existing read_request() to optionally limit the read to
> a specified number of bytes?

I'll check it a bit separately.

-- 
Max

Re: [PATCH v4 4/4] worktree: make add dwim

[Thomas Gummerer]

On 11/25, Randall S. Becker wrote:
> On November 25, 2017 3:06 PM Thomas Gummerer wrote:
> 
> >however we currently document one behaviour, which I would like to change
> >(I usually have branches
> >without a / in that I want to look at) we currently document one behaviour,
> >which I'd like to change.  So 
> >in that case we are a bit worried about backwards compatibility, and how
> >this will affect current users
> >that have a certain expectation of how the command is supposed to work,
> >hence the discussion of
> >whether to hide the new behaviour behind a flag or not.
> 
> >Either way, if we do put the behaviour behind a flag, I'll also add a
> >configuration variable, which can
> >be set to enable the new behaviour so one doesn't have to type out the flag
> >all the time.
> 
> To be consistent with other commands, you could put path after -- and the
> ambiguity with refs containing '/' goes away, as refs before the -- would
> always be considered refs while after you have paths.
>
> What I don't like is the current add syntax of:
> git worktree add [-f] [--detach] [--checkout] [--lock] [-b ]
>  []
> 
> where the path-spec precedes branch making things a bit icky. It might be
> better to have an alternate syntax of:
> 
> git worktree add [-f] [--detach] [--checkout] [--lock] [-b ]
>  []
> git worktree add [-f] [--detach] [--checkout] [--lock] [-b ]
> [] -- 

Hmm I don't think there is any ambiguity there, the first argument
always needs to be a path, and the second one is always a commit-ish.
So this way there is no disambiguation needed.

I'm not convinced the alternative syntax would buy us much, at least
not in the context of what this series is trying to do.

> But since we only have one path, that may be redundant. Just a thought, as
> -- avoids a lot of interpretation evils. While we're here, I wonder whether
>  should be changed to  for more general use. Consider
> release identifiers using tags, and using the tag instead of branch to
> define commit on which the worktree is based.

'git worktree add' can already take a commit-ish, it's just not
documented that way.  I'll add a patch updating the documentation to
the series.

> Cheers,
> Randall
> 
> -- Brief whoami: NonStop developer since approximately
> UNIX(421664400)/NonStop(2112884442) 
> -- In my real life, I talk too much.
> 
> 
> 
> 
> 

Re: [PATCH] Makefile: check that tcl/tk is installed

[Christian Couder]

On Mon, Nov 20, 2017 at 6:15 PM, Christian Couder
 wrote:
> By default running `make install` in the root directory of the
> project will set TCLTK_PATH to `wish` and then go into the "git-gui"
> and "gitk-git" sub-directories to build and install these 2
> sub-projects.

Has this patch fallen through the cracks or is there an unresolved issue?

RE: [PATCH v4 4/4] worktree: make add dwim

[Randall S. Becker]

On November 25, 2017 3:06 PM Thomas Gummerer wrote:

>however we currently document one behaviour, which I would like to change
(I usually have branches
>without a / in that I want to look at) we currently document one behaviour,
which I'd like to change.  So 
>in that case we are a bit worried about backwards compatibility, and how
this will affect current users
>that have a certain expectation of how the command is supposed to work,
hence the discussion of
>whether to hide the new behaviour behind a flag or not.

>Either way, if we do put the behaviour behind a flag, I'll also add a
configuration variable, which can
>be set to enable the new behaviour so one doesn't have to type out the flag
all the time.

To be consistent with other commands, you could put path after -- and the
ambiguity with refs containing '/' goes away, as refs before the -- would
always be considered refs while after you have paths.

What I don't like is the current add syntax of:
git worktree add [-f] [--detach] [--checkout] [--lock] [-b ]
 []

where the path-spec precedes branch making things a bit icky. It might be
better to have an alternate syntax of:

git worktree add [-f] [--detach] [--checkout] [--lock] [-b ]
 []
git worktree add [-f] [--detach] [--checkout] [--lock] [-b ]
[] -- 

But since we only have one path, that may be redundant. Just a thought, as
-- avoids a lot of interpretation evils. While we're here, I wonder whether
 should be changed to  for more general use. Consider
release identifiers using tags, and using the tag instead of branch to
define commit on which the worktree is based.

Cheers,
Randall

-- Brief whoami: NonStop developer since approximately
UNIX(421664400)/NonStop(2112884442) 
-- In my real life, I talk too much.

git-p4: cloning with a change number does not import all files

[Patrick Rouleau]

Hi,

I created a git repository with these commands:
  git p4 clone //perforce/path@123456 repo
  cd repo
  git p4 rebase

Some files created before the change 123456 do not exist in git
history. I do see why,
but those files were not modified after that change number.

If I use "git p4 sync --detect-branches" instead of "git p4 rebase",
the branch contains
all the files, but not the main trunk, and they appear to be added in
the first commit of
the branch.

To avoid the problem, I must clone with "@all" or with the change number when
//perforce/path was created, which is significantly longer.

Regards,
P. Rouleau

Re: [PATCH v4 4/4] worktree: make add dwim

[Thomas Gummerer]

On 11/25, Paul Smith wrote:
> On Sat, 2017-11-25 at 17:50 +, Thomas Gummerer wrote:
> > This would be the output in the new version:
> > 
> >  $ git worktree add ../bla
> >  Branch 'bla' set up to track remote branch 'bla' from 'origin'.
> >  Preparing ../bla (identifier bla)
> >  HEAD is now at 4aade43 bla
> > 
> > vs. the output without the changed behaviour:
> > 
> >  $ git worktree add ../bla
> >  Preparing ../bla (identifier bla)
> >  HEAD is now at 0f215c9 initial import
> > 
> > Of course that assumes that it's used directly, not in scripts, and
> > that users will actually read the output of the command when they
> > invoke it.  Maybe these are not safe assumptions to make though, and
> > we'd rather not have this on by default then.  As I mentioned
> > previously I would prefer having this as default, but I'm happy to
> > hide this behaviour behind a flag if we want to be more careful about
> > introducing this.  Dunno?
> 
> Speaking as a simple user, I find the current behavior of Git worktree
> add very frustrating; I am constantly wanting to create worktrees for
> other peoples' branches so I can look at the code there without messing
> up my workspace, and it's really inconvenient to do that now.
> 
> Also, the current special handling of the directory name as a putative
> branch name is not helpful for me because many of the branches I need
> to examine use "/" as their separator.  I don't begrudge making that
> feature more "DWIM" for those that can use it, but hopefully some help
> is forthcoming for those who can't.
> 
> For example, I need to create a local worktree for the remote rel/1.0
> branch... what do I do?
> 
> What I want to work is this:
> 
> git worktree add ../1.0 rel/1.0
> 
> and have it create a worktree at ../1.0, then do the equivalent of "git
> checkout rel/1.0" which includes setting up to track the remote branch.
>  But of course this doesn't work at all; I get:
> 
> fatal: invalid reference: rel/1.0
> 
> Personally I would think it odd to have to add an extra flag to get
> what I would expect would be "normal" behavior (checkout).
> 
> But maybe that's just me.

This part is getting done in 3/4, and is definitely going to work
without an additional flag, so this is (hopefully) soon going to work
just as you want :)

This is less controversial because as you mentioned this currently
doesn't work at all, so there are no backwards compatibility worries.

For the other case of

git worktree add ../foo

however we currently document one behaviour, which I would like to
change (I usually have branches without a / in that I want to look at)
we currently document one behaviour, which I'd like to change.  So in
that case we are a bit worried about backwards compatibility, and how
this will affect current users that have a certain expectation of how
the command is supposed to work, hence the discussion of whether to
hide the new behaviour behind a flag or not.

Either way, if we do put the behaviour behind a flag, I'll also add a
configuration variable, which can be set to enable the new behaviour
so one doesn't have to type out the flag all the time.

[PATCH] submodule--helper.c: i18n: add a missing space in message

[Jean-Noel Avila]

The message spans over 2 lines but the C conconcatenation does not add
the needed space between the two lines.

Signed-off-by: Jean-Noel Avila 
---
 builtin/submodule--helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 2086f0eb0..a5c4a8a69 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -623,7 +623,7 @@ static void status_submodule(const char *path, const struct 
object_id *ce_oid,
 
if (refs_head_ref(get_submodule_ref_store(path),
  handle_submodule_head_ref, ))
-   die(_("could not resolve HEAD ref inside the"
+   die(_("could not resolve HEAD ref inside the "
  "submodule '%s'"), path);
 
print_status(flags, '+', path, , displaypath);
-- 
2.15.0

[PATCH] submodule--helper.c: i18n: add a missing space in message

[Jean-Noel Avila]

The message spans over 2 lines but the C conconcatenation does not add
the needed space between the two lines.
---
 builtin/submodule--helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 2086f0eb0..a5c4a8a69 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -623,7 +623,7 @@ static void status_submodule(const char *path, const struct 
object_id *ce_oid,
 
if (refs_head_ref(get_submodule_ref_store(path),
  handle_submodule_head_ref, ))
-   die(_("could not resolve HEAD ref inside the"
+   die(_("could not resolve HEAD ref inside the "
  "submodule '%s'"), path);
 
print_status(flags, '+', path, , displaypath);
-- 
2.15.0

Re: [PATCH v4 4/4] worktree: make add dwim

[Paul Smith]

On Sat, 2017-11-25 at 17:50 +, Thomas Gummerer wrote:
> This would be the output in the new version:
> 
>  $ git worktree add ../bla
>  Branch 'bla' set up to track remote branch 'bla' from 'origin'.
>  Preparing ../bla (identifier bla)
>  HEAD is now at 4aade43 bla
> 
> vs. the output without the changed behaviour:
> 
>  $ git worktree add ../bla
>  Preparing ../bla (identifier bla)
>  HEAD is now at 0f215c9 initial import
> 
> Of course that assumes that it's used directly, not in scripts, and
> that users will actually read the output of the command when they
> invoke it.  Maybe these are not safe assumptions to make though, and
> we'd rather not have this on by default then.  As I mentioned
> previously I would prefer having this as default, but I'm happy to
> hide this behaviour behind a flag if we want to be more careful about
> introducing this.  Dunno?

Speaking as a simple user, I find the current behavior of Git worktree
add very frustrating; I am constantly wanting to create worktrees for
other peoples' branches so I can look at the code there without messing
up my workspace, and it's really inconvenient to do that now.

Also, the current special handling of the directory name as a putative
branch name is not helpful for me because many of the branches I need
to examine use "/" as their separator.  I don't begrudge making that
feature more "DWIM" for those that can use it, but hopefully some help
is forthcoming for those who can't.

For example, I need to create a local worktree for the remote rel/1.0
branch... what do I do?

What I want to work is this:

git worktree add ../1.0 rel/1.0

and have it create a worktree at ../1.0, then do the equivalent of "git
checkout rel/1.0" which includes setting up to track the remote branch.
 But of course this doesn't work at all; I get:

fatal: invalid reference: rel/1.0

Personally I would think it odd to have to add an extra flag to get
what I would expect would be "normal" behavior (checkout).

But maybe that's just me.

Re: [PATCH] RelNotes: minor typo fixes in 2.15.1 draft

[Todd Zullinger]

Apologies for the duplicate.  I used the `-n` option, mistakenly 
thinking it was a synonym for `--dry-run` and didn't pay enough 
attention to see that it sent.  (The only indication is s/OK./Dry &/ 
which I missed.)


It was mildly surprising that the script didn't warn or complain about 
an unknown option.  After a quick look, that seems to be due to the 
Getopt::Long pass_through option which sends unknown options to 
format-patch.


That doesn't remove the user-error on my part, of course. ;)

--
Todd
~~
Cogito cogito ergo cogito sum --
I think that I think, therefore I think that I am.
   -- Ambrose Bierce, "The Devil's Dictionary"

Re: [PATCH v4 4/4] worktree: make add dwim

[Thomas Gummerer]

On 11/24, Junio C Hamano wrote:
> Thomas Gummerer  writes:
> 
> > Currently 'git worktree add ' creates a new branch named after the
> > basename of the , that matches the HEAD of whichever worktree we
> > were on when calling "git worktree add ".
> >
> > Make 'git worktree add  behave more like the dwim machinery in
> > 'git checkout ', i.e. check if the new branch name uniquely
> > matches the branch name of a remote tracking branch, and if so check out
> > that branch and set the upstream to the remote tracking branch.
> >
> > This is a change of behaviour compared to the current behaviour, where
> > we create a new branch matching HEAD.  However as 'git worktree' is
> > still an experimental feature, and it's easy to notice/correct the
> > behaviour in case it's not what the user desired it's probably okay to
> > break existing behaviour here.
> 
> Is it "easy to notice"?  I doubt it.  Even if you assume that
> everybody uses bash prompt that shows the name of the branch, the
> user sees the same name of the branch in either mode.

With "easy" I meant at creation time, looking at the output of 'git
worktree add', which with the new version shows that the the new
branch has been set up to track the remote branch, and also shows the
commit HEAD now points to.

This would be the output in the new version:

 $ git worktree add ../bla
 Branch 'bla' set up to track remote branch 'bla' from 'origin'.
 Preparing ../bla (identifier bla)
 HEAD is now at 4aade43 bla

vs. the output without the changed behaviour:

 $ git worktree add ../bla
 Preparing ../bla (identifier bla)
 HEAD is now at 0f215c9 initial import

Of course that assumes that it's used directly, not in scripts, and
that users will actually read the output of the command when they
invoke it.  Maybe these are not safe assumptions to make though, and
we'd rather not have this on by default then.  As I mentioned
previously I would prefer having this as default, but I'm happy to
hide this behaviour behind a flag if we want to be more careful about
introducing this.  Dunno?


> > In order to also satisfy users who want the current behaviour of
> > creating a new branch from HEAD, add a '--no-track' flag, which disables
> > the new behaviour, and keeps the old behaviour of creating a new branch
> > from the head of the current worktree.
> 
> I am not sure if this is a good match for "--track/--no-track";
> which branch is to be checked out (either "automatically from the
> unique remote-tracking branch" or "the current one") is one choice,
> and whether the resulting branch is marked explicitly as integrating
> with the remote or not is another choice within one branch of the
> first choice.  IOW, this makes it impossible to say "create the branch
> based on the unique remote-tracking branch, but do not add the two
> branch.*.{merge,remote} variables".

Hmm good point.  Maybe we'll need another flag for this.  Maybe
--[no-]guess-remote would work, and a corresponding
worktree.guessRemote config would work?  That's the best I could come
up with, better suggestions are definitely welcome.

> Also, you have several mention of "remote tracking branch" in these
> patches.  Please consistently spell them as "remote-tracking branch"
> to be consistent with Documentation/glossary-content.txt and avoid a
> casual/careful reference to "tracking branch" if possible, unless it
> is quite clear to the readers that you are being loose for the sake
> of brevity.  Some people used "tracking branch" to mean the local
> branch that is marked as the branch to integrate with the work on
> a branch at a remote that caused user confusion in the past.

I must admit I wasn't aware of Documentation/glossary-content.txt and
have seen "tracking branch" in other places, so I was just repeating
the pattern.

> That is
> 
> refs/remotes/origin/topic is a remote-tracking branch for the
> branch 'topic' that came from the 'origin' remote.
> 
> when you have branch.foo.remote=origin and
> branch.foo.merge=refs/heads/topic, then your local branch foo is
> marked to integrate with the 'topic' branch at the 'origin'
> remote.
> 
> and these two are quite different things that people in the past and
> over time loosely used a phrase "tracking branch" to cause confusion.


Thanks for the clarification, will fix in the re-roll.

[PATCH] RelNotes: minor typo fixes in 2.15.1 draft

[Todd Zullinger]

Signed-off-by: Todd Zullinger 
---
 Documentation/RelNotes/2.15.1.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/RelNotes/2.15.1.txt 
b/Documentation/RelNotes/2.15.1.txt
index 47f23b56fe..81dd64b4ad 100644
--- a/Documentation/RelNotes/2.15.1.txt
+++ b/Documentation/RelNotes/2.15.1.txt
@@ -13,7 +13,7 @@ Fixes since v2.15
latter, which has been fixed.
 
  * The experimental "color moved lines differently in diff output"
-   feature was buggy around "ignore whitespace changes" edges, whihch
+   feature was buggy around "ignore whitespace changes" edges, which
has been corrected.
 
  * Instead of using custom line comparison and hashing functions to
@@ -24,7 +24,7 @@ Fixes since v2.15
HEAD points at, which have been fixed.
 
  * "git commit", after making a commit, did not check for errors when
-   asking on what branch it made the commit, which has been correted.
+   asking on what branch it made the commit, which has been corrected.
 
  * "git status --ignored -u" did not stop at a working tree of a
separate project that is embedded in an ignored directory and
@@ -35,7 +35,7 @@ Fixes since v2.15
--recurse-submodules" has been fixed.
 
  * A recent regression in "git rebase -i" that broke execution of git
-   commands from subdirectories via "exec" insn has been fixed.
+   commands from subdirectories via "exec" instruction has been fixed.
 
  * "git check-ref-format --branch @{-1}" bit a "BUG()" when run
outside a repository for obvious reasons; clarify the documentation
-- 
2.15.0

[PATCH] RelNotes: minor typo fixes in 2.15.1 draft

[Todd Zullinger]

Signed-off-by: Todd Zullinger 
---
 Documentation/RelNotes/2.15.1.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/RelNotes/2.15.1.txt 
b/Documentation/RelNotes/2.15.1.txt
index 47f23b56fe..81dd64b4ad 100644
--- a/Documentation/RelNotes/2.15.1.txt
+++ b/Documentation/RelNotes/2.15.1.txt
@@ -13,7 +13,7 @@ Fixes since v2.15
latter, which has been fixed.
 
  * The experimental "color moved lines differently in diff output"
-   feature was buggy around "ignore whitespace changes" edges, whihch
+   feature was buggy around "ignore whitespace changes" edges, which
has been corrected.
 
  * Instead of using custom line comparison and hashing functions to
@@ -24,7 +24,7 @@ Fixes since v2.15
HEAD points at, which have been fixed.
 
  * "git commit", after making a commit, did not check for errors when
-   asking on what branch it made the commit, which has been correted.
+   asking on what branch it made the commit, which has been corrected.
 
  * "git status --ignored -u" did not stop at a working tree of a
separate project that is embedded in an ignored directory and
@@ -35,7 +35,7 @@ Fixes since v2.15
--recurse-submodules" has been fixed.
 
  * A recent regression in "git rebase -i" that broke execution of git
-   commands from subdirectories via "exec" insn has been fixed.
+   commands from subdirectories via "exec" instruction has been fixed.
 
  * "git check-ref-format --branch @{-1}" bit a "BUG()" when run
outside a repository for obvious reasons; clarify the documentation
-- 
2.15.0

Re: [PATCH v4 2/4] worktree: add --[no-]track option to the add subcommand

[Thomas Gummerer]

On 11/24, Junio C Hamano wrote:
> Thomas Gummerer  writes:
> 
> > diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> > index b5c47ac602..53042ce565 100755
> > --- a/t/t2025-worktree-add.sh
> > +++ b/t/t2025-worktree-add.sh
> > @@ -313,5 +313,60 @@ test_expect_success 'checkout a branch under bisect' '
> >  test_expect_success 'rename a branch under bisect not allowed' '
> > test_must_fail git branch -M under-bisect bisect-with-new-name
> >  '
> > +# Is branch "refs/heads/$1" set to pull from "$2/$3"?
> > +test_branch_upstream () {
> > +   printf "%s\n" "$2" "refs/heads/$3" >expect.upstream &&
> > +   {
> > +   git config "branch.$1.remote" &&
> > +   git config "branch.$1.merge"
> > +   } >actual.upstream &&
> > +   test_cmp expect.upstream actual.upstream
> > +}
> 
> OK.
> 
> > +test_expect_success '--track sets up tracking' '
> > +   test_when_finished rm -rf track &&
> > +   git worktree add --track -b track track master &&
> > +   git config "branch.track.merge" &&
> > +   (
> > +   test_branch_upstream track . master
> > +   )
> > +'
> 
> Is this "git config" necessary, or is it a remnant of a debugging
> session?  It is tested in the helper that branch.track.merge is set
> to something, and otherwise the helper would fail the same way as
> this standalnoe "git config" would, no?

It's a remnant of a debugging session, sorry.  It would indeed fail in
the same way, so just leaving the 'test_branch_upstream' is enough.
Also looking at that, there's no need for it to be in a subshell, will
fix that as well.


> > +# setup remote repository $1 and repository $2 with $1 set up as
> > +# remote.  The remote has two branches, master and foo.
> > +setup_remote_repo () {
> > +   git init $1 &&
> > +   (
> > +   cd $1 &&
> > +   test_commit $1_master &&
> > +   git checkout -b foo &&
> > +   test_commit upstream_foo
> > +   ) &&
> > +   git init $2 &&
> > +   (
> > +   cd $2 &&
> > +   test_commit $2_master &&
> > +   git remote add $1 ../$1 &&
> > +   git config remote.$1.fetch \
> > +   "refs/heads/*:refs/remotes/$1/*" &&
> > +   git fetch --all
> > +   )
> > +}
> > +
> > +test_expect_success '--no-track avoids setting up tracking' '
> > +   test_when_finished rm -rf repo_upstream repo_local foo &&
> > +   setup_remote_repo repo_upstream repo_local &&
> > +   (
> > +   cd repo_local &&
> > +   git worktree add --no-track -b foo ../foo repo_upstream/foo
> > +   ) &&
> > +   (
> > +   cd foo &&
> > +   ! test_branch_upstream foo repo_upstream foo &&
> 
> It is true that this test helper must yield failure.  But what you
> expect probably is more than that, no?  For example, the test helper
> would fail even if branch.foo.remote is set to the upstream as long
> as branch.foo.merge is not set to point at their foo, but what you
> really want to make sure is that neither configuration variable is
> set.

Yeah you're right, this test is a bit too loose.  Will fix that in the
re-roll.  Thanks!

> > +   git rev-parse repo_upstream/foo >expect &&
> > +   git rev-parse foo >actual &&
> > +   test_cmp expect actual
> > +   )
> > +'
> >  
> >  test_done

RE: Clone repository computer A to remote B doenst work

[Randall S. Becker]

On November 25, 2017 4:31 AM Roberto Garcia wrote:

>I'm trying clone in windows a git repository to other remote machine (NAS 
>Linux based).
>I have installed git for windows but i didn't installed nothing in the other 
>remote machine (NAS Linux based).

You have two choices:
1. Install git on your LINYX machine, which you probably can't do if it's a 
pure NAS outside of your control.
2. Run everything off Windows as git in local mode. Mount the NAS as a windows 
drive. In a command terminal:
a. cd X:\Share\repo.git #you'll have to mkdir this
b. git init --bare #creates a new empty repo on your NAS
c. cd C:\MyStuff #where you keep your clones
d. git clone -l X:\Share\repo.git #clone the bare repository
e. Start adding stuff (git add, git commit)
f. git push   # to move commits to your NAS repo.

Then you have your relationship and can push/pull from your NAS entirely from 
within Windows executing objects. Change directories and drive letters 
accordingly. -l means local, so git won't be starting any git-upload-pack 
processes remotely. Variations on this should work.

Good luck.

Randall

Xmas Loan Bonaza

[Shell Federal Credit Union]

Are You In Need Of A Private Or Business Loans At 2% Rate For Various Purposes? 
If Yes; Contact us with this info below.

Full Name:   
Amount Needed:  
Duration:  
Country 
Cell No:
Sex:   

Best Regards

Re: [PATCH] checkout: include the checkout.h header file

[Thomas Gummerer]

On 11/24, Ramsay Jones wrote:
> 
> Signed-off-by: Ramsay Jones 
> ---
> 
> Hi Thomas,
> 
> If you need to re-roll your 'tg/worktree-create-tracking' branch, could
> you please squash this into the relevant patch (commit 6736ae9593,
> "checkout: factor out functions to new lib file", 2017-11-22).
> 
> [noticed by sparse]

Thanks for noticing this, I'll squash it into my re-roll.

> Thanks!
> 
> ATB,
> Ramsay Jones
> 
>  checkout.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/checkout.c b/checkout.c
> index b0c744d37..ac42630f7 100644
> --- a/checkout.c
> +++ b/checkout.c
> @@ -1,5 +1,6 @@
>  #include "cache.h"
>  #include "remote.h"
> +#include "checkout.h"
>  
>  struct tracking_name_data {
>   /* const */ char *src_ref;
> -- 
> 2.15.0

Clone repository computer A to remote B doenst work

[Roberto Garcia]

Hello all,

I'm trying clone in windows a git repository to other remote machine
(NAS Linux based).
I have installed git for windows but i didn't installed nothing in the
other remote machine (NAS Linux based).

I open git-bash and I try do this: "git clone
user@IP:/directory/where/i/want/clone".
This doesnt work because I obtain "sh: git-upload-pack: command not found".

I search in google and I found a possible solution, I need add -u
/path/to/git-upload-pack.
When i try again writing git clone -u
/c/Git/mingw64/bin/git-upload-pack
user@IP:/directory/where/i/want/clone
After I Write the password I obtain sh:
C:/Git/mingw64/bin/git-upload-pack: No such file or directory. But it
exists! (in my local computer windows)

I'm newbie with Git and Unix system. What can i do ?
Thanks a lot!

HOPETO HEAR FROM YOU

[Miss Fatima]

Hello dear how are you?

Nice to meet you,my name is Miss Fatima, can we become friends? hope
to hear from you so that we can know each other very well,love matters
mostly in life,i will also send you my pictures and tell you more
about myself, my email address is, fatima.ab...@yahoo.com

waiting to hear from you soon.
Miss.Fatima