[PATCH] branch.c: Allow "-" as a short-hand for "@{-1}" in "git branch -d @{-1}"

2016-03-04 Thread Dinesh Polathula
From: Dinesh 

The "-" shorthand can be used as a replacement for "@{-1}" to refer to the 
previous branch the user was on in the "git branch -d @{-1}" command.
Replace "-" argument with "@{-1}" when the command line arguments are parsed.

Signed-off-by: Dinesh 
---
 builtin/branch.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 7b45b6b..98d2c4b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -24,7 +24,7 @@
 static const char * const builtin_branch_usage[] = {
N_("git branch [] [-r | -a] [--merged | --no-merged]"),
N_("git branch [] [-l] [-f]  []"),
-   N_("git branch [] [-r] (-d | -D) ..."),
+   N_("git branch [] [-r] (-d | -D) [-] ..."),
N_("git branch [] (-m | -M) [] "),
N_("git branch [] [-r | -a] [--points-at]"),
NULL
@@ -658,8 +658,13 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
filter.abbrev = -1;
 
if (argc == 2 && !strcmp(argv[1], "-h"))
-   usage_with_options(builtin_branch_usage, options);
-
+   {
+   usage_with_options(builtin_branch_usage, options);  
+   }
+   if (argc == 3 && !strcmp(argv[2], "-"))
+   {
+   argv[2] = "@{-1}";  
+   }
git_config(git_branch_config, NULL);
 
track = git_branch_track;
-- 
2.8.0.rc0

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


[PATCH] Allow "-" as a short-hand for "@{-1}" for branch deletion

2016-03-04 Thread Dinesh Polathula
From: Dinesh 

***
This patch allows the usage of "-" as a short-hand for "@{-1}" in "git branch 
-d @{-1}".

Note : This is a microproject that is part of the Google Summer of Code 
application process.
I am interested in working on the git Beginner mode implementation as part of 
Google Summer of Code. The mentor details for this particular project are not 
available on the Ideas page. The mentors are likely on this mailing list, so I 
request the mentors to drop me a mail so I can get in contact with you to 
further discuss the git Beginner mode project.
***

Dinesh (1):
  branch.c: Allow "-" as a short-hand for "@{-1}" in "git branch -d
@{-1}"

 builtin/branch.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

-- 
2.8.0.rc0

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


Re: [PATCH 1/3] git reset --hard gives clean working tree

2016-03-04 Thread Torsten Bögershausen


On 11.02.16 19:49, Junio C Hamano wrote:
> tbo...@web.de writes:
>
>> From: Torsten Bögershausen 
>>
>> We define the working tree file is clean if either:
>>
>>   * the result of running convert_to_git() on the working tree
>> contents matches what is in the index (because that would mean
>> doing another "git add" on the path is a no-op); OR
>>
>>   * the result of running convert_to_working_tree() on the content
>> in the index matches what is in the working tree (because that
>> would mean doing another "git checkout -f" on the path is a
>> no-op).
>>
>> Add an extra check in ce_compare_data() in read_cache.c, and adjust
>> the test cases in t0025:
>> When a file has CRLF in the index, and is checked out into the working tree,
>> but left unchabged, it is not normalized at the next commit.
>> Whenever the file is changed in the working tree, a line is added/deleted
>> or dos2unix is run, it may be normalized at the next commit,
>> depending on .gitattributes.
>>
>> This patch is a result of a longer discussion on the mailing list,
>> how to fix the flaky t0025.
> Currently, the codepaths that want to stop if it would lose
> information from the index and/or the working tree for the path run
> an equivalent of "diff-files path" to see there is any difference.
> This indeed is overly strict for a path that has contents in the
> index that wouldn't have been created by "clean" conversion (I am
> using this word to mean anything convert_to_git() does, not limited
> to the "clean" filter).
>
> And it is sensible to allow them to proceed if the contents in the
> working tree file for the path match what would be created by
> "smudge" conversion of the contents in the index.
>
> But breaking "diff-files" is not the right way to do so.  Teaching
> "Am I safe to proceed" callers that paths that do not pass
> "diff-files" test may still be safe to work on is.
>
> I did not continue the approach I illustrated because I realized and
> finally convinced myself that touching ce_compare_data() is a wrong
> solution--it breaks "diff-files".
>
> Imagine if you have contents in the index that wouldn't have been
> left by a "clean" conversion of what is in the working tree.  You
> then run "git checkout -f".  Now the contents in the working tree
> will still not convert back to what is in the index with another
> "clean" conversion, but it should pass the "Am I safe to proceed"
> check, namely, it matches what convert_to_worktree() would give.
>
> But imagine further what would happen when you add an extra blank
> line at the end of the file in the working tree (i.e. "echo >>file")
> and then run "diff-files -p".
>
> The illustration patch I gave broke "diff-files" in such a way that
> before such an addition of an extra blank line, it would have said
> "No changes".  And if you run "diff-files" after adding that extra
> blank line, you will see whole bunch of changes, not just the extra
> blank line at the end.
>
> This is sufficient to convince me that the approach is broken.
[]
Would something like this make sense?
(The main part is in diff.c, the rest needs some polishing)

commit e494c31fd2f0f8a638ff14d1b8ae3f3a6d56a107
Author: Torsten Bögershausen 
Date:   Sat Mar 5 07:51:08 2016 +0100

Text files needs to be normalized before diffing
   
Whenever a text file is stored with CRLF in the index, Git changes
CRLF into LF at the next commit.
(text file means the attribute "text" or "crlf" of "eol" is set).
   
"git diff" reports that all lines with CRLF in the index as changed to LF.
After cloning a repo, the work tree is not considered clean by Git, even if
the user didn't change a single line.
   
Avoid to report lines as changed by converting the content from the index 
into
LF before running diff.

diff --git a/convert.c b/convert.c
index f524b8d..af8248d 100644
--- a/convert.c
+++ b/convert.c
@@ -231,9 +231,9 @@ static int has_cr_in_index(const char *path)
 return has_cr;
 }
 
-static int crlf_to_git(const char *path, const char *src, size_t len,
-   struct strbuf *buf,
-   enum crlf_action crlf_action, enum safe_crlf checksafe)
+static int crlf_to_git_internal(const char *path, const char *src, size_t len,
+struct strbuf *buf,
+enum crlf_action crlf_action, enum safe_crlf checksafe)
 {
 struct text_stat stats;
 char *dst;
@@ -852,7 +852,17 @@ const char *get_convert_attr_ascii(const char *path)
 return "";
 }
 
-int convert_to_git(const char *path, const char *src, size_t len,
+int convert_crlf_to_git(const char *path, const char *src, size_t len,
+struct strbuf *buf,
+enum safe_crlf checksafe)
+{
+struct conv_attrs ca;
+convert_attrs(, path);
+return crlf_to_git_internal(path, src, len, buf,
+ca.crlf_action, checksafe);
+
+}
+int convert_to_git(const char *path, const char *src, size_t len,
  

Re: [PATCH 1/2] mergetool: honor tempfile configuration when resolving delete conflicts

2016-03-04 Thread Junio C Hamano
David Aguilar  writes:

> Teach resolve_deleted_merge() to honor the mergetool.keepBackup and
> mergetool.keepTemporaries configuration knobs.
>
> This ensures that the worktree is kept pristine when resolving deletion
> conflicts with the variables both set to false.
>
> Signed-off-by: David Aguilar 
> ---
>  git-mergetool.sh | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index 9f77e3a..615265d 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -126,7 +126,12 @@ resolve_deleted_merge () {
>   case "$ans" in
>   [mMcC]*)
>   git add -- "$MERGED"
> - cleanup_temp_files --save-backup
> + if "$merge_keep_backup" = "true"

The command run as the "if" condition is probably "test", like in
the other hunk?

> + then
> + cleanup_temp_files --save-backup
> + else
> + cleanup_temp_files
> + fi
>   return 0
>   ;;
>   [dD]*)
> @@ -135,6 +140,10 @@ resolve_deleted_merge () {
>   return 0
>   ;;
>   [aA]*)
> + if test "$merge_keep_temporaries" = "false"
> + then
> + cleanup_temp_files
> + fi
>   return 1
>   ;;
>   esac
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] mergetool: support delete/delete conflicts

2016-03-04 Thread David Aguilar
If two branches each move a file into different directories then
mergetool will fail because it assumes that the file being merged, and
its parent directory, are present in the worktree.

Create the merge file's parent directory to allow using the
deleted base version of the file for merge resolution when
encountering a delete/delete conflict.

The end result is that a delete/delete conflict is presented for the
user to resolve.

Reported-by: Joe Einertson 
Signed-off-by: David Aguilar 
---
 git-mergetool.sh | 14 +++---
 t/t7610-mergetool.sh | 30 ++
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 615265d..000f167 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -291,8 +291,14 @@ merge_file () {
return
fi
 
-   mv -- "$MERGED" "$BACKUP"
-   cp -- "$BACKUP" "$MERGED"
+   if test -f "$MERGED"
+   then
+   mv -- "$MERGED" "$BACKUP"
+   cp -- "$BACKUP" "$MERGED"
+   fi
+   # Create a parent directory to handle delete/delete conflicts
+   # where the base's directory no longer exists.
+   mkdir -p "$(dirname "$MERGED")"
 
checkout_staged_file 1 "$MERGED" "$BASE"
checkout_staged_file 2 "$MERGED" "$LOCAL"
@@ -304,7 +310,9 @@ merge_file () {
describe_file "$local_mode" "local" "$LOCAL"
describe_file "$remote_mode" "remote" "$REMOTE"
resolve_deleted_merge
-   return
+   status=$?
+   rmdir -p "$(dirname "$MERGED")" 2>/dev/null
+   return $status
fi
 
if is_symlink "$local_mode" || is_symlink "$remote_mode"
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 6f12b23..f1668be 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -243,6 +243,36 @@ test_expect_success 'mergetool takes partial path' '
git reset --hard
 '
 
+test_expect_success 'mergetool delete/delete conflict' '
+   git checkout -b delete-base branch1 &&
+   mkdir -p a/a &&
+   (echo one; echo two; echo 3; echo 4) >a/a/file.txt &&
+   git add a/a/file.txt &&
+   git commit -m"base file" &&
+   git checkout -b move-to-b delete-base &&
+   mkdir -p b/b &&
+   git mv a/a/file.txt b/b/file.txt &&
+   (echo one; echo two; echo 4) >b/b/file.txt &&
+   git commit -a -m"move to b" &&
+   git checkout -b move-to-c delete-base &&
+   mkdir -p c/c &&
+   git mv a/a/file.txt c/c/file.txt &&
+   (echo one; echo two; echo 3) >c/c/file.txt &&
+   git commit -a -m"move to c" &&
+   ! git merge move-to-b &&
+   echo d | git mergetool a/a/file.txt &&
+   ! test -f a/a/file.txt &&
+   git reset --hard HEAD &&
+   ! git merge move-to-b &&
+   echo m | git mergetool a/a/file.txt &&
+   test -f b/b/file.txt &&
+   git reset --hard HEAD &&
+   ! git merge move-to-b &&
+   ! echo a | git mergetool a/a/file.txt &&
+   ! test -f a/a/file.txt &&
+   git reset --hard HEAD
+'
+
 test_expect_success 'deleted vs modified submodule' '
git checkout -b test6 branch1 &&
git submodule update -N &&
-- 
2.8.0.rc1.2.g28ba210

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


[PATCH 1/2] mergetool: honor tempfile configuration when resolving delete conflicts

2016-03-04 Thread David Aguilar
Teach resolve_deleted_merge() to honor the mergetool.keepBackup and
mergetool.keepTemporaries configuration knobs.

This ensures that the worktree is kept pristine when resolving deletion
conflicts with the variables both set to false.

Signed-off-by: David Aguilar 
---
 git-mergetool.sh | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index 9f77e3a..615265d 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -126,7 +126,12 @@ resolve_deleted_merge () {
case "$ans" in
[mMcC]*)
git add -- "$MERGED"
-   cleanup_temp_files --save-backup
+   if "$merge_keep_backup" = "true"
+   then
+   cleanup_temp_files --save-backup
+   else
+   cleanup_temp_files
+   fi
return 0
;;
[dD]*)
@@ -135,6 +140,10 @@ resolve_deleted_merge () {
return 0
;;
[aA]*)
+   if test "$merge_keep_temporaries" = "false"
+   then
+   cleanup_temp_files
+   fi
return 1
;;
esac
-- 
2.8.0.rc1.2.g28ba210

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


Re: Git clone sends first an empty authorization header

2016-03-04 Thread Guilherme
Hey Bryan,

Yes that will happen he will get a prompet for username/password but
he already provided them in the URL and it worked before. He could
clone. I think this is a little bit confusing.

My problem is that the tool I'm trying to build is trying to provide
the username used to log in via an environment variable. And due to
the fact that the anonymous login is always the first to be tried,
even if the user provides an username/password on the URL I'm not able
to retrieve it.

On Sat, Mar 5, 2016 at 11:20 AM, Bryan Turner  wrote:
> On Fri, Mar 4, 2016 at 9:51 PM, Guilherme  wrote:
>> Hi,
>>
>> When doing basic authentication using git clone by passing the
>> username and password in the url git clone will first send a GET
>> request without the authorization header set.
>>
>> Am i seeing this right?
>
> I believe this is an intentional behavior in either cURL or how Git
> uses it. Credentials aren't sent until the server returns a challenge
> for them, even if you include them in your clone URL or elsewhere. So
> yes, you're seeing it right.
>
>>
>> This means that if the counterpart allows anonymous cloning but not
>> pushing and the user provided a wrong usernam/password, it has two
>> options:
>
> I'm not sure why this would be true. If the remote server allows
> anonymous clone/fetch, then you never get prompted for credentials
> and, even if they're supplied, they're never sent to the remote
> server. If you then try to push, if the server is working correctly it
> should detect that anonymous users can't push and it should return a
> 401 with a WWW-Authenticate header. When the client receives the 401,
> it should send the credentials it has (or prompt for them if it
> doesn't have them) and the push should work without issue.
>
> Perhaps there's an issue with how your server is setup to handle permissions?
>
>>
>> 1. Allow the access and leave the user to figure out why he is not able to 
>> push.
>>
>> 2. Reply by setting the WWW-Authentication header and see if a
>> password/username is provided. This has the downside that if no
>> username and password is provided the user will still get a login
>> prompt for password and username. Upon entering twice nothing he will
>> still be able to clone. This can be confusing.
>>
>> Can this behaviour of git clone (and I guess all the other parts that
>> do basic auth) be changed to provide the authentication header right
>> on the first request? Or am I doing/interpreting it wrong?
>>
>> Thank you.
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git clone sends first an empty authorization header

2016-03-04 Thread Bryan Turner
On Fri, Mar 4, 2016 at 9:51 PM, Guilherme  wrote:
> Hi,
>
> When doing basic authentication using git clone by passing the
> username and password in the url git clone will first send a GET
> request without the authorization header set.
>
> Am i seeing this right?

I believe this is an intentional behavior in either cURL or how Git
uses it. Credentials aren't sent until the server returns a challenge
for them, even if you include them in your clone URL or elsewhere. So
yes, you're seeing it right.

>
> This means that if the counterpart allows anonymous cloning but not
> pushing and the user provided a wrong usernam/password, it has two
> options:

I'm not sure why this would be true. If the remote server allows
anonymous clone/fetch, then you never get prompted for credentials
and, even if they're supplied, they're never sent to the remote
server. If you then try to push, if the server is working correctly it
should detect that anonymous users can't push and it should return a
401 with a WWW-Authenticate header. When the client receives the 401,
it should send the credentials it has (or prompt for them if it
doesn't have them) and the push should work without issue.

Perhaps there's an issue with how your server is setup to handle permissions?

>
> 1. Allow the access and leave the user to figure out why he is not able to 
> push.
>
> 2. Reply by setting the WWW-Authentication header and see if a
> password/username is provided. This has the downside that if no
> username and password is provided the user will still get a login
> prompt for password and username. Upon entering twice nothing he will
> still be able to clone. This can be confusing.
>
> Can this behaviour of git clone (and I guess all the other parts that
> do basic auth) be changed to provide the authentication header right
> on the first request? Or am I doing/interpreting it wrong?
>
> Thank you.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Git clone sends first an empty authorization header

2016-03-04 Thread Guilherme
Hi,

When doing basic authentication using git clone by passing the
username and password in the url git clone will first send a GET
request without the authorization header set.

Am i seeing this right?

This means that if the counterpart allows anonymous cloning but not
pushing and the user provided a wrong usernam/password, it has two
options:

1. Allow the access and leave the user to figure out why he is not able to push.

2. Reply by setting the WWW-Authentication header and see if a
password/username is provided. This has the downside that if no
username and password is provided the user will still get a login
prompt for password and username. Upon entering twice nothing he will
still be able to clone. This can be confusing.

Can this behaviour of git clone (and I guess all the other parts that
do basic auth) be changed to provide the authentication header right
on the first request? Or am I doing/interpreting it wrong?

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


Re: [PATCH] l10n: TEAMS: update Ralf Thielow's email address

2016-03-04 Thread Jiang Xin
That's fine.  Already pulled to git-po.

2016-03-04 4:48 GMT+08:00 Ralf Thielow :
> Signed-off-by: Ralf Thielow 
> ---
>  po/TEAMS | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/po/TEAMS b/po/TEAMS
> index df12b58..56274ad 100644
> --- a/po/TEAMS
> +++ b/po/TEAMS
> @@ -11,7 +11,7 @@ Leader:   Alex Henrie 
>
>  Language:  de (German)
>  Repository:https://github.com/ralfth/git-po-de
> -Leader:Ralf Thielow 
> +Leader:Ralf Thielow 
>  Members:   Thomas Rast 
> Jan Krüger 
> Christian Stimming 
> --
> 2.8.0.rc0.159.g391b917
>



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


Re: Change in .gitignore handling: intended or bug?

2016-03-04 Thread Junio C Hamano
Charles Strahan  writes:

> ...as Duy suggests, I think the new behavior makes a bit
> more sense.

After re-reading your original example, I am inclined to agree with
this.

> Either way, of course, I'd like for it to not change back and
> forth between releases :).
>
> Perhaps just an announcement of the new behavior would suffice -
> 2.7.0 has been out for a while anyway. If people were going to
> complain, I figure they would have done so by now.

Yup, I think a documentation update to clarify how positive and
negative ignore patterns interact with each other may be necessary,
with some examples.

Care to work on a patch?

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


Re: [PATCH 2/2] xdiff/xprepare: fix a memory leak

2016-03-04 Thread Ramsay Jones


On 04/03/16 23:50, Junio C Hamano wrote:
> Ramsay Jones  writes:
> 
>> The xdl_prepare_env() function may initialise an xdlclassifier_t
>> data structure via xdl_init_classifier(), which allocates memory
>> to several fields, for example 'rchash', 'rcrecs' and 'ncha'.
>> If this function later exits due to the failure of xdl_optimize_ctxs(),
>> then this xdlclassifier_t structure, and the memory allocated to it,
>> is not cleaned up.
>>
>> In order to fix the memory leak, insert a call to xdl_free_classifier()
>> before returning.
>>
>> Signed-off-by: Ramsay Jones 
>> ---
>>  xdiff/xprepare.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
>> index 5ffcf99..13b55ab 100644
>> --- a/xdiff/xprepare.c
>> +++ b/xdiff/xprepare.c
>> @@ -301,6 +301,7 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, 
>> xpparam_t const *xpp,
>>  
>>  xdl_free_ctx(>xdf2);
>>  xdl_free_ctx(>xdf1);
>> +xdl_free_classifier();
>>  return -1;
>>  }
> 
> This looks obviously correct from the pattern of prepare's and
> free's in the code that this part follows.  This potential leak has
> been that way since 3443546f (Use a *real* built-in diff generator,
> 2006-03-24), i.e. the very beginning.
> 
> I find it somewhat strange that the call to xdl_free_classifier() at
> the end of this function is made conditional to XDF_HISTOGRAM_DIFF,
> though.  I can half-buy the argument "that is because we do not call
> init-classifier for XDF_HISTOGRAM_DIFF", but in the error path we
> call free-classifier unconditionally, so the code clearly knows that
> it is safe to call free-classifier on a classifier that is cleared
> with the initial memset(, 0, sizeof cf).

Indeed, this is actually why I noticed that XDF_DIFF_ALG() wasn't used.
Rather than doing patch #1, I did consider making this call unconditional.
I can't remember why I didn't. (Hmm, perhaps I just chickened out! ;-))

ATB,
Ramsay Jones


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


[PATCH v4] fetch-pack: fix object_id of exact sha1

2016-03-04 Thread Gabriel Souza Franco
Commit 58f2ed0 (remote-curl: pass ref SHA-1 to fetch-pack as well,
2013-12-05) added support for specifying a SHA-1 as well as a ref name.
Add support for specifying just a SHA-1 and set the ref name to the same
value in this case.

Signed-off-by: Gabriel Souza Franco 
---
 Documentation/git-fetch-pack.txt |  4 
 builtin/fetch-pack.c | 16 +---
 t/t5500-fetch-pack.sh| 14 ++
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-fetch-pack.txt b/Documentation/git-fetch-pack.txt
index 8680f45..239623c 100644
--- a/Documentation/git-fetch-pack.txt
+++ b/Documentation/git-fetch-pack.txt
@@ -104,6 +104,10 @@ be in a separate packet, and the list must end with a 
flush packet.
The remote heads to update from. This is relative to
$GIT_DIR (e.g. "HEAD", "refs/heads/master").  When
unspecified, update from all heads the remote side has.
++
+If the remote has enabled the options `uploadpack.allowTipSHA1InWant` or
+`uploadpack.allowReachableSHA1InWant`, they may alternatively be 40-hex
+sha1s present on the remote.
 
 SEE ALSO
 
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 79a611f..50c9901 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -16,10 +16,20 @@ static void add_sought_entry(struct ref ***sought, int *nr, 
int *alloc,
struct ref *ref;
struct object_id oid;
 
-   if (!get_oid_hex(name, ) && name[GIT_SHA1_HEXSZ] == ' ')
-   name += GIT_SHA1_HEXSZ + 1;
-   else
+   if (!get_oid_hex(name, )) {
+   if (name[GIT_SHA1_HEXSZ] == ' ') {
+   /*  , find refname */
+   name += GIT_SHA1_HEXSZ + 1;
+   } else if (name[GIT_SHA1_HEXSZ] == '\0') {
+   /* , leave sha1 as name */
+   } else {
+   /* , clear cruft from oid */
+   oidclr();
+   }
+   } else {
+   /* , clear cruft from get_oid_hex */
oidclr();
+   }
 
ref = alloc_ref(name);
oidcpy(>old_oid, );
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index e5f83bf..9b9bec4 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -531,6 +531,20 @@ test_expect_success 'shallow fetch with tags does not 
break the repository' '
git fsck
)
 '
+
+test_expect_success 'fetch-pack can fetch a raw sha1' '
+   git init hidden &&
+   (
+   cd hidden &&
+   test_commit 1 &&
+   test_commit 2 &&
+   git update-ref refs/hidden/one HEAD^ &&
+   git config transfer.hiderefs refs/hidden &&
+   git config uploadpack.allowtipsha1inwant true
+   ) &&
+   git fetch-pack hidden $(git -C hidden rev-parse refs/hidden/one)
+'
+
 check_prot_path () {
cat >expected <<-EOF &&
Diag: url=$1
-- 
2.7.2

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


Re: Change in .gitignore handling: intended or bug?

2016-03-04 Thread Charles Strahan
The fix on my side was quite easy (and my .gitignore is probably a _lot_
hairier
than most), and as Duy suggests, I think the new behavior makes a bit
more
sense. Personally, I would be pleased with keeping the new behavior, and
chalking it up to an unintentional bug fix (the best kind).

Either way, of course, I'd like for it to not change back and forth
between
releases :).

Perhaps just an announcement of the new behavior would suffice - 2.7.0
has been
out for a while anyway. If people were going to complain, I figure they
would
have done so by now.

-Charles


On Fri, Mar 4, 2016, at 07:50 PM, Duy Nguyen wrote:
> typo fixes
> 
> On Sat, Mar 5, 2016 at 7:43 AM, Duy Nguyen  wrote:
> > On Sat, Mar 5, 2016 at 12:28 AM, Junio C Hamano  wrote:
> >> Duy Nguyen  writes:
> >>
> >>> On Fri, Mar 4, 2016 at 6:56 PM, Kevin Daudt  wrote:
>  Verified that it's different in 2.7.0, but 2.7.2 gives expected output.
> >>>
> >>> Thanks. 2.7.1 reverts the faulty commit from 2.7.0 that generated two
> >>> other regression reports before this one. I guess it's all good then
> >>> (except for the people still on 2.7.0)
> >>
> >> Are we good at 2.8.0-rc0, too?  Somehow I had an impression that we
> >> queued "another attempt to do it differently" or something.
> >>
> >>  ... goes and looks ...
> >>
> >> $ rungit maint status -suall
> >> ?? baz/quux/corge/wibble.txt
> >> ?? baz/quux/grault.txt
> >> ?? foo/bar.txt
> >> $ rungit master status -suall
> >> ?? baz/quux/corge/wibble.txt
> >> ?? baz/quux/grault.txt
> >> ?? baz/waldo.txt
> >> ?? foo/bar.txt
> >> ?? foo/garply.txt
> >
> > If you swap a60ea8f and bac65a2 so that we can have tracing even
> > without the problematic commit a60ea8f (dir.c: fix match_pathname() -
> > 2016-02-15). Without a60ea8f I got
> >
> > GIT_TRACE_EXCLUDE=1 ~/w/git/temp/git status -suall 2>&1 >/dev/null
> > |grep waldo
> > 07:25:05.639445 dir.c:952   exclude: baz/waldo.txt vs * at
> > line 1 => yes
> >
> > (meaning, baz/waldo.txt matches "*" rule and is decided to be excluded)
> >
> > with a60ea8f
> >
> >> /tmp/abc $ GIT_TRACE_EXCLUDE=1 ~/w/git/temp/git status -suall 2>&1 
> >> >/dev/null |grep waldo
> > 07:25:24.425125 dir.c:952   exclude: baz/waldo.txt vs /baz
> > at line 4 => no
> >
> > the decision is not taken earlier from line "!/baz" and it's decided
> 
> s/not taken/taken/
> 
> > to be re-included. Which I would argue is the correct thing because
> > you ask to re-include the whole directory "baz". It should behave this
> > way because exclude rules without '!' behave this way.
> >
> > Because positive any negative rules can be nested, by adding a rule to
> 
> s/any/and/
> 
> > reinclude what's inside baz..
> >
> > *
> > !/foo
> > !/foo/bar.txt
> > !/baz
> > /baz/*
> > !/baz/quux
> > !/baz/quux/**/*
> >
> > you'll get baz/waldo.txt excluded.
> >
> > Yes it's a behavior change. I think the old behavior is unintended.
> > But it's probably out there long enough that many .gitignore files may
> > rely on it and by making it right, I break them. Whether to revert the
> > series is up to you. Let me know if I should send the revert patch.
> > --
> > Duy
> 
> 
> 
> -- 
> Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Change in .gitignore handling: intended or bug?

2016-03-04 Thread Duy Nguyen
typo fixes

On Sat, Mar 5, 2016 at 7:43 AM, Duy Nguyen  wrote:
> On Sat, Mar 5, 2016 at 12:28 AM, Junio C Hamano  wrote:
>> Duy Nguyen  writes:
>>
>>> On Fri, Mar 4, 2016 at 6:56 PM, Kevin Daudt  wrote:
 Verified that it's different in 2.7.0, but 2.7.2 gives expected output.
>>>
>>> Thanks. 2.7.1 reverts the faulty commit from 2.7.0 that generated two
>>> other regression reports before this one. I guess it's all good then
>>> (except for the people still on 2.7.0)
>>
>> Are we good at 2.8.0-rc0, too?  Somehow I had an impression that we
>> queued "another attempt to do it differently" or something.
>>
>>  ... goes and looks ...
>>
>> $ rungit maint status -suall
>> ?? baz/quux/corge/wibble.txt
>> ?? baz/quux/grault.txt
>> ?? foo/bar.txt
>> $ rungit master status -suall
>> ?? baz/quux/corge/wibble.txt
>> ?? baz/quux/grault.txt
>> ?? baz/waldo.txt
>> ?? foo/bar.txt
>> ?? foo/garply.txt
>
> If you swap a60ea8f and bac65a2 so that we can have tracing even
> without the problematic commit a60ea8f (dir.c: fix match_pathname() -
> 2016-02-15). Without a60ea8f I got
>
> GIT_TRACE_EXCLUDE=1 ~/w/git/temp/git status -suall 2>&1 >/dev/null
> |grep waldo
> 07:25:05.639445 dir.c:952   exclude: baz/waldo.txt vs * at
> line 1 => yes
>
> (meaning, baz/waldo.txt matches "*" rule and is decided to be excluded)
>
> with a60ea8f
>
>> /tmp/abc $ GIT_TRACE_EXCLUDE=1 ~/w/git/temp/git status -suall 2>&1 
>> >/dev/null |grep waldo
> 07:25:24.425125 dir.c:952   exclude: baz/waldo.txt vs /baz
> at line 4 => no
>
> the decision is not taken earlier from line "!/baz" and it's decided

s/not taken/taken/

> to be re-included. Which I would argue is the correct thing because
> you ask to re-include the whole directory "baz". It should behave this
> way because exclude rules without '!' behave this way.
>
> Because positive any negative rules can be nested, by adding a rule to

s/any/and/

> reinclude what's inside baz..
>
> *
> !/foo
> !/foo/bar.txt
> !/baz
> /baz/*
> !/baz/quux
> !/baz/quux/**/*
>
> you'll get baz/waldo.txt excluded.
>
> Yes it's a behavior change. I think the old behavior is unintended.
> But it's probably out there long enough that many .gitignore files may
> rely on it and by making it right, I break them. Whether to revert the
> series is up to you. Let me know if I should send the revert patch.
> --
> Duy



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


Re: Change in .gitignore handling: intended or bug?

2016-03-04 Thread Duy Nguyen
On Sat, Mar 5, 2016 at 12:28 AM, Junio C Hamano  wrote:
> Duy Nguyen  writes:
>
>> On Fri, Mar 4, 2016 at 6:56 PM, Kevin Daudt  wrote:
>>> Verified that it's different in 2.7.0, but 2.7.2 gives expected output.
>>
>> Thanks. 2.7.1 reverts the faulty commit from 2.7.0 that generated two
>> other regression reports before this one. I guess it's all good then
>> (except for the people still on 2.7.0)
>
> Are we good at 2.8.0-rc0, too?  Somehow I had an impression that we
> queued "another attempt to do it differently" or something.
>
>  ... goes and looks ...
>
> $ rungit maint status -suall
> ?? baz/quux/corge/wibble.txt
> ?? baz/quux/grault.txt
> ?? foo/bar.txt
> $ rungit master status -suall
> ?? baz/quux/corge/wibble.txt
> ?? baz/quux/grault.txt
> ?? baz/waldo.txt
> ?? foo/bar.txt
> ?? foo/garply.txt

If you swap a60ea8f and bac65a2 so that we can have tracing even
without the problematic commit a60ea8f (dir.c: fix match_pathname() -
2016-02-15). Without a60ea8f I got

GIT_TRACE_EXCLUDE=1 ~/w/git/temp/git status -suall 2>&1 >/dev/null
|grep waldo
07:25:05.639445 dir.c:952   exclude: baz/waldo.txt vs * at
line 1 => yes

(meaning, baz/waldo.txt matches "*" rule and is decided to be excluded)

with a60ea8f

> /tmp/abc $ GIT_TRACE_EXCLUDE=1 ~/w/git/temp/git status -suall 2>&1 >/dev/null 
> |grep waldo
07:25:24.425125 dir.c:952   exclude: baz/waldo.txt vs /baz
at line 4 => no

the decision is not taken earlier from line "!/baz" and it's decided
to be re-included. Which I would argue is the correct thing because
you ask to re-include the whole directory "baz". It should behave this
way because exclude rules without '!' behave this way.

Because positive any negative rules can be nested, by adding a rule to
reinclude what's inside baz..

*
!/foo
!/foo/bar.txt
!/baz
/baz/*
!/baz/quux
!/baz/quux/**/*

you'll get baz/waldo.txt excluded.

Yes it's a behavior change. I think the old behavior is unintended.
But it's probably out there long enough that many .gitignore files may
rely on it and by making it right, I break them. Whether to revert the
series is up to you. Let me know if I should send the revert patch.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Diff filename has trailing tab if filename contains space

2016-03-04 Thread Junio C Hamano
乙酸鋰  writes:

> Hi,
>
> Using git 2.7.1
>
> Diff filename has trailing tab if filename contains space

Thanks; that is very much deliberate and has been with us forever.

commit 1a9eb3b9d50367bee8fe85022684d812816fe531
Author: Junio C Hamano 
Date:   Fri Sep 22 16:17:58 2006 -0700

git-diff/git-apply: make diff output a bit friendlier to GNU patch (part 2)

Somebody was wondering on #git channel why a git generated diff
does not apply with GNU patch when the filename contains a SP.
It is because GNU patch expects to find TAB (and trailing timestamp)
on ---/+++ (old_name and new_name) lines after the filenames.

The "diff --git" output format was carefully designed to be
compatible with GNU patch where it can, but whitespace
characters were always a pain.

This adds an extra TAB (but not trailing timestamp) to old_name
and new_name lines of git-diff output when the filename has a SP
in it.  An earlier patch updated git-apply to prepare for this.

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


Re: [PATCH 2/2] xdiff/xprepare: fix a memory leak

2016-03-04 Thread Junio C Hamano
Ramsay Jones  writes:

> The xdl_prepare_env() function may initialise an xdlclassifier_t
> data structure via xdl_init_classifier(), which allocates memory
> to several fields, for example 'rchash', 'rcrecs' and 'ncha'.
> If this function later exits due to the failure of xdl_optimize_ctxs(),
> then this xdlclassifier_t structure, and the memory allocated to it,
> is not cleaned up.
>
> In order to fix the memory leak, insert a call to xdl_free_classifier()
> before returning.
>
> Signed-off-by: Ramsay Jones 
> ---
>  xdiff/xprepare.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
> index 5ffcf99..13b55ab 100644
> --- a/xdiff/xprepare.c
> +++ b/xdiff/xprepare.c
> @@ -301,6 +301,7 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, 
> xpparam_t const *xpp,
>  
>   xdl_free_ctx(>xdf2);
>   xdl_free_ctx(>xdf1);
> + xdl_free_classifier();
>   return -1;
>   }

This looks obviously correct from the pattern of prepare's and
free's in the code that this part follows.  This potential leak has
been that way since 3443546f (Use a *real* built-in diff generator,
2006-03-24), i.e. the very beginning.

I find it somewhat strange that the call to xdl_free_classifier() at
the end of this function is made conditional to XDF_HISTOGRAM_DIFF,
though.  I can half-buy the argument "that is because we do not call
init-classifier for XDF_HISTOGRAM_DIFF", but in the error path we
call free-classifier unconditionally, so the code clearly knows that
it is safe to call free-classifier on a classifier that is cleared
with the initial memset(, 0, sizeof cf).

I think the latter funkiness came from 9f37c275.






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


Diff filename has trailing tab if filename contains space

2016-03-04 Thread 乙酸鋰
Hi,

Using git 2.7.1

Diff filename has trailing tab if filename contains space
Please run below shell script
and look at the output diff file 1.diff
There is trailing tab chars after these lines:
--- a/8 1/8.txt
+++ b/8 1/8.txt
 b/9 86
+++ b/9 86



#!/bin/sh
set -e
git init

echo a >> "normal"
git add "normal"
echo y >> "9 86"
git add "9 86" # file name has space
mkdir x
echo d >> "x/normal"
git add "x/normal"
mkdir "8 1"
echo u >> "8 1/8.txt" # directory name has space
echo k >> "8 1/8.txt"
git add "8 1/8.txt"
git commit -m "Initial commit"

echo b >> "normal"
git add "normal"
echo c >> "9 86"
git add "9 86"
echo e >> "x/normal"
git add "x/normal"
echo h >> "8 1/8.txt"
git add "8 1/8.txt"
git commit -m "Edit files"
git diff master~1 > 1.diff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] xdiff/xprepare: fix a memory leak

2016-03-04 Thread Ramsay Jones

The xdl_prepare_env() function may initialise an xdlclassifier_t
data structure via xdl_init_classifier(), which allocates memory
to several fields, for example 'rchash', 'rcrecs' and 'ncha'.
If this function later exits due to the failure of xdl_optimize_ctxs(),
then this xdlclassifier_t structure, and the memory allocated to it,
is not cleaned up.

In order to fix the memory leak, insert a call to xdl_free_classifier()
before returning.

Signed-off-by: Ramsay Jones 
---
 xdiff/xprepare.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
index 5ffcf99..13b55ab 100644
--- a/xdiff/xprepare.c
+++ b/xdiff/xprepare.c
@@ -301,6 +301,7 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t 
const *xpp,
 
xdl_free_ctx(>xdf2);
xdl_free_ctx(>xdf1);
+   xdl_free_classifier();
return -1;
}
 
-- 
2.7.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] xdiff/xprepare: use the XDF_DIFF_ALG() macro to access flag bits

2016-03-04 Thread Ramsay Jones

Commit 307ab20b3 ("xdiff: PATIENCE/HISTOGRAM are not independent option
bits", 19-02-2012) introduced the XDF_DIFF_ALG() macro to access the
flag bits used to represent the diff algorithm requested. In addition,
code which had used explicit manipulation of the flag bits was changed
to use the macros.

However, one example of direct manipulation remains. Update this code to
use the XDF_DIFF_ALG() macro.

Signed-off-by: Ramsay Jones 
---
 xdiff/xprepare.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
index 63a22c6..5ffcf99 100644
--- a/xdiff/xprepare.c
+++ b/xdiff/xprepare.c
@@ -304,7 +304,7 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t 
const *xpp,
return -1;
}
 
-   if (!(xpp->flags & XDF_HISTOGRAM_DIFF))
+   if (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF)
xdl_free_classifier();
 
return 0;
-- 
2.7.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/2] diff: fix a memory leak

2016-03-04 Thread Ramsay Jones

A recent memory leak patch from Patrick, commit 4867f1184
("xdiff/xmerge: fix memory leak in xdl_merge", 23-02-2016),
reminded me that I had a similar patch lying around.

After checking that it wasn't the same one, I dusted it off, and
split it into these two patches.

One of the reasons for not sending it earlier was that it is
not very likely to happen (without very large files), and I had
to use the debugger to confirm the leak and the fix.

[If you want to do so yourself, then I suggest that you use
two files that actually have changes and do 'run --no-pager diff
--no-index file-1 file-2'. set breakpoints on xdl_prepare_env,
xdl_init_classifier, xdl_free_classifier, xdl_optimize_ctxs and
xdl_cleanup_records. You have to force xdl_cleanup_records to
return -1 to simulate an OOM. Since xdl_free_classifier does
not set the allocated fields to NULL, you have to make sure that
you do/don't hit that breakpoint before the return at line #304/305.]

Ramsay Jones (2):
  xdiff/xprepare: use the XDF_DIFF_ALG() macro to access flag bits
  xdiff/xprepare: fix a memory leak

 xdiff/xprepare.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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


[ANNOUNCE] Git v2.8.0-rc1

2016-03-04 Thread Junio C Hamano
A release candidate Git v2.8.0-rc1 is now available for testing
at the usual places.  It is comprised of 453 non-merge commits
since v2.7.0, contributed by 59 people, 19 of which are new faces.

There still is a known regression around "git status" (and "ls-files
-o") relative to v2.7.2, which we may end up resolving by reverting
a topic before v2.8.0 final.  We'll see how it goes.

The tarballs are found at:

https://www.kernel.org/pub/software/scm/git/testing/

The following public repositories all have a copy of the
'v2.8.0-rc1' tag and the 'master' branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git

New contributors whose contributions weren't in v2.7.0 are as follows.
Welcome to the Git development community!

  마누엘, Andrew Wheeler, Changwoo Ryu, Christoph Egger,
  Dan Aloni, Dave Ware, David A. Wheeler, Dickson Wong, Felipe
  Gonçalves Assis, GyuYong Jung, Jon Griffiths, Kazutoshi Satoda,
  Lars Vogel, Martin Amdisen, Matthew Kraai, Paul Wagland, Rob
  Mayoff, Romain Picard, and Victor Leschuk.

Returning contributors who helped this release are as follows.
Thanks for your continued support.

  Alexander Kuleshov, Alex Henrie, brian m. carlson, Christian
  Couder, David A. Greene, David Turner, Dennis Kaarsemaker,
  Edmundo Carmona Antoranz, Elia Pinto, Eric Wong, Jacob Keller,
  Jeff King, Johannes Schindelin, Johannes Sixt, John Keeping,
  Jonathan Nieder, Junio C Hamano, Karsten Blees, Karthik Nayak,
  Knut Franke, Lars Schneider, Matthieu Moy, Matt McCutchen,
  Michael J Gruber, Mike Hommey, Nguyễn Thái Ngọc Duy,
  Øyvind A. Holm, Patrick Steinhardt, Pat Thoyts, Sebastian
  Schuberth, Shawn O. Pearce, Stefan Beller, Stephen P. Smith,
  SZEDER Gábor, Thomas Ackermann, Thomas Braun, Thomas Gummerer,
  Tobias Klauser, Torsten Bögershausen, and Will Palmer.



Git 2.8 Release Notes (draft)
=

Backward compatibility note
---

The rsync:// transport has been removed.


Updates since v2.7
--

UI, Workflows & Features

 * It turns out "git clone" over rsync transport has been broken when
   the source repository has packed references for a long time, and
   nobody noticed nor complained about it.

 * "branch --delete" has "branch -d" but "push --delete" does not.

 * "git blame" learned to produce the progress eye-candy when it takes
   too much time before emitting the first line of the result.

 * "git grep" can now be configured (or told from the command line)
   how many threads to use when searching in the working tree files.

 * Some "git notes" operations, e.g. "git log --notes=", should
   be able to read notes from any tree-ish that is shaped like a notes
   tree, but the notes infrastructure required that the argument must
   be a ref under refs/notes/.  Loosen it to require a valid ref only
   when the operation would update the notes (in which case we must
   have a place to store the updated notes tree, iow, a ref).

 * "git grep" by default does not fall back to its "--no-index"
   behaviour outside a directory under Git's control (otherwise the
   user may by mistake end up running a huge recursive search); with a
   new configuration (set in $HOME/.gitconfig--by definition this
   cannot be set in the config file per project), this safety can be
   disabled.

 * "git pull --rebase" has been extended to allow invoking
   "rebase -i".

 * "git p4" learned to cope with the type of a file getting changed.

 * "git format-patch" learned to notice format.outputDirectory
   configuration variable.  This allows "-o " option to be
   omitted on the command line if you always use the same directory in
   your workflow.

 * "interpret-trailers" has been taught to optionally update a file in
   place, instead of always writing the result to the standard output.

 * Many commands that read files that are expected to contain text
   that is generated (or can be edited) by the end user to control
   their behaviour (e.g. "git grep -f ") have been updated
   to be more tolerant to lines that are terminated with CRLF (they
   used to treat such a line to contain payload that ends with CR,
   which is usually not what the users expect).

 * "git notes merge" used to limit the source of the merged notes tree
   to somewhere under refs/notes/ hierarchy, which was too limiting
   when inventing a workflow to exchange notes with remote
   repositories using remote-tracking notes trees (located in e.g.
   refs/remote-notes/ or somesuch).

 * "git ls-files" learned a new "--eol" option to help diagnose
   end-of-line problems.

 * "ls-remote" learned an option to show which branch the remote
   repository advertises as its primary by pointing 

What's cooking in git.git (Mar 2016, #02; Fri, 4)

2016-03-04 Thread 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.

v2.8-rc1 has been tagged.  There still is a known regression around
"git status" (and "ls-files -o") relative to 2.7.2, which we may end
up resolving by reverting a topic by 2.8 final.  We'll see.

Again, the topics that have not been cooked sufficiently in 'next'
at this point will not be considered for 2.8 final, even though I
might later succumb to the temptation to pick up ones that look
trivially correct.  Those who have their topics merged to 'master'
since v2.7 are expected to focus on responding to regressions and
remaining bugs in their topics in 'master', and strongly encouraged
to actively hunt for regressions and remaining bugs there, not in a
random shiny new feature, during the pre-release period.

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

--
[Graduated to "master"]

* jk/pack-idx-corruption-safety (2016-02-27) 4 commits
  (merged to 'next' on 2016-03-01 at 49e08d3)
 + sha1_file.c: mark strings for translation
  (merged to 'next' on 2016-02-26 at ef0d6de)
 + use_pack: handle signed off_t overflow
 + nth_packed_object_offset: bounds-check extended offset
 + t5313: test bounds-checks of corrupted/malicious pack/idx files

 The code to read the pack data using the offsets stored in the pack
 idx file has been made more carefully check the validity of the
 data in the idx.


* jk/tighten-alloc (2016-02-29) 1 commit
  (merged to 'next' on 2016-03-01 at f4df936)
 + compat/mingw: brown paper bag fix for 50a6c8e


* js/pthread-exit-emu-windows (2016-03-02) 1 commit
  (merged to 'next' on 2016-03-02 at 391b917)
 + Mark win32's pthread_exit() as NORETURN


* mg/httpd-tests-update-for-apache-2.4 (2016-02-25) 1 commit
  (merged to 'next' on 2016-03-01 at d2f7e8c)
 + t/lib-httpd: load mod_unixd
 (this branch is used by jk/submodule-c-credential.)

 The way the test scripts configure the Apache web server has been
 updated to work also for Apache 2.4 running on RedHat derived
 distros.


* nd/clear-gitenv-upon-use-of-alias (2016-03-03) 1 commit
  (merged to 'next' on 2016-03-03 at 1c1c50f)
 + t0001: fix GIT_* environment variable check under --valgrind

 Hotfix for a test breakage made between 2.7 and 'master'.


* nd/i18n-2.8.0 (2016-02-29) 4 commits
  (merged to 'next' on 2016-03-01 at cdf4675)
 + trailer.c: mark strings for translation
 + ref-filter.c: mark strings for translation
 + builtin/clone.c: mark strings for translation
 + builtin/checkout.c: mark strings for translation


* sb/submodule-parallel-fetch (2016-03-01) 1 commit
  (merged to 'next' on 2016-03-01 at b47ab6e)
 + run-command: do not pass child process data into callbacks
 (this branch is used by sb/submodule-init and sb/submodule-parallel-update.)

 Simplify the two callback functions that are triggered when the
 child process terminates to avoid misuse of the child-process
 structure that has already been cleaned up.


* tb/avoid-gcc-on-darwin-10-6 (2016-02-28) 1 commit
  (merged to 'next' on 2016-03-01 at e8dd08a)
 + config.mak.uname: use clang for Mac OS X 10.6

 Out of maintenance gcc on OSX 10.6 fails to compile the code in
 'master'; work it around by using clang by default on the platform.

--
[New Topics]

* sb/rebase-summary (2016-03-02) 1 commit
  (merged to 'next' on 2016-03-04 at d40714d)
 + Documentation: reword rebase summary

 Will merge to 'master' by 2.8-rc2.


* jc/index-pack (2016-03-03) 2 commits
 - index-pack: add a helper function to derive .idx/.keep filename
 - Merge branch 'jc/maint-index-pack-keep' into jc/index-pack
 (this branch is used by jc/bundle; uses jc/maint-index-pack-keep; is tangled 
with jc/index-pack-clone-bundle.)

 Code clean-up.

 Will merge to 'next'.


* jc/maint-index-pack-keep (2016-03-03) 1 commit
  (merged to 'next' on 2016-03-04 at bc1d37a)
 + index-pack: correct --keep[=]
 (this branch is used by jc/bundle, jc/index-pack and 
jc/index-pack-clone-bundle.)

 "git index-pack --keep[=] pack-$name.pack" simply did not work.

 Will merge to 'master' after 2.8 final.


* js/close-packs-before-gc (2016-03-04) 1 commit
  (merged to 'next' on 2016-03-04 at fe6f6ed)
 + t5510: do not leave changed cwd

 A small future-proofing of a test added recently.

 Will merge to 'master' by 2.8-rc2.

--
[Stalled]

* ec/annotate-deleted (2015-11-20) 1 commit
 - annotate: skip checking working tree if a revision is provided

 Usability fix for annotate-specific " " syntax with deleted
 files.

 Waiting for review.


* dg/subtree-rebase-test (2016-01-19) 1 commit
 - contrib/subtree: Add a 

git-completion.bash emitting error messages

2016-03-04 Thread Дилян Палаузов

Hello,

I use ./bash 4.3.42 with bundled libreadline and 
git/contrib/completion/git-completion.bash , as modified by commit 
8716bdca268 from 22 Feb 2016.


I cd to git.git (non-bare, with working tree) and type

git b62c.. ori to force TAB-completion. And then on the shell comes 
the output  (on the same line) "error: key does not contain variable 
name: alias.b62c.."  Pressing again TAB repeats the message.  Even if 
TAB is pressed once, the printed text cannot be deleted with the 
backspace, so the only way to do something useful is to press ENTER.


While the command is indeed nonsense, in my understanding tab-completion 
shall do nothing, if completion is not possible.  In particular it shall 
not print error messages and even the user types stupid things, he shall 
not be required to force ENTER to come back to a clear state, where a 
command can by types.


Greetings
  Dilian


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


Re: [PATCH] t9700: fix test for perl older than 5.14

2016-03-04 Thread Christian Couder
On Fri, Mar 4, 2016 at 1:21 PM, Dennis Kaarsemaker
 wrote:
> On vr, 2016-03-04 at 06:43 -0500, Jeff King wrote:
>> On Fri, Mar 04, 2016 at 11:58:24AM +0100, Dennis Kaarsemaker wrote:
>>
>> > On vr, 2016-03-04 at 03:56 -0500, Jeff King wrote:
>> > > ? Those are just guesses, but if we are tickling a bug in perl's parser,
>> > > this might avoid them. I also wondered when "/r" appeared. It was in
>> > > 5.14, so you're presumably good there. The "use" statement at the top of
>> > > the script says "5.008", so perhaps we should be writing it out longhand
>> > > anyway (that version is "only" 5 years old, so I suspect there are still
>> > > systems around with 5.12 or older).
>> >
>> > Knowing the system Christian is testing on, I think the problem is that
>> > the tests are actually being run against perl 5.10, which RHEL 6 ships
>> > as system perl. As that's still a supported OS, writing tests in a form
>> > compatible with it would be a good thing :)
>>
>> That would make sense. `perl` in t9700-perl-git.sh (and all of our
>> scripts) is actually a shell function:
>>
>>   perl () {
>>   command "$PERL_PATH" "$@"
>>   }
>>
>> to make sure we respect PERL_PATH everywhere. And that defaults in the
>> Makefile to /usr/bin/perl. Christian presumably has 5.14 in his $PATH,
>> but /usr/bin/perl is the system 5.10.
>
> Yeah, that's how our systems are set up.

Yeah, you are both right.

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


Re: Change in .gitignore handling: intended or bug?

2016-03-04 Thread Junio C Hamano
Junio C Hamano  writes:

> Duy Nguyen  writes:
>
>> On Fri, Mar 4, 2016 at 6:56 PM, Kevin Daudt  wrote:
>>> Verified that it's different in 2.7.0, but 2.7.2 gives expected output.
>>
>> Thanks. 2.7.1 reverts the faulty commit from 2.7.0 that generated two
>> other regression reports before this one. I guess it's all good then
>> (except for the people still on 2.7.0)
>
> Are we good at 2.8.0-rc0, too?  Somehow I had an impression that we
> queued "another attempt to do it differently" or something.
>
>  ... goes and looks ...
>
> $ rungit maint status -suall
> ?? baz/quux/corge/wibble.txt
> ?? baz/quux/grault.txt
> ?? foo/bar.txt
> $ rungit master status -suall
> ?? baz/quux/corge/wibble.txt
> ?? baz/quux/grault.txt
> ?? baz/waldo.txt
> ?? foo/bar.txt
> ?? foo/garply.txt

It seems to bisect down to this one between maint..master:

commit a60ea8fb66945a886ea53fd3f41e61cc5fb3201e
Author: Nguyễn Thái Ngọc Duy 
Date:   Mon Feb 15 16:03:36 2016 +0700

dir.c: fix match_pathname()

Given the pattern "1/2/3/4" and the path "1/2/3/4/f", the pattern
prefix is "1/2/3/4". We will compare and remove the prefix from both
pattern and path and come to this code

/*
 * If the whole pattern did not have a wildcard,
 * then our prefix match is all we need; we
 * do not need to call fnmatch at all.
 */
if (!patternlen && !namelen)
return 1;

where patternlen is zero (full pattern consumed) and the remaining
path in "name" is "/f". We fail to realize it's matched in this case
and fall back to fnmatch(), which also fails to catch it. Fix it.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 

But I do not think this change alone is the culprit; the change
itself does make sense in the context of the series.

At this point, we have two choices.  Either we revert the merge of
the whole series:

commit 5e57f9c3dfe7dd44a1b56bb5b3327d7a1356ec7c
Merge: e79112d d589a67
Author: Junio C Hamano 
Date:   Wed Feb 24 13:25:59 2016 -0800

Merge branch 'nd/exclusion-regression-fix'

Another try to add support to the ignore mechanism that lets you
say "this is excluded" and then later say "oh, no, this part (that
is a subset of the previous part) is not excluded".

* nd/exclusion-regression-fix:
  dir.c: don't exclude whole dir prematurely
  dir.c: support marking some patterns already matched
  dir.c: support tracing exclude
  dir.c: fix match_pathname()

to go back to the 2.7.2 behaviour, or add a follow-on change to the
nd/exclusion-regression-fix topic to fix what it wanted to fix
without breaking Charles's use case.  I am inclined to go for the
former (unless the follow-on fix is really trivial), but I haven't
dug into the codebase myself yet, so...

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


Re: [PATCH] stripspace: add --line-count flag

2016-03-04 Thread Junio C Hamano
Junio C Hamano  writes:

> Sidhant Sharma  writes:
>
>> This is my first attempt at the small project listed here: 
>> https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#implement_.27--count-lines.27_in_.27git_stripspace.27.
>
>> Comments?
>
> Isn't that page somewhat stale?
>
> http://git.661346.n2.nabble.com/PATCH-0-3-stripspace-Implement-and-use-count-lines-option-tt7641360.html#none

And its reroll:

  http://thread.gmane.org/gmane.comp.version-control.git/279742

The discussion seems to indicate that we weren't in favor of
addition of this feature at least back then, e.g.

http://thread.gmane.org/gmane.comp.version-control.git/279742/focus=279888

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


Re: [PATCH] stripspace: add --line-count flag

2016-03-04 Thread Sidhant Sharma


On Saturday 05 March 2016 12:19 AM, Junio C Hamano wrote:
> Sidhant Sharma  writes:
>
>> This is my first attempt at the small project listed here: 
>> https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#implement_.27--count-lines.27_in_.27git_stripspace.27.
>> Comments?
> Isn't that page somewhat stale?
>
> http://git.661346.n2.nabble.com/PATCH-0-3-stripspace-Implement-and-use-count-lines-option-tt7641360.html#none
Oh, I should've checked first. My bad, I was just looking to get familiar with 
the codebase.


Thanks

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


Re: git diff does not precompose unicode file paths (OS X)

2016-03-04 Thread Ramsay Jones


On 04/03/16 14:37, Alexander Rinass wrote:
> 
>> On 04 Mar 2016, at 13:16, Torsten Bögershausen  wrote:
>>
>> On 03/04/2016 10:07 AM, Alexander Rinass wrote:
[snip]

> 
> Sticking a precompose_argv(argc, argv) into diff.c’s cmd_diff function fixes 
> the issue.
> 
> But I had to disable the check (precomposed_unicode != 1) in precompose_argv 
> to make it work. That’s probably because precompose_argv is usually called 
> from parse_options and is missing some other call before it?
> 

Yes, you need to place it after the configuration is read, but before
calls to diff_no_index() or setup_revisions(). Directly after the call
to git_config() should be fine. [But this begs the question about other
commands, including plumbing, which don't call parse_options().]

Maybe this will work for you (I can't test it, since I don't have any
access to a Mac):

diff --git a/builtin/diff.c b/builtin/diff.c
index 343c6b8..b7a9405 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -320,6 +320,7 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
gitmodules_config();
init_diff_ui_defaults();
git_config(git_diff_ui_config, NULL);
+   precompose_argv(argc, argv);
 
init_revisions(, prefix);
 


ATB,
Ramsay Jones


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


Re: [PATCH] stripspace: add --line-count flag

2016-03-04 Thread Junio C Hamano
Sidhant Sharma  writes:

> This is my first attempt at the small project listed here: 
> https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#implement_.27--count-lines.27_in_.27git_stripspace.27.

> Comments?

Isn't that page somewhat stale?

http://git.661346.n2.nabble.com/PATCH-0-3-stripspace-Implement-and-use-count-lines-option-tt7641360.html#none
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] stripspace: add --line-count flag

2016-03-04 Thread Sidhant Sharma

>  builtin/stripspace.c   | 22 +-
>  git-rebase--interactive.sh |  6 +++---
>  2 files changed, 24 insertions(+), 4 deletions(-)
>
This is my first attempt at the small project listed here: 
https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#implement_.27--count-lines.27_in_.27git_stripspace.27.
With this, --line-count can be used with stripspace, instead of having to pipe 
its output to `wc -l` in git-rebase--interactive.sh. I went with --line-count 
and not --count-lines since its short form (-c) is already in use, and I think 
-l is more apt for this.
Comments?


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


[PATCH] stripspace: add --line-count flag

2016-03-04 Thread Sidhant Sharma [:tk]
When used, this flag outputs number of lines after stripspace has removed 
trailing whitespace.
With `--line-count`, git-rebase--interactive.sh need not rely on `wc -l` for 
line
count.

Signed-off-by: Sidhant Sharma [:tk] 
---

 This the first version of the patch for the small project listed here:
 
https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#implement_.27--count-lines.27_in_.27git_stripspace.27

 builtin/stripspace.c   | 22 +-
 git-rebase--interactive.sh |  6 +++---
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index 15e716e..e08da03 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -13,22 +13,38 @@ static void comment_lines(struct strbuf *buf)
free(msg);
 }

+static void count_lines(struct strbuf *buf)
+{
+   size_t len = 0;
+   int i;
+
+   for (i = 0; i < buf->len; i++)
+   if (buf->buf[i] == '\n')
+   len++;
+
+   sprintf(buf->buf, "%zu", len);
+   buf->len = strlen(buf->buf);
+}
+
 static const char * const stripspace_usage[] = {
N_("git stripspace [-s | --strip-comments]"),
N_("git stripspace [-c | --comment-lines]"),
+   N_("git stripspace [-l | --line-count]"),
NULL
 };

 enum stripspace_mode {
STRIP_DEFAULT = 0,
STRIP_COMMENTS,
-   COMMENT_LINES
+   COMMENT_LINES,
+   LINE_COUNT
 };

 int cmd_stripspace(int argc, const char **argv, const char *prefix)
 {
struct strbuf buf = STRBUF_INIT;
enum stripspace_mode mode = STRIP_DEFAULT;
+   int count = 0;

const struct option options[] = {
OPT_CMDMODE('s', "strip-comments", ,
@@ -37,6 +53,7 @@ int cmd_stripspace(int argc, const char **argv, const char 
*prefix)
OPT_CMDMODE('c', "comment-lines", ,
N_("prepend comment character and space to each 
line"),
COMMENT_LINES),
+   OPT_BOOL('l', "line-count", , N_("count number of 
lines")),
OPT_END()
};

@@ -55,6 +72,9 @@ int cmd_stripspace(int argc, const char **argv, const char 
*prefix)
else
comment_lines();

+   if (count)
+   count_lines();
+
write_or_die(1, buf.buf, buf.len);
strbuf_release();
return 0;
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index c0cfe88..e8bef37 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -120,9 +120,9 @@ mark_action_done () {
sed -e 1q < "$todo" >> "$done"
sed -e 1d < "$todo" >> "$todo".new
mv -f "$todo".new "$todo"
-   new_count=$(git stripspace --strip-comments <"$done" | wc -l)
+   new_count=$(git stripspace --strip-comments --line-count <"$done")
echo $new_count >"$msgnum"
-   total=$(($new_count + $(git stripspace --strip-comments <"$todo" | wc 
-l)))
+   total=$(($new_count + $(git stripspace --strip-comments --line-count 
<"$todo")))
echo $total >"$end"
if test "$last_count" != "$new_count"
then
@@ -1251,7 +1251,7 @@ test -s "$todo" || echo noop >> "$todo"
 test -n "$autosquash" && rearrange_squash "$todo"
 test -n "$cmd" && add_exec_commands "$todo"

-todocount=$(git stripspace --strip-comments <"$todo" | wc -l)
+todocount=$(git stripspace --strip-comments --line-count <"$todo")
 todocount=${todocount##* }

 cat >>"$todo" 

Re: [PATCH 2/2] t5510: do not leave changed cwd

2016-03-04 Thread Junio C Hamano
Michael J Gruber  writes:

> Jeff King venit, vidit, dixit 04.03.2016 12:52:
>> On Fri, Mar 04, 2016 at 11:53:50AM +0100, Michael J Gruber wrote:
>> 
>>> t5510 carefully keeps the cwd at the test root by using either subshells
>>> or explicit cd'ing back to the root. Use a subshell for the last
>>> subtest, too.
>> 
>> I doubt this caused the heisenbug you saw, as we should have an absolute
>> path for the trash-dir, and we "cd" to its containing directory before
>> deleting it. But this is definitely a good thing to be doing anyway, to
>> prevent surprises for new tests added to t5510.
>
> Absolutely ;)

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


Re: git diff does not precompose unicode file paths (OS X)

2016-03-04 Thread Junio C Hamano
Alexander Rinass  writes:

> Sticking a precompose_argv(argc, argv) into diff.c’s cmd_diff
> function fixes the issue.
>
> But I had to disable the check (precomposed_unicode != 1) in
> precompose_argv to make it work. That’s probably because
> precompose_argv is usually called from parse_options and is
> missing some other call before it?

The "precomposed_unicode" bit comes from your configuration file,
so it won't be usable before you call into git_default_core_config
and that happens via a call to "git_config(git_diff_ui_config, NULL)".

So perhaps you'd want to add the argv munging immediately before
init_revisions() call there?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t9700: fix test for perl older than 5.14

2016-03-04 Thread Junio C Hamano
Jeff King  writes:

> One workaround would therefore be for him to tweak PERL_PATH, but
> obviously that does not help anyone else. I think we should do this:
>
> -- >8 --
> Subject: t9700: fix test for perl older than 5.14
>
> Commit d53c2c6 (mingw: fix t9700's assumption about
> directory separators, 2016-01-27) uses perl's "/r" regex
> modifier to do a non-destructive replacement on a string,
> leaving the original unmodified and returning the result.

Will apply on js/mingw-tests and merge to 'master' before -rc1.
Thanks.

>
> This feature was introduced in perl 5.14, but systems with
> older perl are still common (e.g., CentOS 6.5 still has perl
> 5.10). Let's work around it by providing a helper function
> that does the same thing using older syntax.
>
> While we're at it, let's switch to using an alternate regex
> separator, which is slightly more readable.
>
> Reported-by: Christian Couder 
> Helped-by: Dennis Kaarsemaker 
> Signed-off-by: Jeff King 
> ---
> And yes, before anyone checks, the alternate separators are available
> even in ancient versions of perl. :)
>
>  t/t9700/test.pl | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/t/t9700/test.pl b/t/t9700/test.pl
> index 7e8c40b..1b75c91 100755
> --- a/t/t9700/test.pl
> +++ b/t/t9700/test.pl
> @@ -17,6 +17,12 @@ BEGIN {
>  use Cwd;
>  use File::Basename;
>  
> +sub adjust_dirsep {
> + my $path = shift;
> + $path =~ s{\\}{/}g;
> + return $path;
> +}
> +
>  BEGIN { use_ok('Git') }
>  
>  # set up
> @@ -33,7 +39,7 @@ is($r->config_int("test.int"), 2048, "config_int: integer");
>  is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent");
>  ok($r->config_bool("test.booltrue"), "config_bool: true");
>  ok(!$r->config_bool("test.boolfalse"), "config_bool: false");
> -is($r->config_path("test.path") =~ s/\\/\//gr, 
> $r->config("test.pathexpanded"),
> +is(adjust_dirsep($r->config_path("test.path")), 
> $r->config("test.pathexpanded"),
> "config_path: ~/foo expansion");
>  is_deeply([$r->config_path("test.pathmulti")], ["foo", "bar"],
> "config_path: multiple values");
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Change in .gitignore handling: intended or bug?

2016-03-04 Thread Junio C Hamano
Duy Nguyen  writes:

> On Fri, Mar 4, 2016 at 6:56 PM, Kevin Daudt  wrote:
>> Verified that it's different in 2.7.0, but 2.7.2 gives expected output.
>
> Thanks. 2.7.1 reverts the faulty commit from 2.7.0 that generated two
> other regression reports before this one. I guess it's all good then
> (except for the people still on 2.7.0)

Are we good at 2.8.0-rc0, too?  Somehow I had an impression that we
queued "another attempt to do it differently" or something.

 ... goes and looks ...

$ rungit maint status -suall
?? baz/quux/corge/wibble.txt
?? baz/quux/grault.txt
?? foo/bar.txt
$ rungit master status -suall
?? baz/quux/corge/wibble.txt
?? baz/quux/grault.txt
?? baz/waldo.txt
?? foo/bar.txt
?? foo/garply.txt

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


Re: [PATCH] t9700: fix test for perl older than 5.14

2016-03-04 Thread Johannes Schindelin
Hi Peff,

On Fri, 4 Mar 2016, Jeff King wrote:

> Subject: t9700: fix test for perl older than 5.14
> 
> Commit d53c2c6 (mingw: fix t9700's assumption about
> directory separators, 2016-01-27) uses perl's "/r" regex
> modifier to do a non-destructive replacement on a string,
> leaving the original unmodified and returning the result.
> 
> This feature was introduced in perl 5.14, but systems with
> older perl are still common (e.g., CentOS 6.5 still has perl
> 5.10). Let's work around it by providing a helper function
> that does the same thing using older syntax.
> 
> While we're at it, let's switch to using an alternate regex
> separator, which is slightly more readable.

My apologies! And thanks for cleaning up after me.

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


Re: [PATCH v3 3/3] Documentation/git-pull: document --[no-]autostash option

2016-03-04 Thread Paul Tan
On Fri, Mar 4, 2016 at 12:13 AM, Mehul Jain  wrote:
> Signed-off-by: Mehul Jain 
> ---
>  Documentation/git-pull.txt | 15 +++
>  1 file changed, 15 insertions(+)
>
> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
> index a62a2a6..a593972 100644
> --- a/Documentation/git-pull.txt
> +++ b/Documentation/git-pull.txt
> @@ -128,6 +128,21 @@ unless you have read linkgit:git-rebase[1] carefully.
>  --no-rebase::
> Override earlier --rebase.
>
> +--autostash::
> +--no-autostash::
> +   Automatically create a temporary stash before the operation
> +   begins, and apply it after the operation ends. This means
> +   that you can run rebase on a dirty worktree.
> ++
> +This option is only valid when '--rebase' option is used.
> ++
> +The default is --no-autostash, unless rebase.autoStash configuration

By the way, there is a trailing space on this line.

> +is set.
> ++
> +[NOTE]
> +Use with care: the final stash application after a successful
> +rebase might result in non-trivial conflicts.
> +
>  Options related to fetching
>  ~~~
>
> --
> 2.7.1.340.g69eb491.dirty

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


Re: [PATCH v3 1/3] pull --rebase: add --[no-]autostash flag

2016-03-04 Thread Paul Tan
On Fri, Mar 4, 2016 at 12:13 AM, Mehul Jain  wrote:
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 10eff03..b338b83 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -85,6 +85,7 @@ static char *opt_squash;
>  static char *opt_commit;
>  static char *opt_edit;
>  static char *opt_ff;
> +static int opt_autostash = -1;
>  static char *opt_verify_signatures;
>  static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
>  static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
> @@ -146,6 +147,8 @@ static struct option pull_options[] = {
> OPT_PASSTHRU(0, "ff-only", _ff, NULL,
> N_("abort if fast-forward is not possible"),
> PARSE_OPT_NOARG | PARSE_OPT_NONEG),
> +   OPT_BOOL(0, "autostash", _autostash,
> +   N_("automatically stash/stash pop before and after rebase")),
> OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL,
> N_("verify that the named commit has a valid GPG signature"),
> PARSE_OPT_NOARG),
> @@ -789,7 +792,8 @@ static int run_rebase(const unsigned char *curr_head,
> argv_array_pushv(, opt_strategy_opts.argv);
> if (opt_gpg_sign)
> argv_array_push(, opt_gpg_sign);
> -

Minor nit: but when I wrote the code for run_rebase() I separated the
options for "Shared options" and "Options passed to git-rebase" into
different code block groups from the other code, and I would like it
if it remained that way :-(

> +   if (opt_autostash)
> +   argv_array_push(, "--autostash");

Hmm, interesting. If rebase.autostash=true !opt_autostash, we don't
need to pass --no-autostash to git-rebase because it will only stash
if the worktree is dirty, but a dirty worktree will be caught by
git-pull's die_on_unclean_worktree() anyway.

Still, it may be a problem because the worktree may become dirty
in-between our "worktree is clean" check and when git-rebase is run
(during the git-fetch), and the user may be surprised if git-rebase
attempted to stash in that case.

So we may wish to pass "--no-autostash" to git-rebase as well.

> argv_array_push(, "--onto");
> argv_array_push(, sha1_to_hex(merge_head));
>
> @@ -835,18 +839,25 @@ int cmd_pull(int argc, const char **argv, const char 
> *prefix)
> hashclr(orig_head);
>
> if (opt_rebase) {
> -   int autostash = 0;
> -
> if (is_null_sha1(orig_head) && !is_cache_unborn())
> die(_("Updating an unborn branch with changes added 
> to the index."));
>
> -   git_config_get_bool("rebase.autostash", );
> -   if (!autostash)
> +   if (opt_autostash == -1)
> +   git_config_get_bool("rebase.autostash", 
> _autostash);
> +
> +   if (opt_autostash == 0 || opt_autostash == -1)
> die_on_unclean_work_tree(prefix);
>
> if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
> hashclr(rebase_fork_point);
> }
> +   else {

Git code style puts the else on the same line, not on a new one.

> +   /* If --[no-]autostash option is called without --rebase */

Yeah, I agree with Eric that this comment should be dropped,

> +   if (opt_autostash == 0)
> +   die(_("--no-autostash option is only valid with 
> --rebase."));
> +   else if (opt_autostash == 1)
> +   die(_("--autostash option is only valid with 
> --rebase."));
> +   }

and these error messages combined.

>
> if (run_fetch(repo, refspecs))
> return 1;
> --
> 2.7.1.340.g69eb491.dirty

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


Re: [PATCH] index-pack: --clone-bundle option

2016-03-04 Thread Jeff King
On Thu, Mar 03, 2016 at 03:20:20PM -0800, Junio C Hamano wrote:

> Junio C Hamano  writes:
> 
> > Note that this name choice does not matter very much in the larger
> > picture.  As an initial clone that bootstraps from a clone-bundle is
> > expected to do a rough equivalent of:
> >
> > # create a new repository
> > git init new-repository &&
> > git remote add origin $URL &&
> >
> > # prime the object store and anchor the history to temporary
> > # references
> > git fetch $bundle 'refs/*:refs/temporary/*' &&
> >
> > # fetch the more recent history from the true origin
> > git fetch origin &&
> > git checkout -f &&
> >
> > # remove the temporary refs
> > git for-each-ref -z --format=%(refname) refs/temporary/ |
> > xargs -0 git update-ref -d
> >
> > the names recorded in the bundle will not really matter to the end
> > result.
> 
> Actually, the real implementation of "bootstrap with clone-bundle"
> is more likely to go like this:
> 
> * The client gets redirected to $name.bndl file, and obtains a
>   fairly full $name.pack file by downloading them as static
>   files;
> 
> * The client initializes an empty repository;
> 
> * The pack file is stored at .git/objects/pack/pack-$sha1.pack;
> 
> * When the client does a "git fetch origin" to fill the more
>   recent part, fetch-pack.c::find_common() would read from the
>   "git bundle list-heads $name.bndl" to learn the "reference"
>   objects.  These are thrown at rev_list_insert_ref() and are
>   advertised as "have"s, just like we advertise objects at the
>   tip of refs in alternate repository.
> 
> So there will be no refs/temporary/* hierarchy we would need to
> worry about cleaning up.

I don't think details like this matter much to the bundle-generation
side, so this is pretty academic at this point. But I think unless we
want to do a lot of surgery to git-clone, we'll end up more with
something like:

  1. init empty repository

  2. contact the other side; find out they can redirect us to an
 alternate url

  3. fetch the alternate url; it turns out to be a split bundle. Grab
 the header, and then spool the data into a temp packfile. When it's
 all there, we can "index-pack --fix-thin" it in-place.

The reason I think we'll end up with this approach is that it keeps the
details of split-bundle fetching inside remote-curl. That keeps clone
cleaner, and also means we can grab a split-bundle for a fetch, too.

> Another possible variant is to redirect the client directly to
> download pack-$sha1.pack; "index-pack" needs to be run on the client
> side anyway to create pack-$sha1.idx, so at that time it could do
> the equivalent of "--clone-bundle" processing (it is not strictly
> necessary to create a split bundle) to find the tips of histories,
> and use that information when running "git fetch origin".
> 
> So, even though I started working from "split bundle", we may not
> have to have such a feature after all to support CDN offloadable and
> resumable clone.

Yeah. And I think we'd support this in my step (3) by responding to what
we get at the URL. I.e., "it turns out to be..." can have many outcomes,
and one of them is "a packfile".

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


Re: [PATCH] index-pack: --clone-bundle option

2016-03-04 Thread Jeff King
On Thu, Mar 03, 2016 at 02:57:08PM -0800, Junio C Hamano wrote:

> Teach a new option "--clone-bundle" to "git index-pack" to create a
> split bundle file that uses an existing packfile as its data part.
> 
> The expected "typical" preparation for helping initial clone would
> start by preparing a packfile that contains most of the history and
> add another packfile that contains the remainder (e.g. the objects
> that are only reachable from reflog entries).  The first pack can
> then be used as the data part of a split bundle and these two files
> can be served as static files to bootstrap the clients without
> incurring any more CPU cycles to the server side.

Just FYI, because I know our setup is anything but typical, but this
probably _won't_ be what we do at GitHub. We try to keep everything in a
single packfile that contains many "islands", which are not allowed to
delta between each other[1]. Individual forks of a repo get their own
islands, unreachable objects are in another one, and so forth. So we'd
probably never want to provide a whole packfile, as it has way too much
data. But we _would_ be able to take a bitmap over the set of packed
objects such that if you blindly spew out every object with its bit set,
the resulting pack has the desired objects.

That's not as turn-key for serving with a dumb http server, obviously.
And I don't expect you to write that part. I just wanted to let you know
where I foresee having to take this for GitHub to get it deployed.

[1] It's a little more complicated than "don't delta with each other".
Each object has its own bitmap of which islands it is in, and the
rule is that you can use a delta base iff it is in a subset of your
own islands. The point is that a clone of a particular island should
never have to throw away on-disk deltas.

Cleaning up and sharing that code upstream has been on my todo list
for about 2 years, but somehow there's always new code to be
written. :-/ I'm happy to share if people want to look at the messy
state.

> Among the objects in the packfile, the ones that are not referenced
> by no other objects are identified and recorded as the "references"

s/no/any/ (double negative)

> in the resulting bundle.  As the packfile does not record any ref
> information, however, the names of the "references" recorded in the
> bundle need to be synthesized; we arbitrarily choose to record the
> object whose name is $SHA1 as refs/objects/$SHA1.

Makes sense. In an "island" world I'd want to write one bitmap per
island, and then reconstruct that list on the fly by referencing the
island bitmap with the sha1 names in the pack revidx.

> +static void write_bundle_file(const char *filename, unsigned char *sha1,
> +   const char *packname)
> +{
> + int fd = open(filename, O_CREAT|O_EXCL|O_WRONLY, 0600);
> + struct strbuf buf = STRBUF_INIT;
> + struct stat st;
> + int i;
> +

We should probably bail here if "fd < 0", though I guess technically we
will notice in write_in_full(). :)

> + if (stat(packname, ))
> + die(_("cannot stat %s"), packname);
> +
> + strbuf_addstr(, "# v3 git bundle\n");
> + strbuf_addf(, "size: %lu\n", (unsigned long) st.st_size);
> + strbuf_addf(, "sha1: %s\n", sha1_to_hex(sha1));
> + strbuf_addf(, "data: %s\n\n", packname);

This "sha1" field is the sha1 of the packfile, right? If so, I think
it's redundant with the pack trailer found in the pack at "packname".
I'd prefer not to include that here, because it makes it harder to
generate these v3 bundle headers dynamically (you have to actually
checksum the pack subset to come up with sha1 up front, as opposed to
checksumming as you stream the pack out).

It _could_ be used as an http etag, but I think it would make more sense
to push implementations toward using a unique value for the "packname"
(i.e., so that fetching "https://example.com/foo/1234abcd.pack; is
basically immutable). And I think that comes basically for free, because
the packname has that same hash in it already.

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


Re: git diff does not precompose unicode file paths (OS X)

2016-03-04 Thread Alexander Rinass

> On 04 Mar 2016, at 13:16, Torsten Bögershausen  wrote:
> 
> On 03/04/2016 10:07 AM, Alexander Rinass wrote:
>> Hallo,
>> 
>> It appears that the git diff command does not precompose file path 
>> arguments, even if the option core.precomposeunicode is set to true (which 
>> is the default on OS X).
>> 
>> Passing the decomposed form of a file path to the git diff command will 
>> yield no diff for a modified file.
>> 
>> In my case, the decomposed form of the file path is sent by the OS X Cocoa 
>> framework's NSTask, wich I am using in an application. It can be simulated 
>> on OS X by using $(iconv -f utf-8 -t utf-8-mac <<< FILE_PATH) as file path 
>> argument on the shell.
>> 
>> Git commands like add, log, ls-tree, ls-files, mv, ... accept both file path 
>> forms, git diff does not.
>> 
>> It can be tested with the following setup on OS X (as iconv's utf-8-mac 
>> encoding is only available on OS X):
>> 
>> git init .
>> git config core.quotepath true
>> git config core.precomposeunicode true # (default on OS X)
>> touch .gitignore && git add .gitignore && git commit -m "Initial commit"
>>  echo "." >> Ä
>> git add Ä
>> git commit -m "Create commit with unicode file path"
>>  echo "." >> Ä
>> This gives the following status, showing the precomposed form of "Ä":
>> 
>> git status --short
>>  M "\303\204"
>> Running git add with both forms does work as expected:
>> 
>> git add Ä
>> git status --short
>> M  "\303\204"
>>  git reset HEAD -- Ä
>>  git add $(iconv -f utf-8 -t utf-8-mac <<< Ä)
>> git status --short
>> M  "\303\204"
>>  git reset HEAD -- Ä
>> However, running git diff only works with the precomposed form:
>> 
>> git status --short
>>  M "\303\204"
>>  git --no-pager diff -- Ä
>> [...shows diff...]
>>  git --no-pager diff -- $(iconv -f utf-8 -t utf-8-mac <<< Ä)
>> [...shows NO diff...]
>> 
>> I took a look at the Git source code, and the builtin/diff*.c do not contain 
>> the parse_options call (which does the precompose_argv call) that the other 
>> builtins use.
>> 
>> But I am not really familiar with either C or the Git project structure, so 
>> this may not mean anything.
>> 
>> Best regards,
>> Alexander Rinass
>> 
> Good analyzes, and thanks for the report.
> It should be possible to stick in a
> 
> precompose_arrgv(argc, argv)
> 
> into builtin/diff.c
> 
> Do you you can test that ?
> 


Sticking a precompose_argv(argc, argv) into diff.c’s cmd_diff function fixes 
the issue.

But I had to disable the check (precomposed_unicode != 1) in precompose_argv to 
make it work. That’s probably because precompose_argv is usually called from 
parse_options and is missing some other call before it?

I think it is clear that diff.c and friends are definitely missing the 
precomposing step. I am not sure about the right way to fix though (should 
parse_options be used in the end?) and my C skills are basic at best, otherwise 
I would create a patch.

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


Re: [PATCH 1/2] test-lib: quote TRASH_DIRECTORY

2016-03-04 Thread Jeff King
On Fri, Mar 04, 2016 at 02:03:15PM +0100, Michael J Gruber wrote:

> >> -test ! -z "$debug" || remove_trash=$TRASH_DIRECTORY
> >> +test ! -z "$debug" || remove_trash="$TRASH_DIRECTORY"
> > 
> > I don't think this does anything. The shell doesn't do whitespace
> > splitting on the right-hand side of a variable assignment:
> > 
> >   $ foo='lots of spaces and "!'\'' funky chars'
> >   $ bar=$foo
> >   $ echo "$bar"
> >   lots of spaces and "!' funky chars
> > 
> > Of course we _do_ need quotes when we refer to $remove_trash as an
> > argument (as with "$bar" above), but it looks like we do so correctly
> > everywhere.
> 
> I'm used to that behavior, yes, but:
> 
> - Is this true for every shell that we support?

It better be, because we rely on it all through our scripts. :)

But yes, it is in POSIX, and I think any shell which did not respect it
would be broken enough to be unusable (I don't have a copy of Solaris
/bin/sh handy, but that's usually our go-to for "unusably broken").

> - Having quotes there, too, is a good reminder to have it also where
> necessary.

We do already quote variable assignments unnecessarily in lots of
places, so I don't mind it too much (it's definitely not _wrong_ to do
so).

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


Re: [PATCH 2/2] t5510: do not leave changed cwd

2016-03-04 Thread Michael J Gruber
Jeff King venit, vidit, dixit 04.03.2016 12:52:
> On Fri, Mar 04, 2016 at 11:53:50AM +0100, Michael J Gruber wrote:
> 
>> t5510 carefully keeps the cwd at the test root by using either subshells
>> or explicit cd'ing back to the root. Use a subshell for the last
>> subtest, too.
> 
> I doubt this caused the heisenbug you saw, as we should have an absolute
> path for the trash-dir, and we "cd" to its containing directory before
> deleting it. But this is definitely a good thing to be doing anyway, to
> prevent surprises for new tests added to t5510.

Absolutely ;)

Michael

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


Re: [PATCH 1/2] test-lib: quote TRASH_DIRECTORY

2016-03-04 Thread Michael J Gruber
Jeff King venit, vidit, dixit 04.03.2016 12:51:
> On Fri, Mar 04, 2016 at 11:53:49AM +0100, Michael J Gruber wrote:
> 
>> We always quote $TRASH_DIRECTORY to guard against funky path names. Do
>> so in one more spot
>>
>> Signed-off-by: Michael J Gruber 
>> ---
>>  t/test-lib.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 0b47eb6..8957916 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -868,7 +868,7 @@ case "$TRASH_DIRECTORY" in
>>  /*) ;; # absolute path is good
>>   *) TRASH_DIRECTORY="$TEST_OUTPUT_DIRECTORY/$TRASH_DIRECTORY" ;;
>>  esac
>> -test ! -z "$debug" || remove_trash=$TRASH_DIRECTORY
>> +test ! -z "$debug" || remove_trash="$TRASH_DIRECTORY"
> 
> I don't think this does anything. The shell doesn't do whitespace
> splitting on the right-hand side of a variable assignment:
> 
>   $ foo='lots of spaces and "!'\'' funky chars'
>   $ bar=$foo
>   $ echo "$bar"
>   lots of spaces and "!' funky chars
> 
> Of course we _do_ need quotes when we refer to $remove_trash as an
> argument (as with "$bar" above), but it looks like we do so correctly
> everywhere.

I'm used to that behavior, yes, but:

- Is this true for every shell that we support?
- Having quotes there, too, is a good reminder to have it also where
necessary.

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


Re: Change in .gitignore handling: intended or bug?

2016-03-04 Thread Duy Nguyen
On Fri, Mar 4, 2016 at 6:56 PM, Kevin Daudt  wrote:
> Verified that it's different in 2.7.0, but 2.7.2 gives expected output.

Thanks. 2.7.1 reverts the faulty commit from 2.7.0 that generated two
other regression reports before this one. I guess it's all good then
(except for the people still on 2.7.0)
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t9700: fix test for perl older than 5.14

2016-03-04 Thread Dennis Kaarsemaker
On vr, 2016-03-04 at 06:43 -0500, Jeff King wrote:
> On Fri, Mar 04, 2016 at 11:58:24AM +0100, Dennis Kaarsemaker wrote:
> 
> > On vr, 2016-03-04 at 03:56 -0500, Jeff King wrote:
> > > ? Those are just guesses, but if we are tickling a bug in perl's parser,
> > > this might avoid them. I also wondered when "/r" appeared. It was in
> > > 5.14, so you're presumably good there. The "use" statement at the top of
> > > the script says "5.008", so perhaps we should be writing it out longhand
> > > anyway (that version is "only" 5 years old, so I suspect there are still
> > > systems around with 5.12 or older).
> > 
> > Knowing the system Christian is testing on, I think the problem is that
> > the tests are actually being run against perl 5.10, which RHEL 6 ships
> > as system perl. As that's still a supported OS, writing tests in a form
> > compatible with it would be a good thing :)
> 
> That would make sense. `perl` in t9700-perl-git.sh (and all of our
> scripts) is actually a shell function:
> 
>   perl () {
>   command "$PERL_PATH" "$@"
>   }
> 
> to make sure we respect PERL_PATH everywhere. And that defaults in the
> Makefile to /usr/bin/perl. Christian presumably has 5.14 in his $PATH,
> but /usr/bin/perl is the system 5.10.

Yeah, that's how our systems are set up.

> One workaround would therefore be for him to tweak PERL_PATH, but
> obviously that does not help anyone else. I think we should do this:

Tested against 5.10 and 5.18 and works with both. I also tested the /r
variant with 5.18 and that works as expected.

-- 
Dennis Kaarsemaker
http://www.kaarsemaker.net


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


Re: git diff does not precompose unicode file paths (OS X)

2016-03-04 Thread Torsten Bögershausen

On 03/04/2016 10:07 AM, Alexander Rinass wrote:

Hallo,

It appears that the git diff command does not precompose file path arguments, 
even if the option core.precomposeunicode is set to true (which is the default 
on OS X).

Passing the decomposed form of a file path to the git diff command will yield 
no diff for a modified file.

In my case, the decomposed form of the file path is sent by the OS X Cocoa framework's 
NSTask, wich I am using in an application. It can be simulated on OS X by using $(iconv 
-f utf-8 -t utf-8-mac <<< FILE_PATH) as file path argument on the shell.

Git commands like add, log, ls-tree, ls-files, mv, ... accept both file path 
forms, git diff does not.

It can be tested with the following setup on OS X (as iconv's utf-8-mac 
encoding is only available on OS X):

 git init .
 git config core.quotepath true
 git config core.precomposeunicode true # (default on OS X)
 touch .gitignore && git add .gitignore && git commit -m "Initial commit"
 
 echo "." >> Ä

 git add Ä
 git commit -m "Create commit with unicode file path"
 
 echo "." >> Ä
 
This gives the following status, showing the precomposed form of "Ä":


 git status --short
  M "\303\204"
 
Running git add with both forms does work as expected:


 git add Ä
 git status --short
 M  "\303\204"
 
 git reset HEAD -- Ä
 
 git add $(iconv -f utf-8 -t utf-8-mac <<< Ä)

 git status --short
 M  "\303\204"
 
 git reset HEAD -- Ä
 
However, running git diff only works with the precomposed form:


 git status --short
  M "\303\204"
 
 git --no-pager diff -- Ä

 [...shows diff...]
 
 git --no-pager diff -- $(iconv -f utf-8 -t utf-8-mac <<< Ä)

 [...shows NO diff...]

I took a look at the Git source code, and the builtin/diff*.c do not contain 
the parse_options call (which does the precompose_argv call) that the other 
builtins use.

But I am not really familiar with either C or the Git project structure, so 
this may not mean anything.

Best regards,
Alexander Rinass


Good analyzes, and thanks for the report.
It should be possible to stick in a

precompose_arrgv(argc, argv)

into builtin/diff.c

Do you you can test that ?





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


Re: Change in .gitignore handling: intended or bug?

2016-03-04 Thread Kevin Daudt
On Fri, Mar 04, 2016 at 01:12:37AM -0500, Charles Strahan wrote:
> I'm on 2.7.0.
> 
> Here's a quick sanity check:
> 
> ├── baz
> │   ├── quux
> │   │   ├── corge
> │   │   │   └── wibble.txt
> │   │   └── grault.txt
> │   └── waldo.txt
> └── foo
> ├── bar.txt
> └── garply.txt
> 
> $ git --version
> git version 2.7.0
> 
> $ git status -sb -uall
> ## Initial commit on master
> ?? baz/quux/corge/wibble.txt
> ?? baz/quux/grault.txt
> ?? baz/waldo.txt
> ?? foo/bar.txt
> ?? foo/garply.txt
> 
> 
> For the lazy (such as myself), this will set up an identical tree:
> 
> mkdir -p foo
> mkdir -p baz/quux/corge
> touch foo/bar.txt
> touch foo/garply.txt
> touch baz/waldo.txt
> touch baz/quux/grault.txt
> touch baz/quux/corge/wibble.txt
> cat <<"EOF" > .gitignore
> *
> !/foo
> !/foo/bar.txt
> !/baz
> !/baz/quux
> !/baz/quux/**/*
> EOF
> 
> 
> I just checked https://git-scm.com/docs/gitignore and the example at the
> bottom
> suggests that this behavior may be expected:
> 
> $ cat .gitignore
> # exclude everything except directory foo/bar
> /*
> !/foo
> /foo/*
> !/foo/bar
> 
> Note the /foo/*, explicitly ignoring the entries below /foo.
> 
> This wasn't always the case, though, so I'd love to hear if it was
> intentional
> (or if I've lost my mind, which is quite possible).
> 
> -Charles
> 
> 
> 
> On Fri, Mar 4, 2016, at 12:51 AM, Kevin Daudt wrote:
> > On Thu, Mar 03, 2016 at 09:11:56PM -0500, Charles Strahan wrote:
> > > Hello,
> > > 
> > > I've found a change in the way .gitignore works, and I'm not sure if
> > > it's a bug
> > > or intended.
> > > 
> > > Previously, one could use the following .gitignore:
> > > 
> > > *
> > > !/foo
> > > !/foo/bar.txt
> > > !/baz
> > > !/baz/quux
> > > !/baz/quux/**/*
> > > 
> > > And these files would be seen by git:
> > > 
> > > foo/bar.txt
> > > baz/quux/grault.txt
> > > baz/quux/corge/wibble.txt
> > > 
> > > And these files would be ignored:
> > > 
> > > foo/garply.txt
> > > baz/waldo.txt
> > > 
> > > At some point (between git 2.6.0 and 2.7.0, I think), the behavior
> > > changed such
> > > that _none_ of the files above would be ignored. Previously, git would
> > > treat
> > > !/foo as an indication that it should not prune /foo, but that
> > > _wouldn't_ be
> > > sufficient to un-ignore the contents thereof. Now, it seems the new
> > > scheme
> > > treats !/foo as functionally equivalent to !/foo followed by !/foo/**/*
> > > in the
> > > old scheme.
> > > 
> > > I manage my home directory by making it a git repo, and using
> > > ~/.gitignore to
> > > selectively permit certain files or subdirectories to be seen by git.
> > > The recent
> > > change in behavior has resulted in sensitive directories like ~/.gpg
> > > being
> > > un-ignored. For reference, I've appended my .gitignore to the end of
> > > this email.
> > > 
> > > So, is this behavior intended, or is this a bug? If the former, is there
> > > an
> > > announcement explaining this change?
> > > 
> > > -Charles
> > > 
> > > [snip]
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe git" in
> > > the body of a message to majord...@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > Works as intended for me:
> > 
> > ├── baz
> > │   ├── quux
> > │   │   ├── corge
> > │   │   │   └── wibble.txt
> > │   │   └── grault.txt
> > │   └── waldo.txt
> > └── foo
> > ├── bar.txt
> > └── garply.txt
> > 
> > $ git status -s -uall
> > ?? baz/quux/corge/wibble.txt
> > ?? baz/quux/grault.txt
> > ?? foo/bar.txt
> > 
> > garply.txt and waldo.txt are ignore, but the rest is still tracked.
> > 
> > I'm on 2.7.2.
> --

Verified that it's different in 2.7.0, but 2.7.2 gives expected output.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] t5510: do not leave changed cwd

2016-03-04 Thread Jeff King
On Fri, Mar 04, 2016 at 11:53:50AM +0100, Michael J Gruber wrote:

> t5510 carefully keeps the cwd at the test root by using either subshells
> or explicit cd'ing back to the root. Use a subshell for the last
> subtest, too.

I doubt this caused the heisenbug you saw, as we should have an absolute
path for the trash-dir, and we "cd" to its containing directory before
deleting it. But this is definitely a good thing to be doing anyway, to
prevent surprises for new tests added to t5510.

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


Re: [PATCH 1/2] test-lib: quote TRASH_DIRECTORY

2016-03-04 Thread Jeff King
On Fri, Mar 04, 2016 at 11:53:49AM +0100, Michael J Gruber wrote:

> We always quote $TRASH_DIRECTORY to guard against funky path names. Do
> so in one more spot
> 
> Signed-off-by: Michael J Gruber 
> ---
>  t/test-lib.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 0b47eb6..8957916 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -868,7 +868,7 @@ case "$TRASH_DIRECTORY" in
>  /*) ;; # absolute path is good
>   *) TRASH_DIRECTORY="$TEST_OUTPUT_DIRECTORY/$TRASH_DIRECTORY" ;;
>  esac
> -test ! -z "$debug" || remove_trash=$TRASH_DIRECTORY
> +test ! -z "$debug" || remove_trash="$TRASH_DIRECTORY"

I don't think this does anything. The shell doesn't do whitespace
splitting on the right-hand side of a variable assignment:

  $ foo='lots of spaces and "!'\'' funky chars'
  $ bar=$foo
  $ echo "$bar"
  lots of spaces and "!' funky chars

Of course we _do_ need quotes when we refer to $remove_trash as an
argument (as with "$bar" above), but it looks like we do so correctly
everywhere.

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


Re: t9700-perl-git.sh is broken on some configurations

2016-03-04 Thread Jeff King
On Fri, Mar 04, 2016 at 11:30:36AM +0100, Christian Couder wrote:

> > Those are just guesses, but if we are tickling a bug in perl's parser,
> > this might avoid them. I also wondered when "/r" appeared. It was in
> > 5.14, so you're presumably good there.
> 
> If I just remove the "r" at the end of "s/\\/\//gr", I get with both
> Perl versions:
> 
> Can't modify non-lvalue subroutine call at t/t9700/test.pl line 36.

Right, because the string being operated on is the return value of a
function, so we can't do substitution on it (unless with "r", whose
purpose is to allow exactly such a thing).

> > The "use" statement at the top of
> > the script says "5.008", so perhaps we should be writing it out longhand
> > anyway (that version is "only" 5 years old, so I suspect there are still
> > systems around with 5.12 or older).
> 
> Yeah, it would work.

Thanks for confirming the longhand does work; I think the patch I just
posted elsewhere in the thread should be good for you, then.

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


[PATCH] t9700: fix test for perl older than 5.14

2016-03-04 Thread Jeff King
On Fri, Mar 04, 2016 at 11:58:24AM +0100, Dennis Kaarsemaker wrote:

> On vr, 2016-03-04 at 03:56 -0500, Jeff King wrote:
> > ? Those are just guesses, but if we are tickling a bug in perl's parser,
> > this might avoid them. I also wondered when "/r" appeared. It was in
> > 5.14, so you're presumably good there. The "use" statement at the top of
> > the script says "5.008", so perhaps we should be writing it out longhand
> > anyway (that version is "only" 5 years old, so I suspect there are still
> > systems around with 5.12 or older).
> 
> Knowing the system Christian is testing on, I think the problem is that
> the tests are actually being run against perl 5.10, which RHEL 6 ships
> as system perl. As that's still a supported OS, writing tests in a form
> compatible with it would be a good thing :)

That would make sense. `perl` in t9700-perl-git.sh (and all of our
scripts) is actually a shell function:

  perl () {
  command "$PERL_PATH" "$@"
  }

to make sure we respect PERL_PATH everywhere. And that defaults in the
Makefile to /usr/bin/perl. Christian presumably has 5.14 in his $PATH,
but /usr/bin/perl is the system 5.10.

One workaround would therefore be for him to tweak PERL_PATH, but
obviously that does not help anyone else. I think we should do this:

-- >8 --
Subject: t9700: fix test for perl older than 5.14

Commit d53c2c6 (mingw: fix t9700's assumption about
directory separators, 2016-01-27) uses perl's "/r" regex
modifier to do a non-destructive replacement on a string,
leaving the original unmodified and returning the result.

This feature was introduced in perl 5.14, but systems with
older perl are still common (e.g., CentOS 6.5 still has perl
5.10). Let's work around it by providing a helper function
that does the same thing using older syntax.

While we're at it, let's switch to using an alternate regex
separator, which is slightly more readable.

Reported-by: Christian Couder 
Helped-by: Dennis Kaarsemaker 
Signed-off-by: Jeff King 
---
And yes, before anyone checks, the alternate separators are available
even in ancient versions of perl. :)

 t/t9700/test.pl | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index 7e8c40b..1b75c91 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -17,6 +17,12 @@ BEGIN {
 use Cwd;
 use File::Basename;
 
+sub adjust_dirsep {
+   my $path = shift;
+   $path =~ s{\\}{/}g;
+   return $path;
+}
+
 BEGIN { use_ok('Git') }
 
 # set up
@@ -33,7 +39,7 @@ is($r->config_int("test.int"), 2048, "config_int: integer");
 is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent");
 ok($r->config_bool("test.booltrue"), "config_bool: true");
 ok(!$r->config_bool("test.boolfalse"), "config_bool: false");
-is($r->config_path("test.path") =~ s/\\/\//gr, $r->config("test.pathexpanded"),
+is(adjust_dirsep($r->config_path("test.path")), 
$r->config("test.pathexpanded"),
"config_path: ~/foo expansion");
 is_deeply([$r->config_path("test.pathmulti")], ["foo", "bar"],
"config_path: multiple values");
-- 
2.8.0.rc0.309.g6677de9

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


Re: t9700-perl-git.sh is broken on some configurations

2016-03-04 Thread Dennis Kaarsemaker
On vr, 2016-03-04 at 03:56 -0500, Jeff King wrote:
> ? Those are just guesses, but if we are tickling a bug in perl's parser,
> this might avoid them. I also wondered when "/r" appeared. It was in
> 5.14, so you're presumably good there. The "use" statement at the top of
> the script says "5.008", so perhaps we should be writing it out longhand
> anyway (that version is "only" 5 years old, so I suspect there are still
> systems around with 5.12 or older).

Knowing the system Christian is testing on, I think the problem is that
the tests are actually being run against perl 5.10, which RHEL 6 ships
as system perl. As that's still a supported OS, writing tests in a form
compatible with it would be a good thing :)

-- 
Dennis Kaarsemaker
http://www.kaarsemaker.net


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


[PATCH/RFC 0/2] Test trash dir sanitizing

2016-03-04 Thread Michael J Gruber
I encountered a Heisenbug[*] where t5510 would leave its trash directory without
cleanup, though not reproducibly so. This mini series cleans up two spots which
may or may not be related, but should be goog cleanup anyways.

[*] Running tests with prove -j4.

Michael J Gruber (2):
  test-lib: quote TRASH_DIRECTORY
  t5510: do not leave changed cwd

 t/t5510-fetch.sh | 10 ++
 t/test-lib.sh|  2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)

-- 
2.8.0.rc0.181.g163d81c

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


[PATCH 1/2] test-lib: quote TRASH_DIRECTORY

2016-03-04 Thread Michael J Gruber
We always quote $TRASH_DIRECTORY to guard against funky path names. Do
so in one more spot

Signed-off-by: Michael J Gruber 
---
 t/test-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0b47eb6..8957916 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -868,7 +868,7 @@ case "$TRASH_DIRECTORY" in
 /*) ;; # absolute path is good
  *) TRASH_DIRECTORY="$TEST_OUTPUT_DIRECTORY/$TRASH_DIRECTORY" ;;
 esac
-test ! -z "$debug" || remove_trash=$TRASH_DIRECTORY
+test ! -z "$debug" || remove_trash="$TRASH_DIRECTORY"
 rm -fr "$TRASH_DIRECTORY" || {
GIT_EXIT_OK=t
echo >&5 "FATAL: Cannot prepare test area"
-- 
2.8.0.rc0.181.g163d81c

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


[PATCH 2/2] t5510: do not leave changed cwd

2016-03-04 Thread Michael J Gruber
t5510 carefully keeps the cwd at the test root by using either subshells
or explicit cd'ing back to the root. Use a subshell for the last
subtest, too.

Signed-off-by: Michael J Gruber 
---
 t/t5510-fetch.sh | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 0c10c85..38321d1 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -679,10 +679,12 @@ test_expect_success 'fetching with auto-gc does not lock 
up' '
EOF
git clone "file://$D" auto-gc &&
test_commit test2 &&
-   cd auto-gc &&
-   git config gc.autoPackLimit 1 &&
-   GIT_ASK_YESNO="$D/askyesno" git fetch >fetch.out 2>&1 &&
-   ! grep "Should I try again" fetch.out
+   (
+   cd auto-gc &&
+   git config gc.autoPackLimit 1 &&
+   GIT_ASK_YESNO="$D/askyesno" git fetch >fetch.out 2>&1 &&
+   ! grep "Should I try again" fetch.out
+   )
 '
 
 test_done
-- 
2.8.0.rc0.181.g163d81c

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


Re: t9700-perl-git.sh is broken on some configurations

2016-03-04 Thread Christian Couder
On Fri, Mar 4, 2016 at 9:56 AM, Jeff King  wrote:
> On Fri, Mar 04, 2016 at 09:13:51AM +0100, Christian Couder wrote:
>
>> Indeed on the command line I get:
>>
>> 
>> $ t/t9700/test.pl
>> ok 2 - use Git;
>> Bareword found where operator expected at t/t9700/test.pl line 36,
>> near "s/\\/\//gr"
>> syntax error at t/t9700/test.pl line 36, near "s/\\/\//gr"
>> Execution of t/t9700/test.pl aborted due to compilation errors.
>> 
>>
>> A quick look at t/t9700/test.pl line 36 was not enough for me to spot
>> the problem.
>>
>> Perl version is: perl 5, version 18, subversion 2 (v5.18.2) built for
>> x86_64-linux
>>
>> The machine is running CentOS 6.5.
>
> I can't reproduce on any of the machines I have handy (perl 5.14, 5.20,
> and 5.22). I don't have 5.18 handy. The line in question looks fine to
> me, so perhaps it is a temporary regression in 5.18.

It is strange because on the same machine there is also v5.10.1
installed and I get the same error with it.

> Does it help to wrap it in parentheses, like:
>
> diff --git a/t/t9700/test.pl b/t/t9700/test.pl
> index 7e8c40b..edeeb0e 100755
> --- a/t/t9700/test.pl
> +++ b/t/t9700/test.pl
> @@ -33,7 +33,7 @@ is($r->config_int("test.int"), 2048, "config_int: integer");
>  is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent");
>  ok($r->config_bool("test.booltrue"), "config_bool: true");
>  ok(!$r->config_bool("test.boolfalse"), "config_bool: false");
> -is($r->config_path("test.path") =~ s/\\/\//gr, 
> $r->config("test.pathexpanded"),
> +is(($r->config_path("test.path") =~ s/\\/\//gr), 
> $r->config("test.pathexpanded"),
> "config_path: ~/foo expansion");
>  is_deeply([$r->config_path("test.pathmulti")], ["foo", "bar"],
> "config_path: multiple values");

No, parentheses don't help.

> or even write it out longhand without "/r":
>
> diff --git a/t/t9700/test.pl b/t/t9700/test.pl
> index 7e8c40b..52471cf 100755
> --- a/t/t9700/test.pl
> +++ b/t/t9700/test.pl
> @@ -33,7 +33,9 @@ is($r->config_int("test.int"), 2048, "config_int: integer");
>  is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent");
>  ok($r->config_bool("test.booltrue"), "config_bool: true");
>  ok(!$r->config_bool("test.boolfalse"), "config_bool: false");
> -is($r->config_path("test.path") =~ s/\\/\//gr, 
> $r->config("test.pathexpanded"),
> +my $test_path = $r->config_path("test.path");
> +$test_path =~ s/\\/\//g;
> +is($test_path, $r->config("test.pathexpanded"),
> "config_path: ~/foo expansion");
>  is_deeply([$r->config_path("test.pathmulti")], ["foo", "bar"],
> "config_path: multiple values");
>
> ?

Yeah, it works like the above with both perl versions.

> Those are just guesses, but if we are tickling a bug in perl's parser,
> this might avoid them. I also wondered when "/r" appeared. It was in
> 5.14, so you're presumably good there.

If I just remove the "r" at the end of "s/\\/\//gr", I get with both
Perl versions:

Can't modify non-lvalue subroutine call at t/t9700/test.pl line 36.

> The "use" statement at the top of
> the script says "5.008", so perhaps we should be writing it out longhand
> anyway (that version is "only" 5 years old, so I suspect there are still
> systems around with 5.12 or older).

Yeah, it would work.

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


git diff does not precompose unicode file paths (OS X)

2016-03-04 Thread Alexander Rinass
Hallo,

It appears that the git diff command does not precompose file path arguments, 
even if the option core.precomposeunicode is set to true (which is the default 
on OS X).

Passing the decomposed form of a file path to the git diff command will yield 
no diff for a modified file.

In my case, the decomposed form of the file path is sent by the OS X Cocoa 
framework's NSTask, wich I am using in an application. It can be simulated on 
OS X by using $(iconv -f utf-8 -t utf-8-mac <<< FILE_PATH) as file path 
argument on the shell.

Git commands like add, log, ls-tree, ls-files, mv, ... accept both file path 
forms, git diff does not. 

It can be tested with the following setup on OS X (as iconv's utf-8-mac 
encoding is only available on OS X):

git init .
git config core.quotepath true
git config core.precomposeunicode true # (default on OS X)
touch .gitignore && git add .gitignore && git commit -m "Initial commit"

echo "." >> Ä
git add Ä
git commit -m "Create commit with unicode file path"

echo "." >> Ä

This gives the following status, showing the precomposed form of "Ä":

git status --short
 M "\303\204"

Running git add with both forms does work as expected:

git add Ä
git status --short
M  "\303\204"

git reset HEAD -- Ä

git add $(iconv -f utf-8 -t utf-8-mac <<< Ä)
git status --short
M  "\303\204"

git reset HEAD -- Ä

However, running git diff only works with the precomposed form:

git status --short
 M "\303\204"

git --no-pager diff -- Ä
[...shows diff...]

git --no-pager diff -- $(iconv -f utf-8 -t utf-8-mac <<< Ä)
[...shows NO diff...]

I took a look at the Git source code, and the builtin/diff*.c do not contain 
the parse_options call (which does the precompose_argv call) that the other 
builtins use.

But I am not really familiar with either C or the Git project structure, so 
this may not mean anything. 

Best regards,
Alexander Rinass

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


Re: t9700-perl-git.sh is broken on some configurations

2016-03-04 Thread Jeff King
On Fri, Mar 04, 2016 at 09:13:51AM +0100, Christian Couder wrote:

> Indeed on the command line I get:
> 
> 
> $ t/t9700/test.pl
> ok 2 - use Git;
> Bareword found where operator expected at t/t9700/test.pl line 36,
> near "s/\\/\//gr"
> syntax error at t/t9700/test.pl line 36, near "s/\\/\//gr"
> Execution of t/t9700/test.pl aborted due to compilation errors.
> 
> 
> A quick look at t/t9700/test.pl line 36 was not enough for me to spot
> the problem.
> 
> Perl version is: perl 5, version 18, subversion 2 (v5.18.2) built for
> x86_64-linux
> 
> The machine is running CentOS 6.5.

I can't reproduce on any of the machines I have handy (perl 5.14, 5.20,
and 5.22). I don't have 5.18 handy. The line in question looks fine to
me, so perhaps it is a temporary regression in 5.18.

Does it help to wrap it in parentheses, like:

diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index 7e8c40b..edeeb0e 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -33,7 +33,7 @@ is($r->config_int("test.int"), 2048, "config_int: integer");
 is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent");
 ok($r->config_bool("test.booltrue"), "config_bool: true");
 ok(!$r->config_bool("test.boolfalse"), "config_bool: false");
-is($r->config_path("test.path") =~ s/\\/\//gr, $r->config("test.pathexpanded"),
+is(($r->config_path("test.path") =~ s/\\/\//gr), 
$r->config("test.pathexpanded"),
"config_path: ~/foo expansion");
 is_deeply([$r->config_path("test.pathmulti")], ["foo", "bar"],
"config_path: multiple values");

or even write it out longhand without "/r":

diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index 7e8c40b..52471cf 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -33,7 +33,9 @@ is($r->config_int("test.int"), 2048, "config_int: integer");
 is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent");
 ok($r->config_bool("test.booltrue"), "config_bool: true");
 ok(!$r->config_bool("test.boolfalse"), "config_bool: false");
-is($r->config_path("test.path") =~ s/\\/\//gr, $r->config("test.pathexpanded"),
+my $test_path = $r->config_path("test.path");
+$test_path =~ s/\\/\//g;
+is($test_path, $r->config("test.pathexpanded"),
"config_path: ~/foo expansion");
 is_deeply([$r->config_path("test.pathmulti")], ["foo", "bar"],
"config_path: multiple values");

? Those are just guesses, but if we are tickling a bug in perl's parser,
this might avoid them. I also wondered when "/r" appeared. It was in
5.14, so you're presumably good there. The "use" statement at the top of
the script says "5.008", so perhaps we should be writing it out longhand
anyway (that version is "only" 5 years old, so I suspect there are still
systems around with 5.12 or older).

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


t9700-perl-git.sh is broken on some configurations

2016-03-04 Thread Christian Couder
Hi,

It looks like t9700-perl-git.sh is broken on one machine I use but not
on my laptop since commit d53c2c67380f769f91fd45cc8c63a5883245ccca
(mingw: fix t9700's assumption about directory separators, Jan 27
17:19:56 2016).

I get:


Initialized empty Git repository in /home/ccouder/git/git/t/trash
directory.t9700-perl-git/.git/
expecting success: echo "test file 1" > file1 &&
 echo "test file 2" > file2 &&
 mkdir directory1 &&
 echo "in directory1" >> directory1/file &&
 mkdir directory2 &&
 echo "in directory2" >> directory2/file &&
 git add . &&
 git commit -m "first commit" &&

 echo "new file in subdir 2" > directory2/file2 &&
 git add . &&
 git commit -m "commit in directory2" &&

 echo "changed file 1" > file1 &&
 git commit -a -m "second commit" &&

 git config --add color.test.slot1 green &&
 git config --add test.string value &&
 git config --add test.dupstring value1 &&
 git config --add test.dupstring value2 &&
 git config --add test.booltrue true &&
 git config --add test.boolfalse no &&
 git config --add test.boolother other &&
 git config --add test.int 2k &&
 git config --add test.path "~/foo" &&
 git config --add test.pathexpanded "$HOME/foo" &&
 git config --add test.pathmulti foo &&
 git config --add test.pathmulti bar

[master (root-commit) fc41470] first commit
 Author: A U Thor 
 4 files changed, 4 insertions(+)
 create mode 100644 directory1/file
 create mode 100644 directory2/file
 create mode 100644 file1
 create mode 100644 file2
[master 6a30dee] commit in directory2
 Author: A U Thor 
 1 file changed, 1 insertion(+)
 create mode 100644 directory2/file2
[master 33414b1] second commit
 Author: A U Thor 
 1 file changed, 1 insertion(+), 1 deletion(-)
ok 1 - set up test repository

# run 1: Perl API (perl /home/ccouder/git/git/t/t9700/test.pl)
ok 2 - use Git;
# test_external test Perl API failed: perl /home/ccouder/git/git/t/t9700/test.pl
# expecting no stderr from previous command
# test_external_without_stderr test no stderr: Perl API failed: perl
/home/ccouder/git/git/t/t9700/test.pl:
# Stderr is:
Bareword found where operator expected at
/home/ccouder/git/git/t/t9700/test.pl line 36, near "s/\\/\//gr"
syntax error at /home/ccouder/git/git/t/t9700/test.pl line 36, near "s/\\/\//gr"
Execution of /home/ccouder/git/git/t/t9700/test.pl aborted due to
compilation errors.


Indeed on the command line I get:


$ t/t9700/test.pl
ok 2 - use Git;
Bareword found where operator expected at t/t9700/test.pl line 36,
near "s/\\/\//gr"
syntax error at t/t9700/test.pl line 36, near "s/\\/\//gr"
Execution of t/t9700/test.pl aborted due to compilation errors.


A quick look at t/t9700/test.pl line 36 was not enough for me to spot
the problem.

Perl version is: perl 5, version 18, subversion 2 (v5.18.2) built for
x86_64-linux

The machine is running CentOS 6.5.

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