Re: [ANNOUNCE] Git for Windows 2.14.2(3)

[Lars Schneider]

> On 16 Oct 2017, at 20:59, Steve Hoelzer  wrote:
> 
> Johannes,
> 
> On Mon, Oct 16, 2017 at 5:57 AM, Johannes Schindelin
>  wrote:
>> Hi Steve,
>> 
>> On Sun, 15 Oct 2017, Johannes Schindelin wrote:
>> 
>>> On Fri, 13 Oct 2017, Steve Hoelzer wrote:
>>> 
 On Thu, Oct 12, 2017 at 5:53 PM, Johannes Schindelin
  wrote:
> 
> It is my pleasure to announce that Git for Windows 2.14.2(3) is
> available from:
> 
>https://git-for-windows.github.io/
> 
> Changes since Git for Windows v2.14.2(2) (October 5th 2017)
> 
> New Features
> 
>  * Comes with Git LFS v2.3.3.
 
 I just ran "git update" and afterward "git version" reported
 2.14.2(3), but "git lfs version" still said 2.3.2.
 
 I also uninstalled/reinstalled Git for Windows and LFS is still 2.3.2.
>>> 
>>> Ah bummer. I forgot to actually update it in the VM where I build the
>>> releases :-(
>>> 
>>> Will work on it tomorrow.
>> 
>> I'll actually use this opportunity to revamp a part of Git for Windows'
>> release engineering process to try to prevent similar things from
>> happening in the future.
>> 
>> Also, cURL v7.56.1 is slated to be released in exactly one week, and I
>> have some important installer work to do this week, so I'll just defer the
>> new Git for Windows version tentatively to Monday, 23rd.
>> 
>> Git LFS users can always install Git LFS v2.3.3 specifically in the
>> meantime, or use Git for Windows' snapshot versions
>> (https://wingit.blob.core.windows.net/files/index.html).
> 
> Sounds like a good plan.
> 
> I think I have successfully updated to LFS 2.3.3 by copying the new
> git-lfs.exe into C:\Program Files\Git\mingw64\bin. Is that right way
> to do it?

That's how I do it and that's how the Git LFS installer will (hopefully)
do it in the future, too: https://github.com/git-lfs/git-lfs/issues/2587

- Lars

Re: [PATCH] check-ref-format: require a repository for --branch

[Junio C Hamano]

Junio C Hamano  writes:

>> I don't think there is any need to prepare it upon my 4d03f955,
>> though.  I'd think it could simply replace it.
>
> Yeah, it ended up that way, it seems.  Still it needs a bit of doc
> updates to balance the description.

So here is what I have now on 'pu'.  Clearly not a 2.15 material yet.

-- >8 --
Subject: [PATCH] check-ref-format: --branch cannot grok @{-1} outside a 
repository

"git check-ref-format --branch $name" feature was originally
introduced (and was advertised) as a way for scripts to take any
end-user supplied string (like "master", "@{-1}" etc.) and see if
that is usable when Git expects to see a branch name, and also
obtain the concrete branch name that the at-mark magic expands to.

When the user asks to interpret a branch name like "@{-1}", we have
to dig the answer out of the HEAD reflog.  We can obviously only do
that if we have a repository, and indeed, running it outside a
repository causes us to hit a BUG().

Let's disable the "expand @{-n}" half of the feature when it is run
outside a repository, but keep the feature to check the syntax of a
proposed branch name, as "git check-ref-format --branch $name" can
be stricter than "git check-ref-format refs/heads/$name", and
Porcelain scripts need to have a way to check a given name against
the stricter rule.

Helped-by: Jeff King 
Signed-off-by: Junio C Hamano 
---
 Documentation/git-check-ref-format.txt |  9 -
 builtin/check-ref-format.c | 15 ---
 cache.h| 14 ++
 sha1_name.c| 22 +++---
 strbuf.h   |  6 ++
 t/t1402-check-ref-format.sh| 12 
 6 files changed, 71 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-check-ref-format.txt 
b/Documentation/git-check-ref-format.txt
index 92777cef25..cf0a0b7df2 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -77,7 +77,14 @@ reference name expressions (see linkgit:gitrevisions[7]):
 
 . at-open-brace `@{` is used as a notation to access a reflog entry.
 
-With the `--branch` option, it expands the ``previous branch syntax''
+With the `--branch` option, the command takes a name and checks if
+it can be used as a valid branch name (e.g. when creating a new
+branch).  The rule `git check-ref-format --branch $name` implements
+may be stricter than what `git check-ref-format refs/heads/$name`
+says (e.g. a dash may appear at the beginning of a ref component,
+but it is explicitly forbidden at the beginning of a branch name).
+When run with `--branch` option in a repository, the input is first
+expanded for the ``previous branch syntax''
 `@{-n}`.  For example, `@{-1}` is a way to refer the last branch you
 were on.  This option should be used by porcelains to accept this
 syntax anywhere a branch name is expected, so they can act as if you
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index eac499450f..4e62852089 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -38,13 +38,22 @@ static char *collapse_slashes(const char *refname)
 
 static int check_ref_format_branch(const char *arg)
 {
+   int nongit, malformed;
struct strbuf sb = STRBUF_INIT;
-   int nongit;
+   const char *name = arg;
 
setup_git_directory_gently();
-   if (strbuf_check_branch_ref(, arg))
+
+   if (!nongit)
+   malformed = (strbuf_check_branch_ref(, arg) ||
+!skip_prefix(sb.buf, "refs/heads/", ));
+   else
+   malformed = check_branch_ref_format(arg);
+
+   if (malformed)
die("'%s' is not a valid branch name", arg);
-   printf("%s\n", sb.buf + 11);
+   printf("%s\n", name);
+   strbuf_release();
return 0;
 }
 
diff --git a/cache.h b/cache.h
index 52b91f5b64..3815925122 100644
--- a/cache.h
+++ b/cache.h
@@ -1444,6 +1444,20 @@ extern int parse_oid_hex(const char *hex, struct 
object_id *oid, const char **en
 #define INTERPRET_BRANCH_HEAD (1<<2)
 extern int interpret_branch_name(const char *str, int len, struct strbuf *,
 unsigned allowed);
+
+/*
+ * NEEDSWORK: declare strbuf_branchname() and strbuf_check_branch_ref()
+ * here, not in strbuf.h
+ */
+
+/*
+ * Check if a 'name' is suitable to be used as a branch name; this can
+ * be and is stricter than what check_refname_format() returns for a
+ * string that is a concatenation of "name" after "refs/heads/"; a
+ * name that begins with "-" is not allowed, for example.
+ */
+extern int check_branch_ref_format(const char *name);
+
 extern int get_oid_mb(const char *str, struct object_id *oid);
 
 extern int validate_headref(const char *ref);
diff --git a/sha1_name.c b/sha1_name.c
index 5e2ec37b65..c95080258f 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1319,15 

Re: [PATCH] check-ref-format: require a repository for --branch

[Jonathan Nieder]

Hi,

Junio C Hamano wrote:

> Things like @{-1} would not make any sense when the command is run
> outside a repository, and the documentation is quite clear that it
> is the primary reason why we added "--branch" option to the command,
> i.e.
>
> With the `--branch` option, it expands the ``previous branch syntax''
> `@{-n}`.  For example, `@{-1}` is a way to refer the last branch you
> were on.  This option should be used by porcelains to accept this
> syntax anywhere a branch name is expected, so they can act as if you
> typed the branch name.
>
> So I am tempted to take this patch to make sure that we won't gain
> more people who abuse the command outside a repository.

That seems very sensible on its face.  My only worry is that a script
that can be run both inside and outside a repository and does

branch=$(git check-ref-format --branch "$user_supplied_branch_arg")

currently works with user_supplied_branch_arg='master' and would stop
working.  If we have reason to believe that no such scripts exist,
then this would be a good way to go, but I don't believe we can count
on that.

And in that spirit, I think the patch you replied with aims to go in
the right direction, by providing the core functionality when in a
repository while avoiding breaking such a script outside of one
(though I do not understand it fully yet).

> Having said that, there may still be a use case where a Porcelain
> script wants a way to see if a $name it has is appropriate as a
> branch name before it has a repository

This seems like a different goal than "git check-ref-format --branch"
was originally designed to fulfill (even though it fits well with the
check-ref-format name and coincides with --branch behavior when in a
repository).  I think it's fine for us not to fulfill it.

>(e.g. a wrapper to "git
> clone" that wants to verify the name it is going to give to the "-b"
> option), and a check desired in such a context is different from
> (and is stricter than) feeding refs/heads/$name to the same command
> without the "--branch" option.

Can you say more about this example?  E.g. why is this hypothetical
wrapper unable to rely on "git clone -b"'s own error handling?

> So I think the right endgame in the longer term is:
>
>  - Find (or add if it doesn't exist) a way to recommend to Porcelain
>scripts to use to expand an end-user generated string, and to map
>it to a branch name (it may be "rev-parse --symbolic-full-name
>$name"; I dunno).

--symbolic-full-name seems like a good fit.  Do you remember why
check-ref-format was introduced instead?  Was it just a matter of
implementation simplicity, since --symbolic-full-name can handle a
broader class of revision specifications like --remotes?  The commit
message to v1.6.3-rc0~29^2~4 (give Porcelain a way to grok branch
shorthand, 2009-03-21) is appropriately apologetic but doesn't give
more clues.

>  - Keep check-ref-format as (or revert it to be) a tool to "check".
>This would involve split strbuf_check_branch_ref() into two:

Without an example of where this tool would be used, if we consider
"check-ref-format --branch" to be a mistake then I'd rather deprecate
it with a goal of removing it completely.

Ok, time to look in more detail.

Thanks for your thoughtfulness,
Jonathan

Re: [PATCH] check-ref-format: require a repository for --branch

[Kevin Daudt]

On Tue, Oct 17, 2017 at 11:40:17AM +0900, Junio C Hamano wrote:
> Kevin Daudt  writes:
> 
> >> +  setup_git_directory_gently();
> >> +
> >> +  if (!nongit)
> >> +  malformed = (strbuf_check_branch_ref(, arg) ||
> >> +   !skip_prefix(sb.buf, "refs/heads/", ));
> >> +  else
> >> +  malformed = check_branch_ref_format(arg);
> >> +
> >
> > Would it make sense to swap the logic and get rid of the double
> > negative (!nongit)?
> 
> I am trying to follow the pattern "handle the normal case that have
> been supported forever first, and then handle new exception next",
> so that it is easier to see that there is no behaviour change in the
> normal case, so I do not think it makes it easier to see to swap the
> if/else cases.

Ok, thanks for your reasoning, makes sense.
> >
> >> +  if (malformed)
> >>die("'%s' is not a valid branch name", arg);
> >> -  printf("%s\n", sb.buf + 11);
> >> +  printf("%s\n", name);
> >> +  strbuf_release();
> >>return 0;
> >>  }
> >>  

Re: [PATCH 2/1] mention git stash push first in the man page

[Jeff King]

On Thu, Oct 05, 2017 at 09:10:29PM +0100, Thomas Gummerer wrote:

> Because 'stash push' and 'stash save' are so closely related they share one
> section in the man page.  Currently 'stash save' comes first, as that
> was the command that people were historically using.  However this makes
> the newer, more feature rich git stash push very easy to overlook.
> Change the order to give the newer interface for creating a stash the
> more prominent position.

Seems reasonable, though if we are deprecating "save" should we demote
it from being in the synopsis entirely?

-Peff

Re: [RFC] deprecate git stash save? (was: Re: [PATCH 2/3] http-push: fix construction of hex value from path)

[Jeff King]

On Thu, Oct 05, 2017 at 09:00:49PM +0100, Thomas Gummerer wrote:

> Since you were asking :)  With the introduction of 'git stash push',
> my hope was always that we could eventually get rid of 'git stash
> save' and only keep one interface around.
> 
> As there still many references to it around on the internet, it
> probably requires a bit of a longer deprecation plan.  What do you
> think about the following:
> 
> - Change docs/man pages to use 'git stash push' everywhere instead of
>   'git stash save'.
> - Mention the deprecation in the release notes and in the man page.
> - Print a warning when 'git stash save' is used.
> - Eventually get rid of it (maybe something for 3.0?)
> 
> The steps above would all occur sequentially with a few releases
> between each of them.

That sounds like a pretty good plan.

> A patch for the first step below.  I think that even makes sense if we
> don't want to follow through with the deprecation.

Agreed. The patch mostly looks good, except:

> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> index b4d88af133..1c4e44892d 100644
> --- a/Documentation/user-manual.txt
> +++ b/Documentation/user-manual.txt
> @@ -1556,7 +1556,7 @@ so on a different branch and then coming back), unstash 
> the
>  work-in-progress changes.
>  
>  
> -$ git stash save "work in progress for foo feature"
> +$ git stash push "work in progress for foo feature"
>  

This needs "-m", doesn't it?

>  
>  This command will save your changes away to the `stash`, and
> diff --git a/git-stash.sh b/git-stash.sh
> index 8b2ce9afda..8ce6929d7f 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -267,11 +267,11 @@ push_stash () {
>   # translation of "error: " takes in your language. E.g. 
> in
>   # English this is:
>   #
> - #$ git stash save --blah-blah 2>&1 | head -n 2
> - #error: unknown option for 'stash save': --blah-blah
> - #   To provide a message, use git stash save -- 
> '--blah-blah'
> - eval_gettextln "error: unknown option for 'stash save': 
> \$option
> -   To provide a message, use git stash save -- '\$option'"
> + #$ git stash push --blah-blah 2>&1 | head -n 2
> + #error: unknown option for 'stash push': --blah-blah
> + #   To provide a message, use git stash push -- 
> '--blah-blah'
> + eval_gettextln "error: unknown option for 'stash push': 
> \$option
> +   To provide a message, use git stash push -- '\$option'"

And here, too?

-Peff

Re: [PATCH 0/4] pre-merge hook

[Junio C Hamano]

Michael J Gruber  writes:

> This series revives an old suggestion of mine to make merge honor
> pre-commit hook or a separate pre-merge hook

This seems to have become an abandoned loose end, so I'll drop the
topic from my tree for now; revival of the discussion is _not_
unwelcome (aka "dropping without prejudice").

Re: [PATCH] patch reply

[Junio C Hamano]

Thais Diniz  writes:

> +Just to clarify I did not see Marius patch.
> +Did see Marius' comment saying he would look it in the leftoverbits list,
> +but since i didn't see any patch i thought i could work on it and did so 
> based on Stephan's comment 
> +(which i suppose Mario also did and that is why the code resulted to be 
> similar).

In any case, both versions share exactly the same "don't call
get_multi() to grab the same configuration values from inside the
callback of git_config()" issue, so whoever works on it to complete
the topic, it needs further work.

Re: [PATCH][Outreachy] New git config variable to specify string that will be automatically passed as --push-option

[thais braz]

Just to clarify I did not see Marius patch.
Did see Marius' comment saying he would look it in the leftoverbits list,
but since i didn't see any patch i thought i could work on it and did
so based on Stephan's comment
(which i suppose Mario also did and that is why the code resulted to
be similar).

And sorry send this email as patch. Didn't know how to use git
send-email just as reply

On Thu, Oct 12, 2017 at 12:26 AM, Junio C Hamano  wrote:
> Christian Couder  writes:
>
>>> Can somebody explain what is going on?
>>>
>>> I am guessing that Thais and marius are different people (judging by
>>> the fact that one CC's a message to the other).  Are you two
>>> collaborating on this change, or something?
>>
>> I guess that Thais decided to work on this, because we ask Outreachy
>> applicants to search for #leftoverbits mentions in the mailing list
>> archive to find small tasks they could work on.
>>
>> In this case it looks like Marius sent a patch a few hours before
>> Thais also sent one.
>
> ... after seeing Marius's already working on it, I think.
>
>> Thais, I am sorry, but as Marius sent a patch first, I think it is
>> better if you search for another different small task to work on.
>
> In general, I do not mind seeing people working together well, and
> it is one of the more important skills necessary in the open source
> community.  I however tend to agree with you that this is a bit too
> small a topic for multiple people to be working on.
>
>> Also please keep Peff and me in cc.
>
> Yup, that is always a good idea.
>



-- 
Atenciosamente Thais Diniz Braz

[PATCH] fetch doc: src side of refspec could be full SHA-1

[Junio C Hamano]

Since a9d34933 ("Merge branch 'fm/fetch-raw-sha1'", 2015-06-01) we
allow to fetch by an object name when the other side accepts such a
request, but we never updated the documentation to match.

Signed-off-by: Junio C Hamano 
---
 Documentation/pull-fetch-param.txt | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/pull-fetch-param.txt 
b/Documentation/pull-fetch-param.txt
index 1ebbf1d738..733f932479 100644
--- a/Documentation/pull-fetch-param.txt
+++ b/Documentation/pull-fetch-param.txt
@@ -23,9 +23,11 @@ ifdef::git-pull[]
 endif::git-pull[]
 +
 The format of a  parameter is an optional plus
-`+`, followed by the source ref , followed
+`+`, followed by the source , followed
 by a colon `:`, followed by the destination ref .
-The colon can be omitted when  is empty.
+The colon can be omitted when  is empty.   is most
+typically a ref, but it can also be an fully spelled hex object
+name.
 +
 `tag ` means the same as `refs/tags/:refs/tags/`;
 it requests fetching everything up to the given tag.
-- 
2.15.0-rc1-168-g2b9456ab46

[PATCH] patch reply

[Thais Diniz]

From: Thais Diniz Braz 

---
 emailReply | 4 
 1 file changed, 4 insertions(+)
 create mode 100644 emailReply

diff --git a/emailReply b/emailReply
new file mode 100644
index 0..2d591b55b
--- /dev/null
+++ b/emailReply
@@ -0,0 +1,4 @@
+Just to clarify I did not see Marius patch.
+Did see Marius' comment saying he would look it in the leftoverbits list,
+but since i didn't see any patch i thought i could work on it and did so based 
on Stephan's comment 
+(which i suppose Mario also did and that is why the code resulted to be 
similar).
-- 
2.15.0.rc0.39.g2f0e14e.dirty

Re: [PATCH] check-ref-format: require a repository for --branch

[Junio C Hamano]

Jeff King  writes:

> On Tue, Oct 17, 2017 at 10:22:31AM +0900, Junio C Hamano wrote:
>
>> > I like the state this puts us in, but there's one catch: we're
>> > completely changing the meaning of "check-ref-format --branch", aren't
>> > we?
>> >
>> > It is going from "this is how you resolve @{-1}" to "this is how you
>> > check the validity of a potential branch name". Do we need to pick a
>> > different name, and/or have a deprecation period?
>>  ...
>> At least that is what I wanted to happen in the patch.
>
> Ah, OK, I did not read carefully enough then. I think that would be OK,
> and probably close to what Jonathan was asking for.
>
> It leaves unresolved the fact that the resolving feature does not belong
> in check-ref-format in the first place, but we can just accept that as a
> historical wart.

Yup, I actually was in favor of removing that and making it a
"purely checking validity" feature, but given that it has been
advertised in the documentation since 604e0cb5 ("Documentation:
describe check-ref-format --branch", 2009-10-12), it is a bit too
late to tell users that rev-parse is the right/kosher thing to do.

> I don't think there is any need to prepare it upon my 4d03f955,
> though.  I'd think it could simply replace it.

Yeah, it ended up that way, it seems.  Still it needs a bit of doc
updates to balance the description.  Right now we stress on @{-n}
resolution too much.

Perhaps something like this?

 Documentation/git-check-ref-format.txt | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-check-ref-format.txt 
b/Documentation/git-check-ref-format.txt
index 92777cef25..cf0a0b7df2 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -77,7 +77,14 @@ reference name expressions (see linkgit:gitrevisions[7]):
 
 . at-open-brace `@{` is used as a notation to access a reflog entry.
 
-With the `--branch` option, it expands the ``previous branch syntax''
+With the `--branch` option, the command takes a name and checks if
+it can be used as a valid branch name (e.g. when creating a new
+branch).  The rule `git check-ref-format --branch $name` implements
+may be stricter than what `git check-ref-format refs/heads/$name`
+says (e.g. a dash may appear at the beginning of a ref component,
+but it is explicitly forbidden at the beginning of a branch name).
+When run with `--branch` option in a repository, the input is first
+expanded for the ``previous branch syntax''
 `@{-n}`.  For example, `@{-1}` is a way to refer the last branch you
 were on.  This option should be used by porcelains to accept this
 syntax anywhere a branch name is expected, so they can act as if you

Re: [PATCH v2] column: show auto columns when pager is active

[Junio C Hamano]

Kevin Daudt  writes:

> When columns are set to automatic for git tag and the output is
> paginated by git, the output is a single column instead of multiple
> columns.
>
> Standard behaviour in git is to honor auto values when the pager is
> active, which happens for example with commands like git log showing
> colors when being paged.
>
> Since ff1e72483 (tag: change default of `pager.tag` to "on",
> 2017-08-02), the pager has been enabled by default, exposing this
> problem to more people.
>
> finalize_colopts in column.c only checks whether the output is a TTY to
> determine if columns should be enabled with columns set to auto. Also
> check if the pager is active.
>
> Adding a test for git column is possible but requires some care to work
> around a race on stdin. See commit 18d8c2693 (test_terminal: redirect
> child process' stdin to a pty, 2015-08-04). Test git tag instead, since
> that does not involve stdin, and since that was the original motivation
> for this patch.

Nicely done.

> +test_expect_success TTY 'git tag with auto-columns ' '
> + test_commit one &&
> + test_commit two &&
> + test_commit three &&
> + test_commit four &&
> + test_commit five &&
> + cat >expected <<\EOF &&
> +initial  one  two  threefour five
> +EOF
> + test_terminal env PAGER="cat >actual.tag" COLUMNS=80 \
> + git -c column.ui=auto tag --sort=authordate &&
> + test_cmp expected actual.tag
> +'

I'd use <<-\EOF so that here document can be intended like other
tests, and also use expect vs actual that are used in the other
tests in the same script, instead of suddenly becoming creative
in only this single test.  I can do these clean-ups locally when
queuing so no need to resend only to collect these.

Thanks, will queue.

Re: [PATCH] doc: list filter-branch subdirectory-filter first

[Junio C Hamano]

David Glasser  writes:

> From: David Glasser 
>
> The docs claim that filters are applied in the listed order, so
> subdirectory-filter should come first.
> ---
>  Documentation/git-filter-branch.txt | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)

Good.  Could you sign it off?

Somewhat related tangent is that we may want to also reorder the
output from "git filter-branch -h" to the order of filter
application.  For that matter, the order in which the SYNOPSIS
section lists these command line arguments may want to match, both
for consistency and as an extra reminder to the users.

Re: Consider escaping special characters like 'less' does

[Jeff King]

On Tue, Oct 17, 2017 at 10:13:34AM +0900, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > Alternatively, I suppose we could just always escape in
> > add--interactive. I'm trying to think of a case where somebody would
> > really want their diffFilter to see the raw bytes (or vice versa, to
> > show raw bytes produced by their filter), but I'm having trouble coming
> > up with one.
> 
> Your patch below only implements the "tame the raw bytes that come
> out of their filter", which is quite agreeable.

Yes. I think that is probably OK, especially given that we continue to
allow terminal escapes (certainly some filters would want to use colors;
I don't know if any would want to use more exotic control codes).

> > I can't help but feel this is the tip of a larger iceberg, though. E.g.,
> > what about characters outside of the terminal's correct encoding? Or
> > broken UTF-8 characters?
> 
> Hmph.  If you use it as a "built-in" that is a fallback for
> diffFilter, i.e. use it only when the end user does not have one,
> then users can override whatever wrong thing the built-in logic does
> so... ;-)

Yes, and maybe that is the best way to do it. It just seems like it is
opening a can of worms about exactly which things should be filtered and
how.

I also wondered if people would be annoyed that by using a filter, they
don't get the benefit of the escaping, unless their filter implements it
separately on top (and the original purpose of the filter option was for
things like diff-highlight and diff-so-fancy, which do not do such
escaping).

-Peff

Re: [PATCH] check-ref-format: require a repository for --branch

[Jeff King]

On Tue, Oct 17, 2017 at 10:22:31AM +0900, Junio C Hamano wrote:

> > I like the state this puts us in, but there's one catch: we're
> > completely changing the meaning of "check-ref-format --branch", aren't
> > we?
> >
> > It is going from "this is how you resolve @{-1}" to "this is how you
> > check the validity of a potential branch name". Do we need to pick a
> > different name, and/or have a deprecation period?
> 
> That was not my intention.  When used in a repository, it behaves
> exactly the same as before, including @{-1} resolution part.  And by
> using strbuf_check_branch_ref(), it has always been checking the
> validity of a potential branch name, even though it wasn't
> advertised as such.  The documentation needs to be updated, I would
> think.
> 
> When used outside a repository, @{-1} would not have worked anyway,
> and @{-1} continues not to work, but the part that checks the
> validity should continue to work.
> 
> At least that is what I wanted to happen in the patch.

Ah, OK, I did not read carefully enough then. I think that would be OK,
and probably close to what Jonathan was asking for.

It leaves unresolved the fact that the resolving feature does not belong
in check-ref-format in the first place, but we can just accept that as a
historical wart.

I don't think there is any need to prepare it upon my 4d03f955, though.
I'd think it could simply replace it.

-Peff

Re: [PATCH] check-ref-format: require a repository for --branch

[Junio C Hamano]

Kevin Daudt  writes:

>> +setup_git_directory_gently();
>> +
>> +if (!nongit)
>> +malformed = (strbuf_check_branch_ref(, arg) ||
>> + !skip_prefix(sb.buf, "refs/heads/", ));
>> +else
>> +malformed = check_branch_ref_format(arg);
>> +
>
> Would it make sense to swap the logic and get rid of the double
> negative (!nongit)?

I am trying to follow the pattern "handle the normal case that have
been supported forever first, and then handle new exception next",
so that it is easier to see that there is no behaviour change in the
normal case, so I do not think it makes it easier to see to swap the
if/else cases.
>
>> +if (malformed)
>>  die("'%s' is not a valid branch name", arg);
>> -printf("%s\n", sb.buf + 11);
>> +printf("%s\n", name);
>> +strbuf_release();
>>  return 0;
>>  }
>>  

[L10N] Kickoff of translation for Git 2.15.0 round 2

[Jiang Xin]

Hi,

Git v2.15.0-rc1 released with a typo fix from commit dfab1eac23
("i18n: add a missing space in message", Sun Oct 8 14:18:39 2017 +0200).
This time there are 2 updated messages need to be translated since last
update.  Let's start new round of translation for Git 2.15.0.

You can get it from the usual place:

https://github.com/git-l10n/git-po/

As how to update your XX.po and help to translate Git, please see
"Updating a XX.po file" and other sections in “po/README" file.

--
Jiang Xin

Re: [PATCH v4 0/3] implement fetching of moved submodules

[Junio C Hamano]

Heiko Voigt  writes:

> The previous RFC iteration can be found here:
>
> https://public-inbox.org/git/20171006222544.GA26642@sandbox/
>
> This should now be in a state ready for review for inclusion.
>
> The main difference from last iteration is that we now also support
> unconfigured gitlinks for push and fetch for backwards compatibility.
>
> To implement this compatibility we construct a default name for gitlinks
> if there is a repository found at their location in the worktree.

I do not remember the details of the patch in the previous round
that corresponds to PATCH 2/3 here, so I cannot comment on the
incremental improvement between the two, but the fallback in 2/3
looks like a sensible thing to do.

Let's see what others, especially those who are interested in the
"--recurse-submodules" area, say.

Thanks.

Re: What's cooking in git.git (Oct 2017, #03; Mon, 16)

[brian m. carlson]

On Mon, Oct 16, 2017 at 03:54:56PM +0900, Junio C Hamano wrote:
> * bc/hash-algo (2017-10-04) 6 commits
>  - fixup! hash-algo: integrate hash algorithm support with repo setup
>  - hash-algo: switch empty tree and blob lookups to use hash abstraction
>  - hash-algo: integrate hash algorithm support with repo setup
>  - hash-algo: add structure representing hash algorithm
>  - setup: expose enumerated repo info
>  - Merge branch 'bc/vcs-svn-cleanup' into bc/hash-algo
> 
>  RFC
>  cf. <2017082122.26729-1-sand...@crustytoothpaste.net>

I do plan to reroll this, but might not get to it for a while.  Since it
breaks compilation with ASan and friends, feel free to drop it if that
is convenient for you or others, and I'll resubmit with some fixes.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [PATCH] check-ref-format: require a repository for --branch

[Kevin Daudt]

On Mon, Oct 16, 2017 at 07:45:46PM +0900, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
> [..]
> 
> diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
> index 1e5f9835f0..4e62852089 100644
> --- a/builtin/check-ref-format.c
> +++ b/builtin/check-ref-format.c
> @@ -38,12 +38,22 @@ static char *collapse_slashes(const char *refname)
>  
>  static int check_ref_format_branch(const char *arg)
>  {
> + int nongit, malformed;
>   struct strbuf sb = STRBUF_INIT;
> + const char *name = arg;
>  
> - setup_git_directory();
> - if (strbuf_check_branch_ref(, arg))
> + setup_git_directory_gently();
> +
> + if (!nongit)
> + malformed = (strbuf_check_branch_ref(, arg) ||
> +  !skip_prefix(sb.buf, "refs/heads/", ));
> + else
> + malformed = check_branch_ref_format(arg);
> +

Would it make sense to swap the logic and get rid of the double
negative (!nongit)?

> + if (malformed)
>   die("'%s' is not a valid branch name", arg);
> - printf("%s\n", sb.buf + 11);
> + printf("%s\n", name);
> + strbuf_release();
>   return 0;
>  }
>  

Re: [PATCH] check-ref-format: require a repository for --branch

[Junio C Hamano]

Jeff King  writes:

> On Mon, Oct 16, 2017 at 07:45:46PM +0900, Junio C Hamano wrote:
>
>> > So I think the right endgame in the longer term is:
>> > ...
>> 
>> Here is to illustrate what I mean in a patch form.  It resurrects
>> the gentle setup, and uses a purely textual format check when we are
>> outside the repository, while bypassing the @{magic} interpolation
>> codepath that requires us to be in a repository.  When we are in a
>> repository, we operate the same way as before.
>
> I like the state this puts us in, but there's one catch: we're
> completely changing the meaning of "check-ref-format --branch", aren't
> we?
>
> It is going from "this is how you resolve @{-1}" to "this is how you
> check the validity of a potential branch name". Do we need to pick a
> different name, and/or have a deprecation period?

That was not my intention.  When used in a repository, it behaves
exactly the same as before, including @{-1} resolution part.  And by
using strbuf_check_branch_ref(), it has always been checking the
validity of a potential branch name, even though it wasn't
advertised as such.  The documentation needs to be updated, I would
think.

When used outside a repository, @{-1} would not have worked anyway,
and @{-1} continues not to work, but the part that checks the
validity should continue to work.

At least that is what I wanted to happen in the patch.

Re: [PATCH v3 00/25] object_id part 10

[Junio C Hamano]

"brian m. carlson"  writes:

> On Mon, Oct 16, 2017 at 11:15:34AM +0900, Junio C Hamano wrote:
>> With a hope that this might help other reviewers, here is the
>> interdiff between "the previous one merged with v2.15-rc1" and "this
>> round applied on v2.15-rc1 directly".  
>> 
>> The changes all looked sensible to me.  Thanks.
>
> Is there a reasonably straightforward tool or workflow to generate
> interdiffs?  If so, I can include them in the future.

To me, it was straightforward to do:

$ git checkout master^0
$ git merge bc/object-id
... free conflict resolution thanks to rerere ...
$ git commit -a -m 'old one'
$ OLD=$(git describe)
$ git checkout master^0
$ git am bc-object-id.mbox
$ git diff $OLD

If you had a copy of my 'pu' branch, then you would have had a merge
commit that merges your previous version of the topic to it, and you
can feed that to contrib/rerere-train.sh to tell your rerere database
how I resolved the conflicts there, which may apply to the reproduction
of the above procedure yourself.

Re: Consider escaping special characters like 'less' does

[Junio C Hamano]

Jeff King  writes:

> Alternatively, I suppose we could just always escape in
> add--interactive. I'm trying to think of a case where somebody would
> really want their diffFilter to see the raw bytes (or vice versa, to
> show raw bytes produced by their filter), but I'm having trouble coming
> up with one.

Your patch below only implements the "tame the raw bytes that come
out of their filter", which is quite agreeable.

> I.e., something like this would probably help your case without hurting
> anybody:
> ...
>
> I can't help but feel this is the tip of a larger iceberg, though. E.g.,
> what about characters outside of the terminal's correct encoding? Or
> broken UTF-8 characters?

Hmph.  If you use it as a "built-in" that is a fallback for
diffFilter, i.e. use it only when the end user does not have one,
then users can override whatever wrong thing the built-in logic does
so... ;-)

Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration

[Junio C Hamano]

Jeff King  writes:

> On Sat, Oct 14, 2017 at 12:01:46PM +0900, Junio C Hamano wrote:
>
>> > That takes us back to the pre-regression state. The ancient bug from
>> > 4c7f1819 still exists, but that would be OK for v2.15. We'd probably
>> > want to bump the -rc cycle a bit to give more confidence that (2) caught
>> > everything.
>> 
>> Yes, I think that is the approach I was pushing initially with the
>> jc/ref-filter-colors-fix topic that was later retracted; the result
>> of your 4-patch series more or less matches that one, modulo that I
>> didn't treat for-each-ref as a plumbing.
>
> Ah, right, I forgot about that one while I was putting it together. So
> many alternatives floating around.
>
>> I do share the worry that
>> it is hard to make sure that these post-revert adjustment caught
>> everything; after all, that was a major part of the reason why my
>> earlier attempt was retracted.  I still think this is the _right_
>> direction to go in, even though it is harder to get right.
>
> To be honest, I'm not actually very worried. I think missing a
> post-revert adjustment is the main risk, but my general sense is that
> there hasn't been a lot going on with color fixes outside of my recent
> work. Famous last words and all that, I'm sure. :)
>
>> True.  Let's see what others think.  I know Jonathan is running
>> the fork at $work with "downgrade always to auto" patches, and while
>> I think both approaches would probably work well in practice, I have
>> preference for this "harder but right" approach, so I'd want to see
>> different views discussed on the list before we decide.
>
> After pondering over it, I have a slight preference for that, too. But
> I'm also happy to hear more input.

OK, so it seems we both have slight preference for the "peel back"
approach.  Adding Jonathan to Cc:

Re: [PATCH v3 00/25] object_id part 10

[Kevin Daudt]

On Mon, Oct 16, 2017 at 11:49:13PM +, brian m. carlson wrote:
> On Mon, Oct 16, 2017 at 11:15:34AM +0900, Junio C Hamano wrote:
> > With a hope that this might help other reviewers, here is the
> > interdiff between "the previous one merged with v2.15-rc1" and "this
> > round applied on v2.15-rc1 directly".  
> > 
> > The changes all looked sensible to me.  Thanks.
> 
> Is there a reasonably straightforward tool or workflow to generate
> interdiffs?  If so, I can include them in the future.
> -- 
> brian m. carlson / brian with sandals: Houston, Texas, US
> https://www.crustytoothpaste.net/~bmc | My opinion only
> OpenPGP: https://keybase.io/bk2204

tbdiff: https://github.com/trast/tbdiff

Re: [PATCH v3 00/25] object_id part 10

[brian m. carlson]

On Mon, Oct 16, 2017 at 11:15:34AM +0900, Junio C Hamano wrote:
> With a hope that this might help other reviewers, here is the
> interdiff between "the previous one merged with v2.15-rc1" and "this
> round applied on v2.15-rc1 directly".  
> 
> The changes all looked sensible to me.  Thanks.

Is there a reasonably straightforward tool or workflow to generate
interdiffs?  If so, I can include them in the future.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Adding a target prefix to git filter-branch --subdirectory-filter

[David Glasser]

git filter-branch --subdirectory-filter is really useful and easy to
use.  It's a commonly used step as part of moving a directory from one
repo to another.  It lets you move a subdirectory to the root of the
repo.

I've found that, when moving directories between repos, I often want
to do a task that is very similar to --subdirectory-filter but not
quite the same — I want to put the subdirectory under a prefix (and
maybe in this case the "subdirectory" should just be the entire repo).

Searching the web for  shows
that wanting to do something like this is pretty common.

It's certainly possible to do this with --index-filter or
--tree-filter, and the man page even has an example:

   git filter-branch --index-filter \
   'git ls-files -s | sed "s-\t\"*-/-" |
   GIT_INDEX_FILE=$GIT_INDEX_FILE.new \
   git update-index --index-info &&
mv "$GIT_INDEX_FILE.new" "$GIT_INDEX_FILE"' HEAD

But gosh, this is just a pain to write.

I'd like to add direct support to git filter-branch for having a
non-root target for subdirectory-filter.

I have a couple questions:

(1) What interface should this have? I'd lean towards having this just
be part of --subdirectory-filter as a separate option
--subdirectory-target-prefix.  For the "move root to subdir" you maybe
would have to type --subdirectory-filter=/, or maybe empty
--subdirectory-filter combined with --subdirectory-target-prefix does
the trick.

Alternatively, it could be a new filter type specific to moving
subdirectories around, but I don't know that that makes sense.

(2) The way I'd imagine I would implement this would be to replace the
current `git read-tree -i -m $commit:"$filter_subdir"` with `git
read-tree --empty && git read-tree --prefix=PREFIX/
$commit:"$filter_subdir"`.  --prefix is incompatible with -m, and I
don't really understand the importance of the stat reuse that is done
by `-i -m` single-tree merge. Is it OK to just drop the -i -m?

--dave

[PATCH] doc: list filter-branch subdirectory-filter first

[David Glasser]

From: David Glasser 

The docs claim that filters are applied in the listed order, so
subdirectory-filter should come first.
---
 Documentation/git-filter-branch.txt | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-filter-branch.txt 
b/Documentation/git-filter-branch.txt
index 9e5169aa64f4f..605583c0ad2b5 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -89,6 +89,11 @@ OPTIONS
can be used or modified in the following filter steps except
the commit filter, for technical reasons.
 
+--subdirectory-filter ::
+   Only look at the history which touches the given subdirectory.
+   The result will contain that directory (and only that) as its
+   project root. Implies <>.
+
 --env-filter ::
This filter may be used if you only need to modify the environment
in which the commit will be performed.  Specifically, you might
@@ -167,11 +172,6 @@ be removed, buyer beware. There is also no support for 
changing the
 author or timestamp (or the tag message for that matter). Tags which point
 to other tags will be rewritten to point to the underlying commit.
 
---subdirectory-filter ::
-   Only look at the history which touches the given subdirectory.
-   The result will contain that directory (and only that) as its
-   project root. Implies <>.
-
 --prune-empty::
Some filters will generate empty commits that leave the tree untouched.
This option instructs git-filter-branch to remove such commits if they

--
https://github.com/git/git/pull/415

Re: [PATCH/RFC] git-post: the opposite of git-cherry-pick

[Rafael Ascensao]

> This is worth discussing, though not my preference. The picture to "pick
> cherries" has become quite common, and now that we use it for the name of
> the command, "cherry-pick", the direction of flow is quite obvious and
> strongly implied: from somewhere else to me (and not to somebody else).

What if we borrow '--onto' from rebase and make it cherry-pick --onto
?

This would keep the "pick cherries" analogy, but add the "they're not
for me" intention.
It also carries a bit of the "transplant" meaning of rebase.

-Rafael Ascensão

Re: [PATCH] check-ref-format: require a repository for --branch

[Jonathan Nieder]

Jeff King wrote:
> On Mon, Oct 16, 2017 at 07:45:46PM +0900, Junio C Hamano wrote:

>> Here is to illustrate what I mean in a patch form.  It resurrects
>> the gentle setup, and uses a purely textual format check when we are
>> outside the repository, while bypassing the @{magic} interpolation
>> codepath that requires us to be in a repository.  When we are in a
>> repository, we operate the same way as before.
>
> I like the state this puts us in, but there's one catch: we're
> completely changing the meaning of "check-ref-format --branch", aren't
> we?
>
> It is going from "this is how you resolve @{-1}" to "this is how you
> check the validity of a potential branch name". Do we need to pick a
> different name, and/or have a deprecation period?

Sorry to take so long on picking this up.  I'll try to make an
alternate patch today.

For what it's worth, I don't agree with this repurposing of
"check-ref-format --branch" at all.  The old command already existed.
No one asked for the new command.  At most, we could get rid of the
old command after a deprecation period.  I don't understand at all why
it's worth the confusion of changing its meaning.

Thanks,
Jonathan

Re: Consider escaping special characters like 'less' does

[Jeff King]

On Tue, Oct 17, 2017 at 12:47:01AM +0200, Andreas Schwab wrote:

> On Okt 16 2017, Jeff King  wrote:
> 
> > I can't help but feel this is the tip of a larger iceberg, though. E.g.,
> > what about characters outside of the terminal's correct encoding? Or
> > broken UTF-8 characters?
> 
> Or correctly encoded UTF-8 characters that look confusing?  Or blobs
> with embedded escape sequences?

Yes, leaving ESC unfiltered here made me hesitate, too. We must allow it
to show colors, but showing diffs with raw terminal codes can be quite
confusing.

My general advice on committing unprintable characters is: don't.

-Peff

Re: Minor man page weirdness?

[Jeff King]

On Mon, Oct 16, 2017 at 07:16:49AM -0700, Lars Schneider wrote:

> Hi,
> 
> I just noticed that a space between "-f" and "git" is missing in `man 
> git-branch`.
> The space is present in "Documentation/git-branch.txt", though. I am using 
> `man`
> version 1.6c on macOS.
> 
> -f, --force
>Reset  to  if  exists already. 
> Without
>-fgit branch refuses to change an existing branch. In combination 
> with -d (or
> ^^
> 
> Can you reproduce the "problem"?

I don't see it on my copy (Debian man-db 2.7.6.1-2) . What does:

  cd Documentation
  make git-branch.1
  grep Without git-branch.xml

show? I see:

  ... -f git branch ...

If there's no space there, then the problem is in asciidoc. If not, then
we can further check:

  grep -A3 Without git-branch.1

I get:

  Reset  to  if  exists already\&. Without
  \fB\-f\fR
  \fIgit branch\fR
  refuses to change an existing branch\&. In combination with

Since there's no space there, I think we're relying on roff to insert
one between lines. I'm not familiar enough with roff to say if that's a
reasonable expectation or not. But if the problem is at this level, it's
actually an issue between docbook and roff, and there's probably not a
lot we can do on the Git side.

We do have some hacks/workarounds for broken versions of the toolchain.
You can try tweaking various knobs you find in Documentation/Makefile).
DOCBOOK_SUPPRESS_SP sounds promising, but I think it actually does the
opposite (removes extra spaces).

-Peff

Re: Consider escaping special characters like 'less' does

[Andreas Schwab]

On Okt 16 2017, Jeff King  wrote:

> I can't help but feel this is the tip of a larger iceberg, though. E.g.,
> what about characters outside of the terminal's correct encoding? Or
> broken UTF-8 characters?

Or correctly encoded UTF-8 characters that look confusing?  Or blobs
with embedded escape sequences?

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

Re: [PATCH] check-ref-format: require a repository for --branch

[Jeff King]

On Mon, Oct 16, 2017 at 07:45:46PM +0900, Junio C Hamano wrote:

> > So I think the right endgame in the longer term is:
> > ...
> 
> Here is to illustrate what I mean in a patch form.  It resurrects
> the gentle setup, and uses a purely textual format check when we are
> outside the repository, while bypassing the @{magic} interpolation
> codepath that requires us to be in a repository.  When we are in a
> repository, we operate the same way as before.

I like the state this puts us in, but there's one catch: we're
completely changing the meaning of "check-ref-format --branch", aren't
we?

It is going from "this is how you resolve @{-1}" to "this is how you
check the validity of a potential branch name". Do we need to pick a
different name, and/or have a deprecation period?

-Peff

Re: [PATCH] check-ref-format: require a repository for --branch

[Jeff King]

On Mon, Oct 16, 2017 at 03:44:08PM +0900, Junio C Hamano wrote:

> I threw this topic in stalled category, hoping that one or the other
> opinion eventually turns out to be more prevalent, but it didn't
> seem to have happened X-<.

I think it's sufficiently obscure that nobody really cares.

I admit that _I_ don't actually care myself. We should fix the BUG(),
obviously, but between the two I could live with it either way. Mostly I
didn't want to go to the work to write the patch for the direction that
I didn't think was right, and was hoping Jonathan would if he felt
strongly about it.

> So I think the right endgame in the longer term is:

I won't quote the rest of your message because I agree with it
completely, in terms of the endgame we'd like to see.

I'll address a few specific comments on your followup patch.

-Peff

Re: Consider escaping special characters like 'less' does

[Jeff King]

On Sun, Oct 15, 2017 at 11:37:04PM +0200, Joris Valette wrote:

> 2017-10-15 22:06 GMT+02:00 Jeff King :
> > Git's diff generation will never do such escaping by default, because it
> > means creating a patch that cannot be applied to get back the original
> > content.
> 
> Indeed this would be a problem. That's where my knowledge of git's
> source code ends, but in that case, can't the output be discriminated
> against the command that was executed?
> Command that outputs an applicable patch -> don't escape
> Command that outputs a diff to see changes -> escape

Speaking in a general sense, people use "git diff" for both purposes,
and we can't necessarily tell which[1]. As a matter of fact, that's a
potential problem with textconv filters as well (which are enabled by
default for git-diff, but not for plumbing like diff-tree, format-patch,
etc). I think the feature isn't widely used enough for people to run
into problems (and they also use it only on things that they don't
_expect_ to be able to make patches for, since they're binaries).

[1] Of course we can come up with heuristics, like "is stdout going to a
a terminal"? But in such a case we already kick in the pager, and it
does the exact escaping you're asking for. :)

For a command like add--interactive, it knows which invocations are for
showing to the user and which are for applying (and it already uses
"--color" selectively, for instance). So if there were an "escape this"
option in git's diff generation, we could selectively pass it. But
again, I think the right solution there is not building the escaping
into Git, but just passing it through another filter.

> > It doesn't seem out of the question to me to have an out-of-the-box
> > default for interactive.diffFilter which does some basic escaping (we
> > could even implement it inside the perl script for efficiency).
> 
> Yes I read this thread, but I was left unsatisfied because I would
> like something out-of-the-box.
> Your suggestion might be the best solution then: implement some
> default escaping for interactive.diffFilter.

Alternatively, I suppose we could just always escape in
add--interactive. I'm trying to think of a case where somebody would
really want their diffFilter to see the raw bytes (or vice versa, to
show raw bytes produced by their filter), but I'm having trouble coming
up with one.

I.e., something like this would probably help your case without hurting
anybody:

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 28b325d754..d44e5ea459 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -714,6 +714,16 @@ sub parse_diff {
push @{$hunk[-1]{DISPLAY}},
(@colored ? $colored[$i] : $diff[$i]);
}
+
+   foreach my $hunk (@hunk) {
+   foreach my $line (@{$hunk->{DISPLAY}}) {
+   # all control chars minus newline and ESC (for color)
+   if ($line =~ s/[\000-\011\013-\032\034-\037]/?/g) {
+   $hunk->{CONTROLCHARS} = 1;
+   }
+   }
+   }
+
return @hunk;
 }
 
@@ -1407,6 +1417,9 @@ sub patch_update_file {
if ($hunk[$ix]{TYPE} eq 'hunk') {
$other .= ',e';
}
+   if ($hunk[$ix]->{CONTROLCHARS}) {
+   print "warning: control characters in hunk have been 
replaced by '?'\n";
+   }
for (@{$hunk[$ix]{DISPLAY}}) {
print;
}

I can't help but feel this is the tip of a larger iceberg, though. E.g.,
what about characters outside of the terminal's correct encoding? Or
broken UTF-8 characters?

-Peff

Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration

[Jeff King]

On Sat, Oct 14, 2017 at 12:01:46PM +0900, Junio C Hamano wrote:

> > That takes us back to the pre-regression state. The ancient bug from
> > 4c7f1819 still exists, but that would be OK for v2.15. We'd probably
> > want to bump the -rc cycle a bit to give more confidence that (2) caught
> > everything.
> 
> Yes, I think that is the approach I was pushing initially with the
> jc/ref-filter-colors-fix topic that was later retracted; the result
> of your 4-patch series more or less matches that one, modulo that I
> didn't treat for-each-ref as a plumbing.

Ah, right, I forgot about that one while I was putting it together. So
many alternatives floating around.

> I do share the worry that
> it is hard to make sure that these post-revert adjustment caught
> everything; after all, that was a major part of the reason why my
> earlier attempt was retracted.  I still think this is the _right_
> direction to go in, even though it is harder to get right.

To be honest, I'm not actually very worried. I think missing a
post-revert adjustment is the main risk, but my general sense is that
there hasn't been a lot going on with color fixes outside of my recent
work. Famous last words and all that, I'm sure. :)

> True.  Let's see what others think.  I know Jonathan is running
> the fork at $work with "downgrade always to auto" patches, and while
> I think both approaches would probably work well in practice, I have
> preference for this "harder but right" approach, so I'd want to see
> different views discussed on the list before we decide.

After pondering over it, I have a slight preference for that, too. But
I'm also happy to hear more input.

-Peff

Re: [PATCH] diff: alias -q to --quiet

[Jeff King]

On Sat, Oct 14, 2017 at 11:37:28AM +0900, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > So there are two separate questions/tasks:
> >
> >   1. Should we remove the special handling of "-q" leftover from this
> >  deprecation? I think the answer is yes.
> >
> >   2. Should we teach the diff machinery as a whole to treat "-q" as a
> >  synonym for "--quiet".
> 
> Good questions.  And thanks for archaeology.
> 
> The topic #1 above is something that should have happened when "-q" stopped 
> working
> as "--diff-filter=d", and we probably should have started to error
> out then, so that scripts that relied on the original behaviour
> would have been forced to update.  That did not happen which was a
> grave mistake.
> 
> By doing so, we would have made sure any script that uses "-q" died
> out, and after a while, we can talk about reusing it for other
> purposes, like the topic #2 above.
> 
> Is it worth making "-q" error out while doing #1 and keep it error
> out for a few years?  I have a feeling that the answer might be
> unfortunately yes _if_ we want to also do #2.  Even though we broke
> "-q" for the scripts who wanted to see it ignore only the removals 4
> years ago and left it broken since then.  Removals are much rarer
> than modifications and additions, so it wouldn't be surprising if
> the users of these scripts simply did not notice the old breakage,
> but if we made "-q" to mean "--quiet" without doing #1, they will
> break, as all diffs these scripts work on will suddenly give an
> empty output.

Yeah, after thinking about it, I do think we'd want to restart the
deprecation period. For some features it would be fine, but this one is
sufficiently subtle that I agree there's a good chance scripts might
have been broken without anybody noticing them.

> If we aren't doing #2, then I do not think we need to make "-q"
> error out when we do #1, though.

I don't think we'd add an explicit error-out. But by removing the
leftover code, we would naturally say "no such option: -q", which
amounts to the same thing.

> In any case, if we were to do both of the above two, they must
> happen in that order, not the other way around.

Yep, agreed.

-Peff

Re: [PATCH 3/3] branch: forbid refs/heads/HEAD

[Jeff King]

On Sat, Oct 14, 2017 at 11:20:11AM +0900, Junio C Hamano wrote:

> Junio C Hamano  writes:
> 
> >> Should we test that:
> >>
> >>   git update-ref refs/heads/HEAD HEAD^
> >>
> >> continues to work?
> >
> > Perhaps.  Also we may want to make sure that "git branch -D HEAD"
> > still works as a way to recover from historical mistakes.
> 
> The only difference is improved tests where we use show-ref to make
> sure refs/heads/HEAD does not exist when it shouldn't, exercise
> update-ref to create and delete refs/heads/HEAD, and also make sure
> it can be deleted with "git branch -d".

Thanks, this looks good to me.

-Peff

Re: [PATCH v4 03/11] protocol: introduce protocol extension mechanisms

[Kevin Daudt]

On Mon, Oct 16, 2017 at 10:55:24AM -0700, Brandon Williams wrote:
> Create protocol.{c,h} and provide functions which future servers and
> clients can use to determine which protocol to use or is being used.
> 
> Also introduce the 'GIT_PROTOCOL' environment variable which will be
> used to communicate a colon separated list of keys with optional values
> to a server.  Unknown keys and values must be tolerated.  This mechanism
> is used to communicate which version of the wire protocol a client would
> like to use with a server.
> 
> Signed-off-by: Brandon Williams 
> ---
>  Documentation/config.txt | 17 +++
>  Documentation/git.txt|  6 
>  Makefile |  1 +
>  cache.h  |  8 +
>  protocol.c   | 79 
> 
>  protocol.h   | 33 
>  6 files changed, 144 insertions(+)
>  create mode 100644 protocol.c
>  create mode 100644 protocol.h
>
> [...]
> 
> diff --git a/protocol.h b/protocol.h
> new file mode 100644
> index 0..1b2bc94a8
> --- /dev/null
> +++ b/protocol.h
> @@ -0,0 +1,33 @@
> +#ifndef PROTOCOL_H
> +#define PROTOCOL_H
> +
> +enum protocol_version {
> + protocol_unknown_version = -1,
> + protocol_v0 = 0,
> + protocol_v1 = 1,
> +};
> +
> +/*
> + * Used by a client to determine which protocol version to request be used 
> when
> + * communicating with a server, reflecting the configured value of the
> + * 'protocol.version' config.  If unconfigured, a value of 'protocol_v0' is
> + * returned.
> + */

The first sentence reads a little weird to me around 'which version to
request be used'. 

Re: [ANNOUNCE] Git for Windows 2.14.2(3)

[Steve Hoelzer]

Johannes,

On Mon, Oct 16, 2017 at 5:57 AM, Johannes Schindelin
 wrote:
> Hi Steve,
>
> On Sun, 15 Oct 2017, Johannes Schindelin wrote:
>
>> On Fri, 13 Oct 2017, Steve Hoelzer wrote:
>>
>> > On Thu, Oct 12, 2017 at 5:53 PM, Johannes Schindelin
>> >  wrote:
>> > >
>> > > It is my pleasure to announce that Git for Windows 2.14.2(3) is
>> > > available from:
>> > >
>> > > https://git-for-windows.github.io/
>> > >
>> > > Changes since Git for Windows v2.14.2(2) (October 5th 2017)
>> > >
>> > > New Features
>> > >
>> > >   * Comes with Git LFS v2.3.3.
>> >
>> > I just ran "git update" and afterward "git version" reported
>> > 2.14.2(3), but "git lfs version" still said 2.3.2.
>> >
>> > I also uninstalled/reinstalled Git for Windows and LFS is still 2.3.2.
>>
>> Ah bummer. I forgot to actually update it in the VM where I build the
>> releases :-(
>>
>> Will work on it tomorrow.
>
> I'll actually use this opportunity to revamp a part of Git for Windows'
> release engineering process to try to prevent similar things from
> happening in the future.
>
> Also, cURL v7.56.1 is slated to be released in exactly one week, and I
> have some important installer work to do this week, so I'll just defer the
> new Git for Windows version tentatively to Monday, 23rd.
>
> Git LFS users can always install Git LFS v2.3.3 specifically in the
> meantime, or use Git for Windows' snapshot versions
> (https://wingit.blob.core.windows.net/files/index.html).

Sounds like a good plan.

I think I have successfully updated to LFS 2.3.3 by copying the new
git-lfs.exe into C:\Program Files\Git\mingw64\bin. Is that right way
to do it?

Thanks,
Steve

[PATCH v2] column: show auto columns when pager is active

[Kevin Daudt]

When columns are set to automatic for git tag and the output is
paginated by git, the output is a single column instead of multiple
columns.

Standard behaviour in git is to honor auto values when the pager is
active, which happens for example with commands like git log showing
colors when being paged.

Since ff1e72483 (tag: change default of `pager.tag` to "on",
2017-08-02), the pager has been enabled by default, exposing this
problem to more people.

finalize_colopts in column.c only checks whether the output is a TTY to
determine if columns should be enabled with columns set to auto. Also
check if the pager is active.

Adding a test for git column is possible but requires some care to work
around a race on stdin. See commit 18d8c2693 (test_terminal: redirect
child process' stdin to a pty, 2015-08-04). Test git tag instead, since
that does not involve stdin, and since that was the original motivation
for this patch.

Helped-by: Rafael Ascensão 
Signed-off-by: Kevin Daudt 
---
Changes since v1:

- Remove redundant -p flag in tests
- Explain why git tag is being tested instead of the more obvious git
  column

 column.c |  3 ++-
 t/t7006-pager.sh | 14 ++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/column.c b/column.c
index ff7bdab1a..ded50337f 100644
--- a/column.c
+++ b/column.c
@@ -5,6 +5,7 @@
 #include "parse-options.h"
 #include "run-command.h"
 #include "utf8.h"
+#include "pager.c"
 
 #define XY2LINEAR(d, x, y) (COL_LAYOUT((d)->colopts) == COL_COLUMN ? \
(x) * (d)->rows + (y) : \
@@ -224,7 +225,7 @@ int finalize_colopts(unsigned int *colopts, int 
stdout_is_tty)
if (stdout_is_tty < 0)
stdout_is_tty = isatty(1);
*colopts &= ~COL_ENABLE_MASK;
-   if (stdout_is_tty)
+   if (stdout_is_tty || pager_in_use())
*colopts |= COL_ENABLED;
}
return 0;
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index f0f1abd1c..e985b6873 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -570,4 +570,18 @@ test_expect_success 'command with underscores does not 
complain' '
test_cmp expect actual
 '
 
+test_expect_success TTY 'git tag with auto-columns ' '
+   test_commit one &&
+   test_commit two &&
+   test_commit three &&
+   test_commit four &&
+   test_commit five &&
+   cat >expected <<\EOF &&
+initial  one  two  threefour five
+EOF
+   test_terminal env PAGER="cat >actual.tag" COLUMNS=80 \
+   git -c column.ui=auto tag --sort=authordate &&
+   test_cmp expected actual.tag
+'
+
 test_done
-- 
2.14.2

Re: slight addition to t.gummerer's proposed "git stash" patch

[Thomas Gummerer]

On 10/11, Robert P. J. Day wrote:
> On Wed, 11 Oct 2017, Thomas Gummerer wrote:
> 
> > On 10/11, Robert P. J. Day wrote:
> > >
> > >   was perusing thomas gummerer's proposed "git stash" patch here:
> > >
> > > https://www.spinics.net/lists/git/msg313993.html
> > >
> > > and i'd make one more change -- i'd separate the OPTIONS entries for
> > > "git stash push" and "git stash save" so they don't end up being
> > > rendered all crushed together when displaying the man page:
> >
> > I for one would like that.  I sent a patch recently [1] that would
> > show git stash push first on the man page, which didn't seem to get
> > much traction.  This goes a bit further than that, which I'd be happy
> > with.
> >
> > [1]: 
> > https://public-inbox.org/git/20171005201029.4173-1-t.gumme...@gmail.com/
> 
>   ... snip ...
> 
> if you want, just crush my suggestion into your earlier patch and
> resubmit it.

Thanks, before doing that let me see where that discussion goes.  My
plan was to be a bit more careful and first get rid of mentions of
'git stash save', and mark it deprecated as a next step.  In which
case I'd submit a patch with your suggestions in a few cycles.

> rday
> 
> -- 
> 
> 
> Robert P. J. Day Ottawa, Ontario, CANADA
> http://crashcourse.ca
> 
> Twitter:   http://twitter.com/rpjday
> LinkedIn:   http://ca.linkedin.com/in/rpjday
> 

Minor man page weirdness?

[Lars Schneider]

Hi,

I just noticed that a space between "-f" and "git" is missing in `man 
git-branch`.
The space is present in "Documentation/git-branch.txt", though. I am using `man`
version 1.6c on macOS.

-f, --force
   Reset  to  if  exists already. 
Without
   -fgit branch refuses to change an existing branch. In combination 
with -d (or
^^

Can you reproduce the "problem"?

Cheers,
Lars

Re: [RFC] deprecate git stash save?

[Thomas Gummerer]

On 10/12, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
> > Thomas Gummerer  writes:
> >
> >> git stash push is the newer interface for creating a stash.  While we
> >> are still keeping git stash save around for the time being, it's better
> >> to point new users of git stash to the more modern (and more feature
> >> rich) interface, instead of teaching them the older version that we
> >> might want to phase out in the future.
> >
> > With devil's advocate hat on, because the primary point of "stash"
> > being "clear the desk quickly", I do not necessarily agree that
> > "more feature rich" is a plus and something we should nudge newbies
> > towards.
> 
> I do not particulary view "feature richness" is the primary benefit
> of 'push' that makes it shine.  'save' has a quirk in the command
> line option syntax, but 'push' corrects it, and that is why we want
> to move users towards the latter.

I agree it's not the primary benefit (hence why it's in braces :)),
but I at least some users will eventually want the features 'git stash
push' has to offer, and it's better if they don't have to re-train
their fingers at that point.

But yeah, fixing the quirky command line interface is definitely the
stronger reason for deprecating it.

> IOW, I do not object to the agenda; it is just I found the
> justification used to push the agenda forward was flawed.

I'm happy as long as we agree on the agenda here.  Any opinions about
the patches themselves?  Would you prefer me to resend with an updated
description?

> Thanks.

[PATCH v4 11/11] Documentation: document Extra Parameters

[Brandon Williams]

From: Jonathan Tan 

Document the server support for Extra Parameters, additional information
that the client can send in its first message to the server during a
Git client-server interaction.

Signed-off-by: Jonathan Tan 
Signed-off-by: Brandon Williams 
---
 Documentation/technical/http-protocol.txt |  8 ++
 Documentation/technical/pack-protocol.txt | 43 ++-
 2 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/Documentation/technical/http-protocol.txt 
b/Documentation/technical/http-protocol.txt
index 1c561bdd9..a0e45f288 100644
--- a/Documentation/technical/http-protocol.txt
+++ b/Documentation/technical/http-protocol.txt
@@ -219,6 +219,10 @@ smart server reply:
S: 003c2cb58b79488a98d2721cea644875a8dd0026b115 refs/tags/v1.0\n
S: 003fa3c2e2402b99163d1d59756e5f207ae21cccba4c refs/tags/v1.0^{}\n
 
+The client may send Extra Parameters (see
+Documentation/technical/pack-protocol.txt) as a colon-separated string
+in the Git-Protocol HTTP header.
+
 Dumb Server Response
 
 Dumb servers MUST respond with the dumb server reply format.
@@ -269,7 +273,11 @@ the C locale ordering.  The stream SHOULD include the 
default ref
 named `HEAD` as the first ref.  The stream MUST include capability
 declarations behind a NUL on the first ref.
 
+The returned response contains "version 1" if "version=1" was sent as an
+Extra Parameter.
+
   smart_reply =  PKT-LINE("# service=$servicename" LF)
+*1("version 1")
 ref_list
 ""
   ref_list=  empty_list / non_empty_list
diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index ed1eae8b8..cd31edc91 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -39,6 +39,19 @@ communicates with that invoked process over the SSH 
connection.
 The file:// transport runs the 'upload-pack' or 'receive-pack'
 process locally and communicates with it over a pipe.
 
+Extra Parameters
+
+
+The protocol provides a mechanism in which clients can send additional
+information in its first message to the server. These are called "Extra
+Parameters", and are supported by the Git, SSH, and HTTP protocols.
+
+Each Extra Parameter takes the form of `=` or ``.
+
+Servers that receive any such Extra Parameters MUST ignore all
+unrecognized keys. Currently, the only Extra Parameter recognized is
+"version=1".
+
 Git Transport
 -
 
@@ -46,18 +59,25 @@ The Git transport starts off by sending the command and 
repository
 on the wire using the pkt-line format, followed by a NUL byte and a
 hostname parameter, terminated by a NUL byte.
 
-   0032git-upload-pack /project.git\0host=myserver.com\0
+   0033git-upload-pack /project.git\0host=myserver.com\0
+
+The transport may send Extra Parameters by adding an additional NUL
+byte, and then adding one or more NUL-terminated strings:
+
+   003egit-upload-pack /project.git\0host=myserver.com\0\0version=1\0
 
 --
-   git-proto-request = request-command SP pathname NUL [ host-parameter NUL ]
+   git-proto-request = request-command SP pathname NUL
+  [ host-parameter NUL ] [ NUL extra-parameters ]
request-command   = "git-upload-pack" / "git-receive-pack" /
   "git-upload-archive"   ; case sensitive
pathname  = *( %x01-ff ) ; exclude NUL
host-parameter= "host=" hostname [ ":" port ]
+   extra-parameters  = 1*extra-parameter
+   extra-parameter   = 1*( %x01-ff ) NUL
 --
 
-Only host-parameter is allowed in the git-proto-request. Clients
-MUST NOT attempt to send additional parameters. It is used for the
+host-parameter is used for the
 git-daemon name based virtual hosting.  See --interpolated-path
 option to git daemon, with the %H/%CH format characters.
 
@@ -117,6 +137,12 @@ we execute it without the leading '/'.
 v
ssh u...@example.com "git-upload-pack '~alice/project.git'"
 
+Depending on the value of the `protocol.version` configuration variable,
+Git may attempt to send Extra Parameters as a colon-separated string in
+the GIT_PROTOCOL environment variable. This is done only if
+the `ssh.variant` configuration variable indicates that the ssh command
+supports passing environment variables as an argument.
+
 A few things to remember here:
 
 - The "command name" is spelled with dash (e.g. git-upload-pack), but
@@ -137,11 +163,13 @@ Reference Discovery
 ---
 
 When the client initially connects the server will immediately respond
-with a listing of each reference it has (all branches and tags) along
+with a version number (if "version=1" is sent as an Extra Parameter),
+and a listing of each reference it has (all branches and tags) along
 with the object name that each reference currently points to.
 
-   $ echo -e -n 

[PATCH v4 10/11] ssh: introduce a 'simple' ssh variant

[Brandon Williams]

When using the 'ssh' transport, the '-o' option is used to specify an
environment variable which should be set on the remote end.  This allows
git to send additional information when contacting the server,
requesting the use of a different protocol version via the
'GIT_PROTOCOL' environment variable like so: "-o SendEnv=GIT_PROTOCOL".

Unfortunately not all ssh variants support the sending of environment
variables to the remote end.  To account for this, only use the '-o'
option for ssh variants which are OpenSSH compliant.  This is done by
checking that the basename of the ssh command is 'ssh' or the ssh
variant is overridden to be 'ssh' (via the ssh.variant config).

Other options like '-p' and '-P', which are used to specify a specific
port to use, or '-4' and '-6', which are used to indicate that IPV4 or
IPV6 addresses should be used, may also not be supported by all ssh
variants.

Currently if an ssh command's basename wasn't 'plink' or
'tortoiseplink' git assumes that the command is an OpenSSH variant.
Since user configured ssh commands may not be OpenSSH compliant, tighten
this constraint and assume a variant of 'simple' if the basename of the
command doesn't match the variants known to git.  The new ssh variant
'simple' will only have the host and command to execute ([username@]host
command) passed as parameters to the ssh command.

Update the Documentation to better reflect the command-line options sent
to ssh commands based on their variant.

Reported-by: Jeffrey Yasskin 
Signed-off-by: Brandon Williams 
---
 Documentation/config.txt |  27 ++--
 Documentation/git.txt|   9 ++--
 connect.c| 108 ++-
 t/t5601-clone.sh |  26 +---
 t/t5700-protocol-v1.sh   |   2 +
 5 files changed, 111 insertions(+), 61 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index b78747abc..0460af37e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2084,12 +2084,31 @@ ssh.variant::
Depending on the value of the environment variables `GIT_SSH` or
`GIT_SSH_COMMAND`, or the config setting `core.sshCommand`, Git
auto-detects whether to adjust its command-line parameters for use
-   with plink or tortoiseplink, as opposed to the default (OpenSSH).
+   with ssh (OpenSSH), plink or tortoiseplink, as opposed to the default
+   (simple).
 +
 The config variable `ssh.variant` can be set to override this auto-detection;
-valid values are `ssh`, `plink`, `putty` or `tortoiseplink`. Any other value
-will be treated as normal ssh. This setting can be overridden via the
-environment variable `GIT_SSH_VARIANT`.
+valid values are `ssh`, `simple`, `plink`, `putty` or `tortoiseplink`. Any
+other value will be treated as normal ssh. This setting can be overridden via
+the environment variable `GIT_SSH_VARIANT`.
++
+The current command-line parameters used for each variant are as
+follows:
++
+--
+
+* `ssh` - [-p port] [-4] [-6] [-o option] [username@]host command
+
+* `simple` - [username@]host command
+
+* `plink` or `putty` - [-P port] [-4] [-6] [username@]host command
+
+* `tortoiseplink` - [-P port] [-4] [-6] -batch [username@]host command
+
+--
++
+Except for the `simple` variant, command-line parameters are likely to
+change as git gains new features.
 
 i18n.commitEncoding::
Character encoding the commit messages are stored in; Git itself
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 7518ea3af..8bc3f2147 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -518,11 +518,10 @@ other
If either of these environment variables is set then 'git fetch'
and 'git push' will use the specified command instead of 'ssh'
when they need to connect to a remote system.
-   The command will be given exactly two or four arguments: the
-   'username@host' (or just 'host') from the URL and the shell
-   command to execute on that remote system, optionally preceded by
-   `-p` (literally) and the 'port' from the URL when it specifies
-   something other than the default SSH port.
+   The command-line parameters passed to the configured command are
+   determined by the ssh variant.  See `ssh.variant` option in
+   linkgit:git-config[1] for details.
+
 +
 `$GIT_SSH_COMMAND` takes precedence over `$GIT_SSH`, and is interpreted
 by the shell, which allows additional arguments to be included.
diff --git a/connect.c b/connect.c
index b8695a2fa..7fbd396b3 100644
--- a/connect.c
+++ b/connect.c
@@ -776,37 +776,44 @@ static const char *get_ssh_command(void)
return NULL;
 }
 
-static int override_ssh_variant(int *port_option, int *needs_batch)
+enum ssh_variant {
+   VARIANT_SIMPLE,
+   VARIANT_SSH,
+   VARIANT_PLINK,
+   VARIANT_PUTTY,
+   VARIANT_TORTOISEPLINK,
+};
+
+static int override_ssh_variant(enum ssh_variant *ssh_variant)
 

[PATCH v4 00/11] protocol transition

[Brandon Williams]

Changes in v4:
 * Added more tests for the new handeling of ssh variants.
 * Removed the 'default' case in upload_pack and receive_pack and instead
   ensured that all enum values were accounted for.  This way when a new
   protocol version is introduced the compiler will throw an error if the new
   protocol version isn't accounted for in these switch statements.
 * Added Jonathan's Documentation patch ontop of the series (with the small
   change I pointed out in reply to the patch itself)
 * A few other small changes due to reviewer comments.


Brandon Williams (9):
  pkt-line: add packet_write function
  protocol: introduce protocol extension mechanisms
  daemon: recognize hidden request arguments
  upload-pack, receive-pack: introduce protocol version 1
  connect: teach client to recognize v1 server response
  connect: tell server that the client understands v1
  http: tell server that the client understands v1
  i5700: add interop test for protocol transition
  ssh: introduce a 'simple' ssh variant

Jonathan Tan (2):
  connect: in ref advertisement, shallows are last
  Documentation: document Extra Parameters

 Documentation/config.txt  |  44 +++-
 Documentation/git.txt |  15 +-
 Documentation/technical/http-protocol.txt |   8 +
 Documentation/technical/pack-protocol.txt |  43 +++-
 Makefile  |   1 +
 builtin/receive-pack.c|  17 ++
 cache.h   |  10 +
 connect.c | 354 --
 daemon.c  |  71 +-
 http.c|  18 ++
 pkt-line.c|   6 +
 pkt-line.h|   1 +
 protocol.c|  79 +++
 protocol.h|  33 +++
 t/interop/i5700-protocol-transition.sh|  68 ++
 t/lib-httpd/apache.conf   |   7 +
 t/t5601-clone.sh  |  26 ++-
 t/t5700-protocol-v1.sh| 294 +
 upload-pack.c |  20 +-
 19 files changed, 967 insertions(+), 148 deletions(-)
 create mode 100644 protocol.c
 create mode 100644 protocol.h
 create mode 100755 t/interop/i5700-protocol-transition.sh
 create mode 100755 t/t5700-protocol-v1.sh

--- interdiff with 'origin/bw/protocol-v1'


diff --git a/Documentation/technical/http-protocol.txt 
b/Documentation/technical/http-protocol.txt
index 1c561bdd9..a0e45f288 100644
--- a/Documentation/technical/http-protocol.txt
+++ b/Documentation/technical/http-protocol.txt
@@ -219,6 +219,10 @@ smart server reply:
S: 003c2cb58b79488a98d2721cea644875a8dd0026b115 refs/tags/v1.0\n
S: 003fa3c2e2402b99163d1d59756e5f207ae21cccba4c refs/tags/v1.0^{}\n
 
+The client may send Extra Parameters (see
+Documentation/technical/pack-protocol.txt) as a colon-separated string
+in the Git-Protocol HTTP header.
+
 Dumb Server Response
 
 Dumb servers MUST respond with the dumb server reply format.
@@ -269,7 +273,11 @@ the C locale ordering.  The stream SHOULD include the 
default ref
 named `HEAD` as the first ref.  The stream MUST include capability
 declarations behind a NUL on the first ref.
 
+The returned response contains "version 1" if "version=1" was sent as an
+Extra Parameter.
+
   smart_reply =  PKT-LINE("# service=$servicename" LF)
+*1("version 1")
 ref_list
 ""
   ref_list=  empty_list / non_empty_list
diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index ed1eae8b8..cd31edc91 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -39,6 +39,19 @@ communicates with that invoked process over the SSH 
connection.
 The file:// transport runs the 'upload-pack' or 'receive-pack'
 process locally and communicates with it over a pipe.
 
+Extra Parameters
+
+
+The protocol provides a mechanism in which clients can send additional
+information in its first message to the server. These are called "Extra
+Parameters", and are supported by the Git, SSH, and HTTP protocols.
+
+Each Extra Parameter takes the form of `=` or ``.
+
+Servers that receive any such Extra Parameters MUST ignore all
+unrecognized keys. Currently, the only Extra Parameter recognized is
+"version=1".
+
 Git Transport
 -
 
@@ -46,18 +59,25 @@ The Git transport starts off by sending the command and 
repository
 on the wire using the pkt-line format, followed by a NUL byte and a
 hostname parameter, terminated by a NUL byte.
 
-   0032git-upload-pack /project.git\0host=myserver.com\0
+   0033git-upload-pack /project.git\0host=myserver.com\0
+
+The transport may send Extra Parameters by adding an additional NUL
+byte, and then adding one or more NUL-terminated 

[PATCH v4 08/11] http: tell server that the client understands v1

[Brandon Williams]

Tell a server that protocol v1 can be used by sending the http header
'Git-Protocol' with 'version=1' indicating this.

Also teach the apache http server to pass through the 'Git-Protocol'
header as an environment variable 'GIT_PROTOCOL'.

Signed-off-by: Brandon Williams 
---
 cache.h |  2 ++
 http.c  | 18 +
 t/lib-httpd/apache.conf |  7 +
 t/t5700-protocol-v1.sh  | 69 +
 4 files changed, 96 insertions(+)

diff --git a/cache.h b/cache.h
index c74b73671..3a6b869c2 100644
--- a/cache.h
+++ b/cache.h
@@ -452,6 +452,8 @@ static inline enum object_type object_type(unsigned int 
mode)
  * ignored.
  */
 #define GIT_PROTOCOL_ENVIRONMENT "GIT_PROTOCOL"
+/* HTTP header used to handshake the wire protocol */
+#define GIT_PROTOCOL_HEADER "Git-Protocol"
 
 /*
  * This environment variable is expected to contain a boolean indicating
diff --git a/http.c b/http.c
index 9e40a465f..ffb719216 100644
--- a/http.c
+++ b/http.c
@@ -12,6 +12,7 @@
 #include "gettext.h"
 #include "transport.h"
 #include "packfile.h"
+#include "protocol.h"
 
 static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
 #if LIBCURL_VERSION_NUM >= 0x070a08
@@ -897,6 +898,21 @@ static void set_from_env(const char **var, const char 
*envname)
*var = val;
 }
 
+static void protocol_http_header(void)
+{
+   if (get_protocol_version_config() > 0) {
+   struct strbuf protocol_header = STRBUF_INIT;
+
+   strbuf_addf(_header, GIT_PROTOCOL_HEADER ": 
version=%d",
+   get_protocol_version_config());
+
+
+   extra_http_headers = curl_slist_append(extra_http_headers,
+  protocol_header.buf);
+   strbuf_release(_header);
+   }
+}
+
 void http_init(struct remote *remote, const char *url, int proactive_auth)
 {
char *low_speed_limit;
@@ -927,6 +943,8 @@ void http_init(struct remote *remote, const char *url, int 
proactive_auth)
if (remote)
var_override(_proxy_authmethod, 
remote->http_proxy_authmethod);
 
+   protocol_http_header();
+
pragma_header = curl_slist_append(http_copy_default_headers(),
"Pragma: no-cache");
no_pragma_header = curl_slist_append(http_copy_default_headers(),
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 0642ae7e6..df1943631 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -67,6 +67,9 @@ LockFile accept.lock
 
LoadModule unixd_module modules/mod_unixd.so
 
+
+   LoadModule setenvif_module modules/mod_setenvif.so
+
 
 
 PassEnv GIT_VALGRIND
@@ -76,6 +79,10 @@ PassEnv ASAN_OPTIONS
 PassEnv GIT_TRACE
 PassEnv GIT_CONFIG_NOSYSTEM
 
+= 2.4>
+   SetEnvIf Git-Protocol ".*" GIT_PROTOCOL=$0
+
+
 Alias /dumb/ www/
 Alias /auth/dumb/ www/auth/dumb/
 
diff --git a/t/t5700-protocol-v1.sh b/t/t5700-protocol-v1.sh
index 6551932da..b0779d362 100755
--- a/t/t5700-protocol-v1.sh
+++ b/t/t5700-protocol-v1.sh
@@ -220,4 +220,73 @@ test_expect_success 'push with ssh:// using protocol v1' '
grep "push< version 1" log
 '
 
+# Test protocol v1 with 'http://' transport
+#
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
+
+test_expect_success 'create repo to be served by http:// transport' '
+   git init "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+   git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" config http.receivepack 
true &&
+   test_commit -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" one
+'
+
+test_expect_success 'clone with http:// using protocol v1' '
+   GIT_TRACE_PACKET=1 GIT_TRACE_CURL=1 git -c protocol.version=1 \
+   clone "$HTTPD_URL/smart/http_parent" http_child 2>log &&
+
+   git -C http_child log -1 --format=%s >actual &&
+   git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" log -1 --format=%s 
>expect &&
+   test_cmp expect actual &&
+
+   # Client requested to use protocol v1
+   grep "Git-Protocol: version=1" log &&
+   # Server responded using protocol v1
+   grep "git< version 1" log
+'
+
+test_expect_success 'fetch with http:// using protocol v1' '
+   test_commit -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" two &&
+
+   GIT_TRACE_PACKET=1 git -C http_child -c protocol.version=1 \
+   fetch 2>log &&
+
+   git -C http_child log -1 --format=%s origin/master >actual &&
+   git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" log -1 --format=%s 
>expect &&
+   test_cmp expect actual &&
+
+   # Server responded using protocol v1
+   grep "git< version 1" log
+'
+
+test_expect_success 'pull with http:// using protocol v1' '
+   GIT_TRACE_PACKET=1 git -C http_child -c protocol.version=1 \
+   pull 2>log &&
+
+   git -C http_child log -1 --format=%s >actual &&
+   git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" log -1 --format=%s 
>expect &&
+   test_cmp 

[PATCH v4 01/11] connect: in ref advertisement, shallows are last

[Brandon Williams]

From: Jonathan Tan 

Currently, get_remote_heads() parses the ref advertisement in one loop,
allowing refs and shallow lines to intersperse, despite this not being
allowed by the specification. Refactor get_remote_heads() to use two
loops instead, enforcing that refs come first, and then shallows.

This also makes it easier to teach get_remote_heads() to interpret other
lines in the ref advertisement, which will be done in a subsequent
patch.

As part of this change, this patch interprets capabilities only on the
first line in the ref advertisement, printing a warning message when
encountering capabilities on other lines.

Signed-off-by: Jonathan Tan 
Signed-off-by: Brandon Williams 
---
 connect.c | 189 --
 1 file changed, 123 insertions(+), 66 deletions(-)

diff --git a/connect.c b/connect.c
index df56c0cbf..8e2e276b6 100644
--- a/connect.c
+++ b/connect.c
@@ -11,6 +11,7 @@
 #include "string-list.h"
 #include "sha1-array.h"
 #include "transport.h"
+#include "strbuf.h"
 
 static char *server_capabilities;
 static const char *parse_feature_value(const char *, const char *, int *);
@@ -107,6 +108,104 @@ static void annotate_refs_with_symref_info(struct ref 
*ref)
string_list_clear(, 0);
 }
 
+/*
+ * Read one line of a server's ref advertisement into packet_buffer.
+ */
+static int read_remote_ref(int in, char **src_buf, size_t *src_len,
+  int *responded)
+{
+   int len = packet_read(in, src_buf, src_len,
+ packet_buffer, sizeof(packet_buffer),
+ PACKET_READ_GENTLE_ON_EOF |
+ PACKET_READ_CHOMP_NEWLINE);
+   const char *arg;
+   if (len < 0)
+   die_initial_contact(*responded);
+   if (len > 4 && skip_prefix(packet_buffer, "ERR ", ))
+   die("remote error: %s", arg);
+
+   *responded = 1;
+
+   return len;
+}
+
+#define EXPECTING_FIRST_REF 0
+#define EXPECTING_REF 1
+#define EXPECTING_SHALLOW 2
+
+static void process_capabilities(int *len)
+{
+   int nul_location = strlen(packet_buffer);
+   if (nul_location == *len)
+   return;
+   server_capabilities = xstrdup(packet_buffer + nul_location + 1);
+   *len = nul_location;
+}
+
+static int process_dummy_ref(void)
+{
+   struct object_id oid;
+   const char *name;
+
+   if (parse_oid_hex(packet_buffer, , ))
+   return 0;
+   if (*name != ' ')
+   return 0;
+   name++;
+
+   return !oidcmp(_oid, ) && !strcmp(name, "capabilities^{}");
+}
+
+static void check_no_capabilities(int len)
+{
+   if (strlen(packet_buffer) != len)
+   warning("Ignoring capabilities after first line '%s'",
+   packet_buffer + strlen(packet_buffer));
+}
+
+static int process_ref(int len, struct ref ***list, unsigned int flags,
+  struct oid_array *extra_have)
+{
+   struct object_id old_oid;
+   const char *name;
+
+   if (parse_oid_hex(packet_buffer, _oid, ))
+   return 0;
+   if (*name != ' ')
+   return 0;
+   name++;
+
+   if (extra_have && !strcmp(name, ".have")) {
+   oid_array_append(extra_have, _oid);
+   } else if (!strcmp(name, "capabilities^{}")) {
+   die("protocol error: unexpected capabilities^{}");
+   } else if (check_ref(name, flags)) {
+   struct ref *ref = alloc_ref(name);
+   oidcpy(>old_oid, _oid);
+   **list = ref;
+   *list = >next;
+   }
+   check_no_capabilities(len);
+   return 1;
+}
+
+static int process_shallow(int len, struct oid_array *shallow_points)
+{
+   const char *arg;
+   struct object_id old_oid;
+
+   if (!skip_prefix(packet_buffer, "shallow ", ))
+   return 0;
+
+   if (get_oid_hex(arg, _oid))
+   die("protocol error: expected shallow sha-1, got '%s'", arg);
+   if (!shallow_points)
+   die("repository on the other end cannot be shallow");
+   oid_array_append(shallow_points, _oid);
+   check_no_capabilities(len);
+   return 1;
+}
+
 /*
  * Read all the refs from the other end
  */
@@ -123,76 +222,34 @@ struct ref **get_remote_heads(int in, char *src_buf, 
size_t src_len,
 * willing to talk to us.  A hang-up before seeing any
 * response does not necessarily mean an ACL problem, though.
 */
-   int saw_response;
-   int got_dummy_ref_with_capabilities_declaration = 0;
+   int responded = 0;
+   int len;
+   int state = EXPECTING_FIRST_REF;
 
*list = NULL;
-   for (saw_response = 0; ; saw_response = 1) {
-   struct ref *ref;
-   struct object_id old_oid;
-   char *name;
-   int len, name_len;
-   char *buffer = 

[PATCH v4 06/11] connect: teach client to recognize v1 server response

[Brandon Williams]

Teach a client to recognize that a server understands protocol v1 by
looking at the first pkt-line the server sends in response.  This is
done by looking for the response "version 1" send by upload-pack or
receive-pack.

Signed-off-by: Brandon Williams 
---
 connect.c | 30 ++
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/connect.c b/connect.c
index 8e2e276b6..a5e708a61 100644
--- a/connect.c
+++ b/connect.c
@@ -12,6 +12,7 @@
 #include "sha1-array.h"
 #include "transport.h"
 #include "strbuf.h"
+#include "protocol.h"
 
 static char *server_capabilities;
 static const char *parse_feature_value(const char *, const char *, int *);
@@ -129,9 +130,23 @@ static int read_remote_ref(int in, char **src_buf, size_t 
*src_len,
return len;
 }
 
-#define EXPECTING_FIRST_REF 0
-#define EXPECTING_REF 1
-#define EXPECTING_SHALLOW 2
+#define EXPECTING_PROTOCOL_VERSION 0
+#define EXPECTING_FIRST_REF 1
+#define EXPECTING_REF 2
+#define EXPECTING_SHALLOW 3
+
+/* Returns 1 if packet_buffer is a protocol version pkt-line, 0 otherwise. */
+static int process_protocol_version(void)
+{
+   switch (determine_protocol_version_client(packet_buffer)) {
+   case protocol_v1:
+   return 1;
+   case protocol_v0:
+   return 0;
+   default:
+   die("server is speaking an unknown protocol");
+   }
+}
 
 static void process_capabilities(int *len)
 {
@@ -224,12 +239,19 @@ struct ref **get_remote_heads(int in, char *src_buf, 
size_t src_len,
 */
int responded = 0;
int len;
-   int state = EXPECTING_FIRST_REF;
+   int state = EXPECTING_PROTOCOL_VERSION;
 
*list = NULL;
 
while ((len = read_remote_ref(in, _buf, _len, ))) {
switch (state) {
+   case EXPECTING_PROTOCOL_VERSION:
+   if (process_protocol_version()) {
+   state = EXPECTING_FIRST_REF;
+   break;
+   }
+   state = EXPECTING_FIRST_REF;
+   /* fallthrough */
case EXPECTING_FIRST_REF:
process_capabilities();
if (process_dummy_ref()) {
-- 
2.15.0.rc0.271.g36b669edcc-goog

[PATCH v4 02/11] pkt-line: add packet_write function

[Brandon Williams]

Add a function which can be used to write the contents of an arbitrary
buffer.  This makes it easy to build up data in a buffer before writing
the packet instead of formatting the entire contents of the packet using
'packet_write_fmt()'.

Signed-off-by: Brandon Williams 
---
 pkt-line.c | 6 ++
 pkt-line.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/pkt-line.c b/pkt-line.c
index 647bbd3bc..7006b3587 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -188,6 +188,12 @@ static int packet_write_gently(const int fd_out, const 
char *buf, size_t size)
return 0;
 }
 
+void packet_write(int fd_out, const char *buf, size_t size)
+{
+   if (packet_write_gently(fd_out, buf, size))
+   die_errno("packet write failed");
+}
+
 void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
 {
va_list args;
diff --git a/pkt-line.h b/pkt-line.h
index 66ef610fc..3dad583e2 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -22,6 +22,7 @@
 void packet_flush(int fd);
 void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format 
(printf, 2, 3)));
 void packet_buf_flush(struct strbuf *buf);
+void packet_write(int fd_out, const char *buf, size_t size);
 void packet_buf_write(struct strbuf *buf, const char *fmt, ...) 
__attribute__((format (printf, 2, 3)));
 int packet_flush_gently(int fd);
 int packet_write_fmt_gently(int fd, const char *fmt, ...) 
__attribute__((format (printf, 2, 3)));
-- 
2.15.0.rc0.271.g36b669edcc-goog

[PATCH v4 04/11] daemon: recognize hidden request arguments

[Brandon Williams]

A normal request to git-daemon is structured as
"command path/to/repo\0host=..\0" and due to a bug introduced in
49ba83fb6 (Add virtualization support to git-daemon, 2006-09-19) we
aren't able to place any extra arguments (separated by NULs) besides the
host otherwise the parsing of those arguments would enter an infinite
loop.  This bug was fixed in 73bb33a94 (daemon: Strictly parse the
"extra arg" part of the command, 2009-06-04) but a check was put in
place to disallow extra arguments so that new clients wouldn't trigger
this bug in older servers.

In order to get around this limitation teach git-daemon to recognize
additional request arguments hidden behind a second NUL byte.  Requests
can then be structured like:
"command path/to/repo\0host=..\0\0version=1\0key=value\0".  git-daemon
can then parse out the extra arguments and set 'GIT_PROTOCOL'
accordingly.

By placing these extra arguments behind a second NUL byte we can skirt
around both the infinite loop bug in 49ba83fb6 (Add virtualization
support to git-daemon, 2006-09-19) as well as the explicit disallowing
of extra arguments introduced in 73bb33a94 (daemon: Strictly parse the
"extra arg" part of the command, 2009-06-04) because both of these
versions of git-daemon check for a single NUL byte after the host
argument before terminating the argument parsing.

Signed-off-by: Brandon Williams 
---
 daemon.c | 71 
 1 file changed, 62 insertions(+), 9 deletions(-)

diff --git a/daemon.c b/daemon.c
index 30747075f..e37e343d0 100644
--- a/daemon.c
+++ b/daemon.c
@@ -282,7 +282,7 @@ static const char *path_ok(const char *directory, struct 
hostinfo *hi)
return NULL;/* Fallthrough. Deny by default */
 }
 
-typedef int (*daemon_service_fn)(void);
+typedef int (*daemon_service_fn)(const struct argv_array *env);
 struct daemon_service {
const char *name;
const char *config_name;
@@ -363,7 +363,7 @@ static int run_access_hook(struct daemon_service *service, 
const char *dir,
 }
 
 static int run_service(const char *dir, struct daemon_service *service,
-  struct hostinfo *hi)
+  struct hostinfo *hi, const struct argv_array *env)
 {
const char *path;
int enabled = service->enabled;
@@ -422,7 +422,7 @@ static int run_service(const char *dir, struct 
daemon_service *service,
 */
signal(SIGTERM, SIG_IGN);
 
-   return service->fn();
+   return service->fn(env);
 }
 
 static void copy_to_log(int fd)
@@ -462,25 +462,34 @@ static int run_service_command(struct child_process *cld)
return finish_command(cld);
 }
 
-static int upload_pack(void)
+static int upload_pack(const struct argv_array *env)
 {
struct child_process cld = CHILD_PROCESS_INIT;
argv_array_pushl(, "upload-pack", "--strict", NULL);
argv_array_pushf(, "--timeout=%u", timeout);
+
+   argv_array_pushv(_array, env->argv);
+
return run_service_command();
 }
 
-static int upload_archive(void)
+static int upload_archive(const struct argv_array *env)
 {
struct child_process cld = CHILD_PROCESS_INIT;
argv_array_push(, "upload-archive");
+
+   argv_array_pushv(_array, env->argv);
+
return run_service_command();
 }
 
-static int receive_pack(void)
+static int receive_pack(const struct argv_array *env)
 {
struct child_process cld = CHILD_PROCESS_INIT;
argv_array_push(, "receive-pack");
+
+   argv_array_pushv(_array, env->argv);
+
return run_service_command();
 }
 
@@ -573,8 +582,11 @@ static void canonicalize_client(struct strbuf *out, const 
char *in)
 
 /*
  * Read the host as supplied by the client connection.
+ *
+ * Returns a pointer to the character after the NUL byte terminating the host
+ * arguemnt, or 'extra_args' if there is no host arguemnt.
  */
-static void parse_host_arg(struct hostinfo *hi, char *extra_args, int buflen)
+static char *parse_host_arg(struct hostinfo *hi, char *extra_args, int buflen)
 {
char *val;
int vallen;
@@ -602,6 +614,43 @@ static void parse_host_arg(struct hostinfo *hi, char 
*extra_args, int buflen)
if (extra_args < end && *extra_args)
die("Invalid request");
}
+
+   return extra_args;
+}
+
+static void parse_extra_args(struct hostinfo *hi, struct argv_array *env,
+char *extra_args, int buflen)
+{
+   const char *end = extra_args + buflen;
+   struct strbuf git_protocol = STRBUF_INIT;
+
+   /* First look for the host argument */
+   extra_args = parse_host_arg(hi, extra_args, buflen);
+
+   /* Look for additional arguments places after a second NUL byte */
+   for (; extra_args < end; extra_args += strlen(extra_args) + 1) {
+   const char *arg = extra_args;
+
+   /*
+* Parse the extra arguments, adding most to 'git_protocol'
+ 

[PATCH v4 05/11] upload-pack, receive-pack: introduce protocol version 1

[Brandon Williams]

Teach upload-pack and receive-pack to understand and respond using
protocol version 1, if requested.

Protocol version 1 is simply the original and current protocol (what I'm
calling version 0) with the addition of a single packet line, which
precedes the ref advertisement, indicating the protocol version being
spoken.

Signed-off-by: Brandon Williams 
---
 builtin/receive-pack.c | 17 +
 upload-pack.c  | 20 +++-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index dd06b3fb4..839c1462d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -24,6 +24,7 @@
 #include "tmp-objdir.h"
 #include "oidset.h"
 #include "packfile.h"
+#include "protocol.h"
 
 static const char * const receive_pack_usage[] = {
N_("git receive-pack "),
@@ -1963,6 +1964,22 @@ int cmd_receive_pack(int argc, const char **argv, const 
char *prefix)
else if (0 <= receive_unpack_limit)
unpack_limit = receive_unpack_limit;
 
+   switch (determine_protocol_version_server()) {
+   case protocol_v1:
+   /*
+* v1 is just the original protocol with a version string,
+* so just fall through after writing the version string.
+*/
+   if (advertise_refs || !stateless_rpc)
+   packet_write_fmt(1, "version 1\n");
+
+   /* fallthrough */
+   case protocol_v0:
+   break;
+   case protocol_unknown_version:
+   BUG("unknown protocol version");
+   }
+
if (advertise_refs || !stateless_rpc) {
write_head_info();
}
diff --git a/upload-pack.c b/upload-pack.c
index 7efff2fbf..ef99a029c 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -18,6 +18,7 @@
 #include "parse-options.h"
 #include "argv-array.h"
 #include "prio-queue.h"
+#include "protocol.h"
 
 static const char * const upload_pack_usage[] = {
N_("git upload-pack [] "),
@@ -1067,6 +1068,23 @@ int cmd_main(int argc, const char **argv)
die("'%s' does not appear to be a git repository", dir);
 
git_config(upload_pack_config, NULL);
-   upload_pack();
+
+   switch (determine_protocol_version_server()) {
+   case protocol_v1:
+   /*
+* v1 is just the original protocol with a version string,
+* so just fall through after writing the version string.
+*/
+   if (advertise_refs || !stateless_rpc)
+   packet_write_fmt(1, "version 1\n");
+
+   /* fallthrough */
+   case protocol_v0:
+   upload_pack();
+   break;
+   case protocol_unknown_version:
+   BUG("unknown protocol version");
+   }
+
return 0;
 }
-- 
2.15.0.rc0.271.g36b669edcc-goog

[PATCH v4 07/11] connect: tell server that the client understands v1

[Brandon Williams]

Teach the connection logic to tell a serve that it understands protocol
v1.  This is done in 2 different ways for the builtin transports, both
of which ultimately set 'GIT_PROTOCOL' to 'version=1' on the server.

1. git://
   A normal request to git-daemon is structured as
   "command path/to/repo\0host=..\0" and due to a bug introduced in
   49ba83fb6 (Add virtualization support to git-daemon, 2006-09-19) we
   aren't able to place any extra arguments (separated by NULs) besides
   the host otherwise the parsing of those arguments would enter an
   infinite loop.  This bug was fixed in 73bb33a94 (daemon: Strictly
   parse the "extra arg" part of the command, 2009-06-04) but a check
   was put in place to disallow extra arguments so that new clients
   wouldn't trigger this bug in older servers.

   In order to get around this limitation git-daemon was taught to
   recognize additional request arguments hidden behind a second
   NUL byte.  Requests can then be structured like:
   "command path/to/repo\0host=..\0\0version=1\0key=value\0".
   git-daemon can then parse out the extra arguments and set
   'GIT_PROTOCOL' accordingly.

   By placing these extra arguments behind a second NUL byte we can
   skirt around both the infinite loop bug in 49ba83fb6 (Add
   virtualization support to git-daemon, 2006-09-19) as well as the
   explicit disallowing of extra arguments introduced in 73bb33a94
   (daemon: Strictly parse the "extra arg" part of the command,
   2009-06-04) because both of these versions of git-daemon check for a
   single NUL byte after the host argument before terminating the
   argument parsing.

2. ssh://, file://
   Set 'GIT_PROTOCOL' environment variable with the desired protocol
   version.  With the file:// transport, 'GIT_PROTOCOL' can be set
   explicitly in the locally running git-upload-pack or git-receive-pack
   processes.  With the ssh:// transport and OpenSSH compliant ssh
   programs, 'GIT_PROTOCOL' can be sent across ssh by using '-o
   SendEnv=GIT_PROTOCOL' and having the server whitelist this
   environment variable.

Signed-off-by: Brandon Williams 
---
 connect.c  |  37 ++--
 t/t5700-protocol-v1.sh | 223 +
 2 files changed, 255 insertions(+), 5 deletions(-)
 create mode 100755 t/t5700-protocol-v1.sh

diff --git a/connect.c b/connect.c
index a5e708a61..b8695a2fa 100644
--- a/connect.c
+++ b/connect.c
@@ -871,6 +871,7 @@ struct child_process *git_connect(int fd[2], const char 
*url,
printf("Diag: path=%s\n", path ? path : "NULL");
conn = NULL;
} else if (protocol == PROTO_GIT) {
+   struct strbuf request = STRBUF_INIT;
/*
 * Set up virtual host information based on where we will
 * connect, unless the user has overridden us in
@@ -898,13 +899,25 @@ struct child_process *git_connect(int fd[2], const char 
*url,
 * Note: Do not add any other headers here!  Doing so
 * will cause older git-daemon servers to crash.
 */
-   packet_write_fmt(fd[1],
-"%s %s%chost=%s%c",
-prog, path, 0,
-target_host, 0);
+   strbuf_addf(,
+   "%s %s%chost=%s%c",
+   prog, path, 0,
+   target_host, 0);
+
+   /* If using a new version put that stuff here after a second 
null byte */
+   if (get_protocol_version_config() > 0) {
+   strbuf_addch(, '\0');
+   strbuf_addf(, "version=%d%c",
+   get_protocol_version_config(), '\0');
+   }
+
+   packet_write(fd[1], request.buf, request.len);
+
free(target_host);
+   strbuf_release();
} else {
struct strbuf cmd = STRBUF_INIT;
+   const char *const *var;
 
conn = xmalloc(sizeof(*conn));
child_process_init(conn);
@@ -917,7 +930,9 @@ struct child_process *git_connect(int fd[2], const char 
*url,
sq_quote_buf(, path);
 
/* remove repo-local variables from the environment */
-   conn->env = local_repo_env;
+   for (var = local_repo_env; *var; var++)
+   argv_array_push(>env_array, *var);
+
conn->use_shell = 1;
conn->in = conn->out = -1;
if (protocol == PROTO_SSH) {
@@ -971,6 +986,14 @@ struct child_process *git_connect(int fd[2], const char 
*url,
}
 
argv_array_push(>args, ssh);
+
+   if (get_protocol_version_config() > 0) {
+   argv_array_push(>args, "-o");
+   argv_array_push(>args, "SendEnv=" 

[PATCH v4 09/11] i5700: add interop test for protocol transition

[Brandon Williams]

Signed-off-by: Brandon Williams 
---
 t/interop/i5700-protocol-transition.sh | 68 ++
 1 file changed, 68 insertions(+)
 create mode 100755 t/interop/i5700-protocol-transition.sh

diff --git a/t/interop/i5700-protocol-transition.sh 
b/t/interop/i5700-protocol-transition.sh
new file mode 100755
index 0..97e8e580e
--- /dev/null
+++ b/t/interop/i5700-protocol-transition.sh
@@ -0,0 +1,68 @@
+#!/bin/sh
+
+VERSION_A=.
+VERSION_B=v2.0.0
+
+: ${LIB_GIT_DAEMON_PORT:=5700}
+LIB_GIT_DAEMON_COMMAND='git.b daemon'
+
+test_description='clone and fetch by client who is trying to use a new 
protocol'
+. ./interop-lib.sh
+. "$TEST_DIRECTORY"/lib-git-daemon.sh
+
+start_git_daemon --export-all
+
+repo=$GIT_DAEMON_DOCUMENT_ROOT_PATH/repo
+
+test_expect_success "create repo served by $VERSION_B" '
+   git.b init "$repo" &&
+   git.b -C "$repo" commit --allow-empty -m one
+'
+
+test_expect_success "git:// clone with $VERSION_A and protocol v1" '
+   GIT_TRACE_PACKET=1 git.a -c protocol.version=1 clone 
"$GIT_DAEMON_URL/repo" child 2>log &&
+   git.a -C child log -1 --format=%s >actual &&
+   git.b -C "$repo" log -1 --format=%s >expect &&
+   test_cmp expect actual &&
+   grep "version=1" log
+'
+
+test_expect_success "git:// fetch with $VERSION_A and protocol v1" '
+   git.b -C "$repo" commit --allow-empty -m two &&
+   git.b -C "$repo" log -1 --format=%s >expect &&
+
+   GIT_TRACE_PACKET=1 git.a -C child -c protocol.version=1 fetch 2>log &&
+   git.a -C child log -1 --format=%s FETCH_HEAD >actual &&
+
+   test_cmp expect actual &&
+   grep "version=1" log &&
+   ! grep "version 1" log
+'
+
+stop_git_daemon
+
+test_expect_success "create repo served by $VERSION_B" '
+   git.b init parent &&
+   git.b -C parent commit --allow-empty -m one
+'
+
+test_expect_success "file:// clone with $VERSION_A and protocol v1" '
+   GIT_TRACE_PACKET=1 git.a -c protocol.version=1 clone 
--upload-pack="git.b upload-pack" parent child2 2>log &&
+   git.a -C child2 log -1 --format=%s >actual &&
+   git.b -C parent log -1 --format=%s >expect &&
+   test_cmp expect actual &&
+   ! grep "version 1" log
+'
+
+test_expect_success "file:// fetch with $VERSION_A and protocol v1" '
+   git.b -C parent commit --allow-empty -m two &&
+   git.b -C parent log -1 --format=%s >expect &&
+
+   GIT_TRACE_PACKET=1 git.a -C child2 -c protocol.version=1 fetch 
--upload-pack="git.b upload-pack" 2>log &&
+   git.a -C child2 log -1 --format=%s FETCH_HEAD >actual &&
+
+   test_cmp expect actual &&
+   ! grep "version 1" log
+'
+
+test_done
-- 
2.15.0.rc0.271.g36b669edcc-goog

[PATCH v4 03/11] protocol: introduce protocol extension mechanisms

[Brandon Williams]

Create protocol.{c,h} and provide functions which future servers and
clients can use to determine which protocol to use or is being used.

Also introduce the 'GIT_PROTOCOL' environment variable which will be
used to communicate a colon separated list of keys with optional values
to a server.  Unknown keys and values must be tolerated.  This mechanism
is used to communicate which version of the wire protocol a client would
like to use with a server.

Signed-off-by: Brandon Williams 
---
 Documentation/config.txt | 17 +++
 Documentation/git.txt|  6 
 Makefile |  1 +
 cache.h  |  8 +
 protocol.c   | 79 
 protocol.h   | 33 
 6 files changed, 144 insertions(+)
 create mode 100644 protocol.c
 create mode 100644 protocol.h

diff --git a/Documentation/config.txt b/Documentation/config.txt
index dc4e3f58a..b78747abc 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2517,6 +2517,23 @@ The protocol names currently used by git are:
 `hg` to allow the `git-remote-hg` helper)
 --
 
+protocol.version::
+   Experimental. If set, clients will attempt to communicate with a
+   server using the specified protocol version.  If unset, no
+   attempt will be made by the client to communicate using a
+   particular protocol version, this results in protocol version 0
+   being used.
+   Supported versions:
++
+--
+
+* `0` - the original wire protocol.
+
+* `1` - the original wire protocol with the addition of a version string
+  in the initial response from the server.
+
+--
+
 pull.ff::
By default, Git does not create an extra merge commit when merging
a commit that is a descendant of the current commit. Instead, the
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 6e3a6767e..7518ea3af 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -697,6 +697,12 @@ of clones and fetches.
which feed potentially-untrusted URLS to git commands.  See
linkgit:git-config[1] for more details.
 
+`GIT_PROTOCOL`::
+   For internal use only.  Used in handshaking the wire protocol.
+   Contains a colon ':' separated list of keys with optional values
+   'key[=value]'.  Presence of unknown keys and values must be
+   ignored.
+
 Discussion[[Discussion]]
 
 
diff --git a/Makefile b/Makefile
index ed4ca438b..9ce68cded 100644
--- a/Makefile
+++ b/Makefile
@@ -842,6 +842,7 @@ LIB_OBJS += pretty.o
 LIB_OBJS += prio-queue.o
 LIB_OBJS += progress.o
 LIB_OBJS += prompt.o
+LIB_OBJS += protocol.o
 LIB_OBJS += quote.o
 LIB_OBJS += reachable.o
 LIB_OBJS += read-cache.o
diff --git a/cache.h b/cache.h
index 49b083ee0..c74b73671 100644
--- a/cache.h
+++ b/cache.h
@@ -445,6 +445,14 @@ static inline enum object_type object_type(unsigned int 
mode)
 #define GIT_ICASE_PATHSPECS_ENVIRONMENT "GIT_ICASE_PATHSPECS"
 #define GIT_QUARANTINE_ENVIRONMENT "GIT_QUARANTINE_PATH"
 
+/*
+ * Environment variable used in handshaking the wire protocol.
+ * Contains a colon ':' separated list of keys with optional values
+ * 'key[=value]'.  Presence of unknown keys and values must be
+ * ignored.
+ */
+#define GIT_PROTOCOL_ENVIRONMENT "GIT_PROTOCOL"
+
 /*
  * This environment variable is expected to contain a boolean indicating
  * whether we should or should not treat:
diff --git a/protocol.c b/protocol.c
new file mode 100644
index 0..43012b7eb
--- /dev/null
+++ b/protocol.c
@@ -0,0 +1,79 @@
+#include "cache.h"
+#include "config.h"
+#include "protocol.h"
+
+static enum protocol_version parse_protocol_version(const char *value)
+{
+   if (!strcmp(value, "0"))
+   return protocol_v0;
+   else if (!strcmp(value, "1"))
+   return protocol_v1;
+   else
+   return protocol_unknown_version;
+}
+
+enum protocol_version get_protocol_version_config(void)
+{
+   const char *value;
+   if (!git_config_get_string_const("protocol.version", )) {
+   enum protocol_version version = parse_protocol_version(value);
+
+   if (version == protocol_unknown_version)
+   die("unknown value for config 'protocol.version': %s",
+   value);
+
+   return version;
+   }
+
+   return protocol_v0;
+}
+
+enum protocol_version determine_protocol_version_server(void)
+{
+   const char *git_protocol = getenv(GIT_PROTOCOL_ENVIRONMENT);
+   enum protocol_version version = protocol_v0;
+
+   /*
+* Determine which protocol version the client has requested.  Since
+* multiple 'version' keys can be sent by the client, indicating that
+* the client is okay to speak any of them, select the greatest version
+* that the client has requested.  This is due to the assumption that
+* the most recent protocol version will be 

Re: [PATCH v1 1/1] Introduce git add --renormalize .

[Junio C Hamano]

tbo...@web.de writes:

> From: Torsten Bögershausen 
>
> Make it safer to normalize the line endings in a repository:
> Files that had been commited with CRLF will be commited with LF.
> (Unless core.autorclf and .gitattributes specify that Git
>  should not do line ending conversions)

A few issues I saw after a quick read:

 - The log message tells us old and new ways, but does not make it
   clear why users are encouraged to use the new way at all.  You
   didn't make the implementation of "add --renormalize" to just
   start "git add" without calling read_cache() and letting all
   files added new to the index (which is how the old way worked) to
   give a sugarcoated equivalent of the old way for a reason, and
   that should be desribed in the log.

 - An ugly global variable is introduced instead of passing
   necessary information through the callchain properly, but the
   title does not say PATCH/RFC.

 - The documentation makes it sound as if this new feature is _only_
   about CRLF vs LF.  SHouldn't this equally apply after the user
   changes .gitattributes settings that govern the "clean" side of
   the filter and makes what is in the index "unclean"?

The second point is a showstopper from maintainability's point of
view, but none of the above should be insurmojntable.

Thanks.

Re: [PATCH v3 10/10] ssh: introduce a 'simple' ssh variant

[Brandon Williams]

On 10/03, Jonathan Nieder wrote:
> Hi,
> 
> Brandon Williams wrote:
> 
> > When using the 'ssh' transport, the '-o' option is used to specify an
> > environment variable which should be set on the remote end.  This allows
> > git to send additional information when contacting the server,
> > requesting the use of a different protocol version via the
> > 'GIT_PROTOCOL' environment variable like so: "-o SendEnv=GIT_PROTOCOL"
> >
> > Unfortunately not all ssh variants support the sending of environment
> > variables to the remote end.  To account for this, only use the '-o'
> > option for ssh variants which are OpenSSH compliant.  This is done by
> > checking that the basename of the ssh command is 'ssh' or the ssh
> > variant is overridden to be 'ssh' (via the ssh.variant config).
> 
> This also affects -p (port), right?

Yeah I'll add a comment in the commit msg indicating that options like
-p and -4 -6 are are only supported by some variants.

> 
> What happens if I specify a ssh://host:port/path URL and the SSH
> implementation is of 'simple' type?

The port would only be sent if your ssh command supported it.

> 
> > Previously if an ssh command's basename wasn't 'plink' or
> 
> Git's commit messages use the present tense to describe the current
> state of the code, so this is "Currently". :)

I'll fix this :)

> 
> > 'tortoiseplink' git assumed that the command was an OpenSSH variant.
> > Since user configured ssh commands may not be OpenSSH compliant, tighten
> > this constraint and assume a variant of 'simple' if the basename of the
> > command doesn't match the variants known to git.  The new ssh variant
> > 'simple' will only have the host and command to execute ([username@]host
> > command) passed as parameters to the ssh command.
> >
> > Update the Documentation to better reflect the command-line options sent
> > to ssh commands based on their variant.
> >
> > Reported-by: Jeffrey Yasskin 
> > Signed-off-by: Brandon Williams 
> 
> Thanks for working on this.
> 
> For background, the GIT_SSH implementation that motivated this is
> https://github.com/travis-ci/dpl/blob/6c3fddfda1f2a85944c56b068bac0a77c049/lib/dpl/provider.rb#L215,
> which does not support -p or -4/-6, either.
> 
> > ---
> >  Documentation/config.txt |  27 ++--
> >  Documentation/git.txt|   9 ++--
> >  connect.c| 107 
> > ++-
> >  t/t5601-clone.sh |   9 ++--
> >  t/t5700-protocol-v1.sh   |   2 +
> >  5 files changed, 95 insertions(+), 59 deletions(-)
> [...]
> > --- a/connect.c
> > +++ b/connect.c
> > @@ -776,37 +776,44 @@ static const char *get_ssh_command(void)
> [...]
> > +static enum ssh_variant determine_ssh_variant(const char *ssh_command,
> > + int is_cmdline)
> [...]
> > -   if (!strcasecmp(variant, "plink") ||
> > -   !strcasecmp(variant, "plink.exe"))
> > -   *port_option = 'P';
> > +   if (!strcasecmp(variant, "ssh"))
> > +   ssh_variant = VARIANT_SSH;
> 
> Could this handle ssh.exe, too?

Yeah I'll add the additional comparison.

> 
> [...]
> > --- a/t/t5601-clone.sh
> > +++ b/t/t5601-clone.sh
> 
> Can this get tests for the new defaulting behavior?  E.g.
> 
>  - default is "simple"
>  - how "simple" treats an ssh://host:port/path URL
>  - how "simple" treats ipv4/ipv6 switching
>  - ssh defaults to "ssh"
>  - if GIT_SSH=ssh, can set ssh.variant to "simple" to force the "simple"
>mode

I'll look to adding a few more tests.

> 
> One other worry: this (intentionally) changes the behavior of a
> previously-working GIT_SSH=ssh-wrapper that wants to support
> OpenSSH-style options but does not declare ssh.variant=ssh.  When
> discovering this change, what should the author of such an ssh-wrapper
> do?
> 
> They could instruct their users to set ssh.variant or GIT_SSH_VARIANT
> to "ssh", but then they are at the mercy of future additional options
> supported by OpenSSH we may want to start using in the future (e.g.,
> maybe we will start passing "--" to separate options from the
> hostname).  So this is not a futureproof option for them.
> 
> They could take the new default behavior or instruct their users to
> set ssh.variant or GIT_SSH_VARIANT to "simple", but then they lose
> support for handling alternate ports, ipv4/ipv6, and specifying -o
> SendEnv to propagate GIT_PROTOCOL or other envvars.  They can handle
> GIT_PROTOCOL propagation manually, but losing port support seems like
> a heavy cost.
> 
> They could send a patch to define yet another variant that is
> forward-compatible, for example using an interface similar to what
> git-credential(1) uses.  Then they can set GIT_SSH to their
> OpenSSH-style helper and GIT_FANCY_NEW_SSH to their more modern
> helper, so that old Git versions could use GIT_SSH and new Git
> versions could use GIT_FANCY_NEW_SSH.  This might be their best
> option.  It feels odd to say that their only good 

Re: Does Git build things during 'make install"?

[Johannes Sixt]

Am 16.10.2017 um 10:23 schrieb Junio C Hamano:
> Johannes Sixt  writes:
> 
>> Yes, running "sudo make install" is a nightmare. sudo clears the path,
>> and the git command is not found by the make invoked with root
>> permissions. This changes the version string that gets compiled into
>> the executable, which finally triggers a complete rebuild under
>> root. Sad...
> 
> In the meantime, would it help to intall as yourself under DESTDIR
> set to where you can write into, and then limit the potential
> damange done while pretending to be a privileged user to "cp -R" (or
> "tar cf" in $DESTDIR piped to "tar xf" in /)?
> 
> It appears that some dependencies are screwed up around "perl"
> related things, which may want to get fixed.  I agree that "make &&
> make install" that runs two 'make' under the same environment and
> user shouldn't (re)build anything during the latter 'make', but we
> somehow seem to do so.

We do so only, if 'make install' does not run in the same environment
and if there is no version file.

I use the patch below. It works for me, but I could imagine that it
suffers from the original problem if there is no git in PATH and there
is no version file, i.e., when the source is not a release tarball.

- 8< -
Subject: [PATCH] version-gen: Use just built git if no other git is in PATH

Signed-off-by: Johannes Sixt 
---
 GIT-VERSION-GEN | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 0e88e23653..b610aa3249 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -3,6 +3,9 @@
 GVF=GIT-VERSION-FILE
 DEF_VER=v2.15.0-rc1
 
+# use git that was just compiled if there is no git elsewhere in PATH
+PATH=$PATH:.
+
 LF='
 '
 
-- 
2.14.2.808.g3bc32f2729

[PATCH v1 1/1] Introduce git add --renormalize .

[tboegi]

From: Torsten Bögershausen 

Make it safer to normalize the line endings in a repository:
Files that had been commited with CRLF will be commited with LF.
(Unless core.autorclf and .gitattributes specify that Git
 should not do line ending conversions)

The old way to normalize a repo was like this:
 # Make sure that there are not untracked files
 $ echo "* text=auto" >.gitattributes
 $ git read-tree --empty
 $ git add .
 $ git commit -m "Introduce end-of-line normalization"

The new method is one step shorter, more intuitive and does not
add untracked files:
 $ echo "* text=auto" >.gitattributes
 $ git add --renormalize .
 $ git commit -m "Introduce end-of-line normalization"

Note that "git add --renormalize " is the short form for
"git add -u --renormalize ".

Signed-off-by: Torsten Bögershausen 
---
 Documentation/git-add.txt   |  8 +++-
 Documentation/gitattributes.txt |  3 +--
 builtin/add.c   | 27 +--
 cache.h |  1 +
 convert.c   |  1 +
 environment.c   |  1 +
 read-cache.c| 24 ++--
 t/t0025-crlf-renormalize.sh | 30 ++
 8 files changed, 80 insertions(+), 15 deletions(-)
 create mode 100755 t/t0025-crlf-renormalize.sh

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index f4169fb1ec..b6e431903d 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git add' [--verbose | -v] [--dry-run | -n] [--force | -f] [--interactive | 
-i] [--patch | -p]
  [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
- [--intent-to-add | -N] [--refresh] [--ignore-errors] 
[--ignore-missing]
+ [--intent-to-add | -N] [--refresh] [--ignore-errors] 
[--ignore-missing] [--renormalize]
  [--chmod=(+|-)x] [--] [...]
 
 DESCRIPTION
@@ -172,6 +172,12 @@ for "git add --no-all ...", i.e. ignored removed 
files.
warning (e.g., if you are manually performing operations on
submodules).
 
+--renormalize::
+   Normalizes the line endings from CRLF to LF of tracked files.
+   This applies to files which are either "text" or "text=auto"
+   in .gitattributes (or core.autocrlf is true or input)
+--renormalize implies -u
+
 --chmod=(+|-)x::
Override the executable bit of the added files.  The executable
bit is only changed in the index, the files on disk are left
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 4c68bc19d5..071dec2bc4 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -232,8 +232,7 @@ From a clean working directory:
 
 -
 $ echo "* text=auto" >.gitattributes
-$ git read-tree --empty   # Clean index, force re-scan of working directory
-$ git add .
+$ git add --renormalize .
 $ git status# Show files that will be normalized
 $ git commit -m "Introduce end-of-line normalization"
 -
diff --git a/builtin/add.c b/builtin/add.c
index a648cf4c56..ee8e756fdc 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -123,6 +123,25 @@ int add_files_to_cache(const char *prefix,
return !!data.add_errors;
 }
 
+static int renormalize_tracked_files(const struct pathspec *pathspec, int 
flags)
+{
+   int i, retval = 0;
+
+   for (i = 0; i < active_nr; i++) {
+   struct cache_entry *ce = active_cache[i];
+
+   if (ce_stage(ce))
+   continue; /* do not touch unmerged paths */
+   if (!S_ISREG(ce->ce_mode) && !S_ISLNK(ce->ce_mode))
+   continue; /* do not touch non blobs */
+   if (pathspec && !ce_path_match(ce, pathspec, NULL))
+   continue;
+   retval |= add_file_to_cache(ce->name, flags);
+   }
+
+   return retval;
+}
+
 static char *prune_directory(struct dir_struct *dir, struct pathspec 
*pathspec, int prefix)
 {
char *seen;
@@ -276,6 +295,7 @@ static struct option builtin_add_options[] = {
OPT_BOOL('e', "edit", _interactive, N_("edit current diff and 
apply")),
OPT__FORCE(_too, N_("allow adding otherwise ignored files")),
OPT_BOOL('u', "update", _worktree_changes, N_("update tracked 
files")),
+   OPT_BOOL(0, "renormalize", _renormalize, N_("renormalize EOL of 
tracked files (implies -u)")),
OPT_BOOL('N', "intent-to-add", _to_add, N_("record only the fact 
that the path will be added later")),
OPT_BOOL('A', "all", _explicit, N_("add changes from all 
tracked and untracked files")),
{ OPTION_CALLBACK, 0, "ignore-removal", _explicit,
@@ -406,7 +426,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
  chmod_arg[1] != 'x' || chmod_arg[2]))

Greetings in the name of God, Business proposal in God we trust

[Mrs, Suran Yoda]

Greetings in the name of God

Dear Friend


Greetings in the name of God,please let this not sound strange to you
for my only surviving lawyer who would have done this died early this
year.I prayed and got your email id from your country guestbook. I am
Mrs Suran Yoda from London,I am 72 years old,i am suffering from a
long time cancer of the lungs which also affected my brain,from all
indication my conditions is really deteriorating and it is quite
obvious that,according to my doctors they have advised me that i may
not live for the next two months,this is because the cancer stage has
gotten to a very bad stage.I am married to (Dr Andrews Yoda) who
worked with the Embassy of United Kingdom in South

Africa for nine years,Before he died in 2004.

I was bred up from a motherless babies home and was married to my late
husband for Thirty years without a child,my husband died in a fatal
motor accident Before his death we were true believers.Since his death
I decided not to re-marry,I sold all my inherited belongings and
deposited all the sum of $6.5 Million dollars with Bank in South
Africa.Though what disturbs me mostly is the cancer. Having known my
condition I decided to donate this fund to church,i want you as God
fearing person,to also use this money to fund church,orphanages and
widows,I took this decision,before i rest in peace because my time
will so on be up.

The Bible made us to understand that blessed are the hands that
giveth. I took this decision because I don`t have any child that will
inherit this money and my husband's relatives are not Christians and I
don`t want my husband hard earned money to be misused by unbelievers.
I don`t want a situation where these money will be used in an ungodly
manner,hence the reason for taking this bold decision.I am not afraid
of death hence i know where am going.Presently,I'm with my laptop in a
hospital here in London where I have been undergoing treatment for
cancer of the lungs.

As soon as I receive your reply I shall give you the contact of the
Bank.I will also issue you a letter of authority that will prove you
as the new beneficiary of my fund.Please assure me that you will act
accordingly as I stated.Hoping to hear from you soon.

Remain blessed in the name of the Lord.

Yours in Christ,
Mrs Suran Yoda

[PATCH v4 2/3] implement fetching of moved submodules

[Heiko Voigt]

We store the changed submodules paths to calculate which submodule needs
fetching. This does not work for moved submodules since their paths do
not stay the same in case of a moved submodules. In case of new
submodules we do not have a path in the current checkout, since they
just appeared in this fetch.

It is more general to collect the submodule names for changes instead of
their paths to include the above cases. If we do not have a
configuration for a gitlink we rely on constructing a default name from
the path if a git repository can be found at its path. We skip
non-configured gitlinks whose default name collides with a configured
one.

With the change described above we implement 'on-demand' fetching of
changes in moved submodules.

Signed-off-by: Heiko Voigt 
---
 submodule-config.h  |   3 +
 submodule.c | 138 
 t/t5526-fetch-submodules.sh |  35 +++
 3 files changed, 138 insertions(+), 38 deletions(-)

diff --git a/submodule-config.h b/submodule-config.h
index e3845831f6..a5503a5d17 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -22,6 +22,9 @@ struct submodule {
int recommend_shallow;
 };
 
+#define SUBMODULE_INIT { NULL, NULL, NULL, RECURSE_SUBMODULES_NONE, \
+   NULL, NULL, SUBMODULE_UPDATE_STRATEGY_INIT, {0}, -1 };
+
 struct submodule_cache;
 struct repository;
 
diff --git a/submodule.c b/submodule.c
index 63e7094e16..71d1773e2e 100644
--- a/submodule.c
+++ b/submodule.c
@@ -21,7 +21,7 @@
 #include "parse-options.h"
 
 static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
-static struct string_list changed_submodule_paths = STRING_LIST_INIT_DUP;
+static struct string_list changed_submodule_names = STRING_LIST_INIT_DUP;
 static int initialized_fetch_ref_tips;
 static struct oid_array ref_tips_before_fetch;
 static struct oid_array ref_tips_after_fetch;
@@ -674,11 +674,11 @@ const struct submodule *submodule_from_ce(const struct 
cache_entry *ce)
 }
 
 static struct oid_array *submodule_commits(struct string_list *submodules,
-  const char *path)
+  const char *name)
 {
struct string_list_item *item;
 
-   item = string_list_insert(submodules, path);
+   item = string_list_insert(submodules, name);
if (item->util)
return (struct oid_array *) item->util;
 
@@ -687,39 +687,67 @@ static struct oid_array *submodule_commits(struct 
string_list *submodules,
return (struct oid_array *) item->util;
 }
 
+struct collect_changed_submodules_cb_data {
+   struct string_list *changed;
+   const struct object_id *commit_oid;
+};
+
+/*
+ * this would normally be two functions: default_name_from_path() and
+ * path_from_default_name(). Since the default name is the same as
+ * the submodule path we can get away with just one function which only
+ * checks whether there is a submodule in the working directory at that
+ * location.
+ */
+static const char *default_name_or_path(const char *path_or_name)
+{
+   int error_code;
+
+   if (!is_submodule_populated_gently(path_or_name, _code))
+   return NULL;
+
+   return path_or_name;
+}
+
 static void collect_changed_submodules_cb(struct diff_queue_struct *q,
  struct diff_options *options,
  void *data)
 {
+   struct collect_changed_submodules_cb_data *me = data;
+   struct string_list *changed = me->changed;
+   const struct object_id *commit_oid = me->commit_oid;
int i;
-   struct string_list *changed = data;
 
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
struct oid_array *commits;
+   const struct submodule *submodule;
+   const char *name;
+
if (!S_ISGITLINK(p->two->mode))
continue;
 
-   if (S_ISGITLINK(p->one->mode)) {
-   /*
-* NEEDSWORK: We should honor the name configured in
-* the .gitmodules file of the commit we are examining
-* here to be able to correctly follow submodules
-* being moved around.
-*/
-   commits = submodule_commits(changed, p->two->path);
-   oid_array_append(commits, >two->oid);
-   } else {
-   /* Submodule is new or was moved here */
-   /*
-* NEEDSWORK: When the .git directories of submodules
-* live inside the superprojects .git directory some
-* day we should fetch new submodules directly into
-* that location too when config or options request
-* that so 

[PATCH v4 3/3] submodule: simplify decision tree whether to or not to fetch

[Heiko Voigt]

To make extending this logic later easier.

Signed-off-by: Heiko Voigt 
---
 submodule.c | 74 ++---
 1 file changed, 37 insertions(+), 37 deletions(-)

diff --git a/submodule.c b/submodule.c
index 71d1773e2e..82d206eb65 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1187,6 +1187,31 @@ struct submodule_parallel_fetch {
 };
 #define SPF_INIT {0, ARGV_ARRAY_INIT, NULL, NULL, 0, 0, 0, 0}
 
+static int get_fetch_recurse_config(const struct submodule *submodule,
+   struct submodule_parallel_fetch *spf)
+{
+   if (spf->command_line_option != RECURSE_SUBMODULES_DEFAULT)
+   return spf->command_line_option;
+
+   if (submodule) {
+   char *key;
+   const char *value;
+
+   int fetch_recurse = submodule->fetch_recurse;
+   key = xstrfmt("submodule.%s.fetchRecurseSubmodules", 
submodule->name);
+   if (!repo_config_get_string_const(the_repository, key, )) 
{
+   fetch_recurse = parse_fetch_recurse_submodules_arg(key, 
value);
+   }
+   free(key);
+
+   if (fetch_recurse != RECURSE_SUBMODULES_NONE)
+   /* local config overrules everything except commandline 
*/
+   return fetch_recurse;
+   }
+
+   return spf->default_option;
+}
+
 static int get_next_submodule(struct child_process *cp,
  struct strbuf *err, void *data, void **task_cb)
 {
@@ -1214,46 +1239,21 @@ static int get_next_submodule(struct child_process *cp,
}
}
 
-   default_argv = "yes";
-   if (spf->command_line_option == RECURSE_SUBMODULES_DEFAULT) {
-   int fetch_recurse = RECURSE_SUBMODULES_NONE;
-
-   if (submodule) {
-   char *key;
-   const char *value;
-
-   fetch_recurse = submodule->fetch_recurse;
-   key = 
xstrfmt("submodule.%s.fetchRecurseSubmodules", submodule->name);
-   if 
(!repo_config_get_string_const(the_repository, key, )) {
-   fetch_recurse = 
parse_fetch_recurse_submodules_arg(key, value);
-   }
-   free(key);
-   }
-
-   if (fetch_recurse != RECURSE_SUBMODULES_NONE) {
-   if (fetch_recurse == RECURSE_SUBMODULES_OFF)
-   continue;
-   if (fetch_recurse == 
RECURSE_SUBMODULES_ON_DEMAND) {
-   if 
(!unsorted_string_list_lookup(_submodule_names,
-
submodule->name))
-   continue;
-   default_argv = "on-demand";
-   }
-   } else {
-   if (spf->default_option == 
RECURSE_SUBMODULES_OFF)
-   continue;
-   if (spf->default_option == 
RECURSE_SUBMODULES_ON_DEMAND) {
-   if 
(!unsorted_string_list_lookup(_submodule_names,
- 
submodule->name))
-   continue;
-   default_argv = "on-demand";
-   }
-   }
-   } else if (spf->command_line_option == 
RECURSE_SUBMODULES_ON_DEMAND) {
-   if 
(!unsorted_string_list_lookup(_submodule_names,
+   switch (get_fetch_recurse_config(submodule, spf))
+   {
+   default:
+   case RECURSE_SUBMODULES_DEFAULT:
+   case RECURSE_SUBMODULES_ON_DEMAND:
+   if (!submodule || 
!unsorted_string_list_lookup(_submodule_names,
 submodule->name))
continue;
default_argv = "on-demand";
+   break;
+   case RECURSE_SUBMODULES_ON:
+   default_argv = "yes";
+   break;
+   case RECURSE_SUBMODULES_OFF:
+   continue;
}
 
strbuf_addf(_path, "%s/%s", spf->work_tree, ce->name);
-- 
2.14.1.145.gb3622a4

[PATCH v4 1/3] fetch: add test to make sure we stay backwards compatible

[Heiko Voigt]

The current implementation of submodules supports on-demand fetch if
there is no .gitmodules entry for a submodule. Let's add a test to
document this behavior.

Signed-off-by: Heiko Voigt 
---
 t/t5526-fetch-submodules.sh | 42 +-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 42251f7f3a..43a22f680f 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -478,7 +478,47 @@ test_expect_success "don't fetch submodule when newly 
recorded commits are alrea
git fetch >../actual.out 2>../actual.err
) &&
! test -s actual.out &&
-   test_i18ncmp expect.err actual.err
+   test_i18ncmp expect.err actual.err &&
+   (
+   cd submodule &&
+   git checkout -q master
+   )
+'
+
+test_expect_success "'fetch.recurseSubmodules=on-demand' works also without 
.gitmodule entry" '
+   (
+   cd downstream &&
+   git fetch --recurse-submodules
+   ) &&
+   add_upstream_commit &&
+   head1=$(git rev-parse --short HEAD) &&
+   git add submodule &&
+   git rm .gitmodules &&
+   git commit -m "new submodule without .gitmodules" &&
+   printf "" >expect.out &&
+   head2=$(git rev-parse --short HEAD) &&
+   echo "From $pwd/." >expect.err.2 &&
+   echo "   $head1..$head2  master -> origin/master" >>expect.err.2 &&
+   head -3 expect.err >>expect.err.2 &&
+   (
+   cd downstream &&
+   rm .gitmodules &&
+   git config fetch.recurseSubmodules on-demand &&
+   # fake submodule configuration to avoid skipping submodule 
handling
+   git config -f .gitmodules submodule.fake.path fake &&
+   git config -f .gitmodules submodule.fake.url fakeurl &&
+   git add .gitmodules &&
+   git config --unset submodule.submodule.url &&
+   git fetch >../actual.out 2>../actual.err &&
+   # cleanup
+   git config --unset fetch.recurseSubmodules &&
+   git reset --hard
+   ) &&
+   test_i18ncmp expect.out actual.out &&
+   test_i18ncmp expect.err.2 actual.err &&
+   git checkout HEAD^ -- .gitmodules &&
+   git add .gitmodules &&
+   git commit -m "new submodule restored .gitmodules"
 '
 
 test_expect_success 'fetching submodules respects parallel settings' '
-- 
2.14.1.145.gb3622a4

[PATCH v4 0/3] implement fetching of moved submodules

[Heiko Voigt]

The previous RFC iteration can be found here:

https://public-inbox.org/git/20171006222544.GA26642@sandbox/

This should now be in a state ready for review for inclusion.

The main difference from last iteration is that we now also support
unconfigured gitlinks for push and fetch for backwards compatibility.

To implement this compatibility we construct a default name for gitlinks
if there is a repository found at their location in the worktree.

Cheers Heiko

Heiko Voigt (3):
  fetch: add test to make sure we stay backwards compatible
  implement fetching of moved submodules
  submodule: simplify decision tree whether to or not to fetch

 submodule-config.h  |   3 +
 submodule.c | 200 +---
 t/t5526-fetch-submodules.sh |  77 -
 3 files changed, 210 insertions(+), 70 deletions(-)

-- 
2.14.1.145.gb3622a4

Re: [PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash()

[Junio C Hamano]

"Philip Oakley"  writes:

> Hi, 'Truncate' is real English, but it is not that common in normal usage.
>
> My dictionary suggests that it means 'cut off at the tip' such as a
> truncated cone. However the thesaurus is far more relaxed about the
> common idioms that truncate at the tail such as: clip, crop, cut
> short, trim, abbreviate, curtail, etc.
>
> So perhaps "could not trim '%s'".

Truncate is fine, as there is already another instance that barfs
with "cannot truncate" upon an error from ftruncate(), and the patch
merely matches the two error messages.

Re: [PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash()

[Philip Oakley]

From: "Johannes Schindelin" 

On Mon, 16 Oct 2017, Junio C Hamano wrote:


Ralf Thielow  writes:

> When the write opertion fails, we write that we could
> not read. Change the error message to match the operation
> and remove the full stop at the end.
>
> When ftruncate() fails, we write that we couldn't finish
> the operation on the todo file. It is more accurate to write
> that we couldn't truncate as we do in other calls of ftruncate().

Wouldn't it be more accurate to say we couldn't ftruncate, though?


This is an end-user facing error message, right? Should we not let users
who are happily oblivious of POSIX nomenclature remain happily oblivious?

In other words, I would be finer with "truncate" than with "ftruncate...
wait, huh? Is that even English?"


Hi, 'Truncate' is real English, but it is not that common in normal usage.

My dictionary suggests that it means 'cut off at the tip' such as a 
truncated cone. However the thesaurus is far more relaxed about the common 
idioms that truncate at the tail such as: clip, crop, cut short, trim, 
abbreviate, curtail, etc.


So perhaps "could not trim '%s'".

--
Philip

Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name

[Johannes Schindelin]

Hi Junio,

On Mon, 16 Oct 2017, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> >> >> -Also respects `:remotename` to state the name of the *remote* instead 
> >> >> of
> >> >> -the ref.
> >> >> +Also respects `:remotename` to state the name of the *remote* instead
> >> >> +of the ref, and `:remoteref` to state the name of the *reference* as
> >> >> +locally known by the remote.
> >> >
> >> > What does "locally known by the remote" mean in this sentence?
> >> 
> >> Good question.  I cannot offhand offer a better and concise
> >> phrasing, but I think can explain what it wants to describe ;-).
> >
> > Yep, described it well.
> >
> > Maybe "and `:remoteref` to state the name by which the remote knows the
> > *reference*"?
> 
> I dunno.  
> 
> The original and your update both seem to come from a worldview
> where there is a single conceptual thing called "reference" that is
> shared between our repository and the remote repository we pull from
> (or push to), and the name we call it is "refs/remotes/origin/devel"
> while the name they use to call it is "refs/heads/devel".  If you
> subscribe to that worldview, then the updated sentence might make it
> clearer than the original.
> 
> But I do not share that worldview, and I am not sure (note: I am
> truly unsure---it's not like I am convinced it is a good idea but
> sugarcoating my expression, at least in this case) if subscribing to
> the worldview helps users' understanding.
> 
> In my view "refs/remotes/origin/devel" is a reference we use to keep
> track of (or "a reference that corresponds to") the reference they
> have called "refs/heads/devel" at the remote, and these are two
> separate entities, so it's not like "this single thing is called
> differently by us and them".
> 
> Stepping back a bit; here is how the description begins.
> 
> upstream::
> The name of a local ref which can be considered ``upstream''
> from the displayed ref.
> 
> So 'upstream' is like 'refs/remotes/origin/devel' in the example in
> the message you are responding to.  Perhaps we can make it clear in
> the description, and then add :remote* modifiers are about asking
> where that remote-tracking branch comes from.  For example, instead
> of these "Also respects...", something like:
> 
> For a %(upstream) that is a remote-tracking branch, the name of
> the remote repository it is copied from can be obtained with
> %(upstream:remotename).  Simiarly, the branch at the remote
> repository whose tip is copioed to this remote-tracking branch
> can be obtined with %(upstream:remoteref) as a full refname.
> 
> may reduce the chance of confusion?

Let's take two more steps back.

First, for-each-ref is a low-level command (I do not remember whether our
nickname for "low-level" is "plumbing" or "porcelain" or anything, so I
stick with "low-level" which I deem descriptive enough). That is, users of
this command (and therefore, readers of this man page) need to be quite
familiar with Git's worldview.

Second, the main purpose of this patch series is to answer the question
"what is the default  in `git push 
:`?" for *many* refs at once.

Maybe it would be better to describe the functionality by describing the
question it tries to answer.

Ciao,
Dscho

Re: [PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash()

[Johannes Schindelin]

Hi Junio,

On Mon, 16 Oct 2017, Junio C Hamano wrote:

> Ralf Thielow  writes:
> 
> > When the write opertion fails, we write that we could
> > not read. Change the error message to match the operation
> > and remove the full stop at the end.
> >
> > When ftruncate() fails, we write that we couldn't finish
> > the operation on the todo file. It is more accurate to write
> > that we couldn't truncate as we do in other calls of ftruncate().
> 
> Wouldn't it be more accurate to say we couldn't ftruncate, though?

This is an end-user facing error message, right? Should we not let users
who are happily oblivious of POSIX nomenclature remain happily oblivious?

In other words, I would be finer with "truncate" than with "ftruncate...
wait, huh? Is that even English?"

Ciao,
Dscho

Re: [ANNOUNCE] Git for Windows 2.14.2(3)

[Johannes Schindelin]

Hi Steve,

On Sun, 15 Oct 2017, Johannes Schindelin wrote:

> On Fri, 13 Oct 2017, Steve Hoelzer wrote:
> 
> > On Thu, Oct 12, 2017 at 5:53 PM, Johannes Schindelin
> >  wrote:
> > >
> > > It is my pleasure to announce that Git for Windows 2.14.2(3) is
> > > available from:
> > >
> > > https://git-for-windows.github.io/
> > >
> > > Changes since Git for Windows v2.14.2(2) (October 5th 2017)
> > >
> > > New Features
> > >
> > >   * Comes with Git LFS v2.3.3.
> > 
> > I just ran "git update" and afterward "git version" reported
> > 2.14.2(3), but "git lfs version" still said 2.3.2.
> > 
> > I also uninstalled/reinstalled Git for Windows and LFS is still 2.3.2.
> 
> Ah bummer. I forgot to actually update it in the VM where I build the
> releases :-(
> 
> Will work on it tomorrow.

I'll actually use this opportunity to revamp a part of Git for Windows'
release engineering process to try to prevent similar things from
happening in the future.

Also, cURL v7.56.1 is slated to be released in exactly one week, and I
have some important installer work to do this week, so I'll just defer the
new Git for Windows version tentatively to Monday, 23rd.

Git LFS users can always install Git LFS v2.3.3 specifically in the
meantime, or use Git for Windows' snapshot versions
(https://wingit.blob.core.windows.net/files/index.html).

Ciao,
Johannes

Re: [PATCH] check-ref-format: require a repository for --branch

[Junio C Hamano]

Junio C Hamano  writes:

> Having said that, there may still be a use case where a Porcelain
> script wants a way to see if a $name it has is appropriate as a
> branch name before it has a repository (e.g. a wrapper to "git
> clone" that wants to verify the name it is going to give to the "-b"
> option), and a check desired in such a context is different from
> (and is stricter than) feeding refs/heads/$name to the same command
> without the "--branch" option.
>
> So I think the right endgame in the longer term is:
> ...

Here is to illustrate what I mean in a patch form.  It resurrects
the gentle setup, and uses a purely textual format check when we are
outside the repository, while bypassing the @{magic} interpolation
codepath that requires us to be in a repository.  When we are in a
repository, we operate the same way as before.

Designed to be applied directly on top of 4d03f955
("check-ref-format: require a repository for --branch", 2017-07-14),
so it is missing the "'HEAD' is not a good branch name" I showed a
few days ago.

 builtin/check-ref-format.c  | 16 +---
 cache.h | 14 ++
 sha1_name.c | 22 +++---
 strbuf.h|  6 ++
 t/t1402-check-ref-format.sh | 10 +-
 5 files changed, 61 insertions(+), 7 deletions(-)

diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 1e5f9835f0..4e62852089 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -38,12 +38,22 @@ static char *collapse_slashes(const char *refname)
 
 static int check_ref_format_branch(const char *arg)
 {
+   int nongit, malformed;
struct strbuf sb = STRBUF_INIT;
+   const char *name = arg;
 
-   setup_git_directory();
-   if (strbuf_check_branch_ref(, arg))
+   setup_git_directory_gently();
+
+   if (!nongit)
+   malformed = (strbuf_check_branch_ref(, arg) ||
+!skip_prefix(sb.buf, "refs/heads/", ));
+   else
+   malformed = check_branch_ref_format(arg);
+
+   if (malformed)
die("'%s' is not a valid branch name", arg);
-   printf("%s\n", sb.buf + 11);
+   printf("%s\n", name);
+   strbuf_release();
return 0;
 }
 
diff --git a/cache.h b/cache.h
index 52b91f5b64..3815925122 100644
--- a/cache.h
+++ b/cache.h
@@ -1444,6 +1444,20 @@ extern int parse_oid_hex(const char *hex, struct 
object_id *oid, const char **en
 #define INTERPRET_BRANCH_HEAD (1<<2)
 extern int interpret_branch_name(const char *str, int len, struct strbuf *,
 unsigned allowed);
+
+/*
+ * NEEDSWORK: declare strbuf_branchname() and strbuf_check_branch_ref()
+ * here, not in strbuf.h
+ */
+
+/*
+ * Check if a 'name' is suitable to be used as a branch name; this can
+ * be and is stricter than what check_refname_format() returns for a
+ * string that is a concatenation of "name" after "refs/heads/"; a
+ * name that begins with "-" is not allowed, for example.
+ */
+extern int check_branch_ref_format(const char *name);
+
 extern int get_oid_mb(const char *str, struct object_id *oid);
 
 extern int validate_headref(const char *ref);
diff --git a/sha1_name.c b/sha1_name.c
index 5e2ec37b65..c95080258f 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1319,15 +1319,31 @@ void strbuf_branchname(struct strbuf *sb, const char 
*name, unsigned allowed)
strbuf_add(sb, name + used, len - used);
 }
 
-int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
+static int strbuf_check_branch_ref_format(struct strbuf *sb)
 {
-   strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
-   if (name[0] == '-')
+   if (*sb->buf == '-')
return -1;
strbuf_splice(sb, 0, 0, "refs/heads/", 11);
return check_refname_format(sb->buf, 0);
 }
 
+int check_branch_ref_format(const char *name)
+{
+   struct strbuf sb = STRBUF_INIT;
+   int result;
+
+   strbuf_addstr(, name);
+   result = strbuf_check_branch_ref_format();
+   strbuf_release();
+   return result;
+}
+
+int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
+{
+   strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
+   return strbuf_check_branch_ref_format(sb);
+}
+
 /*
  * This is like "get_sha1_basic()", except it allows "sha1 expressions",
  * notably "xyz^" for "parent of xyz"
diff --git a/strbuf.h b/strbuf.h
index d785258649..3da95685b2 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -568,6 +568,12 @@ static inline void strbuf_complete_line(struct strbuf *sb)
strbuf_complete(sb, '\n');
 }
 
+/*
+ * NEEDSWORK: the following two functions should not be in this file;
+ * these are about refnames, and should be declared next to
+ * interpret_branch_name() in cache.h
+ */
+
 /*
  * Copy "name" to "sb", expanding any special @-marks as handled by
  * interpret_branch_name(). The result is a non-qualified branch name
diff --git 

Re: Does Git build things during 'make install"?

[Junio C Hamano]

Johannes Sixt  writes:

> Yes, running "sudo make install" is a nightmare. sudo clears the path,
> and the git command is not found by the make invoked with root
> permissions. This changes the version string that gets compiled into
> the executable, which finally triggers a complete rebuild under
> root. Sad...

In the meantime, would it help to intall as yourself under DESTDIR
set to where you can write into, and then limit the potential
damange done while pretending to be a privileged user to "cp -R" (or
"tar cf" in $DESTDIR piped to "tar xf" in /)?

It appears that some dependencies are screwed up around "perl"
related things, which may want to get fixed.  I agree that "make &&
make install" that runs two 'make' under the same environment and
user shouldn't (re)build anything during the latter 'make', but we
somehow seem to do so.

DEAR FRIEND.

[Mr.Nawfal Nagi]

Dear Friend,
   I am Mr.Nawfal Nagi the head of file department of Bank of
Africa(B.O.A) here in Burkina Faso / Ouagadougou. In my department we
discover an abandoned sum of (US$18 million US Dollars) in an account
that belongs to one of our foreign customer who died along with his
family in plane crash. It is therefore upon this discovery that I now
decided to make this business proposal to you and release the money to
you as the next of kin or relation to the deceased for the safety and
subsequent disbursement since nobody is coming for it. I agree that
40% of this money will be for you, while 60% would be for me. Then
after the money is been transferred into your account, I will visit
your country for an investment under your kind control.

You have to contact my Bank directly as the real next of kin of this
deceased account with next of kin application form. You have to send
me those your information below to enable me use it and get you next
of kin application form from bank, so that you will contact Bank for
the transfer of this money into your account.

Your Full Name___
Your Home Address
Your Age ___
Your Handset Number
Your Occupation ___

I am waiting for your urgent respond to enable us proceed further for
the transfer through my private e-mail(mrnawfalnag...@gmail.com)

Yours faithfully,
Mr.Nawfal Nagi

What's cooking in git.git (Oct 2017, #03; Mon, 16)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

2.15-rc1 has been tagged, but 2.15-rc2 is going to slip.  The topics
that are cooking in 'next' that are not urgent fixes are classified
as "Will cook in 'next'", and will not graduate to 'master' until
the final.

We haven't decided how to resolve the "git add -i" regression (see
the thread at
https://public-inbox.org/git/xmqqzi8vvht6@gitster.mtv.corp.google.com/
for the two approaches), and 'next' has one of them ("demote
'always' to 'auto' when given to color.ui from the configuration
file"), while 'pu' has f6b2410f20 that takes a different approach
("It was a mistake to allow plumbing to pay attention to color.ui
config, so revert it to unbreak 'add -i'").

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

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

--
[New Topics]

* jk/ui-color-always-to-auto-maint (2017-10-13) 2 commits
  (merged to 'next' on 2017-10-13 at bb16e1edc8)
 + color: document that "git -c color.*=always" is a bit special
 + color: downgrade "always" to "auto" only for on-disk configuration

 It turns out that "git -c color.ui=always cmd" is relied on by many
 third-party tools as a way to force coloured output no matter what
 the end-user configuration is, and a recent attempt to downgrade
 'always' to 'auto' to fix the regression to "git add -p" broke it.


* jk/ref-filter-colors-fix (2017-10-14) 4 commits
 - tag: respect color.ui config
 - Revert "color: check color.ui in git_default_config()"
 - Revert "t6006: drop "always" color config tests"
 - Revert "color: make "always" the same as "auto" in config"

 This is the "theoretically more correct" approach of simply
 stepping back to the state before plumbing commands started paying
 attention to "color.ui" configuration variable.


* jc/branch-name-sanity (2017-10-14) 3 commits
  (merged to 'next' on 2017-10-16 at 174646d1c3)
 + branch: forbid refs/heads/HEAD
 + branch: split validate_new_branchname() into two
 + branch: streamline "attr_only" handling in validate_new_branchname()

 "git branch" and "git checkout -b" are now forbidden from creating
 a branch whose name is "HEAD".

 Will cook in 'next'.


* jk/revision-pruning-optim (2017-10-14) 1 commit
  (merged to 'next' on 2017-10-16 at 2662baa21d)
 + revision: quit pruning diff more quickly when possible

 Pathspec-limited revision traversal was taught not to keep finding
 unneeded differences once it knows two trees are different inside
 given pathspec.

 Will cook in 'next'.


* js/rebase-i-final (2017-10-16) 1 commit
  (merged to 'next' on 2017-10-16 at 72362f5f9c)
 + sequencer.c: fix and unify error messages in rearrange_squash()

 Error message fix.

 Will merge to 'master'.


* wk/merge-options-gpg-sign-doc (2017-10-12) 1 commit
  (merged to 'next' on 2017-10-16 at ae61d824da)
 + Documentation/merge-options.txt: describe -S/--gpg-sign for 'pull'

 Doc updates.

 Will cook in 'next'.


* wk/pull-signoff (2017-10-13) 1 commit
  (merged to 'next' on 2017-10-16 at 5e48f349d9)
 + pull: pass --signoff/--no-signoff to "git merge"

 "git pull" has been taught to accept "--[no-]signoff" option and
 pass it down to "git merge".

 Will cook in 'next'.


* sb/diff-color-move (2017-10-16) 1 commit
  (merged to 'next' on 2017-10-16 at 69de1bad9d)
 + diff: fix infinite loop with --color-moved --ignore-space-change

 A recently added "--color-moved" feature of "diff" fell into
 infinite loop when ignoring whitespace changes, which has been
 fixed.

 Will merge to 'master'.

--
[Stalled]

* mk/use-size-t-in-zlib (2017-08-10) 1 commit
 . zlib.c: use size_t for size

 The wrapper to call into zlib followed our long tradition to use
 "unsigned long" for sizes of regions in memory, which have been
 updated to use "size_t".

 Needs resurrecting by making sure the fix is good and still applies
 (or adjusted to today's codebase).


* mg/status-in-progress-info (2017-05-10) 2 commits
 - status --short --inprogress: spell it as --in-progress
 - status: show in-progress info for short status

 "git status" learns an option to report various operations
 (e.g. "merging") that the user is in the middle of.

 cf. 


* nd/worktree-move (2017-04-20) 6 commits
 - worktree remove: new command
 - worktree move: refuse to move worktrees with submodules
 - worktree move: accept destination as directory
 - worktree move: new command
 - worktree.c: add update_worktree_location()
 - worktree.c: add validate_worktree()

 "git worktree" learned move and remove subcommands.

 Expecting a reroll.
 cf. <20170420101024.7593-1-pclo...@gmail.com>
 

Re: [PATCH] check-ref-format: require a repository for --branch

[Junio C Hamano]

Jeff King  writes:

> On Mon, Jul 17, 2017 at 10:27:09AM -0700, Jonathan Nieder wrote:
>>  ...
>> I don't think it's right.  Today I can do
>> 
>>  $ cd /tmp
>>  $ git check-ref-format --branch master
>>  master
>> 
>> You might wonder why I'd ever do such a thing.  But that's what "git
>> check-ref-format --branch" is for --- it is for taking a 
>> argument and turning it into a branch name.  For example, if you have
>> a script with an $opt_branch variable that defaults to "master", it
>> may do
>> 
>>  resolved_branch=$(git check-ref-format --branch "$opt_branch")
>> 
>> even though it is in a mode that not going to have to use
>> $resolved_branch and it is not running from a repository.
>
> I'm not sure I buy that. What does "turning it into a branch name" even
> mean when you are not in a repository? Clearly @{-1} must fail. And
> everything else is just going to output the exact input you provided.
> So any script calling "check-ref-format --branch" outside of a repo
> would be better off not calling it at all.

I threw this topic in stalled category, hoping that one or the other
opinion eventually turns out to be more prevalent, but it didn't
seem to have happened X-<.

Things like @{-1} would not make any sense when the command is run
outside a repository, and the documentation is quite clear that it
is the primary reason why we added "--branch" option to the command,
i.e.

With the `--branch` option, it expands the ``previous branch syntax''
`@{-n}`.  For example, `@{-1}` is a way to refer the last branch you
were on.  This option should be used by porcelains to accept this
syntax anywhere a branch name is expected, so they can act as if you
typed the branch name.

So I am tempted to take this patch to make sure that we won't gain
more people who abuse the command outside a repository.

Having said that, there may still be a use case where a Porcelain
script wants a way to see if a $name it has is appropriate as a
branch name before it has a repository (e.g. a wrapper to "git
clone" that wants to verify the name it is going to give to the "-b"
option), and a check desired in such a context is different from
(and is stricter than) feeding refs/heads/$name to the same command
without the "--branch" option.

So I think the right endgame in the longer term is:

 - Find (or add if it doesn't exist) a way to recommend to Porcelain
   scripts to use to expand an end-user generated string, and to map
   it to a branch name (it may be "rev-parse --symbolic-full-name
   $name"; I dunno).

 - Keep check-ref-format as (or revert it to be) a tool to "check".
   This would involve split strbuf_check_branch_ref() into two:

   - one that does not do the @{-1} thing and is used ONLY for
 format validity check (including rejecting a name that begins
 with a dash, which is OK for a random ref but not acceptable as
 a branch name);

   - the other that does @{-1} thing before doing the above.
   
   and then making the code call the former, not the latter.

The end result would be that check-ref-format becomes textual check
only, and can be usable (agian) outside repository, with or without
"--branch".  As the current code does not allow us do that yet, I
think it is safer to forbid use of --branch outside the repository
for now, purely as a bugfix.


[Footnote]

*1* In a sense, @{-1} is not something the scripts need to check its
validity of---it is the branch you came from, so by definition
it must be with a good name.  What the scripts want is instead
see what the branch actually is, which is not what
"check-ref-format" is about.

a31dca03 ("check-ref-format --branch: give Porcelain a way to
grok branch shorthand", 2009-03-21) says:

The command may not be the best place to add this new feature, but

$ git check-ref-format --branch "@{-1}"

allows Porcelains to figure out what branch you were on before the last
branch switching.