Re: [PATCH v16 0/7] config commit verbose

[Jeff King]

On Fri, May 06, 2016 at 08:33:15AM -0700, Junio C Hamano wrote:

> > Then I replied:
> >
> >However, that doesn't mean that we have to spread this badly chosen
> >name from options to config variables, does it?  I think that if we
> >are going to define a new config variable today, then it should be
> >named properly, and it's better not to call it 'commit.verbose', but
> >'commit.showDiff' or something.
> >
> > http://thread.gmane.org/gmane.comp.version-control.git/289027/focus=289303
> >
> > Any thoughts on this?  Before a poorly named config variable enters to
> > the codebase and we'll have to maintain it "forever"...
> 
> My thoughts are --show-diff would probably be a UI mistake of a
> different sort, if you are anticipating that the different kinds of
> information to be shown in verbose modes would proliferate and that
> you would want to give the user flexibility to pick and choose to
> use some while not using some other among them.  You would end up
> having --show-xyzzy --show-frotz --show-nitfol ... options.
> 
> I am not convinced that we would want such a degree of flexibility
> in the first place, but even if we did, we'd be better off giving
> that as "--verbose=diff,xyzzy,frotz...", I would think.
> 
> And commit.verbose that begins its life as a simple boolean, which
> can be extended to become bool-or-string if needed, is better than
> having commit.showDiff, commit.showXyzzy, commit.showFrotz, etc.

I don't think anyone is anticipating more "--show-" options. It is just
that "--verbose" is the opposite of "--quiet" in most other commands,
and pertains to chattiness on the terminal about what is going on.

Whereas in git-commit, is about sticking some data in the commit message
template. Naively I'd expect it to cause commit to spew more data to
stderr about what's being committed, ident info, etc.

If you are thinking that there could be something like "--show-ident" to
replace that, I do not mind that too much. But IMHO that does not
address the root problem that commit's "--verbose" is not very much like
the same option in other commands. And something like
"--verbose=diff,ident" just seems to make that worse by coupling options
that otherwise don't have anything to do with each other.

-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] t: fix duplicate words of "output"

[Ilya Bobyr]

On 5/6/2016 10:26 AM, Junio C Hamano wrote:
> Li Peng  writes:
>
>> Fix duplicate words of "output" in comment.
>>
>> Signed-off-by: Li Peng 
>> ---
>>  t/t-basic.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/t/t-basic.sh b/t/t-basic.sh
>> index 79b9074..60811a3 100755
>> --- a/t/t-basic.sh
>> +++ b/t/t-basic.sh
>> @@ -98,7 +98,7 @@ check_sub_test_lib_test () {
>>  }
>>  
>>  check_sub_test_lib_test_err () {
>> -name="$1" # stdin is the expected output output from the test
>> +name="$1" # stdin is the expected output from the test
> The original is marginally correct.  The first "output" is a noun,
> "expected output", the second one is a pp of verb "to output",
> i.e. "to produce".
>
> I would have written it as "the expected output produced by the
> test", though.

Right, the original form is quite hard to understand %)

--
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] gitweb: fix link to parent diff with pathinfo

[Richard Braun]

Gitweb, when used with PATH_INFO, shows a link to parent diff
like http://somedomain/somerepo.git/commitdiff/somehash?hp=parenthash.
That link reports "400 - Invalid hash parameter".

As I understand it, it should instead directly point to the parent diff,
i.e. turn it into http://somedomain/somerepo.git/commitdiff/parenthash,
and delete 'hash_parent' element from the %params hash once we used it,
otherwise the '?hp=parenthash' string is appended.

Signed-off-by: Richard Braun 
---
 gitweb/gitweb.perl | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 05d7910..f7f7936 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1423,7 +1423,12 @@ sub href {
delete $params{'hash'};
delete $params{'hash_base'};
} elsif (defined $params{'hash'}) {
-   $href .= esc_path_info($params{'hash'});
+   if (defined $params{'hash_parent'}) {
+   $href .= esc_path_info($params{'hash_parent'});
+   delete $params{'hash_parent'};
+   } else {
+   $href .= esc_path_info($params{'hash'});
+   }
delete $params{'hash'};
}
 
-- 
2.1.4

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

Re: [PATCH v5 2/2] bisect: rewrite `check_term_format` shell function in C

[Eric Sunshine]

On Fri, May 6, 2016 at 10:49 AM, Pranit Bauva  wrote:
> Reimplement the `check_term_format` shell function in C and add
> a `--check-term-format` subcommand to `git bisect--helper` to call it
> from git-bisect.sh
>
> Using `--check-term-format` subcommand is a temporary measure to port
> shell function to C so as to use the existing test suite. As more
> functions are ported, this subcommand will loose its existence and will

s/loose/lose/

Though, maybe it would be better to say:

...this subcommand will be retired...

> be called by some other method/subcommand. For eg. In conversion of
> write_terms() of git-bisect.sh, the subcommand will be removed and
> instead check_term_format() will be called in its C implementation while
> a new subcommand will be introduced for write_terms().
>
> Signed-off-by: Pranit Bauva 
> ---
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> @@ -2,16 +2,65 @@
> +/*
> + * To check whether the string `term` belongs to the set of strings
> + * included in the variable arguments.

This is still a sentence fragment as pointed out last time[1]. You can
fix it with: s/To check/Check/

> + */
> +static int one_of(const char *term, ...)
> +{
> +   int res = 0;
> +   va_list matches;
> +   const char *match;
> +
> +   va_start(matches, term);
> +   while (!res && (match=va_arg(matches, const char *)))

Style: add spaces around '='

> +   res = !strcmp(term, match);
> +   va_end(matches);
> +
> +   return res;
> +}

The rest of the patch seems to address the issues raised by my
previous review[1] (aside from my comments[1,2] about this bottom-up
approach repeatedly adding and removing unnecessary complexity as each
little function is ported from shell to C, but that's a separate
philosophical discussion).

[1]: http://thread.gmane.org/gmane.comp.version-control.git/289476/focus=293501
[2]: http://thread.gmane.org/gmane.comp.version-control.git/289476/focus=293517
--
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] t3513: do not compress backup tar file

[Stefan Beller]

On Fri, May 6, 2016 at 3:45 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> Armin Kunaschik  wrote:
>>> I'm trying to compile/test/use git 2.8.2 on AIX 6.1 with
>>> no bash available.
>> ...
>>> make test does not make it through t3513-revert-submodule.sh anymore.
>>> The test is not portable since it uses the z-flags of GNU-tar. When -z
>>> is removed, (and extension is changed back to tar) everything runs and
>>> tests smoothly.
>>
>> Signed-off-by: Stefan Beller 
>> ---
>
> Thanks for a quick fix.  Even though "no bash" and "AIX 6.1" are
> interesting details that are part of a good bug report, these are
> irrelevant noise for a commit that fixes a bug that is unrelated to
> them, so let's rephrase the message and queue it, like this:
>
> t3513: do not compress backup tar file
>
> The test uses the 'z' option, i.e. "compress the output while at
> it", which is GNUism and not portable.
>
> Reported-by: Armin Kunaschik 
> Signed-off-by: Stefan Beller 
> Signed-off-by: Junio C Hamano 

Thanks for rewriting the message. :)

>
>>  t/t3513-revert-submodule.sh | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/t/t3513-revert-submodule.sh b/t/t3513-revert-submodule.sh
>> index a1c4e02..db93781 100755
>> --- a/t/t3513-revert-submodule.sh
>> +++ b/t/t3513-revert-submodule.sh
>> @@ -14,11 +14,11 @@ test_description='revert can handle submodules'
>>  git_revert () {
>>   git status -su >expect &&
>>   ls -1pR * >>expect &&
>> - tar czf "$TRASH_DIRECTORY/tmp.tgz" * &&
>> + tar cf "$TRASH_DIRECTORY/tmp.tar" * &&
>>   git checkout "$1" &&
>>   git revert HEAD &&
>>   rm -rf * &&
>> - tar xzf "$TRASH_DIRECTORY/tmp.tgz" &&
>> + tar xf "$TRASH_DIRECTORY/tmp.tar" &&
>>   git status -su >actual &&
>>   ls -1pR * >>actual &&
>>   test_cmp expect actual &&
>
> This is not a new problem, but these "ls -1pR" and "rm -rf *" makes
> me wonder if it is the best way to test what is being tested.
>
> The title says "revert can handle submodules", but when it sees that
> revert finishes successfully, it discards the resulting working tree
> state with "rm -rf *" (Yuck) and repopulates with the state before
> the 'checkout && revert' sequence, so the 'status' and 'ls' are not
> testing what 'revert' did at all.
>
> Shouldn't it be testing HEAD^{tree} before "checkout && revert" and
> after and make sure they match, and checking the working state left
> by 'revert' without clobbering it with tarball extract?
>

Reading the comment above that function indicates that the middle revert
is not the interesting revert. The later revert is the actually tested revert.
--
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

What's cooking in git.git (May 2016, #02; Fri, 6)

[Junio C Hamano]

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

The 'master' branch now has the tenth batch of topics of this cycle.
On the 'maint' front, 2.8.2 is out and fixes that have been in
'master' accumulates on it for 2.8.3.

Ones with questionable status has a '?' character in their comments.

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

* bc/object-id (2016-04-25) 6 commits
  (merged to 'next' on 2016-04-29 at 02f13a4)
 + match-trees: convert several leaf functions to use struct object_id
 + tree-walk: convert tree_entry_extract() to use struct object_id
 + struct name_entry: use struct object_id instead of unsigned char sha1[20]
 + match-trees: convert shift_tree() and shift_tree_by() to use object_id
 + test-match-trees: convert to use struct object_id
 + sha1-name: introduce a get_oid() function

 Move from unsigned char[20] to struct object_id continues.


* bw/rebase-merge-entire-branch (2016-04-24) 1 commit
  (merged to 'next' on 2016-04-29 at 7a9487f)
 + git-rebase--merge: don't include absent parent as a base

 "git rebase -m" could be asked to rebase an entire branch starting
 from the root, but failed by assuming that there always is a parent
 commit to the first commit on the branch.


* jc/drop-git-spec-in (2016-04-27) 2 commits
  (merged to 'next' on 2016-04-27 at 2b85030)
 + Makefile: remove dependency on git.spec
  (merged to 'next' on 2016-04-22 at 531583f)
 + Makefile: stop pretending to support rpmbuild

 As nobody maintains our in-tree git.spec.in and distros use their
 own spec file, we stopped pretending that we support "make rpm".


* jk/diff-compact-heuristic (2016-05-02) 3 commits
  (merged to 'next' on 2016-05-02 at 2a74763)
 + diff: undocument the compaction heuristic knobs for experimentation
  (merged to 'next' on 2016-04-22 at 0c117ea)
 + xdiff: implement empty line chunk heuristic
 + xdiff: add recs_match helper function
 (this branch is tangled with jc/diff-compact-always-use-blank-heuristics.)

 Patch output from "git diff" and friends has been tweaked to be
 more readable by using a blank line as a strong hint that the
 contents before and after it belong to a logically separate unit.


* js/http-custom-headers (2016-04-27) 1 commit
  (merged to 'next' on 2016-04-27 at 0c97a50)
 + http: support sending custom HTTP headers

 HTTP transport clients learned to throw extra HTTP headers at the
 server, specified via http.extraHeader configuration variable.


* ld/p4-test-py3 (2016-04-26) 3 commits
  (merged to 'next' on 2016-04-27 at d5d5fca)
 + git-p4 tests: time_in_seconds should use $PYTHON_PATH
 + git-p4 tests: work with python3 as well as python2
 + git-p4 tests: cd to / before running python

 The test scripts for "git p4" (but not "git p4" implementation
 itself) has been updated so that they would work even on a system
 where the installed version of Python is python 3.


* ls/p4-lfs-test-fix-2.7.0 (2016-04-29) 2 commits
  (merged to 'next' on 2016-04-29 at da56b67)
 + t9824: fix wrong reference value
  (merged to 'next' on 2016-04-27 at be87c63)
 + t9824: fix broken &&-chain in a subshell

 Fix a broken test.


* sb/clone-shallow-passthru (2016-04-26) 1 commit
  (merged to 'next' on 2016-04-27 at 3112b24)
 + clone: add `--shallow-submodules` flag

 "git clone" learned "--shallow-submodules" option.


* sb/config-exit-status-list (2016-04-26) 1 commit
  (merged to 'next' on 2016-04-27 at 44fe343)
 + config doc: improve exit code listing

 Doc update.

--
[New Topics]

* sb/submodule-deinit-all (2016-05-05) 1 commit
 - submodule deinit: require '--all' instead of '.' for all submodules

 Correct faulty recommendation to use "git submodule deinit ." when
 de-initialising all submodules, which would result in a strange
 error message in a pathological corner case.

 Will merge to 'next'.


* bn/config-doc-tt-varnames (2016-05-05) 1 commit
 - config: consistently format $variables in monospaced font
 (this branch uses jc/config-pathname-type.)

 Doc formatting fixes.


* lp/typofixes (2016-05-06) 1 commit
 - typofix: assorted typofixes in comments, documentation and messages

 Will merge to 'next'.


* sb/z-is-gnutar-ism (2016-05-06) 1 commit
 - t3513: do not compress backup tar file

 Will merge to 'next'.


* jc/test-parse-options-expect (2016-05-06) 4 commits
 - t0040: convert a few tests to use test-parse-options --expect
 - t0040: remove unused test helpers
 - test-parse-options: --expect= option to simplify tests
 - test-parse-options: fix output when callback option fails
 (this branch uses pb/commit-verbose-config.)

Re: [PATCH] t3513: do not compress backup tar file

[Junio C Hamano]

Stefan Beller  writes:

> Armin Kunaschik  wrote:
>> I'm trying to compile/test/use git 2.8.2 on AIX 6.1 with
>> no bash available.
> ...
>> make test does not make it through t3513-revert-submodule.sh anymore.
>> The test is not portable since it uses the z-flags of GNU-tar. When -z
>> is removed, (and extension is changed back to tar) everything runs and
>> tests smoothly.
>
> Signed-off-by: Stefan Beller 
> ---

Thanks for a quick fix.  Even though "no bash" and "AIX 6.1" are
interesting details that are part of a good bug report, these are
irrelevant noise for a commit that fixes a bug that is unrelated to
them, so let's rephrase the message and queue it, like this:

t3513: do not compress backup tar file

The test uses the 'z' option, i.e. "compress the output while at
it", which is GNUism and not portable.

Reported-by: Armin Kunaschik 
Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 

>  t/t3513-revert-submodule.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t3513-revert-submodule.sh b/t/t3513-revert-submodule.sh
> index a1c4e02..db93781 100755
> --- a/t/t3513-revert-submodule.sh
> +++ b/t/t3513-revert-submodule.sh
> @@ -14,11 +14,11 @@ test_description='revert can handle submodules'
>  git_revert () {
>   git status -su >expect &&
>   ls -1pR * >>expect &&
> - tar czf "$TRASH_DIRECTORY/tmp.tgz" * &&
> + tar cf "$TRASH_DIRECTORY/tmp.tar" * &&
>   git checkout "$1" &&
>   git revert HEAD &&
>   rm -rf * &&
> - tar xzf "$TRASH_DIRECTORY/tmp.tgz" &&
> + tar xf "$TRASH_DIRECTORY/tmp.tar" &&
>   git status -su >actual &&
>   ls -1pR * >>actual &&
>   test_cmp expect actual &&

This is not a new problem, but these "ls -1pR" and "rm -rf *" makes
me wonder if it is the best way to test what is being tested.

The title says "revert can handle submodules", but when it sees that
revert finishes successfully, it discards the resulting working tree
state with "rm -rf *" (Yuck) and repopulates with the state before
the 'checkout && revert' sequence, so the 'status' and 'ls' are not
testing what 'revert' did at all.

Shouldn't it be testing HEAD^{tree} before "checkout && revert" and
after and make sure they match, and checking the working state left
by 'revert' without clobbering it with tarball extract?

--
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] gitweb: fix link to parent diff with pathinfo

[Junio C Hamano]

Richard Braun  writes:

> Signed-off-by: Richard Braun 
> ---

Could you justify your change with a bit more than "fix"?  That is,

gitweb, when used with PATH_INFO, shows a link to parent diff
like [fill in the blank].  However, it is wrong because [fill in
the blank].

Make it show it like [fill in the blank].  Because [fill in the
blank], delete 'hash_parent' element from the %params hash once
we used it; otherwise [fill in the blank to describe "this bad
thing happens"].

or something like that.

Thanks.

>  gitweb/gitweb.perl | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 05d7910..f7f7936 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1423,7 +1423,12 @@ sub href {
>   delete $params{'hash'};
>   delete $params{'hash_base'};
>   } elsif (defined $params{'hash'}) {
> - $href .= esc_path_info($params{'hash'});
> + if (defined $params{'hash_parent'}) {
> + $href .= esc_path_info($params{'hash_parent'});
> + delete $params{'hash_parent'};
> + } else {
> + $href .= esc_path_info($params{'hash'});
> + }
>   delete $params{'hash'};
>   }
--
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 v5 0/2] bisect--helper: rewrite of check-term-format()

[Junio C Hamano]

Pranit Bauva  writes:

> A very interesting fact to note:
> I made a mistake by dropping of the first argument 'term' in the function
> one_of() and compiled and the test suite passed fully (which was pointed
> out by Christian). This shows that the test coverage is incomplete. 
> I am
> currently writing tests to improve on that coverage. It will be included
> separately.
>
> Link to v4: http://thread.gmane.org/gmane.comp.version-control.git/293488
>
> Changes wrt v4:
>  * Stick with 'subcommand' in the commit message.
>  * Rename enum as 'subcommand' to make it singular.

I've already said what needs to be said on this.

>  * s/bug/BUG/g
>  * Drop _() in the default of switch case
>  * Use some suggestions for commit message no. 2 and also include why I am
>using subcommand.

I am not particularly impressed by the rationale, though.

As a starter project to show that you learned how to add a new mode
to bisect--helper, check-term-format may be a small enough function
that is easy to dip the toe into the codebase, so in that sense it
may be an appropriate first step, but otherwise it is not all that
interesting, especially when we _know_ that it will be discarded.

--
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/2] rev-parse tests: add tests executed from a subdirectory

[Junio C Hamano]

Michael Rappazzo  writes:

> t1500-rev-parse contains envrionment leaks (changing dir without
> changing back, setting config variables, etc).  Add a test to clean this
> up up so that future tests can be added without worry of any setting
> from a previous test.

This is a wonderful thing to do, but...

>  test_rev_parse toplevel false false true '' .git
>  
> @@ -84,4 +85,41 @@ test_rev_parse 'GIT_DIR=../repo.git, core.bare = true' 
> true false false ''
>  git config --unset core.bare
>  test_rev_parse 'GIT_DIR=../repo.git, core.bare undefined' false false true ''
>  
> +test_expect_success 'cleanup from the previous tests' '
> + cd .. &&
> + rm -r work &&

Instead of cleaning things up like this, could you please please
please fix these existing tests that chdir around without being in a
subshell?  If the "previous tests" failed before going down as this
step expects, the "cd .. && rm -r" can make things worse.

> + mv repo.git .git &&
> + unset GIT_DIR &&
> + unset GIT_CONFIG &&

The spirit of this change is to make the test more independent from
the effects of what happened previously.  Use sane_unset so that
we do not have to worry about previous step that may have failed
before it has a chance to set GIT_DIR and GIT_CONFIG (which would
cause these unset to fail).

> + git config core.bare $original_core_bare

Is this (rather, the capturing of $original_core_bare up above)
necessary?  We are in the default 'trash' repository when the
capturing happens, and we know it is not a bare repository, right?
--
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: /* compiler workaround */ - what was the issue?

[Ramsay Jones]

On 06/05/16 21:21, Ramsay Jones wrote:
> On 06/05/16 19:54, Junio C Hamano wrote:
>> Ramsay Jones  writes:
>>
[snip]

> I still can't get gcc to complain, e.g. (on top of above):
> 
>   $ git diff
>   diff --git a/builtin/rev-list.c b/builtin/rev-list.c
>   index deae1f3..845fcdc 100644
>   --- a/builtin/rev-list.c
>   +++ b/builtin/rev-list.c
>   @@ -377,7 +377,7 @@ int cmd_rev_list(int argc, const char **argv, const 
> char *prefix)
>   mark_edges_uninteresting(, show_edge);
>
>   if (bisect_list) {
>   -   int reaches = 0, all = 0;
>   +   int reaches, all;
>
>   revs.commits = find_bisection(revs.commits, , ,
> bisect_find_all);
>   $ rm builtin/rev-list.o
>   $ make V=1 CFLAGS='-g -O3 -Wall -Wextra -Wuninitialized 
> -Wno-unused-parameter' builtin/rev-list.o
>   cc -o builtin/rev-list.o -c -MF builtin/.depend/rev-list.o.d -MQ 
> builtin/rev-list.o -MMD -MP  -g -O3 -Wall -Wextra -Wuninitialized 
> -Wno-unused-parameter -I. -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND  
> -DHAVE_PATHS_H -DHAVE_DEV_TTY -DXDL_FAST_HASH -DHAVE_CLOCK_GETTIME 
> -DHAVE_CLOCK_MONOTONIC -DHAVE_GETDELIM -DSHA1_HEADER=''  
> -DNO_STRLCPY -DNO_MKSTEMPS -DSHELL_PATH='"/bin/sh"'  builtin/rev-list.c
>   In file included from ./cache.h:4:0,
>from builtin/rev-list.c:1:
>   ./git-compat-util.h: In function ‘xsize_t’:
>   ./git-compat-util.h:838:10: warning: comparison between signed and unsigned 
> integer expressions [-Wsign-compare]
> if (len > (size_t) len)
> ^
>   $ 

BTW, another patch that I have hanging around ... this time the
patch below was built on master from a couple of years ago (so,
I haven't bothered to rebase it, but you should get the idea):

ATB,
Ramsay Jones

-- >8 --
Subject: [PATCH] git-compat-util.h: xsize_t(): avoid signed comparison warnings

Signed-off-by: Ramsay Jones 
---
 git-compat-util.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index c07e0c1..3a9cf6c 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -838,9 +838,10 @@ static inline char *xstrdup_or_null(const char *str)
 
 static inline size_t xsize_t(off_t len)
 {
-   if (len > (size_t) len)
+   size_t r = (size_t)len;
+   if (len != (off_t)r)
die("Cannot handle files this big");
-   return (size_t)len;
+   return r;
 }
 
 __attribute__((format (printf, 3, 4)))
-- 
2.8.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

Re: [PATCH 78/83] Move libified code from builtin/apply.c to apply.{c,h}

[Junio C Hamano]

Christian Couder  writes:

> By the way does someone know how such a patch can be applied?

Perhaps this one?

http://thread.gmane.org/gmane.linux.kernel/1879635



--
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 v16 0/7] config commit verbose

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> On Fri, May 6, 2016 at 6:16 PM, Pranit Bauva  wrote:
>> [+cc:git@vger.kernel.org] Because its an interesting fact to be shared
>> which isn't covered elsewhere.
>>
>> On Fri, May 6, 2016 at 2:53 AM, Ævar Arnfjörð Bjarmason
>>  wrote:
>>> Sending this privately since it's probably covered elsewhere. With
>>> this, if I set the option will "reword" in git rebase -i show me the
>>> patch?
>>>
>>> If so: awesome.
>>
>> Yes, git rebase -i will show the diff in 'reword' if commit.verbose is
>> set to true or a value greater than 0.
>>
>> I dug further in git-rebase--interactive.sh
>> I could find appearances of "git commit --amend" but I was unable to
>> find appearances of "COMMIT_EDITMSG". If COMMIT_EDITMSG was coming
>> into picture, the commit.verbose could not affect it. And that is not
>> the case.
>>
>> I guess this would be a desirable trait for most of the consumers of
>> commit.verbose (like Ævar) so there would not be a need to suppress.
>
> Yeah it's great, it's something I've wanted from interactive rebase
> for a while now.

I can see why "commit -v" may be useful during "rebase -i", but we
should also have rebase.verbose and "rebase -v".  I do not want to
make all my commits with -v, and I suspect I want to do "commit -v"
more often during "rebase -i" than regular commit, for example.


--
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: /* compiler workaround */ - what was the issue?

[Marc Branchaud]

On 2016-05-06 03:57 PM, Junio C Hamano wrote:

Marc Branchaud  writes:


On 2016-05-06 02:54 PM, Junio C Hamano wrote:


I wonder if can we come up with a short and sweet notation to remind
futhre readers that this "initialization" is not initializing but
merely squelching warnings from stupid compilers, and agree to use
it consistently?


Perhaps

#define COMPILER_UNINITIALIZED_WARNING_INITIALIZER 0

or, for short-and-sweet

#define CUWI 0

?

:)


I get that smiley.


Of course, right after I sent that I thought of

#define SPURIOUS_COMPILER_RELATED_UNINITIALIZED_WARNING_INITIALIZER 0

or

#define SCRUWI 0

Which we'd get to pronounce as "screwy".

OK, I'll shut up now.

M.

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

Re: /* compiler workaround */ - what was the issue?

[Ramsay Jones]

On 06/05/16 19:54, Junio C Hamano wrote:
> Ramsay Jones  writes:
> 
>> The patch below applies to master (I haven't checked for any more
>> additions).
>>
>>  if (bisect_list) {
>> -int reaches = reaches, all = all;
>> +int reaches = 0, all = 0;
> 
> One thing that is somewhat sad is that this makes future readers
> wonder if these values '0' are sensible initial values.
> 
> Having to wonder "is it sensible to initialize this variable to 0?
> Shouldn't it be initialized to INT_MAX instead?" is wasting their
> time exactly because we _know_ these are not even "initial values".
> We know these do not have to be initialized, because some more
> appropriate values will get assigned to them before they are used,
> and have these only because some compilers get it wrong.
> 
> The original "reaches = reaches" had the documentation value (at
> least for those who knew the convention) to save the readers from
> wasting their time that way.  Now these 0 are indistinguishable from
> the other initializations that require to be zero.

Ah, I think I remember now why I hadn't sent this patch before. ;-)
[This started off as one patch, was then split into two (int and pointer),
and then back into one again - presumably because I had by that time
forgotten why I split it up!]

I have a very vague recollection of you expressing your dislike of
these parts of the patch before. I had intended to investigate why
gcc was incorrectly issuing these warnings - but I couldn't get my
currently installed compiler to complain. That would have meant
building various gcc versions, so that I could bisect ...

So, that's why I didn't get around to it ... :-D

I still can't get gcc to complain, e.g. (on top of above):

  $ git diff
  diff --git a/builtin/rev-list.c b/builtin/rev-list.c
  index deae1f3..845fcdc 100644
  --- a/builtin/rev-list.c
  +++ b/builtin/rev-list.c
  @@ -377,7 +377,7 @@ int cmd_rev_list(int argc, const char **argv, const char 
*prefix)
  mark_edges_uninteresting(, show_edge);
   
  if (bisect_list) {
  -   int reaches = 0, all = 0;
  +   int reaches, all;
   
  revs.commits = find_bisection(revs.commits, , ,
bisect_find_all);
  $ rm builtin/rev-list.o
  $ make V=1 CFLAGS='-g -O3 -Wall -Wextra -Wuninitialized 
-Wno-unused-parameter' builtin/rev-list.o
  cc -o builtin/rev-list.o -c -MF builtin/.depend/rev-list.o.d -MQ 
builtin/rev-list.o -MMD -MP  -g -O3 -Wall -Wextra -Wuninitialized 
-Wno-unused-parameter -I. -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND  
-DHAVE_PATHS_H -DHAVE_DEV_TTY -DXDL_FAST_HASH -DHAVE_CLOCK_GETTIME 
-DHAVE_CLOCK_MONOTONIC -DHAVE_GETDELIM -DSHA1_HEADER=''  
-DNO_STRLCPY -DNO_MKSTEMPS -DSHELL_PATH='"/bin/sh"'  builtin/rev-list.c
  In file included from ./cache.h:4:0,
   from builtin/rev-list.c:1:
  ./git-compat-util.h: In function ‘xsize_t’:
  ./git-compat-util.h:838:10: warning: comparison between signed and unsigned 
integer expressions [-Wsign-compare]
if (len > (size_t) len)
^
  $ 
  
[Note: gcc (Ubuntu 4.8.4-2ubuntu1~14.04.1) 4.8.4]

> 
>> diff --git a/read-cache.c b/read-cache.c
>> index d9fb78b..978d6b6 100644
>> --- a/read-cache.c
>> +++ b/read-cache.c
>> @@ -1870,7 +1870,7 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, 
>> struct cache_entry *ce,
>>  {
>>  int size;
>>  struct ondisk_cache_entry *ondisk;
>> -int saved_namelen = saved_namelen; /* compiler workaround */
>> +int saved_namelen = 0;
> 
> I wonder if can we come up with a short and sweet notation to remind
> futhre readers that this "initialization" is not initializing but
> merely squelching warnings from stupid compilers, and agree to use
> it consistently?

Nothing comes to mind.

Do current compilers complain?

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 78/83] Move libified code from builtin/apply.c to apply.{c,h}

[Christian Couder]

On Thu, May 5, 2016 at 10:50 AM, Christian Couder
 wrote:
> On Wed, May 4, 2016 at 1:32 PM, Duy Nguyen  wrote:
>> On Wed, May 4, 2016 at 5:39 PM, Christian Couder
>>  wrote:
>>>
>>> Right now I get:
>>>
 git log --summary -M -C -B -1 20f1d27
>>> commit 20f1d274609f5dde2eaaa279e7ee79fd5ef9c849
>>> Author: Christian Couder 
>>> Date:   Fri Apr 22 20:55:46 2016 +0200
>>>
>>> Move libified code from builtin/apply.c to apply.{c,h}
>>>
>>> Signed-off-by: Christian Couder 
>>>
>>>  copy builtin/apply.c => apply.c (96%)
>>>  rewrite builtin/apply.c (96%)
>>
>> Ah.. I forgot about -B to break rename pairs! This looks good. I
>> really need to go back to diff rename hints series, so we can annotate
>> this commit and don't have to use -B every time.
>>
>>> And using format-patch:
>>>
 git format-patch -M -C -B -1  -o 
 ../../patches/test-libify-apply-use-in-am/ 20f1d27
>>> ../../patches/test-libify-apply-use-in-am/0001-Move-libified-code-from-builtin-apply.c-to-apply.-c-.patch
 wc 
 ../../patches/test-libify-apply-use-in-am/0001-Move-libified-code-from-builtin-apply.c-to-apply.-c-.patch
>>>   5264  23426 147133

By the way does someone know how such a patch can be applied?

I get the following:

> git reset --keep 20f1d27~
> git am 
> ../../patches/test-libify-apply-use-in-am/0001-Move-libified-code-from-builtin-apply.c-to-apply.-c-.patch
Applying: Move libified code from builtin/apply.c to apply.{c,h}
error: apply.c: already exists in index
Patch failed at 0001 Move libified code from builtin/apply.c to apply.{c,h}
The copy of the patch that failed is found in:
   /home/christian/git/git/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

With git apply I get:

> git am --abort
> git apply -v 
> ../../patches/test-libify-apply-use-in-am/0001-Move-libified-code-from-builtin-apply.c-to-apply.-c-.patch
Checking patch builtin/apply.c => apply.c...
error: apply.c: already exists in working directory
Checking patch apply.h...
Checking patch builtin/apply.c...
--
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] pathspec: remove check_path_for_gitlink

[Junio C Hamano]

Stefan Beller  writes:

> It was a bug, but now people in the outside world consider it a feature.
> Search for "Git fake submodules" and you'll find a few users who use this
> technique successfully.
>
> I do not think fixing this bug would do good. So maybe we just let it slip?

I am OK with leaving it unfixed, iow, we just say this:

If deep/in is a different repository, whether it is a submodule,
"git add deep/in/the/tree/is-a-leaf.txt" will give an undefined
result.

But that is totally different from accepting it as a feature.  If we
were to accept it as a feature (and we will not), then

I did "git add deep/in/the/tree/is-a-leaf.txt" and have kept the
path tracked.  Today I did "git add deep/*" and then the path
disappeared from my project--I now only have deep/in as a
submodule, which is not what I want.

would become a valid bug report.  I do not want to see that happen.

I.e. I am *NOT* OK with polluting the codebase with a hack to
respond to such a bug report, e.g. by adding a rule that says "if a
file deep/in/the/tree/is-a-leaf.txt is tracked and deep/in is a
repository, 'git add deep/in' must fail".

The stance "It is a bug, but we do not fix it right now.  The
behaviour is undefined" also leaves the door open for a future
enhancement that allows 'git add deep/in/the/tree/is-a-leaf.txt' to
be an equivalent to 'git -C deep/in/the/tree add is-a-leaf.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] submodule: stop sanitizing config options

[Junio C Hamano]

Stefan Beller  writes:

>> Hmph, Stefan, do you want to keep this (if you want to resurrect the
>> function in some future, for example)?
>
> IMHO it is easier to revert or rewrite than to carry unused code?
> Unused code is not tested and untested code is broken code.
> And relying on broken code in the future will guarantee bugs.
> (Sure new code may also have bugs, but I just think that bugs
> in newly written code can be fixed easier)
>
> I would prefer to get rid of it, i.e. taking that patch.

OK.
--
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: /* compiler workaround */ - what was the issue?

[Stefan Beller]

On Fri, May 6, 2016 at 12:57 PM, Junio C Hamano  wrote:
> Marc Branchaud  writes:
>
>> On 2016-05-06 02:54 PM, Junio C Hamano wrote:
>>>
>>> I wonder if can we come up with a short and sweet notation to remind
>>> futhre readers that this "initialization" is not initializing but
>>> merely squelching warnings from stupid compilers, and agree to use
>>> it consistently?
>>
>> Perhaps
>>
>>   #define COMPILER_UNINITIALIZED_WARNING_INITIALIZER 0
>>
>> or, for short-and-sweet
>>

   /* Here could go a longer explanation than the 4 character
define :) */
>>   #define CUWI 0
>>
>> ?
>>
>> :)
>
> I get that smiley.
>
> I was hinting to keep the /* compiler workaround */ comment, but
> in a bit shorter form.
> --
> 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: /* compiler workaround */ - what was the issue?

[Junio C Hamano]

Marc Branchaud  writes:

> On 2016-05-06 02:54 PM, Junio C Hamano wrote:
>>
>> I wonder if can we come up with a short and sweet notation to remind
>> futhre readers that this "initialization" is not initializing but
>> merely squelching warnings from stupid compilers, and agree to use
>> it consistently?
>
> Perhaps
>
>   #define COMPILER_UNINITIALIZED_WARNING_INITIALIZER 0
>
> or, for short-and-sweet
>
>   #define CUWI 0
>
> ?
>
> :)

I get that smiley.

I was hinting to keep the /* compiler workaround */ comment, but
in a bit shorter form.
--
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 v16 0/7] config commit verbose

[Ævar Arnfjörð Bjarmason]

On Fri, May 6, 2016 at 6:16 PM, Pranit Bauva  wrote:
> [+cc:git@vger.kernel.org] Because its an interesting fact to be shared
> which isn't covered elsewhere.
>
> On Fri, May 6, 2016 at 2:53 AM, Ævar Arnfjörð Bjarmason
>  wrote:
>> Sending this privately since it's probably covered elsewhere. With
>> this, if I set the option will "reword" in git rebase -i show me the
>> patch?
>>
>> If so: awesome.
>
> Yes, git rebase -i will show the diff in 'reword' if commit.verbose is
> set to true or a value greater than 0.
>
> I dug further in git-rebase--interactive.sh
> I could find appearances of "git commit --amend" but I was unable to
> find appearances of "COMMIT_EDITMSG". If COMMIT_EDITMSG was coming
> into picture, the commit.verbose could not affect it. And that is not
> the case.
>
> I guess this would be a desirable trait for most of the consumers of
> commit.verbose (like Ævar) so there would not be a need to suppress.

Yeah it's great, it's something I've wanted from interactive rebase
for a while now.
--
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: /* compiler workaround */ - what was the issue?

[Marc Branchaud]

On 2016-05-06 02:54 PM, Junio C Hamano wrote:


I wonder if can we come up with a short and sweet notation to remind
futhre readers that this "initialization" is not initializing but
merely squelching warnings from stupid compilers, and agree to use
it consistently?


Perhaps

#define COMPILER_UNINITIALIZED_WARNING_INITIALIZER 0

or, for short-and-sweet

#define CUWI 0

?

:)

M.

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

Re: [PATCH] pathspec: remove check_path_for_gitlink

[Stefan Beller]

On Fri, May 6, 2016 at 12:02 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> On Fri, May 6, 2016 at 3:30 AM, Duy Nguyen  wrote:
>>> On Fri, May 6, 2016 at 6:27 AM, Junio C Hamano  wrote:
 Stefan Beller  writes:

>> I wonder if the patches mentioned have something to do with the "git
>> add deep/in/the/tree" that fails to notice deep/in/ is an unrelated
>> repository in some way?
>>>
>>> The same functionality is added in 8745024 (parse_pathspec: support
>>> stripping/checking submodule paths - 2013-07-14) so if it didn't fail
>>> to notice that before 5a76aff1a6 and did after, it's a bug.
>>
>> The bug seems to have existed before. However in the bug we are talking
>> about the nested repo is not a submodule yet.
>
> That agrees with Duy's recollection below:
>
>>> I vaguely recall this symptom. It has something to do with the index,
>>> the check we do requires a gitlink in the index, I think. So if the
>>> gitlink entry is not in the index, our protection line fails.
>
> So are we all on the same page that this is a bug now?

It was a bug, but now people in the outside world consider it a feature.
Search for "Git fake submodules" and you'll find a few users who use this
technique successfully.

I do not think fixing this bug would do good. So maybe we just let it slip?
--
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] pathspec: remove check_path_for_gitlink

[Junio C Hamano]

Stefan Beller  writes:

> On Fri, May 6, 2016 at 3:30 AM, Duy Nguyen  wrote:
>> On Fri, May 6, 2016 at 6:27 AM, Junio C Hamano  wrote:
>>> Stefan Beller  writes:
>>>
> I wonder if the patches mentioned have something to do with the "git
> add deep/in/the/tree" that fails to notice deep/in/ is an unrelated
> repository in some way?
>>
>> The same functionality is added in 8745024 (parse_pathspec: support
>> stripping/checking submodule paths - 2013-07-14) so if it didn't fail
>> to notice that before 5a76aff1a6 and did after, it's a bug.
>
> The bug seems to have existed before. However in the bug we are talking
> about the nested repo is not a submodule yet.

That agrees with Duy's recollection below:

>> I vaguely recall this symptom. It has something to do with the index,
>> the check we do requires a gitlink in the index, I think. So if the
>> gitlink entry is not in the index, our protection line fails.

So are we all on the same page that this is a bug now?
--
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 11/15] diff: ignore submodules excluded by groups

[Junio C Hamano]

Stefan Beller  writes:

>> Further suppose that the user needs to view a submodule outside the
>> default group temporarily (imagine: for a day or two), while
>> carrying out some specific task.  Perhaps she is working on the
>> documentation submodule, which is her primary task hence her
>> configuration file specifies it as the default, but needs to see the
>> submodule that houses the implementation to describe the behaviour.
>>
>> So she does "init code-module/"; this has explicit pathspec, so
>> there is no difference between the two.  Now, while reading the code
>> module, she finds a typo in a comment, and wants to fix it.  We will
>> start to see differences.
>
> Another way (3) is to add code-module/ to the "set of interesting
> submodules, i.e. to the default group"

I do not want to force her to do more than "submodule deinit" when
she is done with that temporary task that required her to have
code-module/ checked out, which is what you are doing with such a
change.  So that is a non-starter to me.

>> The amount of "extra" in the first use case necessary for (1) is
>> greater than the amount of "extra" in the second use case necessary
>> for (2), though.  In addition, in the second use case, (1) makes it
>> easier for the user to miss important changes she made outside the
>> area of her usual attention, while (2) forces her to make a
>> conscious effort to exclude them.  These are the reasons why I have
>> a slight preference for (2) over (1).
>>
>
> That makes sense.
>
> So with (2)
>  * there is no need to modify status, diff, log for the default case and the
> --recursive \*default" may come later, so the initial series can be 
> smaller.
>  *

I sense a premature "Send" button here ;-)

In any case, I care much more about the "making it harder to miss
things outside the usual area of attention" benefit than "the user
may have to type less more often" benefit, even though both are
slightly in favor between (1) and (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: /* compiler workaround */ - what was the issue?

[Junio C Hamano]

Ramsay Jones  writes:

> The patch below applies to master (I haven't checked for any more
> additions).
>
>   if (bisect_list) {
> - int reaches = reaches, all = all;
> + int reaches = 0, all = 0;

One thing that is somewhat sad is that this makes future readers
wonder if these values '0' are sensible initial values.

Having to wonder "is it sensible to initialize this variable to 0?
Shouldn't it be initialized to INT_MAX instead?" is wasting their
time exactly because we _know_ these are not even "initial values".
We know these do not have to be initialized, because some more
appropriate values will get assigned to them before they are used,
and have these only because some compilers get it wrong.

The original "reaches = reaches" had the documentation value (at
least for those who knew the convention) to save the readers from
wasting their time that way.  Now these 0 are indistinguishable from
the other initializations that require to be zero.

> diff --git a/read-cache.c b/read-cache.c
> index d9fb78b..978d6b6 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1870,7 +1870,7 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, 
> struct cache_entry *ce,
>  {
>   int size;
>   struct ondisk_cache_entry *ondisk;
> - int saved_namelen = saved_namelen; /* compiler workaround */
> + int saved_namelen = 0;

I wonder if can we come up with a short and sweet notation to remind
futhre readers that this "initialization" is not initializing but
merely squelching warnings from stupid compilers, and agree to use
it consistently?

--
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] t3513: do not compress backup tar file

[Stefan Beller]

Armin Kunaschik  wrote:
> I'm trying to compile/test/use git 2.8.2 on AIX 6.1 with
> no bash available.
...
> make test does not make it through t3513-revert-submodule.sh anymore.
> The test is not portable since it uses the z-flags of GNU-tar. When -z
> is removed, (and extension is changed back to tar) everything runs and
> tests smoothly.

Signed-off-by: Stefan Beller 
---
 t/t3513-revert-submodule.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t3513-revert-submodule.sh b/t/t3513-revert-submodule.sh
index a1c4e02..db93781 100755
--- a/t/t3513-revert-submodule.sh
+++ b/t/t3513-revert-submodule.sh
@@ -14,11 +14,11 @@ test_description='revert can handle submodules'
 git_revert () {
git status -su >expect &&
ls -1pR * >>expect &&
-   tar czf "$TRASH_DIRECTORY/tmp.tgz" * &&
+   tar cf "$TRASH_DIRECTORY/tmp.tar" * &&
git checkout "$1" &&
git revert HEAD &&
rm -rf * &&
-   tar xzf "$TRASH_DIRECTORY/tmp.tgz" &&
+   tar xf "$TRASH_DIRECTORY/tmp.tar" &&
git status -su >actual &&
ls -1pR * >>actual &&
test_cmp expect actual &&
-- 
2.8.2.335.g4bb51ae.dirty

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

Re: /* compiler workaround */ - what was the issue?

[Philip Oakley]

From: "Ramsay Jones" 

On 06/05/16 14:15, Philip Oakley wrote:

From: "Duy Nguyen" 
On Fri, May 6, 2016 at 4:41 AM, Junio C Hamano  
wrote:

"Philip Oakley"  writes:


int saved_namelen = saved_namelen; /* compiler workaround */

Which then becomes an MSVC compile warning C4700: uninitialized local
variable.

I'm wondering what was the compiler workaround being referred to? i.e. 
why
does it need that tweak? There's no mention of the reason in the 
commit

message.


That was a fairly well-known workaround for GCC that issues a false
warning that variable is used before initialized.  I thought we
stopped using it several years ago in new code after doing a bulk
sanitizing


I guess that's 803a777 (cat-file: Fix an gcc -Wuninitialized warning -
2013-03-26) and more commits around that time. The split-index commit
is in 2014. I must have missed the trend.


(I think the new recommended workaround was to initialise
such a variable to the nil value like '0' for integers).


Yep. First Jeff removed the " = xxx" part from "xxx = xxx" then Ramsay
added the " = NULL" back. So we probably just do "int saved_namelen =
0;" in this case.
--

Thanks,

I'll try and work up a patch - probably next week as I'm away for the 
weekend.


Yeah, I don't remember why these were left over from the previous
attempt to clean these up (maybe they conflicted with in-flight
topics?), but I have had a patch hanging around ... :-D

The patch below applies to master (I haven't checked for any more
additions).


Looks good to me. Catches those I've seen.

Have a good weekend all.



ATB,
Ramsay Jones

-- >8 --
From: Ramsay Jones 
Subject: [PATCH] -Wuninitialized: remove a gcc specific workaround

Signed-off-by: Ramsay Jones 
---
builtin/rev-list.c | 2 +-
fast-import.c  | 4 ++--
merge-recursive.c  | 2 +-
read-cache.c   | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 275da0d..deae1f3 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -377,7 +377,7 @@ int cmd_rev_list(int argc, const char **argv, const 
char *prefix)

 mark_edges_uninteresting(, show_edge);

 if (bisect_list) {
- int reaches = reaches, all = all;
+ int reaches = 0, all = 0;

 revs.commits = find_bisection(revs.commits, , ,
   bisect_find_all);
diff --git a/fast-import.c b/fast-import.c
index 9fc7093..ca66d80 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2935,7 +2935,7 @@ static void cat_blob(struct object_entry *oe, 
unsigned char sha1[20])


static void parse_get_mark(const char *p)
{
- struct object_entry *oe = oe;
+ struct object_entry *oe = NULL;
 char output[42];

 /* get-mark SP  LF */
@@ -2952,7 +2952,7 @@ static void parse_get_mark(const char *p)

static void parse_cat_blob(const char *p)
{
- struct object_entry *oe = oe;
+ struct object_entry *oe = NULL;
 unsigned char sha1[20];

 /* cat-blob SP  LF */
diff --git a/merge-recursive.c b/merge-recursive.c
index 06d31ed..9cecc24 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1897,7 +1897,7 @@ int merge_recursive(struct merge_options *o,
{
 struct commit_list *iter;
 struct commit *merged_common_ancestors;
- struct tree *mrtree = mrtree;
+ struct tree *mrtree = NULL;
 int clean;

 if (show(o, 4)) {
diff --git a/read-cache.c b/read-cache.c
index d9fb78b..978d6b6 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1870,7 +1870,7 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, 
struct cache_entry *ce,

{
 int size;
 struct ondisk_cache_entry *ondisk;
- int saved_namelen = saved_namelen; /* compiler workaround */
+ int saved_namelen = 0;
 char *name;
 int result;

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



--
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] utf8: fix duplicate words of "the"

[Junio C Hamano]

Junio C Hamano  writes:

> Jeff King  writes:
>>
>> IMHO it would be fine to just do all of these in a single patch. They're
>> different files, yes, but it's all conceptually the same change.
>
> I can squash them into a single one.  So far, everything except two
> I saw was good.

So I tentatively queued this.

-- >8 --
From: Li Peng 
Date: Fri, 6 May 2016 20:36:46 +0800
Subject: [PATCH] typofix: assorted typofixes in comments, documentation and
 messages

Many instances of duplicate words (e.g. "the the path") and
a few typoes are fixed, originally in multiple patches.

wildmatch: fix duplicate words of "the"
t: fix duplicate words of "output"
transport-helper: fix duplicate words of "read"
Git.pm: fix duplicate words of "return"
path: fix duplicate words of "look"
pack-protocol.txt: fix duplicate words of "the"
precompose-utf8: fix typo of "sequences"
split-index: fix typo
worktree.c: fix typo
remote-ext: fix typo

Signed-off-by: Li Peng 
Signed-off-by: Junio C Hamano 
---
 Documentation/technical/pack-protocol.txt | 2 +-
 builtin/remote-ext.c  | 2 +-
 compat/precompose_utf8.c  | 2 +-
 path.c| 2 +-
 perl/Git.pm   | 2 +-
 split-index.c | 2 +-
 t/t-basic.sh  | 2 +-
 transport-helper.c| 2 +-
 wildmatch.c   | 2 +-
 worktree.c| 2 +-
 10 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index c6977bb..8b36343 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -526,7 +526,7 @@ Push Certificate
 
 A push certificate begins with a set of header lines.  After the
 header and an empty line, the protocol commands follow, one per
-line. Note that the the trailing LF in push-cert PKT-LINEs is _not_
+line. Note that the trailing LF in push-cert PKT-LINEs is _not_
 optional; it must be present.
 
 Currently, the following header fields are defined:
diff --git a/builtin/remote-ext.c b/builtin/remote-ext.c
index 7457c74..88eb8f9 100644
--- a/builtin/remote-ext.c
+++ b/builtin/remote-ext.c
@@ -168,7 +168,7 @@ static int command_loop(const char *child)
size_t i;
if (!fgets(buffer, MAXCOMMAND - 1, stdin)) {
if (ferror(stdin))
-   die("Comammand input error");
+   die("Command input error");
exit(0);
}
/* Strip end of line characters. */
diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
index dfbe6d8..4293b53 100644
--- a/compat/precompose_utf8.c
+++ b/compat/precompose_utf8.c
@@ -147,7 +147,7 @@ struct dirent_prec_psx *precompose_utf8_readdir(PREC_DIR 
*prec_dir)
if (errno || inleft) {
/*
 * iconv() failed and errno could be 
E2BIG, EILSEQ, EINVAL, EBADF
-* MacOS X avoids illegal byte 
sequemces.
+* MacOS X avoids illegal byte 
sequences.
 * If they occur on a mounted drive 
(e.g. NFS) it is not worth to
 * die() for that, but rather let the 
user see the original name
*/
diff --git a/path.c b/path.c
index 969b494..a5e953f 100644
--- a/path.c
+++ b/path.c
@@ -134,7 +134,7 @@ static struct common_dir common_list[] = {
  * definite
  * definition
  *
- * The trie would look look like:
+ * The trie would look like:
  * root: len = 0, children a and d non-NULL, value = NULL.
  *a: len = 2, contents = bc, value = (data for "abc")
  *d: len = 2, contents = ef, children i non-NULL, value = (data for "def")
diff --git a/perl/Git.pm b/perl/Git.pm
index 49eb88a..ce7e4e8 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -393,7 +393,7 @@ sub command_close_pipe {
 Execute the given C in the same way as command_output_pipe()
 does but return both an input pipe filehandle and an output pipe filehandle.
 
-The function will return return C<($pid, $pipe_in, $pipe_out, $ctx)>.
+The function will return C<($pid, $pipe_in, $pipe_out, $ctx)>.
 See C for details.
 
 =cut
diff --git a/split-index.c b/split-index.c
index 968b780..3c75d4b 100644
--- a/split-index.c
+++ b/split-index.c
@@ -60,7 +60,7 @@ static void mark_base_index_entries(struct index_state *base)
 * To keep track of the shared entries between
 * istate->base->cache[] and istate->cache[], base entry
 * position is stored in 

Re: [PATCH 11/15] diff: ignore submodules excluded by groups

[Stefan Beller]

On Thu, May 5, 2016 at 11:08 PM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> The set of submodules you "init" to the working tree are the ones
>> that are interesting to you.  So once the tree is populated, you do
>> not ever have to look at the "defaultGroup" configuration.  You just
>> need to look at the working tree.
>> ...
>
> I forgot to prefix the first few paragraphs of that message with
> "Here is how my version of the world should work."  I did not mean
> to say "Here is how you must make your work work, or I won't talk to
> you."  I was just hurried as I had to tend to other topics.
>
> I actually do not care too deeply (except for the "automatically
> remove" part, which I do not think we should do), as I do not think
> there is a big fundamental difference between the two views.  To
> make sure we are on the same page, let me rephrase the two views I
> have in mind.

Ok, maybe we can leave that automatically remove part out for the
first series. (Eventually submodules should behave like files and we
delete files on checkout all the time, and it's a reasonable default)

So I think to reduce scope to only cover the clone/update first, all
other operations behave like today, i.e.

git clone --recurse-submodules --init-submodule=label
--init-submodule=label2   git://...
# will clone the superproject and recursively
# checkout any submodule being labeled label or label2

git config submodule.defaultGroups default
git config --add submodule.defaultGroups devel
# configure which submodules you are interested in.

git submodule add --label  git://... ..
# record a label while adding a submodule

git submodule update --init-labeled(=*label)
# will update all initialized submodules
# and learn a new switch to also initialize the grouped
# submodules (either the specified group or if none given
# the default group as configured before)




git status
git diff
git submodule summary
# care about all initialized submodules, i.e. (2) below
# a switch for recurse=label will be in a later series.



>
> The difference is what should happen when the user does not give any
> pathspec, *label, or :name to limit the set of submodules to act on,
> which, traditionally meant to work on everything, and we are trying
> to change that to some "default".
>
>  (1) The default set is what the configuration file says is the
>  default group.  The working tree state is ignored.
>
>  (2) The default set is what the configuration file says is the
>  default group, plus those the user showed interest by doing
>  "submodule init".
>
> Suppose that the user has a mostly satisfactory default configured,
> i.e. the set of submodules the configuration file says is the default
> is both necessary and sufficient to carry out her daily task.  Then
> there is no difference between the two.
>
> Further suppose that the user needs to view a submodule outside the
> default group temporarily (imagine: for a day or two), while
> carrying out some specific task.  Perhaps she is working on the
> documentation submodule, which is her primary task hence her
> configuration file specifies it as the default, but needs to see the
> submodule that houses the implementation to describe the behaviour.
>
> So she does "init code-module/"; this has explicit pathspec, so
> there is no difference between the two.  Now, while reading the code
> module, she finds a typo in a comment, and wants to fix it.  We will
> start to see differences.

Another way (3) is to add code-module/ to the "set of interesting
submodules, i.e. to the default group"

git config --all submodule.defaultGroup ./code-module
git submodule update

>
>  * When she wants to get a bird's eye view of everything she cares
>about at the moment, i.e. wants to view the state of her usual
>modules plus the code-module she is visiting, (1) is more
>cumbersome.
>
>With (1), "diff --recursive" will not step outside of her
>configured default, so she says "diff --recursive \*default
>code-module/" to say "I want to see both my default submodule(s)
>and the one I checked out by hand".
>
>With (2), she does not have to do anything special, as manually
>checked out code-module/ will be acted upon, in addition to the
>configured default.

In (3), diff --recursive will also just work fine.

>
>
>  * When she wants to see her usual modules ignoring the one-off
>checkout, (1) is easier.

In (3) you'd do

git config --unset submodule.defaultGroup ./code-module
# optionally: git submodule update

>
>With (1), she can say "diff --recursive" and done.
>
>With (2), she needs to say "diff --recursive \*default" to
>explicitly state "I may have checkouts of other submodules, but
>this time I want to view only the usual default of mine".

and diff --recursive works fine again as well.

>
> The difference 

Re: /* compiler workaround */ - what was the issue?

[Ramsay Jones]

On 06/05/16 14:15, Philip Oakley wrote:
> From: "Duy Nguyen" 
>> On Fri, May 6, 2016 at 4:41 AM, Junio C Hamano  wrote:
>>> "Philip Oakley"  writes:
>>>
 int saved_namelen = saved_namelen; /* compiler workaround */

 Which then becomes an MSVC compile warning C4700: uninitialized local
 variable.

 I'm wondering what was the compiler workaround being referred to? i.e. why
 does it need that tweak? There's no mention of the reason in the commit
 message.
>>>
>>> That was a fairly well-known workaround for GCC that issues a false
>>> warning that variable is used before initialized.  I thought we
>>> stopped using it several years ago in new code after doing a bulk
>>> sanitizing
>>
>> I guess that's 803a777 (cat-file: Fix an gcc -Wuninitialized warning -
>> 2013-03-26) and more commits around that time. The split-index commit
>> is in 2014. I must have missed the trend.
>>
>>> (I think the new recommended workaround was to initialise
>>> such a variable to the nil value like '0' for integers).
>>
>> Yep. First Jeff removed the " = xxx" part from "xxx = xxx" then Ramsay
>> added the " = NULL" back. So we probably just do "int saved_namelen =
>> 0;" in this case.
>> -- 
> Thanks,
> 
> I'll try and work up a patch - probably next week as I'm away for the weekend.

Yeah, I don't remember why these were left over from the previous
attempt to clean these up (maybe they conflicted with in-flight
topics?), but I have had a patch hanging around ... :-D

The patch below applies to master (I haven't checked for any more
additions).

ATB,
Ramsay Jones

-- >8 --
From: Ramsay Jones 
Subject: [PATCH] -Wuninitialized: remove a gcc specific workaround

Signed-off-by: Ramsay Jones 
---
 builtin/rev-list.c | 2 +-
 fast-import.c  | 4 ++--
 merge-recursive.c  | 2 +-
 read-cache.c   | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 275da0d..deae1f3 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -377,7 +377,7 @@ int cmd_rev_list(int argc, const char **argv, const char 
*prefix)
mark_edges_uninteresting(, show_edge);
 
if (bisect_list) {
-   int reaches = reaches, all = all;
+   int reaches = 0, all = 0;
 
revs.commits = find_bisection(revs.commits, , ,
  bisect_find_all);
diff --git a/fast-import.c b/fast-import.c
index 9fc7093..ca66d80 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2935,7 +2935,7 @@ static void cat_blob(struct object_entry *oe, unsigned 
char sha1[20])
 
 static void parse_get_mark(const char *p)
 {
-   struct object_entry *oe = oe;
+   struct object_entry *oe = NULL;
char output[42];
 
/* get-mark SP  LF */
@@ -2952,7 +2952,7 @@ static void parse_get_mark(const char *p)
 
 static void parse_cat_blob(const char *p)
 {
-   struct object_entry *oe = oe;
+   struct object_entry *oe = NULL;
unsigned char sha1[20];
 
/* cat-blob SP  LF */
diff --git a/merge-recursive.c b/merge-recursive.c
index 06d31ed..9cecc24 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1897,7 +1897,7 @@ int merge_recursive(struct merge_options *o,
 {
struct commit_list *iter;
struct commit *merged_common_ancestors;
-   struct tree *mrtree = mrtree;
+   struct tree *mrtree = NULL;
int clean;
 
if (show(o, 4)) {
diff --git a/read-cache.c b/read-cache.c
index d9fb78b..978d6b6 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1870,7 +1870,7 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, struct 
cache_entry *ce,
 {
int size;
struct ondisk_cache_entry *ondisk;
-   int saved_namelen = saved_namelen; /* compiler workaround */
+   int saved_namelen = 0;
char *name;
int result;
 
-- 
2.8.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] t0040: remove unused test helpers

[Junio C Hamano]

9a001381 (Fix tests under GETTEXT_POISON on parseopt, 2012-08-27)
introduced check_i18n, but the helper was never used from the
beginning.

The same commit also introduced check_unknown_i18n to replace the
helper check_unknown and changed all users of the latter to use the
former, but failed to remove check_unknown itself.

Signed-off-by: Junio C Hamano 
---
 t/t0040-parse-options.sh | 24 
 1 file changed, 24 deletions(-)

diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index d678fbf..5c8c72a 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -81,30 +81,6 @@ check() {
test_cmp expect output
 }
 
-check_i18n() {
-   what="$1" &&
-   shift &&
-   expect="$1" &&
-   shift &&
-   sed "s/^$what .*/$what $expect/" expect &&
-   test-parse-options $* >output 2>output.err &&
-   test_must_be_empty output.err &&
-   test_i18ncmp expect output
-}
-
-check_unknown() {
-   case "$1" in
-   --*)
-   echo error: unknown option \`${1#--}\' >expect ;;
-   -*)
-   echo error: unknown switch \`${1#-}\' >expect ;;
-   esac &&
-   cat expect.err >>expect &&
-   test_must_fail test-parse-options $* >output 2>output.err &&
-   test_must_be_empty output &&
-   test_cmp expect output.err
-}
-
 check_unknown_i18n() {
case "$1" in
--*)
-- 
2.8.2-507-g43e827d

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

OK DEAR

[mpmlmmawvx7958]

My greeting to you over there and i hope all is fine, how are you doing, please 
my dear i saw your profile on FB and i
became interested to know more about you, and i hope it will be the same from 
you, please i will like you to contact me
to my email so that i will tell you more about me below,( vivianeri...@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] test-parse-options: --expect= option to simplify tests

[Junio C Hamano]

Stefan Beller  writes:

>> +   if (!item)
>> +   ; /* not among entries being checked */
>> +   else {
>> +   if (strcmp((const char *)item->util, buf.buf)) {
>> +   printf("expected '%s', got '%s'\n",
>> +  (char *)item->util, buf.buf);
>> +   *status = 1;
>> +   }
>> +   }
>> +   }
>> +   strbuf_reset();
>
> strbuf_release ?

Thanks for spotting a leak.

I originally had the buf as static, as all generated strings are
short and of similar length, in an attempt to reuse the already
allocated storage instead of allocating it from scratch every call.

>>
>> return 0;
>
> return ret; ? Otherwise `ret` is unused.

This, too.  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] utf8: fix duplicate words of "the"

[Junio C Hamano]

Jeff King  writes:

> On Fri, May 06, 2016 at 08:31:33PM +0800, Li Peng wrote:
>
>> Fix duplicate words of "the" in comment.
>
> Obviously a good change, along with the other one of mine you found.
>
>> ---
>>  transport-helper.c | 2 +-
>>  utf8.h | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> You seem to be breaking these up by file; was the change in
> transport-helper supposed to be in a different patch?
>
> IMHO it would be fine to just do all of these in a single patch. They're
> different files, yes, but it's all conceptually the same change.
>
> -Peff

I can squash them into a single one.  So far, everything except two
I saw was good.
--
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] t: fix duplicate words of "output"

[Junio C Hamano]

Li Peng  writes:

> Fix duplicate words of "output" in comment.
>
> Signed-off-by: Li Peng 
> ---
>  t/t-basic.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t-basic.sh b/t/t-basic.sh
> index 79b9074..60811a3 100755
> --- a/t/t-basic.sh
> +++ b/t/t-basic.sh
> @@ -98,7 +98,7 @@ check_sub_test_lib_test () {
>  }
>  
>  check_sub_test_lib_test_err () {
> - name="$1" # stdin is the expected output output from the test
> + name="$1" # stdin is the expected output from the test

The original is marginally correct.  The first "output" is a noun,
"expected output", the second one is a pp of verb "to output",
i.e. "to produce".

I would have written it as "the expected output produced by the
test", though.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting

[Junio C Hamano]

Junio C Hamano  writes:

> If you are sure we do not need that, that is one less reason we
> would be better off without mark_as_git_dir().

Oops.  To many rounds of rewriting followed by a rather careless
final round of reading.  I couldn't decide between "That is one less
reason why we need it" and "That is one more reason why we are
better off without it".
--
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] pathspec: remove check_path_for_gitlink

[Stefan Beller]

On Fri, May 6, 2016 at 3:30 AM, Duy Nguyen  wrote:
> On Fri, May 6, 2016 at 6:27 AM, Junio C Hamano  wrote:
>> Stefan Beller  writes:
>>
 I wonder if the patches mentioned have something to do with the "git
 add deep/in/the/tree" that fails to notice deep/in/ is an unrelated
 repository in some way?
>
> The same functionality is added in 8745024 (parse_pathspec: support
> stripping/checking submodule paths - 2013-07-14) so if it didn't fail
> to notice that before 5a76aff1a6 and did after, it's a bug.

The bug seems to have existed before. However in the bug we are talking
about the nested repo is not a submodule yet.

>
>>>
>>> Which is considered a feature now. Maybe we should add tests for that?
>>>
>>> http://debuggable.com/posts/git-fake-submodules:4b563ee4-f3cc-4061-967e-0e48cbdd56cb
>>
>> That is a bug, plain and simple.  Duy any ideas where we went wrong?
>
> I vaguely recall this symptom. It has something to do with the index,
> the check we do requires a gitlink in the index, I think. So if the
> gitlink entry is not in the index, our protection line fails. I think
> doing all this at pathspec level is wrong. We should wait at least
> after read_directory() is done, by then we have a lot more info to
> decide.
>
>> I think we already have code to avoid adding beyond symlinks.
>> "git add deep/in/the/tree" should refuse if deep/in is a symbolic
>> link (and happens to point at a directory that has the/tree in it).
>> We used not to catch that long time ago, but I think we fixed it.
>>
>> The logic and the places to do the checks for "no, that thing may be
>> a directory but is an unrelated repository" should be the same.
> --
> Duy

Here are some tests to have a clear picture of what is happening:
(diff against origin/master)
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index f99f674..c9dfa11 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -37,6 +37,22 @@ test_expect_success 'setup - repository in init
subdirectory' '
git add a &&
git commit -m "submodule commit 1" &&
git tag -a -m "rev-1" rev-1
+   ) &&
+   mkdir init_slash1 &&
+   (
+   cd init_slash1 &&
+   git init &&
+   echo a >a &&
+   git add a &&
+   git commit -m "first commit"
+   ) &&
+   mkdir init_slash2 &&
+   (
+   cd init_slash2 &&
+   git init &&
+   echo a >a &&
+   git add a &&
+   git commit -m "first commit"
)
 '

@@ -44,7 +60,30 @@ test_expect_success 'setup - commit with gitlink' '
echo a >a &&
echo z >z &&
git add a init z &&
-   git commit -m "super commit 1"
+   git commit -m "super commit 1" &&
+   git ls-tree HEAD |grep init >actual &&
+   grep "16 commit" actual
+'
+
+test_expect_success 'setup - commit with gitlink/ in between' '
+   echo a >a &&
+   echo z >z &&
+   git add a init_slash1/ z &&
+   git commit -m "super commit 1" &&
+   git ls-tree HEAD |grep init_slash1 >actual &&
+   grep "16 commit" actual
+'
+
+test_expect_failure 'setup - commit with gitlink/ only' '
+   git add init_slash2/ &&
+   git commit -m "super commit 1" &&
+   git ls-tree HEAD |grep init_slash2 >actual &&
+   grep "16 commit" actual
+'
+
+test_expect_success 'tear down slash tests' '
+   rm -rf init_slash* &&
+   git commit -a -m "removing init_slash*"
 '

 test_expect_success 'setup - hide init subdirectory' '
--
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 v8 10/10] ce_compare_data() did not respect conversion

[Junio C Hamano]

Torsten Bögershausen  writes:

> We only need to pay extra attention when there is an external
> clean/smudge filter defined - otherwise we should be able to skip
> it - after 10/10.

The last two words worries me.  The goal of the suggested fix was to
make sure that

$ git reset --hard && git diff

reports that there _are_ differences in the working tree files, if
the to-git and to-working-tree conversion do not round-trip.  It is
a total opposite of 10/10, which hides the fact that working tree
contents, if added to the index, would change the index.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/4] rev-parse: fix some options when executed from subpath of main tree

[Junio C Hamano]

SZEDER Gábor  writes:

>> I agree with this, but at one point Junio suggested that it should
>> return the relative path[1],
>
> I wasn't aware of Junio's suggestion.

Mike justified his position to change everything to absolute, citing
that it is what "rev-parse --show-toplevel" does when run from a
subdirectory.  I just gave a counter-example that rev-parse does not
always talk in absolute with "rev-parse -C t --show-cdup".

It hinted that his justification may not be strong enough to choose
absolute over relative, but wasn't meant to be a suggestion to
choose relative at all.  At most, I was saying "either is equally
plausible".

> It shouldn't really matter in practice, because both the absolute and
> relative paths will ultimately lead to the same place.  However, I
> still think that for consistency's sake absolute paths would be better
> when executed in a subdir of the working tree.

Sure.

One thing I'd be worried about is that people may be using it to
compute paths and store them in $GIT_DIR/somewhere, and expecting
that they can later do a wholesale "mv" of directory hierarchy and
relative paths still work.  I wouldn't be one of those who does the
"mv" so I personally do not care, but I suspect some do.
--
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: Portability of git shell scripts?

[Armin Kunaschik]

On Wed, May 4, 2016 at 11:20 PM, Jeff King  wrote:
> On Wed, May 04, 2016 at 08:17:38PM +0200, Armin Kunaschik wrote:
>
>> I'm trying to compile/test/use git 2.8.2 on AIX 6.1 with no bash available.
>> /bin/sh is a hard link to /bin/ksh which is a ksh88, a posix shell.
>> Is this supposed to work?
>
> We aim for a practical subset of Bourne shells, including bash, dash,
> ash, ksh, etc. There's at least one Bourne-ish shell known not to work,
> which is Solaris /bin/sh[1]. POSIX is usually a good guide, but we aim
> for practical portability more than adhering strictly to the standards
> document.
>
> I've tested with mksh in the past (though it's possible that we've
> introduced a regression since then). But I think we've run into problems
> with ksh93[2]. I don't know about ksh88, or what construct it doesn't
> like.  It may or may not be easy to work around.

In general ksh (88 or 93) is posix compliant... and bash is moving away
from posix. :-) But I know what you mean.

>> As an example: make test fails on nearly every t34* test and on tests
>> which contain rebase.
>> The installation of bash (and manually changing the shebang to
>> /bin/bash) "fixes" all rebase test failures. So obviously git-rebase
>> is not portable at some point.
>
> Right. Any modern-ish Bourne shell will do, so moving to bash is one way
> to fix it.

My last compile of git 2.2.2 did far better than the current 2.8.2. So
it looks like
there were more recent changes that broke portability.

>> Does it make any sense to put work into making these scripts portable,
>> that is, work with posix shells?
>
> Maybe. :) If you can find what it is that ksh88 is unhappy with, we can
> see how painful it is to adapt to. But given my looking into ksh93 in
> [2], I suspect it will be easier to just use a more modern shell.

Regarding [2] this was a bug which was fixed quite fast. To me this is no
real showstopper. Modernity of ksh93 depends on the letter after the 93 :-)

>> And, as last resort, is it possible to configure git use bash in some
>> or all shell scripts?
>
> You can set SHELL_PATH in your config.mak file.

I tried a build with SHELL_PATH=/bin/bash. Many problems "went away".
Others appeared. I'll give it a few more days to look into it.

First finding:
make test does not make it through t3513-revert-submodule.sh anymore.
The test is not portable since it uses the z-flags of GNU-tar. When -z
is removed,
(and extension is changed back to tar) everything runs and tests smoothly.

Is this report enough to start the magic to change things?

Regards,
Armin
--
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] exec_cmd.c, sideband.c, Makefile: avoid multiple PREFIX definitions

[Junio C Hamano]

Dropped "git-for-windows"  from
the Cc: list, as I seem to be getting bounces from it due to its
moderation policy.

"Philip Oakley"  writes:

> Perhaps EXEC_CMD_PREFIX, for that is what it is?

That name is doubly wrong, I have to say.

This is used only after RUNTIME_PREFIX heuristics to learn the
binary location from argv[0] fails, or the result of it does not
have expected suffix string (i.e. GIT_EXEC_PATH . BINDIR . "git").
The code even says this:

if (!prefix &&
!(prefix = strip_path_suffix(argv0_path, GIT_EXEC_PATH)) &&
!(prefix = strip_path_suffix(argv0_path, BINDIR)) &&
!(prefix = strip_path_suffix(argv0_path, "git"))) {
prefix = PREFIX;
trace_printf("RUNTIME_PREFIX requested, "
"but prefix computation failed.  "
"Using static fallback '%s'.\n", prefix);
}

Notice "static fallback" there?

I have a very strong preference for the name to reflect that fact.
I.e.  send a signal to those who do not use RUNTIME_PREFIX
configuration that they do not have to care.

Also "EXEC" is wrong, too.  The way the 'prefix' variable we see
above is used is that system_path() takes a directory path to
various installed component of the Git package, e.g. GIT_MAN_PATH
is the location for manual pages, as its "path" parameter, and
then 

strbuf_addf(, "%s/%s", prefix, path);

is used to formulate the absolute path for it.  A name with "EXEC"
in it would incorrectly hint that it points at a rough equivalent to
/usr/local/bin/ or /usr/local/libexec/git/, but PREFIX corresponds
more to /usr/local/.

Even if J6t's point about these two separate PREFIXes should never
exist at the same time is correct, I think it is a good change to
use a more explicit name for this variable that is used to
communicate between Makefile and the *.c source.

As to your "RUNTIME_PREFIX_FALLBACK is very long" objection, I do
not care ;-) More seriously, this is not something typed very often.
It appears only twice in this codepath and having clear names to
tell readers what it is about is much more important.

I do agree the most logical name, after understanding all of the
above, which is RUNTIME_PREFIX_STATIC_FALLBACK, may be a bit too
long, though.

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

Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting

[Junio C Hamano]

Johannes Schindelin  writes:

>> I agree with the goal of the change, but I am having a hard time
>> justifying this addition.  Primarily because I do not understand the
>> need for this.
>> 
>> In order to be prepared to handle HIDE_DOTFILES_TRUE case,
>> mingw_mkdir() needs to be taught about needs_hiding() and
>> make_hidden() already.  Why isn't it sufficient to teach
>> needs_hiding() to check with !strcmp(basename(path, ".git"))
>> under HIDE_DOTFILES_DOTGITONLY?
>
> The reason was that I wanted to avoid to compare a name unnecessarily when
> I already had a code path that knew perfectly well that a given directory
> is the .git/ directory.
>
> I made the change. It was more painful than I expected, as two bugs were
> uncovered, both introduced after the original patch by Erik.
> ...
> It worries me slightly that the new code is so different from the code
> that was tried and tested through all those years (although admittedly it
> is unlikely anybody ever ran with core.hideDotFiles = true, given above
> findings). But I guess that cannot be helped. Not unless we reintroduce
> those two bugs.

I have a huge preference for a code that has been production for
years over a new code that would cook at most two weeks in 'next'.
As I said, the part regarding the mark_as_git_dir() in the message
you are responding to was me asking you to explain, not me objecting
to the code.

> I had a look in the mail archives, and it looks as if I wanted to support
> `git clone -c core.hideDotFiles...`. I introduced a new regression test
> and verified that we no longer need to write that config setting
> explicitly.

If you are sure we do not need that, that is one less reason we
would be better off without mark_as_git_dir().  It was another way
how a $GIT_DIR creation codepath behaved differently from other
mingw_mkdir() codepath in the patch.

Having said that, I actually was leaning towards an opinion that it
might actually be better to have mark_as_git_dir() there.  Platforms
may need to do other things in their implementation of the function
to tweak things inside $GIT_DIR, just like you had to have a place
to add configuration variables and mark_as_git_dir() was the perfect
place for it.

But mark_as_git_dir() is not a generic enough name to express its
purpose.  It wasn't even when all it did was to see the
HIDE_DOTFILES configuration and hide it (you are not marking it, in
the sense that after you return, you cannot tell which directories
are "marked" as "git_dir" by only looking at the resulting
filesystem entities), and as an all-purpose place to hook platform
specific tweaks, it is even less about "marking it as a $GIT_DIR".

A name that hints the fact that it is a place to do a platform
specific extra preparation of $GIT_DIR would be more appropriate.

So given the knowledge that

 - I am not fundamentally opposed to having an extra call there;
 - in fact, I suspect it may even be a good thing to have one;
 - I am not entirely happy with the name mark_as_git_dir; and
 - the rewrite to lose that call was more painful than anticipated.

would you still choose to lose the extra call and go with
!stricmp(basename(path), ".git")?  The best approach for v2 might be
to

 - Keep the two bugfixes that was uncovered during this exercise;
 - keep the change to init_db() to add a call to mark_as_git_dir();
 - optionally, come up with a better name for that function; and
 - drop the setting of configuration varaibles that was unnecessary.

That is what I think, with the new knowledge I learned from your
message.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 33/33] lock_ref_sha1_basic(): only handle REF_NODEREF mode

[Michael Haggerty]

Now lock_ref_sha1_basic() is only called with flags==REF_NODEREF. So we
don't have to handle other cases anymore.

This enables several simplifications, the most interesting of which come
from the fact that ref_lock::orig_ref_name is now always the same as
ref_lock::ref_name:

* Remove ref_lock::orig_ref_name
* Remove local variable orig_refname from lock_ref_sha1_basic()
* ref_name can be initialize once and its value reused
* commit_ref_update() never has to write to the reflog for
  lock->orig_ref_name

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 54 +++-
 1 file changed, 20 insertions(+), 34 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index a180b9e..71e068b 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -7,7 +7,6 @@
 
 struct ref_lock {
char *ref_name;
-   char *orig_ref_name;
struct lock_file *lk;
struct object_id old_oid;
 };
@@ -1522,7 +1521,6 @@ static void unlock_ref(struct ref_lock *lock)
if (lock->lk)
rollback_lock_file(lock->lk);
free(lock->ref_name);
-   free(lock->orig_ref_name);
free(lock);
 }
 
@@ -1576,7 +1574,6 @@ static int lock_raw_ref(const char *refname, int 
mustexist,
*lock_p = lock = xcalloc(1, sizeof(*lock));
 
lock->ref_name = xstrdup(refname);
-   lock->orig_ref_name = xstrdup(refname);
strbuf_git_path(_file, "%s", refname);
 
 retry:
@@ -1964,14 +1961,13 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
struct strbuf *err)
 {
struct strbuf ref_file = STRBUF_INIT;
-   struct strbuf orig_ref_file = STRBUF_INIT;
-   const char *orig_refname = refname;
struct ref_lock *lock;
int last_errno = 0;
-   int lflags = 0;
+   int lflags = LOCK_NO_DEREF;
int mustexist = (old_sha1 && !is_null_sha1(old_sha1));
-   int resolve_flags = 0;
+   int resolve_flags = RESOLVE_REF_NO_RECURSE;
int attempts_remaining = 3;
+   int resolved;
 
assert(err);
 
@@ -1981,46 +1977,39 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
resolve_flags |= RESOLVE_REF_READING;
if (flags & REF_DELETING)
resolve_flags |= RESOLVE_REF_ALLOW_BAD_NAME;
-   if (flags & REF_NODEREF) {
-   resolve_flags |= RESOLVE_REF_NO_RECURSE;
-   lflags |= LOCK_NO_DEREF;
-   }
 
-   refname = resolve_ref_unsafe(refname, resolve_flags,
-lock->old_oid.hash, type);
-   if (!refname && errno == EISDIR) {
+   strbuf_git_path(_file, "%s", refname);
+   resolved = !!resolve_ref_unsafe(refname, resolve_flags,
+   lock->old_oid.hash, type);
+   if (!resolved && errno == EISDIR) {
/*
 * we are trying to lock foo but we used to
 * have foo/bar which now does not exist;
 * it is normal for the empty directory 'foo'
 * to remain.
 */
-   strbuf_git_path(_ref_file, "%s", orig_refname);
-   if (remove_empty_directories(_ref_file)) {
+   if (remove_empty_directories(_file)) {
last_errno = errno;
-   if (!verify_refname_available_dir(orig_refname, extras, 
skip,
+   if (!verify_refname_available_dir(refname, extras, skip,
  
get_loose_refs(_cache), err))
strbuf_addf(err, "there are still refs under 
'%s'",
-   orig_refname);
+   refname);
goto error_return;
}
-   refname = resolve_ref_unsafe(orig_refname, resolve_flags,
-lock->old_oid.hash, type);
+   resolved = !!resolve_ref_unsafe(refname, resolve_flags,
+   lock->old_oid.hash, type);
}
-   if (!refname) {
+   if (!resolved) {
last_errno = errno;
if (last_errno != ENOTDIR ||
-   !verify_refname_available_dir(orig_refname, extras, skip,
+   !verify_refname_available_dir(refname, extras, skip,
  get_loose_refs(_cache), 
err))
strbuf_addf(err, "unable to resolve reference '%s': %s",
-   orig_refname, strerror(last_errno));
+   refname, strerror(last_errno));
 
goto error_return;
}
 
-   if (flags & REF_NODEREF)
-   refname = orig_refname;
-
/*
 * If the ref did not exist and we are creating 

[PATCH v2 22/33] refs: allow log-only updates

[Michael Haggerty]

From: David Turner 

The refs infrastructure learns about log-only ref updates, which only
update the reflog.  Later, we will use this to separate symbolic
reference resolution from ref updating.

Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 16 ++--
 refs/refs-internal.h |  7 +++
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index f4f7953..3f546d0 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2683,7 +2683,7 @@ static int commit_ref_update(struct ref_lock *lock,
}
}
}
-   if (commit_ref(lock)) {
+   if (!(flags & REF_LOG_ONLY) && commit_ref(lock)) {
strbuf_addf(err, "couldn't set '%s'", lock->ref_name);
unlock_ref(lock);
return -1;
@@ -3101,7 +3101,8 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
goto cleanup;
}
if ((update->flags & REF_HAVE_NEW) &&
-   !(update->flags & REF_DELETING)) {
+   !(update->flags & REF_DELETING) &&
+   !(update->flags & REF_LOG_ONLY)) {
int overwriting_symref = ((update->type & REF_ISSYMREF) 
&&
  (update->flags & 
REF_NODEREF));
 
@@ -3133,8 +3134,9 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
}
if (!(update->flags & REF_NEEDS_COMMIT)) {
/*
-* We didn't have to write anything to the lockfile.
-* Close it to free up the file descriptor:
+* We didn't call write_ref_to_lockfile(), so
+* the lockfile is still open. Close it to
+* free up the file descriptor:
 */
if (close_ref(update->lock)) {
strbuf_addf(err, "couldn't close '%s.lock'",
@@ -3149,7 +3151,8 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
for (i = 0; i < transaction->nr; i++) {
struct ref_update *update = updates[i];
 
-   if (update->flags & REF_NEEDS_COMMIT) {
+   if (update->flags & REF_NEEDS_COMMIT ||
+   update->flags & REF_LOG_ONLY) {
if (commit_ref_update(update->lock,
  update->new_sha1, update->msg,
  update->flags, err)) {
@@ -3168,7 +3171,8 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
for (i = 0; i < transaction->nr; i++) {
struct ref_update *update = updates[i];
 
-   if (update->flags & REF_DELETING) {
+   if (update->flags & REF_DELETING &&
+   !(update->flags & REF_LOG_ONLY)) {
if (delete_ref_loose(update->lock, update->type, err)) {
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 1f94f7a..85f4650 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -43,6 +43,13 @@
  */
 
 /*
+ * Used as a flag in ref_update::flags when we want to log a ref
+ * update but not actually perform it.  This is used when a symbolic
+ * ref update is split up.
+ */
+#define REF_LOG_ONLY 0x80
+
+/*
  * Return true iff refname is minimally safe. "Safe" here means that
  * deleting a loose reference by this name will not do any damage, for
  * example by causing a file that is not a reference to be deleted.
-- 
2.8.1

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

[PATCH v2 20/33] ref_transaction_commit(): correctly report close_ref() failure

[Michael Haggerty]

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 7cc680a..f4f7953 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3139,6 +3139,7 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
if (close_ref(update->lock)) {
strbuf_addf(err, "couldn't close '%s.lock'",
update->refname);
+   ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
}
}
-- 
2.8.1

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

[PATCH v2 09/33] commit_ref_update(): write error message to *err, not stderr

[Michael Haggerty]

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 0cc116d..2d3a8c6 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2719,7 +2719,7 @@ static int commit_ref_update(struct ref_lock *lock,
}
}
if (commit_ref(lock)) {
-   error("Couldn't set %s", lock->ref_name);
+   strbuf_addf(err, "Couldn't set %s", lock->ref_name);
unlock_ref(lock);
return -1;
}
-- 
2.8.1

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

[PATCH v2 18/33] refs: make error messages more consistent

[Michael Haggerty]

* Always start error messages with a lower-case letter.

* Always enclose reference names in single quotes.

Signed-off-by: Michael Haggerty 
---
 refs.c|  8 
 refs/files-backend.c  | 32 
 t/t1400-update-ref.sh |  4 ++--
 3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/refs.c b/refs.c
index b18d995..ba14105 100644
--- a/refs.c
+++ b/refs.c
@@ -504,7 +504,7 @@ static int write_pseudoref(const char *pseudoref, const 
unsigned char *sha1,
filename = git_path("%s", pseudoref);
fd = hold_lock_file_for_update(, filename, LOCK_DIE_ON_ERROR);
if (fd < 0) {
-   strbuf_addf(err, "Could not open '%s' for writing: %s",
+   strbuf_addf(err, "could not open '%s' for writing: %s",
filename, strerror(errno));
return -1;
}
@@ -515,14 +515,14 @@ static int write_pseudoref(const char *pseudoref, const 
unsigned char *sha1,
if (read_ref(pseudoref, actual_old_sha1))
die("could not read ref '%s'", pseudoref);
if (hashcmp(actual_old_sha1, old_sha1)) {
-   strbuf_addf(err, "Unexpected sha1 when writing %s", 
pseudoref);
+   strbuf_addf(err, "unexpected sha1 when writing '%s'", 
pseudoref);
rollback_lock_file();
goto done;
}
}
 
if (write_in_full(fd, buf.buf, buf.len) != buf.len) {
-   strbuf_addf(err, "Could not write to '%s'", filename);
+   strbuf_addf(err, "could not write to '%s'", filename);
rollback_lock_file();
goto done;
}
@@ -792,7 +792,7 @@ int ref_transaction_update(struct ref_transaction 
*transaction,
 
if (new_sha1 && !is_null_sha1(new_sha1) &&
check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
-   strbuf_addf(err, "refusing to update ref with bad name %s",
+   strbuf_addf(err, "refusing to update ref with bad name '%s'",
refname);
return -1;
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 97377c7..5a597bb 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1701,7 +1701,7 @@ static int verify_lock(struct ref_lock *lock,
  lock->old_oid.hash, NULL)) {
if (old_sha1) {
int save_errno = errno;
-   strbuf_addf(err, "can't verify ref %s", lock->ref_name);
+   strbuf_addf(err, "can't verify ref '%s'", 
lock->ref_name);
errno = save_errno;
return -1;
} else {
@@ -1710,7 +1710,7 @@ static int verify_lock(struct ref_lock *lock,
}
}
if (old_sha1 && hashcmp(lock->old_oid.hash, old_sha1)) {
-   strbuf_addf(err, "ref %s is at %s but expected %s",
+   strbuf_addf(err, "ref '%s' is at %s but expected %s",
lock->ref_name,
sha1_to_hex(lock->old_oid.hash),
sha1_to_hex(old_sha1));
@@ -1790,7 +1790,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
if (last_errno != ENOTDIR ||
!verify_refname_available_dir(orig_refname, extras, skip,
  get_loose_refs(_cache), 
err))
-   strbuf_addf(err, "unable to resolve reference %s: %s",
+   strbuf_addf(err, "unable to resolve reference '%s': %s",
orig_refname, strerror(last_errno));
 
goto error_return;
@@ -1828,7 +1828,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
/* fall through */
default:
last_errno = errno;
-   strbuf_addf(err, "unable to create directory for %s",
+   strbuf_addf(err, "unable to create directory for '%s'",
ref_file.buf);
goto error_return;
}
@@ -2473,7 +2473,7 @@ static int log_ref_setup(const char *refname, struct 
strbuf *logfile, struct str
strbuf_git_path(logfile, "logs/%s", refname);
if (force_create || should_autocreate_reflog(refname)) {
if (safe_create_leading_directories(logfile->buf) < 0) {
-   strbuf_addf(err, "unable to create directory for %s: "
+   strbuf_addf(err, "unable to create directory for '%s': "
"%s", logfile->buf, strerror(errno));
return -1;
}
@@ -2487,7 +2487,7 @@ static int log_ref_setup(const char *refname, struct 
strbuf *logfile, struct str
 
if (errno == EISDIR) {
if 

[PATCH v2 30/33] lock_ref_for_update(): don't re-read non-symbolic references

[Michael Haggerty]

Before the previous patch, our first read of the reference happened
before the reference was locked, so we couldn't trust its value and had
to read it again. But now that our first read of the reference happens
after acquiring the lock, there is no need to read it a second time. So
move the read_ref_full() call into the (update->type & REF_ISSYMREF)
block.

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 52 
 1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 799866f..a9066ba 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3430,33 +3430,45 @@ static int lock_ref_for_update(struct ref_update 
*update,
 
lock = update->lock;
 
-   if (read_ref_full(update->refname,
- mustexist ? RESOLVE_REF_READING : 0,
- lock->old_oid.hash, NULL)) {
-   if (update->flags & REF_HAVE_OLD) {
-   strbuf_addf(err, "cannot lock ref '%s': can't resolve 
old value",
-   update->refname);
-   return TRANSACTION_GENERIC_ERROR;
-   } else {
-   hashclr(lock->old_oid.hash);
-   }
-   }
-   if ((update->flags & REF_HAVE_OLD) &&
-   hashcmp(lock->old_oid.hash, update->old_sha1)) {
-   strbuf_addf(err, "cannot lock ref '%s': is at %s but expected 
%s",
-   update->refname,
-   sha1_to_hex(lock->old_oid.hash),
-   sha1_to_hex(update->old_sha1));
-   return TRANSACTION_GENERIC_ERROR;
-   }
-
if (update->type & REF_ISSYMREF) {
+   if (read_ref_full(update->refname,
+ mustexist ? RESOLVE_REF_READING : 0,
+ lock->old_oid.hash, NULL)) {
+   if (update->flags & REF_HAVE_OLD) {
+   strbuf_addf(err, "cannot lock ref '%s': can't 
resolve old value",
+   update->refname);
+   return TRANSACTION_GENERIC_ERROR;
+   } else {
+   hashclr(lock->old_oid.hash);
+   }
+   }
+   if ((update->flags & REF_HAVE_OLD) &&
+   hashcmp(lock->old_oid.hash, update->old_sha1)) {
+   strbuf_addf(err, "cannot lock ref '%s': is at %s but 
expected %s",
+   update->refname,
+   sha1_to_hex(lock->old_oid.hash),
+   sha1_to_hex(update->old_sha1));
+   return TRANSACTION_GENERIC_ERROR;
+   }
+
if (!(update->flags & REF_NODEREF)) {
ret = split_symref_update(update, referent.buf, 
transaction,
  affected_refnames, err);
if (ret)
return ret;
}
+   } else if ((update->flags & REF_HAVE_OLD) &&
+  hashcmp(lock->old_oid.hash, update->old_sha1)) {
+   if (is_null_sha1(update->old_sha1))
+   strbuf_addf(err, "cannot lock ref '%s': reference 
already exists",
+   update->refname);
+   else
+   strbuf_addf(err, "cannot lock ref '%s': is at %s but 
expected %s",
+   update->refname,
+   sha1_to_hex(lock->old_oid.hash),
+   sha1_to_hex(update->old_sha1));
+
+   return TRANSACTION_GENERIC_ERROR;
}
 
if ((update->flags & REF_HAVE_NEW) &&
-- 
2.8.1

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

[PATCH v2 11/33] ref_transaction_commit(): remove local variable n

[Michael Haggerty]

This microoptimization doesn't make a significant difference in speed.
And it causes problems if somebody ever wants to modify the function to
add updates to a transaction as part of processing it, as will happen
shortly.

Make the same change in initial_ref_transaction_commit().

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 20 +---
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 80d346f..93a94af 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3076,7 +3076,6 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
   struct strbuf *err)
 {
int ret = 0, i;
-   int n = transaction->nr;
struct ref_update **updates = transaction->updates;
struct string_list refs_to_delete = STRING_LIST_INIT_NODUP;
struct string_list_item *ref_to_delete;
@@ -3087,13 +3086,13 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
if (transaction->state != REF_TRANSACTION_OPEN)
die("BUG: commit called for transaction that is not open");
 
-   if (!n) {
+   if (!transaction->nr) {
transaction->state = REF_TRANSACTION_CLOSED;
return 0;
}
 
/* Fail if a refname appears more than once in the transaction: */
-   for (i = 0; i < n; i++)
+   for (i = 0; i < transaction->nr; i++)
string_list_append(_refnames, updates[i]->refname);
string_list_sort(_refnames);
if (ref_update_reject_duplicates(_refnames, err)) {
@@ -3107,7 +3106,7 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
 * lockfiles, ready to be activated. Only keep one lockfile
 * open at a time to avoid running out of file descriptors.
 */
-   for (i = 0; i < n; i++) {
+   for (i = 0; i < transaction->nr; i++) {
struct ref_update *update = updates[i];
 
if ((update->flags & REF_HAVE_NEW) &&
@@ -3178,7 +3177,7 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
}
 
/* Perform updates first so live commits remain referenced */
-   for (i = 0; i < n; i++) {
+   for (i = 0; i < transaction->nr; i++) {
struct ref_update *update = updates[i];
 
if (update->flags & REF_NEEDS_COMMIT) {
@@ -3197,7 +3196,7 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
}
 
/* Perform deletes now that updates are safely completed */
-   for (i = 0; i < n; i++) {
+   for (i = 0; i < transaction->nr; i++) {
struct ref_update *update = updates[i];
 
if (update->flags & REF_DELETING) {
@@ -3223,7 +3222,7 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
 cleanup:
transaction->state = REF_TRANSACTION_CLOSED;
 
-   for (i = 0; i < n; i++)
+   for (i = 0; i < transaction->nr; i++)
if (updates[i]->lock)
unlock_ref(updates[i]->lock);
string_list_clear(_to_delete, 0);
@@ -3243,7 +3242,6 @@ int initial_ref_transaction_commit(struct ref_transaction 
*transaction,
   struct strbuf *err)
 {
int ret = 0, i;
-   int n = transaction->nr;
struct ref_update **updates = transaction->updates;
struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
 
@@ -3253,7 +3251,7 @@ int initial_ref_transaction_commit(struct ref_transaction 
*transaction,
die("BUG: commit called for transaction that is not open");
 
/* Fail if a refname appears more than once in the transaction: */
-   for (i = 0; i < n; i++)
+   for (i = 0; i < transaction->nr; i++)
string_list_append(_refnames, updates[i]->refname);
string_list_sort(_refnames);
if (ref_update_reject_duplicates(_refnames, err)) {
@@ -3276,7 +3274,7 @@ int initial_ref_transaction_commit(struct ref_transaction 
*transaction,
if (for_each_rawref(ref_present, _refnames))
die("BUG: initial ref transaction called with existing refs");
 
-   for (i = 0; i < n; i++) {
+   for (i = 0; i < transaction->nr; i++) {
struct ref_update *update = updates[i];
 
if ((update->flags & REF_HAVE_OLD) &&
@@ -3297,7 +3295,7 @@ int initial_ref_transaction_commit(struct ref_transaction 
*transaction,
goto cleanup;
}
 
-   for (i = 0; i < n; i++) {
+   for (i = 0; i < transaction->nr; i++) {
struct ref_update *update = updates[i];
 
if ((update->flags & REF_HAVE_NEW) &&
-- 
2.8.1

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

[PATCH v2 19/33] ref_transaction_create(): disallow recursive pruning

[Michael Haggerty]

It is nonsensical (and a little bit dangerous) to use REF_ISPRUNING
without REF_NODEREF. Forbid it explicitly. Change the one REF_ISPRUNING
caller to pass REF_NODEREF too.

Signed-off-by: Michael Haggerty 
---
 refs.c   | 3 +++
 refs/files-backend.c | 2 +-
 refs/refs-internal.h | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index ba14105..5dc2473 100644
--- a/refs.c
+++ b/refs.c
@@ -790,6 +790,9 @@ int ref_transaction_update(struct ref_transaction 
*transaction,
if (transaction->state != REF_TRANSACTION_OPEN)
die("BUG: update called for transaction that is not open");
 
+   if ((flags & REF_ISPRUNING) && !(flags & REF_NODEREF))
+   die("BUG: REF_ISPRUNING set without REF_NODEREF");
+
if (new_sha1 && !is_null_sha1(new_sha1) &&
check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
strbuf_addf(err, "refusing to update ref with bad name '%s'",
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 5a597bb..7cc680a 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2087,7 +2087,7 @@ static void prune_ref(struct ref_to_prune *r)
transaction = ref_transaction_begin();
if (!transaction ||
ref_transaction_delete(transaction, r->name, r->sha1,
-  REF_ISPRUNING, NULL, ) ||
+  REF_ISPRUNING | REF_NODEREF, NULL, ) ||
ref_transaction_commit(transaction, )) {
ref_transaction_free(transaction);
error("%s", err.buf);
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index de7722e..1f94f7a 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -15,7 +15,7 @@
 
 /*
  * Used as a flag in ref_update::flags when a loose ref is being
- * pruned.
+ * pruned. This flag must only be used when REF_NODEREF is set.
  */
 #define REF_ISPRUNING  0x04
 
-- 
2.8.1

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

[PATCH v2 10/33] rename_ref(): remove unneeded local variable

[Michael Haggerty]

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 2d3a8c6..80d346f 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2360,20 +2360,17 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
struct ref_lock *lock;
struct stat loginfo;
int log = !lstat(git_path("logs/%s", oldrefname), );
-   const char *symref = NULL;
struct strbuf err = STRBUF_INIT;
 
if (log && S_ISLNK(loginfo.st_mode))
return error("reflog for %s is a symlink", oldrefname);
 
-   symref = resolve_ref_unsafe(oldrefname, RESOLVE_REF_READING,
-   orig_sha1, );
+   if (!resolve_ref_unsafe(oldrefname, RESOLVE_REF_READING, orig_sha1, 
))
+   return error("refname %s not found", oldrefname);
+
if (flag & REF_ISSYMREF)
return error("refname %s is a symbolic ref, renaming it is not 
supported",
oldrefname);
-   if (!symref)
-   return error("refname %s not found", oldrefname);
-
if (!rename_ref_available(oldrefname, newrefname))
return 1;
 
-- 
2.8.1

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

[PATCH v2 26/33] lock_ref_for_update(): new function

[Michael Haggerty]

Extract a new function, lock_ref_for_update(), from
ref_transaction_commit().

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 152 ---
 1 file changed, 85 insertions(+), 67 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index f0eef9e..1c20af5 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3051,6 +3051,88 @@ static int ref_update_reject_duplicates(struct 
string_list *refnames,
return 0;
 }
 
+/*
+ * Acquire all locks, verify old values if provided, check
+ * that new values are valid, and write new values to the
+ * lockfiles, ready to be activated. Only keep one lockfile
+ * open at a time to avoid running out of file descriptors.
+ */
+static int lock_ref_for_update(struct ref_update *update,
+  struct ref_transaction *transaction,
+  struct string_list *affected_refnames,
+  struct strbuf *err)
+{
+   int ret;
+
+   if ((update->flags & REF_HAVE_NEW) &&
+   is_null_sha1(update->new_sha1))
+   update->flags |= REF_DELETING;
+   update->lock = lock_ref_sha1_basic(
+   update->refname,
+   ((update->flags & REF_HAVE_OLD) ?
+update->old_sha1 : NULL),
+   affected_refnames, NULL,
+   update->flags,
+   >type,
+   err);
+   if (!update->lock) {
+   char *reason;
+
+   ret = (errno == ENOTDIR)
+   ? TRANSACTION_NAME_CONFLICT
+   : TRANSACTION_GENERIC_ERROR;
+   reason = strbuf_detach(err, NULL);
+   strbuf_addf(err, "cannot lock ref '%s': %s",
+   update->refname, reason);
+   free(reason);
+   return ret;
+   }
+   if ((update->flags & REF_HAVE_NEW) &&
+   !(update->flags & REF_DELETING) &&
+   !(update->flags & REF_LOG_ONLY)) {
+   int overwriting_symref = ((update->type & REF_ISSYMREF) &&
+ (update->flags & REF_NODEREF));
+
+   if (!overwriting_symref &&
+   !hashcmp(update->lock->old_oid.hash, update->new_sha1)) {
+   /*
+* The reference already has the desired
+* value, so we don't need to write it.
+*/
+   } else if (write_ref_to_lockfile(update->lock,
+update->new_sha1,
+err)) {
+   char *write_err = strbuf_detach(err, NULL);
+
+   /*
+* The lock was freed upon failure of
+* write_ref_to_lockfile():
+*/
+   update->lock = NULL;
+   strbuf_addf(err,
+   "cannot update the ref '%s': %s",
+   update->refname, write_err);
+   free(write_err);
+   return TRANSACTION_GENERIC_ERROR;
+   } else {
+   update->flags |= REF_NEEDS_COMMIT;
+   }
+   }
+   if (!(update->flags & REF_NEEDS_COMMIT)) {
+   /*
+* We didn't call write_ref_to_lockfile(), so
+* the lockfile is still open. Close it to
+* free up the file descriptor:
+*/
+   if (close_ref(update->lock)) {
+   strbuf_addf(err, "couldn't close '%s.lock'",
+   update->refname);
+   return TRANSACTION_GENERIC_ERROR;
+   }
+   }
+   return 0;
+}
+
 int ref_transaction_commit(struct ref_transaction *transaction,
   struct strbuf *err)
 {
@@ -3088,74 +3170,10 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
for (i = 0; i < transaction->nr; i++) {
struct ref_update *update = updates[i];
 
-   if ((update->flags & REF_HAVE_NEW) &&
-   is_null_sha1(update->new_sha1))
-   update->flags |= REF_DELETING;
-   update->lock = lock_ref_sha1_basic(
-   update->refname,
-   ((update->flags & REF_HAVE_OLD) ?
-update->old_sha1 : NULL),
-   _refnames, NULL,
-   update->flags,
-   >type,
-   err);
-   if (!update->lock) {
-   char *reason;
-
-   ret = (errno == ENOTDIR)
-   ? 

[PATCH v2 14/33] read_raw_ref(): rename symref argument to referent

[Michael Haggerty]

After all, it doesn't hold the symbolic reference, but rather the
reference referred to.

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 21 +++--
 refs/refs-internal.h |  2 +-
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 8ced104..fbbd48f 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1393,9 +1393,10 @@ static int resolve_missing_loose_ref(const char *refname,
  *
  * If the ref is a sha1, fill in sha1 and return 0.
  *
- * If the ref is symbolic, fill in *symref with the referrent
- * (e.g. "refs/heads/master") and return 0.  The caller is responsible
- * for validating the referrent.  Set REF_ISSYMREF in type.
+ * If the ref is symbolic, fill in *referent with the name of the
+ * branch to which it refers (e.g. "refs/heads/master") and return 0.
+ * The caller is responsible for validating the referent. Set
+ * REF_ISSYMREF in type.
  *
  * If the ref doesn't exist, set errno to ENOENT and return -1.
  *
@@ -1411,15 +1412,15 @@ static int resolve_missing_loose_ref(const char 
*refname,
  *
  * sb_path is workspace: the caller should allocate and free it.
  *
- * It is OK for refname to point into symref. In this case:
- * - if the function succeeds with REF_ISSYMREF, symref will be
+ * It is OK for refname to point into referent. In this case:
+ * - if the function succeeds with REF_ISSYMREF, referent will be
  *   overwritten and the memory pointed to by refname might be changed
  *   or even freed.
- * - in all other cases, symref will be untouched, and therefore
+ * - in all other cases, referent will be untouched, and therefore
  *   refname will still be valid and unchanged.
  */
 int read_raw_ref(const char *refname, unsigned char *sha1,
-struct strbuf *symref, unsigned int *type)
+struct strbuf *referent, unsigned int *type)
 {
struct strbuf sb_contents = STRBUF_INIT;
struct strbuf sb_path = STRBUF_INIT;
@@ -1469,7 +1470,7 @@ stat_ref:
}
if (starts_with(sb_contents.buf, "refs/") &&
!check_refname_format(sb_contents.buf, 0)) {
-   strbuf_swap(_contents, symref);
+   strbuf_swap(_contents, referent);
*type |= REF_ISSYMREF;
ret = 0;
goto out;
@@ -1518,8 +1519,8 @@ stat_ref:
while (isspace(*buf))
buf++;
 
-   strbuf_reset(symref);
-   strbuf_addstr(symref, buf);
+   strbuf_reset(referent);
+   strbuf_addstr(referent, buf);
*type |= REF_ISSYMREF;
ret = 0;
goto out;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 0b047f6..37a1a37 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -210,6 +210,6 @@ int do_for_each_ref(const char *submodule, const char *base,
each_ref_fn fn, int trim, int flags, void *cb_data);
 
 int read_raw_ref(const char *refname, unsigned char *sha1,
-struct strbuf *symref, unsigned int *type);
+struct strbuf *referent, unsigned int *type);
 
 #endif /* REFS_REFS_INTERNAL_H */
-- 
2.8.1

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

[PATCH v2 16/33] read_raw_ref(): move docstring to header file

[Michael Haggerty]

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 38 --
 refs/refs-internal.h | 38 ++
 2 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 73717dd..fc0c7c1 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1388,44 +1388,6 @@ static int resolve_missing_loose_ref(const char *refname,
return -1;
 }
 
-/*
- * Read the specified reference from the filesystem or packed refs
- * file, non-recursively. Set type to describe the reference, and:
- *
- * - If refname is the name of a normal reference, fill in sha1
- *   (leaving referent unchanged).
- *
- * - If refname is the name of a symbolic reference, write the full
- *   name of the reference to which it refers (e.g.
- *   "refs/heads/master") to referent and set the REF_ISSYMREF bit in
- *   type (leaving sha1 unchanged). The caller is responsible for
- *   validating that referent is a valid reference name.
- *
- * WARNING: refname might be used as part of a filename, so it is
- * important from a security standpoint that it be safe in the sense
- * of refname_is_safe(). Moreover, for symrefs this function sets
- * referent to whatever the repository says, which might not be a
- * properly-formatted or even safe reference name. NEITHER INPUT NOR
- * OUTPUT REFERENCE NAMES ARE VALIDATED WITHIN THIS FUNCTION.
- *
- * Return 0 on success. If the ref doesn't exist, set errno to ENOENT
- * and return -1. If the ref exists but is neither a symbolic ref nor
- * a sha1, it is broken; set REF_ISBROKEN in type, set errno to
- * EINVAL, and return -1. If there is another error reading the ref,
- * set errno appropriately and return -1.
- *
- * Backend-specific flags might be set in type as well, regardless of
- * outcome.
- *
- * It is OK for refname to point into referent. If so:
- *
- * - if the function succeeds with REF_ISSYMREF, referent will be
- *   overwritten and the memory formerly pointed to by it might be
- *   changed or even freed.
- *
- * - in all other cases, referent will be untouched, and therefore
- *   refname will still be valid and unchanged.
- */
 int read_raw_ref(const char *refname, unsigned char *sha1,
 struct strbuf *referent, unsigned int *type)
 {
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 37a1a37..de7722e 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -209,6 +209,44 @@ int rename_ref_available(const char *oldname, const char 
*newname);
 int do_for_each_ref(const char *submodule, const char *base,
each_ref_fn fn, int trim, int flags, void *cb_data);
 
+/*
+ * Read the specified reference from the filesystem or packed refs
+ * file, non-recursively. Set type to describe the reference, and:
+ *
+ * - If refname is the name of a normal reference, fill in sha1
+ *   (leaving referent unchanged).
+ *
+ * - If refname is the name of a symbolic reference, write the full
+ *   name of the reference to which it refers (e.g.
+ *   "refs/heads/master") to referent and set the REF_ISSYMREF bit in
+ *   type (leaving sha1 unchanged). The caller is responsible for
+ *   validating that referent is a valid reference name.
+ *
+ * WARNING: refname might be used as part of a filename, so it is
+ * important from a security standpoint that it be safe in the sense
+ * of refname_is_safe(). Moreover, for symrefs this function sets
+ * referent to whatever the repository says, which might not be a
+ * properly-formatted or even safe reference name. NEITHER INPUT NOR
+ * OUTPUT REFERENCE NAMES ARE VALIDATED WITHIN THIS FUNCTION.
+ *
+ * Return 0 on success. If the ref doesn't exist, set errno to ENOENT
+ * and return -1. If the ref exists but is neither a symbolic ref nor
+ * a sha1, it is broken; set REF_ISBROKEN in type, set errno to
+ * EINVAL, and return -1. If there is another error reading the ref,
+ * set errno appropriately and return -1.
+ *
+ * Backend-specific flags might be set in type as well, regardless of
+ * outcome.
+ *
+ * It is OK for refname to point into referent. If so:
+ *
+ * - if the function succeeds with REF_ISSYMREF, referent will be
+ *   overwritten and the memory formerly pointed to by it might be
+ *   changed or even freed.
+ *
+ * - in all other cases, referent will be untouched, and therefore
+ *   refname will still be valid and unchanged.
+ */
 int read_raw_ref(const char *refname, unsigned char *sha1,
 struct strbuf *referent, unsigned int *type);
 
-- 
2.8.1

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

[PATCH v2 21/33] delete_branches(): use resolve_refdup()

[Michael Haggerty]

The return value of resolve_ref_unsafe() is not guaranteed to stay
around as long as we need it, so use resolve_refdup() instead.

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

diff --git a/builtin/branch.c b/builtin/branch.c
index 0adba62..ae55688 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -212,7 +212,7 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
die(_("Couldn't look up commit object for HEAD"));
}
for (i = 0; i < argc; i++, strbuf_release()) {
-   const char *target;
+   char *target = NULL;
int flags = 0;
 
strbuf_branchname(, argv[i]);
@@ -231,11 +231,11 @@ static int delete_branches(int argc, const char **argv, 
int force, int kinds,
}
}
 
-   target = resolve_ref_unsafe(name,
-   RESOLVE_REF_READING
-   | RESOLVE_REF_NO_RECURSE
-   | RESOLVE_REF_ALLOW_BAD_NAME,
-   sha1, );
+   target = resolve_refdup(name,
+   RESOLVE_REF_READING
+   | RESOLVE_REF_NO_RECURSE
+   | RESOLVE_REF_ALLOW_BAD_NAME,
+   sha1, );
if (!target) {
error(remote_branch
  ? _("remote-tracking branch '%s' not found.")
@@ -248,7 +248,7 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
check_branch_commit(bname.buf, name, sha1, head_rev, kinds,
force)) {
ret = 1;
-   continue;
+   goto next;
}
 
if (delete_ref(name, is_null_sha1(sha1) ? NULL : sha1,
@@ -258,7 +258,7 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
  : _("Error deleting branch '%s'"),
  bname.buf);
ret = 1;
-   continue;
+   goto next;
}
if (!quiet) {
printf(remote_branch
@@ -270,6 +270,9 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
   : find_unique_abbrev(sha1, DEFAULT_ABBREV));
}
delete_branch_config(bname.buf);
+
+   next:
+   free(target);
}
 
free(name);
-- 
2.8.1

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

[PATCH v2 02/33] commit_ref(): if there is an empty dir in the way, delete it

[Michael Haggerty]

Part of the bug revealed in the last commit is that resolve_ref_unsafe()
incorrectly returns EISDIR if it finds a directory in the place where it
is looking for a loose reference, even if the corresponding packed
reference exists. lock_ref_sha1_basic() notices the bogus EISDIR, and
use it as an indication that it should call remove_empty_directories()
and call resolve_ref_unsafe() again.

But resolve_ref_unsafe() shouldn't report EISDIR in this case. If we
would simply make that change, then remove_empty_directories() wouldn't
get called anymore, and the empty directory would get in the way when
commit_ref() calls commit_lock_file() to rename the lockfile into place.

So instead of relying on lock_ref_sha1_basic() to delete empty
directories, teach commit_ref(), just before calling commit_lock_file(),
to check whether a directory is in the way, and if so, try to delete it.

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 1f38076..ad9cd86 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2457,6 +2457,30 @@ static int close_ref(struct ref_lock *lock)
 
 static int commit_ref(struct ref_lock *lock)
 {
+   char *path = get_locked_file_path(lock->lk);
+   struct stat st;
+
+   if (!lstat(path, ) && S_ISDIR(st.st_mode)) {
+   /*
+* There is a directory at the path we want to rename
+* the lockfile to. Hopefully it is empty; try to
+* delete it.
+*/
+   size_t len = strlen(path);
+   struct strbuf sb_path = STRBUF_INIT;
+
+   strbuf_attach(_path, path, len, len);
+
+   /*
+* If this fails, commit_lock_file() will also fail
+* and will report the problem.
+*/
+   remove_empty_directories(_path);
+   strbuf_release(_path);
+   } else {
+   free(path);
+   }
+
if (commit_lock_file(lock->lk))
return -1;
return 0;
-- 
2.8.1

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

[PATCH v2 29/33] refs: resolve symbolic refs first

[Michael Haggerty]

Before committing ref updates, split symbolic ref updates into two
parts: an update to the underlying ref, and a log-only update to the
symbolic ref. This ensures that both references are locked correctly
during the transaction, including while their reflogs are updated.

Similarly, if the reference pointed to by HEAD is modified directly, add
a separate log-only update to HEAD, rather than leaving the job of
updating HEAD's reflog to commit_ref_update(). This change ensures that
HEAD is locked correctly while its reflog is being modified, as well as
being cheaper (HEAD only needs to be resolved once).

This makes use of a new function, lock_ref_raw(), which is analogous to
read_ref_raw(), but acquires a lock on the reference before reading it.

This change still has two problems:

* There are redundant read_ref_full() reference lookups.

* It is still possible to get incorrect reflogs for symbolic references
  if there is a concurrent update by another process, since the old_oid
  of a symref is determined before the lock on the pointed-to ref is
  held.

Both problems will soon be fixed.

Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c  | 513 +-
 refs/refs-internal.h  |  11 +-
 t/t1400-update-ref.sh |  35 
 3 files changed, 514 insertions(+), 45 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 8ff76ed..799866f 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1527,6 +1527,228 @@ static void unlock_ref(struct ref_lock *lock)
 }
 
 /*
+ * Lock refname, without following symrefs, and set *lock_p to point
+ * at a newly-allocated lock object. Fill in lock->old_oid, referent,
+ * and type similarly to read_raw_ref().
+ *
+ * The caller must verify that refname is a "safe" reference name (in
+ * the sense of refname_is_safe()) before calling this function.
+ *
+ * If the reference doesn't already exist, verify that refname doesn't
+ * have a D/F conflict with any existing references. extras and skip
+ * are passed to verify_refname_available_dir() for this check.
+ *
+ * If mustexist is not set and the reference is not found or is
+ * broken, lock the reference anyway but clear sha1.
+ *
+ * Return 0 on success. On failure, write an error message to err and
+ * return TRANSACTION_NAME_CONFLICT or TRANSACTION_GENERIC_ERROR.
+ *
+ * Implementation note: This function is basically
+ *
+ * lock reference
+ * read_raw_ref()
+ *
+ * but it includes a lot more code to
+ * - Deal with possible races with other processes
+ * - Avoid calling verify_refname_available_dir() when it can be
+ *   avoided, namely if we were successfully able to read the ref
+ * - Generate informative error messages in the case of failure
+ */
+static int lock_raw_ref(const char *refname, int mustexist,
+   const struct string_list *extras,
+   const struct string_list *skip,
+   struct ref_lock **lock_p,
+   struct strbuf *referent,
+   unsigned int *type,
+   struct strbuf *err)
+{
+   struct ref_lock *lock;
+   struct strbuf ref_file = STRBUF_INIT;
+   int attempts_remaining = 3;
+   int ret = TRANSACTION_GENERIC_ERROR;
+
+   assert(err);
+   *type = 0;
+
+   /* First lock the file so it can't change out from under us. */
+
+   *lock_p = lock = xcalloc(1, sizeof(*lock));
+
+   lock->ref_name = xstrdup(refname);
+   lock->orig_ref_name = xstrdup(refname);
+   strbuf_git_path(_file, "%s", refname);
+
+retry:
+   switch (safe_create_leading_directories(ref_file.buf)) {
+   case SCLD_OK:
+   break; /* success */
+   case SCLD_EXISTS:
+   /*
+* Suppose refname is "refs/foo/bar". We just failed
+* to create the containing directory, "refs/foo",
+* because there was a non-directory in the way. This
+* indicates a D/F conflict, probably because of
+* another reference such as "refs/foo". There is no
+* reason to expect this error to be transitory.
+*/
+   if (verify_refname_available(refname, extras, skip, err)) {
+   if (mustexist) {
+   /*
+* To the user the relevant error is
+* that the "mustexist" reference is
+* missing:
+*/
+   strbuf_reset(err);
+   strbuf_addf(err, "unable to resolve reference 
'%s'",
+   refname);
+   } else {
+   /*
+* 

[PATCH v2 32/33] commit_ref_update(): remove the flags parameter

[Michael Haggerty]

commit_ref_update() is now only called with flags=0. So remove the flags
parameter entirely.

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 08ec293..a180b9e 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2541,7 +2541,7 @@ static int write_ref_to_lockfile(struct ref_lock *lock,
 const unsigned char *sha1, struct strbuf *err);
 static int commit_ref_update(struct ref_lock *lock,
 const unsigned char *sha1, const char *logmsg,
-int flags, struct strbuf *err);
+struct strbuf *err);
 
 int rename_ref(const char *oldrefname, const char *newrefname, const char 
*logmsg)
 {
@@ -2617,7 +2617,7 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
hashcpy(lock->old_oid.hash, orig_sha1);
 
if (write_ref_to_lockfile(lock, orig_sha1, ) ||
-   commit_ref_update(lock, orig_sha1, logmsg, 0, )) {
+   commit_ref_update(lock, orig_sha1, logmsg, )) {
error("unable to write current sha1 into %s: %s", newrefname, 
err.buf);
strbuf_release();
goto rollback;
@@ -2637,7 +2637,7 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
flag = log_all_ref_updates;
log_all_ref_updates = 0;
if (write_ref_to_lockfile(lock, orig_sha1, ) ||
-   commit_ref_update(lock, orig_sha1, NULL, 0, )) {
+   commit_ref_update(lock, orig_sha1, NULL, )) {
error("unable to write current sha1 into %s: %s", oldrefname, 
err.buf);
strbuf_release();
}
@@ -2875,12 +2875,12 @@ static int write_ref_to_lockfile(struct ref_lock *lock,
  */
 static int commit_ref_update(struct ref_lock *lock,
 const unsigned char *sha1, const char *logmsg,
-int flags, struct strbuf *err)
+struct strbuf *err)
 {
clear_loose_ref_cache(_cache);
-   if (log_ref_write(lock->ref_name, lock->old_oid.hash, sha1, logmsg, 
flags, err) < 0 ||
+   if (log_ref_write(lock->ref_name, lock->old_oid.hash, sha1, logmsg, 0, 
err) < 0 ||
(strcmp(lock->ref_name, lock->orig_ref_name) &&
-log_ref_write(lock->orig_ref_name, lock->old_oid.hash, sha1, 
logmsg, flags, err) < 0)) {
+log_ref_write(lock->orig_ref_name, lock->old_oid.hash, sha1, 
logmsg, 0, err) < 0)) {
char *old_msg = strbuf_detach(err, NULL);
strbuf_addf(err, "cannot update the ref '%s': %s",
lock->ref_name, old_msg);
@@ -2916,7 +2916,7 @@ static int commit_ref_update(struct ref_lock *lock,
}
}
}
-   if (!(flags & REF_LOG_ONLY) && commit_ref(lock)) {
+   if (commit_ref(lock)) {
strbuf_addf(err, "couldn't set '%s'", lock->ref_name);
unlock_ref(lock);
return -1;
-- 
2.8.1

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

[PATCH v2 28/33] ref_transaction_update(): check refname_is_safe() at a minimum

[Michael Haggerty]

If the user has asked that a new value be set for a reference, we use
check_refname_format() to verify that the reference name satisfies all
of the rules. But in other cases, at least check that refname_is_safe().

Signed-off-by: Michael Haggerty 
---
 refs.c  | 5 +++--
 t/t1400-update-ref.sh   | 2 +-
 t/t1430-bad-ref-name.sh | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/refs.c b/refs.c
index 7c4eeb1..842c5c7 100644
--- a/refs.c
+++ b/refs.c
@@ -805,8 +805,9 @@ int ref_transaction_update(struct ref_transaction 
*transaction,
 {
assert(err);
 
-   if (new_sha1 && !is_null_sha1(new_sha1) &&
-   check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
+   if ((new_sha1 && !is_null_sha1(new_sha1)) ?
+   check_refname_format(refname, REFNAME_ALLOW_ONELEVEL) :
+   !refname_is_safe(refname)) {
strbuf_addf(err, "refusing to update ref with bad name '%s'",
refname);
return -1;
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 40b0cce..08bd8fd 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -23,7 +23,7 @@ test_expect_success setup '
 m=refs/heads/master
 n_dir=refs/heads/gu
 n=$n_dir/fixes
-outside=foo
+outside=refs/foo
 
 test_expect_success \
"create $m" \
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index 25ddab4..8937e25 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -285,7 +285,7 @@ test_expect_success 'update-ref -d cannot delete non-ref in 
.git dir' '
echo precious >expect &&
test_must_fail git update-ref -d my-private-file >output 2>error &&
test_must_be_empty output &&
-   test_i18ngrep -e "cannot lock .*: unable to resolve reference" error &&
+   test_i18ngrep -e "refusing to update ref with bad name" error &&
test_cmp expect .git/my-private-file
 '
 
-- 
2.8.1

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

[PATCH v2 17/33] lock_ref_sha1_basic(): remove unneeded local variable

[Michael Haggerty]

resolve_ref_unsafe() can cope with being called with NULL passed to its
flags argument. So lock_ref_sha1_basic() can just hand its own type
parameter through.

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index fc0c7c1..97377c7 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1738,7 +1738,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
const unsigned char *old_sha1,
const struct string_list *extras,
const struct string_list *skip,
-   unsigned int flags, int *type_p,
+   unsigned int flags, int *type,
struct strbuf *err)
 {
struct strbuf ref_file = STRBUF_INIT;
@@ -1746,7 +1746,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
const char *orig_refname = refname;
struct ref_lock *lock;
int last_errno = 0;
-   int type;
int lflags = 0;
int mustexist = (old_sha1 && !is_null_sha1(old_sha1));
int resolve_flags = 0;
@@ -1766,7 +1765,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
}
 
refname = resolve_ref_unsafe(refname, resolve_flags,
-lock->old_oid.hash, );
+lock->old_oid.hash, type);
if (!refname && errno == EISDIR) {
/*
 * we are trying to lock foo but we used to
@@ -1784,10 +1783,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
goto error_return;
}
refname = resolve_ref_unsafe(orig_refname, resolve_flags,
-lock->old_oid.hash, );
+lock->old_oid.hash, type);
}
-   if (type_p)
-   *type_p = type;
if (!refname) {
last_errno = errno;
if (last_errno != ENOTDIR ||
-- 
2.8.1

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

[PATCH v2 08/33] refname_is_safe(): insist that the refname already be normalized

[Michael Haggerty]

The reference name is going to be compared to other reference names, so
it should be in its normalized form.

Signed-off-by: Michael Haggerty 
---
 refs.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index ca0280f..b18d995 100644
--- a/refs.c
+++ b/refs.c
@@ -125,14 +125,19 @@ int refname_is_safe(const char *refname)
if (skip_prefix(refname, "refs/", )) {
char *buf;
int result;
+   size_t restlen = strlen(rest);
+
+   /* rest must not be empty, or start or end with "/" */
+   if (!restlen || *rest == '/' || rest[restlen - 1] == '/')
+   return 0;
 
/*
 * Does the refname try to escape refs/?
 * For example: refs/foo/../bar is safe but refs/foo/../../bar
 * is not.
 */
-   buf = xmallocz(strlen(rest));
-   result = !normalize_path_copy(buf, rest);
+   buf = xmallocz(restlen);
+   result = !normalize_path_copy(buf, rest) && !strcmp(buf, rest);
free(buf);
return result;
}
-- 
2.8.1

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

[PATCH v2 31/33] lock_ref_for_update(): don't resolve symrefs

[Michael Haggerty]

If a transaction includes a non-NODEREF update to a symbolic reference,
we don't have to look it up in lock_ref_for_update(). The reference will
be dereferenced anyway when the split-off update is processed.

This change requires that we store a backpointer from the split-off
update to its parent update, for two reasons:

* We still want to report the original reference name in error messages.
  So if an error occurs when checking the split-off update's old_sha1,
  walk the parent_update pointers back to find the original reference
  name, and report that one.

* We still need to write the old_sha1 of the symref to its reflog. So
  after we read the split-off update's reference value, walk the
  parent_update pointers back and fill in their old_sha1 fields.

Aside from eliminating unnecessary reads, this change fixes a
subtle (though not very serious) race condition: in the old code, the
old_sha1 of the symref was resolved before the reference that it pointed
at was locked. So it was possible that the old_sha1 value logged to the
symref's reflog could be wrong if another process changed the downstream
reference before it was locked.

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 108 +--
 refs/refs-internal.h |  17 
 2 files changed, 95 insertions(+), 30 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index a9066ba..08ec293 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3371,8 +3371,15 @@ static int split_symref_update(struct ref_update *update,
update->new_sha1, update->old_sha1,
update->msg);
 
-   /* Change the symbolic ref update to log only: */
+   new_update->parent_update = update;
+
+   /*
+* Change the symbolic ref update to log only. Also, it
+* doesn't need to check its old SHA-1 value, as that will be
+* done when new_update is processed.
+*/
update->flags |= REF_LOG_ONLY | REF_NODEREF;
+   update->flags &= ~REF_HAVE_OLD;
 
item->util = new_update;
 
@@ -3380,6 +3387,17 @@ static int split_symref_update(struct ref_update *update,
 }
 
 /*
+ * Return the refname under which update was originally requested.
+ */
+static const char *original_update_refname(struct ref_update *update)
+{
+   while (update->parent_update)
+   update = update->parent_update;
+
+   return update->refname;
+}
+
+/*
  * Prepare for carrying out update:
  * - Lock the reference referred to by update.
  * - Read the reference under lock.
@@ -3431,44 +3449,74 @@ static int lock_ref_for_update(struct ref_update 
*update,
lock = update->lock;
 
if (update->type & REF_ISSYMREF) {
-   if (read_ref_full(update->refname,
- mustexist ? RESOLVE_REF_READING : 0,
- lock->old_oid.hash, NULL)) {
-   if (update->flags & REF_HAVE_OLD) {
-   strbuf_addf(err, "cannot lock ref '%s': can't 
resolve old value",
-   update->refname);
+   if (update->flags & REF_NODEREF) {
+   /*
+* We won't be reading the referent as part of
+* the transaction, so we have to read it here
+* to record and possibly check old_sha1:
+*/
+   if (read_ref_full(update->refname,
+ mustexist ? RESOLVE_REF_READING : 0,
+ lock->old_oid.hash, NULL)) {
+   if (update->flags & REF_HAVE_OLD) {
+   strbuf_addf(err, "cannot lock ref '%s': 
"
+   "can't resolve old value",
+   update->refname);
+   return TRANSACTION_GENERIC_ERROR;
+   } else {
+   hashclr(lock->old_oid.hash);
+   }
+   }
+   if ((update->flags & REF_HAVE_OLD) &&
+   hashcmp(lock->old_oid.hash, update->old_sha1)) {
+   strbuf_addf(err, "cannot lock ref '%s': "
+   "is at %s but expected %s",
+   update->refname,
+   sha1_to_hex(lock->old_oid.hash),
+   sha1_to_hex(update->old_sha1));
return TRANSACTION_GENERIC_ERROR;
-   } else {
-   hashclr(lock->old_oid.hash);
}
-   }
-   if ((update->flags & 

[PATCH v2 25/33] add_update(): initialize the whole ref_update

[Michael Haggerty]

Change add_update() to initialize all of the fields in the new
ref_update object. Rename the function to ref_transaction_add_update(),
and increase its visibility to all of the refs-related code.

All of this makes the function more useful for other future callers.

Signed-off-by: Michael Haggerty 
---
 refs.c   | 48 ++--
 refs/refs-internal.h | 14 ++
 2 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/refs.c b/refs.c
index 5dc2473..7c4eeb1 100644
--- a/refs.c
+++ b/refs.c
@@ -766,13 +766,33 @@ void ref_transaction_free(struct ref_transaction 
*transaction)
free(transaction);
 }
 
-static struct ref_update *add_update(struct ref_transaction *transaction,
-const char *refname)
+struct ref_update *ref_transaction_add_update(
+   struct ref_transaction *transaction,
+   const char *refname, unsigned int flags,
+   const unsigned char *new_sha1,
+   const unsigned char *old_sha1,
+   const char *msg)
 {
struct ref_update *update;
+
+   if (transaction->state != REF_TRANSACTION_OPEN)
+   die("BUG: update called for transaction that is not open");
+
+   if ((flags & REF_ISPRUNING) && !(flags & REF_NODEREF))
+   die("BUG: REF_ISPRUNING set without REF_NODEREF");
+
FLEX_ALLOC_STR(update, refname, refname);
ALLOC_GROW(transaction->updates, transaction->nr + 1, 
transaction->alloc);
transaction->updates[transaction->nr++] = update;
+
+   update->flags = flags;
+
+   if (flags & REF_HAVE_NEW)
+   hashcpy(update->new_sha1, new_sha1);
+   if (flags & REF_HAVE_OLD)
+   hashcpy(update->old_sha1, old_sha1);
+   if (msg)
+   update->msg = xstrdup(msg);
return update;
 }
 
@@ -783,16 +803,8 @@ int ref_transaction_update(struct ref_transaction 
*transaction,
   unsigned int flags, const char *msg,
   struct strbuf *err)
 {
-   struct ref_update *update;
-
assert(err);
 
-   if (transaction->state != REF_TRANSACTION_OPEN)
-   die("BUG: update called for transaction that is not open");
-
-   if ((flags & REF_ISPRUNING) && !(flags & REF_NODEREF))
-   die("BUG: REF_ISPRUNING set without REF_NODEREF");
-
if (new_sha1 && !is_null_sha1(new_sha1) &&
check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
strbuf_addf(err, "refusing to update ref with bad name '%s'",
@@ -800,18 +812,10 @@ int ref_transaction_update(struct ref_transaction 
*transaction,
return -1;
}
 
-   update = add_update(transaction, refname);
-   if (new_sha1) {
-   hashcpy(update->new_sha1, new_sha1);
-   flags |= REF_HAVE_NEW;
-   }
-   if (old_sha1) {
-   hashcpy(update->old_sha1, old_sha1);
-   flags |= REF_HAVE_OLD;
-   }
-   update->flags = flags;
-   if (msg)
-   update->msg = xstrdup(msg);
+   flags |= (new_sha1 ? REF_HAVE_NEW : 0) | (old_sha1 ? REF_HAVE_OLD : 0);
+
+   ref_transaction_add_update(transaction, refname, flags,
+  new_sha1, old_sha1, msg);
return 0;
 }
 
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 9686e60..babdf27 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -158,6 +158,20 @@ struct ref_update {
 };
 
 /*
+ * Add a ref_update with the specified properties to transaction, and
+ * return a pointer to the new object. This function does not verify
+ * that refname is well-formed. new_sha1 and old_sha1 are only
+ * dereferenced if the REF_HAVE_NEW and REF_HAVE_OLD bits,
+ * respectively, are set in flags.
+ */
+struct ref_update *ref_transaction_add_update(
+   struct ref_transaction *transaction,
+   const char *refname, unsigned int flags,
+   const unsigned char *new_sha1,
+   const unsigned char *old_sha1,
+   const char *msg);
+
+/*
  * Transaction states.
  * OPEN:   The transaction is in a valid state and can accept new updates.
  * An OPEN transaction can be committed.
-- 
2.8.1

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

Re: [PATCH v16 0/7] config commit verbose

[Pranit Bauva]

[+cc:git@vger.kernel.org] Because its an interesting fact to be shared
which isn't covered elsewhere.

On Fri, May 6, 2016 at 2:53 AM, Ævar Arnfjörð Bjarmason
 wrote:
> Sending this privately since it's probably covered elsewhere. With
> this, if I set the option will "reword" in git rebase -i show me the
> patch?
>
> If so: awesome.

Yes, git rebase -i will show the diff in 'reword' if commit.verbose is
set to true or a value greater than 0.

I dug further in git-rebase--interactive.sh
I could find appearances of "git commit --amend" but I was unable to
find appearances of "COMMIT_EDITMSG". If COMMIT_EDITMSG was coming
into picture, the commit.verbose could not affect it. And that is not
the case.

I guess this would be a desirable trait for most of the consumers of
commit.verbose (like Ævar) so there would not be a need to suppress.

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

[PATCH v2 12/33] read_raw_ref(): rename flags argument to type

[Michael Haggerty]

This will hopefully reduce confusion with the "flags" arguments that are
used in many functions in this module as an input parameter to choose
how the function should operate.

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 18 +-
 refs/refs-internal.h |  2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 93a94af..fa8bbd6 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1395,18 +1395,18 @@ static int resolve_missing_loose_ref(const char 
*refname,
  *
  * If the ref is symbolic, fill in *symref with the referrent
  * (e.g. "refs/heads/master") and return 0.  The caller is responsible
- * for validating the referrent.  Set REF_ISSYMREF in flags.
+ * for validating the referrent.  Set REF_ISSYMREF in type.
  *
  * If the ref doesn't exist, set errno to ENOENT and return -1.
  *
  * If the ref exists but is neither a symbolic ref nor a sha1, it is
- * broken. Set REF_ISBROKEN in flags, set errno to EINVAL, and return
+ * broken. Set REF_ISBROKEN in type, set errno to EINVAL, and return
  * -1.
  *
  * If there is another error reading the ref, set errno appropriately and
  * return -1.
  *
- * Backend-specific flags might be set in flags as well, regardless of
+ * Backend-specific flags might be set in type as well, regardless of
  * outcome.
  *
  * sb_path is workspace: the caller should allocate and free it.
@@ -1419,7 +1419,7 @@ static int resolve_missing_loose_ref(const char *refname,
  *   refname will still be valid and unchanged.
  */
 int read_raw_ref(const char *refname, unsigned char *sha1,
-struct strbuf *symref, unsigned int *flags)
+struct strbuf *symref, unsigned int *type)
 {
struct strbuf sb_contents = STRBUF_INIT;
struct strbuf sb_path = STRBUF_INIT;
@@ -1448,7 +1448,7 @@ stat_ref:
if (lstat(path, ) < 0) {
if (errno != ENOENT)
goto out;
-   if (resolve_missing_loose_ref(refname, sha1, flags)) {
+   if (resolve_missing_loose_ref(refname, sha1, type)) {
errno = ENOENT;
goto out;
}
@@ -1469,7 +1469,7 @@ stat_ref:
if (starts_with(sb_contents.buf, "refs/") &&
!check_refname_format(sb_contents.buf, 0)) {
strbuf_swap(_contents, symref);
-   *flags |= REF_ISSYMREF;
+   *type |= REF_ISSYMREF;
ret = 0;
goto out;
}
@@ -1482,7 +1482,7 @@ stat_ref:
 * ref is supposed to be, there could still be a
 * packed ref:
 */
-   if (resolve_missing_loose_ref(refname, sha1, flags)) {
+   if (resolve_missing_loose_ref(refname, sha1, type)) {
errno = EISDIR;
goto out;
}
@@ -1519,7 +1519,7 @@ stat_ref:
 
strbuf_reset(symref);
strbuf_addstr(symref, buf);
-   *flags |= REF_ISSYMREF;
+   *type |= REF_ISSYMREF;
ret = 0;
goto out;
}
@@ -1530,7 +1530,7 @@ stat_ref:
 */
if (get_sha1_hex(buf, sha1) ||
(buf[40] != '\0' && !isspace(buf[40]))) {
-   *flags |= REF_ISBROKEN;
+   *type |= REF_ISBROKEN;
errno = EINVAL;
goto out;
}
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 3a4f634..0b047f6 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -210,6 +210,6 @@ int do_for_each_ref(const char *submodule, const char *base,
each_ref_fn fn, int trim, int flags, void *cb_data);
 
 int read_raw_ref(const char *refname, unsigned char *sha1,
-struct strbuf *symref, unsigned int *flags);
+struct strbuf *symref, unsigned int *type);
 
 #endif /* REFS_REFS_INTERNAL_H */
-- 
2.8.1

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

[PATCH v2 24/33] verify_refname_available(): adjust constness in declaration

[Michael Haggerty]

The two string_list arguments can be const.

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 4 ++--
 refs/refs-internal.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index ad53b6e..f0eef9e 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2299,8 +2299,8 @@ out:
 }
 
 int verify_refname_available(const char *newname,
-struct string_list *extras,
-struct string_list *skip,
+const struct string_list *extras,
+const struct string_list *skip,
 struct strbuf *err)
 {
struct ref_dir *packed_refs = get_packed_refs(_cache);
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 85f4650..9686e60 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -116,8 +116,8 @@ enum peel_status peel_object(const unsigned char *name, 
unsigned char *sha1);
  * extras and skip must be sorted.
  */
 int verify_refname_available(const char *newname,
-struct string_list *extras,
-struct string_list *skip,
+const struct string_list *extras,
+const struct string_list *skip,
 struct strbuf *err);
 
 /*
-- 
2.8.1

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

[PATCH v2 15/33] read_raw_ref(): improve docstring

[Michael Haggerty]

Among other things, document the (important!) requirement that input
refname be checked for safety before calling this function.

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 41 -
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index fbbd48f..73717dd 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1389,33 +1389,40 @@ static int resolve_missing_loose_ref(const char 
*refname,
 }
 
 /*
- * Read a raw ref from the filesystem or packed refs file.
+ * Read the specified reference from the filesystem or packed refs
+ * file, non-recursively. Set type to describe the reference, and:
  *
- * If the ref is a sha1, fill in sha1 and return 0.
+ * - If refname is the name of a normal reference, fill in sha1
+ *   (leaving referent unchanged).
  *
- * If the ref is symbolic, fill in *referent with the name of the
- * branch to which it refers (e.g. "refs/heads/master") and return 0.
- * The caller is responsible for validating the referent. Set
- * REF_ISSYMREF in type.
+ * - If refname is the name of a symbolic reference, write the full
+ *   name of the reference to which it refers (e.g.
+ *   "refs/heads/master") to referent and set the REF_ISSYMREF bit in
+ *   type (leaving sha1 unchanged). The caller is responsible for
+ *   validating that referent is a valid reference name.
  *
- * If the ref doesn't exist, set errno to ENOENT and return -1.
+ * WARNING: refname might be used as part of a filename, so it is
+ * important from a security standpoint that it be safe in the sense
+ * of refname_is_safe(). Moreover, for symrefs this function sets
+ * referent to whatever the repository says, which might not be a
+ * properly-formatted or even safe reference name. NEITHER INPUT NOR
+ * OUTPUT REFERENCE NAMES ARE VALIDATED WITHIN THIS FUNCTION.
  *
- * If the ref exists but is neither a symbolic ref nor a sha1, it is
- * broken. Set REF_ISBROKEN in type, set errno to EINVAL, and return
- * -1.
- *
- * If there is another error reading the ref, set errno appropriately and
- * return -1.
+ * Return 0 on success. If the ref doesn't exist, set errno to ENOENT
+ * and return -1. If the ref exists but is neither a symbolic ref nor
+ * a sha1, it is broken; set REF_ISBROKEN in type, set errno to
+ * EINVAL, and return -1. If there is another error reading the ref,
+ * set errno appropriately and return -1.
  *
  * Backend-specific flags might be set in type as well, regardless of
  * outcome.
  *
- * sb_path is workspace: the caller should allocate and free it.
+ * It is OK for refname to point into referent. If so:
  *
- * It is OK for refname to point into referent. In this case:
  * - if the function succeeds with REF_ISSYMREF, referent will be
- *   overwritten and the memory pointed to by refname might be changed
- *   or even freed.
+ *   overwritten and the memory formerly pointed to by it might be
+ *   changed or even freed.
+ *
  * - in all other cases, referent will be untouched, and therefore
  *   refname will still be valid and unchanged.
  */
-- 
2.8.1

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

[PATCH v2 13/33] read_raw_ref(): clear *type at start of function

[Michael Haggerty]

This is more convenient and less error-prone for callers.

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index fa8bbd6..8ced104 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1430,6 +1430,7 @@ int read_raw_ref(const char *refname, unsigned char *sha1,
int ret = -1;
int save_errno;
 
+   *type = 0;
strbuf_reset(_path);
strbuf_git_path(_path, "%s", refname);
path = sb_path.buf;
-- 
2.8.1

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

[PATCH v2 23/33] refs: don't dereference on rename

[Michael Haggerty]

From: David Turner 

When renaming refs, don't dereference either the origin or the destination
before renaming.

The origin does not need to be dereferenced because it is presently
forbidden to rename symbolic refs.

Not dereferencing the destination fixes a bug where renaming on top of
a broken symref would use the pointed-to ref name for the moved
reflog.

Add a test for the reflog bug.

Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 21 -
 t/t3200-branch.sh|  9 +
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 3f546d0..ad53b6e 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2333,7 +2333,8 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
if (log && S_ISLNK(loginfo.st_mode))
return error("reflog for %s is a symlink", oldrefname);
 
-   if (!resolve_ref_unsafe(oldrefname, RESOLVE_REF_READING, orig_sha1, 
))
+   if (!resolve_ref_unsafe(oldrefname, RESOLVE_REF_READING | 
RESOLVE_REF_NO_RECURSE,
+   orig_sha1, ))
return error("refname %s not found", oldrefname);
 
if (flag & REF_ISSYMREF)
@@ -2351,8 +2352,16 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
goto rollback;
}
 
-   if (!read_ref_full(newrefname, RESOLVE_REF_READING, sha1, NULL) &&
-   delete_ref(newrefname, sha1, REF_NODEREF)) {
+   /*
+* Since we are doing a shallow lookup, sha1 is not the
+* correct value to pass to delete_ref as old_sha1. But that
+* doesn't matter, because an old_sha1 check wouldn't add to
+* the safety anyway; we want to delete the reference whatever
+* its current value.
+*/
+   if (!read_ref_full(newrefname, RESOLVE_REF_READING | 
RESOLVE_REF_NO_RECURSE,
+  sha1, NULL) &&
+   delete_ref(newrefname, NULL, REF_NODEREF)) {
if (errno==EISDIR) {
struct strbuf path = STRBUF_INIT;
int result;
@@ -2376,7 +2385,8 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
 
logmoved = log;
 
-   lock = lock_ref_sha1_basic(newrefname, NULL, NULL, NULL, 0, NULL, );
+   lock = lock_ref_sha1_basic(newrefname, NULL, NULL, NULL, REF_NODEREF,
+  NULL, );
if (!lock) {
error("unable to rename '%s' to '%s': %s", oldrefname, 
newrefname, err.buf);
strbuf_release();
@@ -2394,7 +2404,8 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
return 0;
 
  rollback:
-   lock = lock_ref_sha1_basic(oldrefname, NULL, NULL, NULL, 0, NULL, );
+   lock = lock_ref_sha1_basic(oldrefname, NULL, NULL, NULL, REF_NODEREF,
+  NULL, );
if (!lock) {
error("unable to lock %s for rollback: %s", oldrefname, 
err.buf);
strbuf_release();
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index f3e3b6c..4281160 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -79,6 +79,15 @@ test_expect_success 'git branch -m dumps usage' '
test_i18ngrep "branch name required" err
 '
 
+test_expect_success 'git branch -m m broken_symref should work' '
+   test_when_finished "git branch -D broken_symref" &&
+   git branch -l m &&
+   git symbolic-ref refs/heads/broken_symref refs/heads/i_am_broken &&
+   git branch -m m broken_symref &&
+   git reflog exists refs/heads/broken_symref &&
+   test_must_fail git reflog exists refs/heads/i_am_broken
+'
+
 test_expect_success 'git branch -m m m/m should work' '
git branch -l m &&
git branch -m m m/m &&
-- 
2.8.1

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

[PATCH v2 27/33] unlock_ref(): move definition higher in the file

[Michael Haggerty]

This avoids the need for a forward declaration in the next patch.

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 1c20af5..8ff76ed 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1516,6 +1516,16 @@ out:
return ret;
 }
 
+static void unlock_ref(struct ref_lock *lock)
+{
+   /* Do not free lock->lk -- atexit() still looks at them */
+   if (lock->lk)
+   rollback_lock_file(lock->lk);
+   free(lock->ref_name);
+   free(lock->orig_ref_name);
+   free(lock);
+}
+
 /*
  * Peel the entry (if possible) and return its new peel_status.  If
  * repeel is true, re-peel the entry even if there is an old peeled
@@ -1674,16 +1684,6 @@ int do_for_each_ref(const char *submodule, const char 
*base,
return do_for_each_entry(refs, base, do_one_ref, );
 }
 
-static void unlock_ref(struct ref_lock *lock)
-{
-   /* Do not free lock->lk -- atexit() still looks at them */
-   if (lock->lk)
-   rollback_lock_file(lock->lk);
-   free(lock->ref_name);
-   free(lock->orig_ref_name);
-   free(lock);
-}
-
 /*
  * Verify that the reference locked by lock has the value old_sha1.
  * Fail if the reference doesn't exist and mustexist is set. Return 0
-- 
2.8.1

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

[PATCH v2 07/33] refname_is_safe(): don't allow the empty string

[Michael Haggerty]

Signed-off-by: Michael Haggerty 
---
 refs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 5789152..ca0280f 100644
--- a/refs.c
+++ b/refs.c
@@ -136,11 +136,12 @@ int refname_is_safe(const char *refname)
free(buf);
return result;
}
-   while (*refname) {
+
+   do {
if (!isupper(*refname) && *refname != '_')
return 0;
refname++;
-   }
+   } while (*refname);
return 1;
 }
 
-- 
2.8.1

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

[PATCH v2 00/33] Yet more preparation for reference backends

[Michael Haggerty]

Thanks to David, Junio, and Peff for their comments on v1 of this
patch series [1]. I think I have addressed all of the points that were
brought up. Plus I fixed a pre-existing bug that I noticed myself
while adding some more tests; see the first bullet point below for
more information.

Changes between v1 and v2:

* Prefixed the patch series with three new patches that demonstrate
  and fix some bugs in reference resolution that I noticed while
  inspecting this series:

  * "t1404: demonstrate a bug resolving references" -- this
demonstrates a bug that has existed since at least 2.5.0.
  * "read_raw_ref(): don't get confused by an empty directory"
  * "commit_ref(): if there is an empty dir in the way, delete it"

* Added a patch "read_raw_ref(): move docstring to header file".

* "ref_transaction_create(): disallow recursive pruning": Add a
  comment that `REF_ISPRUNING` must only be used with `REF_NODEREF`.

* "refs: don't dereference on rename": explain why we can't pass an
  `old_sha1` argument to `delete_ref()`. Inline `resolve_flags`
  constant to make the code more transparent.

* "add_update(): initialize the whole ref_update": move some more
  checks from `ref_transaction_update()` to `add_update()`.

* "refs: resolve symbolic refs first":
  * Remove unused `deleting` parameter from `lock_raw_ref()`
  * Fix a comment, add another.

* "lock_ref_sha1_basic(): only handle REF_NODEREF mode": initialize
  `ref_name` only once, as its value can be reused.

This patch series is also available from my GitHub repo [2] as branch
"split-under-lock".

[1] http://thread.gmane.org/gmane.comp.version-control.git/292772
[2] https://github.com/mhagger/git

David Turner (2):
  refs: allow log-only updates
  refs: don't dereference on rename

Michael Haggerty (31):
  t1404: demonstrate a bug resolving references
  commit_ref(): if there is an empty dir in the way, delete it
  read_raw_ref(): don't get confused by an empty directory
  safe_create_leading_directories(): improve docstring
  remove_dir_recursively(): add docstring
  refname_is_safe(): use skip_prefix()
  refname_is_safe(): don't allow the empty string
  refname_is_safe(): insist that the refname already be normalized
  commit_ref_update(): write error message to *err, not stderr
  rename_ref(): remove unneeded local variable
  ref_transaction_commit(): remove local variable n
  read_raw_ref(): rename flags argument to type
  read_raw_ref(): clear *type at start of function
  read_raw_ref(): rename symref argument to referent
  read_raw_ref(): improve docstring
  read_raw_ref(): move docstring to header file
  lock_ref_sha1_basic(): remove unneeded local variable
  refs: make error messages more consistent
  ref_transaction_create(): disallow recursive pruning
  ref_transaction_commit(): correctly report close_ref() failure
  delete_branches(): use resolve_refdup()
  verify_refname_available(): adjust constness in declaration
  add_update(): initialize the whole ref_update
  lock_ref_for_update(): new function
  unlock_ref(): move definition higher in the file
  ref_transaction_update(): check refname_is_safe() at a minimum
  refs: resolve symbolic refs first
  lock_ref_for_update(): don't re-read non-symbolic references
  lock_ref_for_update(): don't resolve symrefs
  commit_ref_update(): remove the flags parameter
  lock_ref_sha1_basic(): only handle REF_NODEREF mode

 builtin/branch.c   |  19 +-
 cache.h|   5 +
 dir.h  |  23 +
 refs.c |  96 ++--
 refs/files-backend.c   | 909 -
 refs/refs-internal.h   |  95 +++-
 t/t1400-update-ref.sh  |  41 +-
 t/t1404-update-ref-df-conflicts.sh |  76 +++-
 t/t1430-bad-ref-name.sh|   2 +-
 t/t3200-branch.sh  |   9 +
 10 files changed, 1013 insertions(+), 262 deletions(-)

-- 
2.8.1

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

[PATCH v2 03/33] read_raw_ref(): don't get confused by an empty directory

[Michael Haggerty]

Even if there is an empty directory where we look for the loose version
of a reference, check for a packed reference before giving up. This
fixes the failing test that was introduced two commits ago.

Signed-off-by: Michael Haggerty 
---
 refs/files-backend.c   | 11 ++-
 t/t1404-update-ref-df-conflicts.sh |  2 +-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index ad9cd86..0cc116d 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1477,7 +1477,16 @@ stat_ref:
 
/* Is it a directory? */
if (S_ISDIR(st.st_mode)) {
-   errno = EISDIR;
+   /*
+* Even though there is a directory where the loose
+* ref is supposed to be, there could still be a
+* packed ref:
+*/
+   if (resolve_missing_loose_ref(refname, sha1, flags)) {
+   errno = EISDIR;
+   goto out;
+   }
+   ret = 0;
goto out;
}
 
diff --git a/t/t1404-update-ref-df-conflicts.sh 
b/t/t1404-update-ref-df-conflicts.sh
index 16752a9..6d869d1 100755
--- a/t/t1404-update-ref-df-conflicts.sh
+++ b/t/t1404-update-ref-df-conflicts.sh
@@ -106,7 +106,7 @@ test_expect_success 'one new ref is a simple prefix of 
another' '
 
 '
 
-test_expect_failure 'empty directory should not fool rev-parse' '
+test_expect_success 'empty directory should not fool rev-parse' '
prefix=refs/e-rev-parse &&
git update-ref $prefix/foo $C &&
git pack-refs --all &&
-- 
2.8.1

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

[PATCH v2 06/33] refname_is_safe(): use skip_prefix()

[Michael Haggerty]

Signed-off-by: Michael Haggerty 
---
 refs.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/refs.c b/refs.c
index 87dc82f..5789152 100644
--- a/refs.c
+++ b/refs.c
@@ -120,17 +120,19 @@ int check_refname_format(const char *refname, int flags)
 
 int refname_is_safe(const char *refname)
 {
-   if (starts_with(refname, "refs/")) {
+   const char *rest;
+
+   if (skip_prefix(refname, "refs/", )) {
char *buf;
int result;
 
-   buf = xmallocz(strlen(refname));
/*
 * Does the refname try to escape refs/?
 * For example: refs/foo/../bar is safe but refs/foo/../../bar
 * is not.
 */
-   result = !normalize_path_copy(buf, refname + strlen("refs/"));
+   buf = xmallocz(strlen(rest));
+   result = !normalize_path_copy(buf, rest);
free(buf);
return result;
}
-- 
2.8.1

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

[PATCH v2 01/33] t1404: demonstrate a bug resolving references

[Michael Haggerty]

Add some tests checking that it is possible to work with a reference
even if there is an empty directory where the loose ref would be stored.

One of the new tests demonstrates a bug that has been with us since at
least 2.5.0--single reference lookup gives up when it sees the
directory, even if the reference exists as a packed ref. This probably
hasn't been reported before because Git usually cleans up empty
directories when packing references.

This bug will be fixed shortly.

Signed-off-by: Michael Haggerty 
---
 t/t1404-update-ref-df-conflicts.sh | 76 +-
 1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/t/t1404-update-ref-df-conflicts.sh 
b/t/t1404-update-ref-df-conflicts.sh
index 66bafb5..16752a9 100755
--- a/t/t1404-update-ref-df-conflicts.sh
+++ b/t/t1404-update-ref-df-conflicts.sh
@@ -28,7 +28,9 @@ Q="'"
 test_expect_success 'setup' '
 
git commit --allow-empty -m Initial &&
-   C=$(git rev-parse HEAD)
+   C=$(git rev-parse HEAD) &&
+   git commit --allow-empty -m Second &&
+   D=$(git rev-parse HEAD)
 
 '
 
@@ -104,4 +106,76 @@ test_expect_success 'one new ref is a simple prefix of 
another' '
 
 '
 
+test_expect_failure 'empty directory should not fool rev-parse' '
+   prefix=refs/e-rev-parse &&
+   git update-ref $prefix/foo $C &&
+   git pack-refs --all &&
+   mkdir -p .git/$prefix/foo/bar/baz &&
+   echo "$C" >expected &&
+   git rev-parse $prefix/foo >actual &&
+   test_cmp expected actual
+'
+
+test_expect_success 'empty directory should not fool for-each-ref' '
+   prefix=refs/e-for-each-ref &&
+   git update-ref $prefix/foo $C &&
+   git for-each-ref $prefix >expected &&
+   git pack-refs --all &&
+   mkdir -p .git/$prefix/foo/bar/baz &&
+   git for-each-ref $prefix >actual &&
+   test_cmp expected actual
+'
+
+test_expect_success 'empty directory should not fool create' '
+   prefix=refs/e-create &&
+   mkdir -p .git/$prefix/foo/bar/baz &&
+   printf "create %s $C\n" $prefix/foo |
+   git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool verify' '
+   prefix=refs/e-verify &&
+   git update-ref $prefix/foo $C &&
+   git pack-refs --all &&
+   mkdir -p .git/$prefix/foo/bar/baz &&
+   printf "verify %s $C\n" $prefix/foo |
+   git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool 1-arg update' '
+   prefix=refs/e-update-1 &&
+   git update-ref $prefix/foo $C &&
+   git pack-refs --all &&
+   mkdir -p .git/$prefix/foo/bar/baz &&
+   printf "update %s $D\n" $prefix/foo |
+   git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool 2-arg update' '
+   prefix=refs/e-update-2 &&
+   git update-ref $prefix/foo $C &&
+   git pack-refs --all &&
+   mkdir -p .git/$prefix/foo/bar/baz &&
+   printf "update %s $D $C\n" $prefix/foo |
+   git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool 0-arg delete' '
+   prefix=refs/e-delete-0 &&
+   git update-ref $prefix/foo $C &&
+   git pack-refs --all &&
+   mkdir -p .git/$prefix/foo/bar/baz &&
+   printf "delete %s\n" $prefix/foo |
+   git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool 1-arg delete' '
+   prefix=refs/e-delete-1 &&
+   git update-ref $prefix/foo $C &&
+   git pack-refs --all &&
+   mkdir -p .git/$prefix/foo/bar/baz &&
+   printf "delete %s $C\n" $prefix/foo |
+   git update-ref --stdin
+'
+
 test_done
-- 
2.8.1

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

[PATCH v2 04/33] safe_create_leading_directories(): improve docstring

[Michael Haggerty]

Document the difference between this function and
safe_create_leading_directories_const(), and that the former restores
path before returning.

Signed-off-by: Michael Haggerty 
---
 cache.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/cache.h b/cache.h
index 2711048..4134f64 100644
--- a/cache.h
+++ b/cache.h
@@ -993,6 +993,11 @@ int adjust_shared_perm(const char *path);
  * directory while we were working.  To be robust against this kind of
  * race, callers might want to try invoking the function again when it
  * returns SCLD_VANISHED.
+ *
+ * safe_create_leading_directories() temporarily changes path while it
+ * is working but restores it before returning.
+ * safe_create_leading_directories_const() doesn't modify path, even
+ * temporarily.
  */
 enum scld_error {
SCLD_OK = 0,
-- 
2.8.1

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

[PATCH v2 05/33] remove_dir_recursively(): add docstring

[Michael Haggerty]

Add a docstring for the remove_dir_recursively() function and the
REMOVE_DIR_* flags that can be passed to it.

Signed-off-by: Michael Haggerty 
---
 dir.h | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/dir.h b/dir.h
index 301b737..5f19acc 100644
--- a/dir.h
+++ b/dir.h
@@ -262,9 +262,32 @@ extern int is_empty_dir(const char *dir);
 
 extern void setup_standard_excludes(struct dir_struct *dir);
 
+
+/* Constants for remove_dir_recursively: */
+
+/*
+ * If a non-directory is found within path, stop and return an error.
+ * (In this case some empty directories might already have been
+ * removed.)
+ */
 #define REMOVE_DIR_EMPTY_ONLY 01
+
+/*
+ * If any Git work trees are found within path, skip them without
+ * considering it an error.
+ */
 #define REMOVE_DIR_KEEP_NESTED_GIT 02
+
+/* Remove the contents of path, but leave path itself. */
 #define REMOVE_DIR_KEEP_TOPLEVEL 04
+
+/*
+ * Remove path and its contents, recursively. flags is a combination
+ * of the above REMOVE_DIR_* constants. Return 0 on success.
+ *
+ * This function uses path as temporary scratch space, but restores it
+ * before returning.
+ */
 extern int remove_dir_recursively(struct strbuf *path, int flag);
 
 /* tries to remove the path with empty directories along it, ignores ENOENT */
-- 
2.8.1

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

[PATCH v3] gitk: Add Portuguese translation

[Vasco Almeida]

Signed-off-by: Vasco Almeida 
---
I think translation work is ready.  Don't expect another re-roll soon.

 po/pt_pt.po | 1376 +++
 1 file changed, 1376 insertions(+)
 create mode 100644 po/pt_pt.po

diff --git a/po/pt_pt.po b/po/pt_pt.po
new file mode 100644
index 000..e85b276
--- /dev/null
+++ b/po/pt_pt.po
@@ -0,0 +1,1376 @@
+# Portuguese translations for gitk package.
+# Copyright (C) 2016 Paul Mackerras
+# This file is distributed under the same license as the gitk package.
+# Vasco Almeida , 2016.
+msgid ""
+msgstr ""
+"Project-Id-Version: gitk\n"
+"Report-Msgid-Bugs-To: \n"
+"POT-Creation-Date: 2016-04-15 16:52+\n"
+"PO-Revision-Date: 2016-05-06 15:35+\n"
+"Last-Translator: Vasco Almeida \n"
+"Language-Team: Portuguese\n"
+"Language: pt\n"
+"MIME-Version: 1.0\n"
+"Content-Type: text/plain; charset=UTF-8\n"
+"Content-Transfer-Encoding: 8bit\n"
+"Plural-Forms: nplurals=2; plural=(n != 1);\n"
+"X-Generator: Virtaal 0.7.1\n"
+
+#: gitk:140
+msgid "Couldn't get list of unmerged files:"
+msgstr "Não foi possível obter lista de ficheiros não integrados:"
+
+#: gitk:212 gitk:2399
+msgid "Color words"
+msgstr "Colorir palavras"
+
+#: gitk:217 gitk:2399 gitk:8239 gitk:8272
+msgid "Markup words"
+msgstr "Marcar palavras"
+
+#: gitk:324
+msgid "Error parsing revisions:"
+msgstr "Erro ao analisar revisões:"
+
+#: gitk:380
+msgid "Error executing --argscmd command:"
+msgstr "Erro ao executar o comando de --argscmd:"
+
+#: gitk:393
+msgid "No files selected: --merge specified but no files are unmerged."
+msgstr ""
+"Nenhum ficheiro selecionado: --merge especificado mas não há ficheiros por "
+"integrar."
+
+#: gitk:396
+msgid ""
+"No files selected: --merge specified but no unmerged files are within file "
+"limit."
+msgstr ""
+"Nenhum ficheiro selecionado: --merge especificado mas não há ficheiros por "
+"integrar ao nível de ficheiro."
+
+#: gitk:418 gitk:566
+msgid "Error executing git log:"
+msgstr "Erro ao executar git log:"
+
+#: gitk:436 gitk:582
+msgid "Reading"
+msgstr "A ler"
+
+#: gitk:496 gitk:4544
+msgid "Reading commits..."
+msgstr "A ler commits..."
+
+#: gitk:499 gitk:1637 gitk:4547
+msgid "No commits selected"
+msgstr "Nenhum commit selecionado"
+
+#: gitk:1445 gitk:4064 gitk:12469
+msgid "Command line"
+msgstr "Linha de comandos"
+
+#: gitk:1511
+msgid "Can't parse git log output:"
+msgstr "Não é possível analisar a saída de git log:"
+
+#: gitk:1740
+msgid "No commit information available"
+msgstr "Não há informação disponível sobre o commit"
+
+#: gitk:1903 gitk:1932 gitk:4334 gitk:9702 gitk:11274 gitk:11554
+msgid "OK"
+msgstr "OK"
+
+#: gitk:1934 gitk:4336 gitk:9215 gitk:9294 gitk:9424 gitk:9473 gitk:9704
+#: gitk:11275 gitk:11555
+msgid "Cancel"
+msgstr "Cancelar"
+
+#: gitk:2083
+msgid ""
+msgstr "At"
+
+#: gitk:2084
+msgid ""
+msgstr ""
+
+#: gitk:2085
+msgid "Reread re"
+msgstr "Reler reências"
+
+#: gitk:2086
+msgid " references"
+msgstr " referências"
+
+#: gitk:2088
+msgid "Start git "
+msgstr "Iniciar git "
+
+#: gitk:2090
+msgid ""
+msgstr ""
+
+#: gitk:2082
+msgid ""
+msgstr ""
+
+#: gitk:2094
+msgid ""
+msgstr "ências"
+
+#: gitk:2093
+msgid ""
+msgstr ""
+
+#: gitk:2098
+msgid " view..."
+msgstr " vista..."
+
+#: gitk:2099
+msgid " view..."
+msgstr " vista..."
+
+#: gitk:2100
+msgid " view"
+msgstr "Elimina vista"
+
+#: gitk:2102
+msgid " files"
+msgstr " os ficheiros"
+
+#: gitk:2097
+msgid ""
+msgstr ""
+
+#: gitk:2107 gitk:2117
+msgid " gitk"
+msgstr " gitk"
+
+#: gitk:2108 gitk:2122
+msgid " bindings"
+msgstr ""
+
+#: gitk:2106 gitk:2121
+msgid ""
+msgstr ""
+
+#: gitk:2199 gitk:8671
+msgid "SHA1 ID:"
+msgstr "ID SHA1:"
+
+#: gitk:2243
+msgid "Row"
+msgstr "Linha"
+
+#: gitk:2281
+msgid "Find"
+msgstr "Procurar"
+
+#: gitk:2309
+msgid "commit"
+msgstr "commit"
+
+#: gitk:2313 gitk:2315 gitk:4706 gitk:4729 gitk:4753 gitk:6774 gitk:6846
+#: gitk:6931
+msgid "containing:"
+msgstr "contendo:"
+
+#: gitk:2316 gitk:3545 gitk:3550 gitk:4782
+msgid "touching paths:"
+msgstr "altera os caminhos:"
+
+#: gitk:2317 gitk:4796
+msgid "adding/removing string:"
+msgstr "adiciona/remove a cadeia:"
+
+#: gitk:2318 gitk:4798
+msgid "changing lines matching:"
+msgstr "altera linhas com:"
+
+#: gitk:2327 gitk:2329 gitk:4785
+msgid "Exact"
+msgstr "Exato"
+
+#: gitk:2329 gitk:4873 gitk:6742
+msgid "IgnCase"
+msgstr "IgnMaiúsculas"
+
+#: gitk:2329 gitk:4755 gitk:4871 gitk:6738
+msgid "Regexp"
+msgstr "Expr. regular"
+
+#: gitk:2331 gitk:2332 gitk:4893 gitk:4923 gitk:4930 gitk:6867 gitk:6935
+msgid "All fields"
+msgstr "Todos os campos"
+
+#: gitk:2332 gitk:4890 gitk:4923 gitk:6805
+msgid "Headline"
+msgstr "Cabeçalho"
+
+#: gitk:2333 gitk:4890 gitk:6805 gitk:6935 gitk:7408
+msgid "Comments"
+msgstr "Comentários"
+
+#: gitk:2333 gitk:4890 gitk:4895 gitk:4930 gitk:6805 gitk:7343 gitk:8849
+#: gitk:8864
+msgid "Author"
+msgstr "Autor"
+
+#: gitk:2333 gitk:4890 

[PATCH v2] git-gui: l10n: add Portuguese translation

[Vasco Almeida]

Add Portuguese glossary.

Signed-off-by: Vasco Almeida 
---
 po/glossary/pt_pt.po |  293 ++
 po/pt_pt.po  | 2716 ++
 2 files changed, 3009 insertions(+)
 create mode 100644 po/glossary/pt_pt.po
 create mode 100644 po/pt_pt.po

diff --git a/po/glossary/pt_pt.po b/po/glossary/pt_pt.po
new file mode 100644
index 000..adc3b54
--- /dev/null
+++ b/po/glossary/pt_pt.po
@@ -0,0 +1,293 @@
+# Portuguese translations for git-gui glossary.
+# Copyright (C) 2016 Shawn Pearce, et al.
+# This file is distributed under the same license as the git package.
+# Vasco Almeida , 2016.
+msgid ""
+msgstr ""
+"Project-Id-Version: git-gui glossary\n"
+"POT-Creation-Date: 2016-05-06 10:22+\n"
+"PO-Revision-Date: 2016-05-06 12:32+\n"
+"Last-Translator: Vasco Almeida \n"
+"Language-Team: Portuguese\n"
+"Language: pt\n"
+"MIME-Version: 1.0\n"
+"Content-Type: text/plain; charset=UTF-8\n"
+"Content-Transfer-Encoding: 8bit\n"
+"Plural-Forms: nplurals=2; plural=(n != 1);\n"
+"X-Generator: Virtaal 0.7.1\n"
+
+#. "English Definition (Dear translator: This file will never be visible to 
the user! It should only serve as a tool for you, the translator. Nothing 
more.)"
+msgid ""
+"English Term (Dear translator: This file will never be visible to the user!)"
+msgstr ""
+"Outro SCM em português:\n"
+"http://svn.code.sf.net/p/tortoisesvn/code/trunk/Languages/pt/TortoiseUI.po e "
+"\n"
+"http://svn.code.sf.net/p/tortoisesvn/code/trunk/Languages/pt/TortoiseDoc.po\n;
+" em html: https://tortoisesvn.net/docs/release/TortoiseSVN_pt/index.html\n;
+"\n"
+"https://translations.launchpad.net/tortoisehg (medíocre)"
+
+#. ""
+msgid "amend"
+msgstr "emendar"
+
+#. ""
+msgid "annotate"
+msgstr "anotar"
+
+#. "A 'branch' is an active line of development."
+msgid "branch [noun]"
+msgstr "ramo"
+
+#. ""
+msgid "branch [verb]"
+msgstr "criar ramo"
+
+#. ""
+msgid "checkout [noun]"
+msgstr "extração"
+
+#. "The action of updating the working tree to a revision which was stored in 
the object database."
+msgid "checkout [verb]"
+msgstr "extrair"
+
+#. ""
+msgid "clone [verb]"
+msgstr "clonar"
+
+#. "A single point in the git history."
+msgid "commit [noun]"
+msgstr "commit"
+
+#. "The action of storing a new snapshot of the project's state in the git 
history."
+msgid "commit [verb]"
+msgstr "submeter"
+
+#. ""
+msgid "diff [noun]"
+msgstr "diferenças"
+
+#. ""
+msgid "diff [verb]"
+msgstr "mostrar diferenças"
+
+#. "A fast-forward is a special type of merge where you have a revision and 
you are merging another branch's changes that happen to be a descendant of what 
you have."
+msgid "fast forward merge"
+msgstr "integração por avanço rápido"
+
+#. "Fetching a branch means to get the branch's head from a remote repository, 
to find out which objects are missing from the local object database, and to 
get them, too."
+msgid "fetch"
+msgstr "obter"
+
+#. "One context of consecutive lines in a whole patch, which consists of many 
such hunks"
+msgid "hunk"
+msgstr "excerto"
+
+#. "A collection of files. The index is a stored version of your working tree."
+msgid "index (in git-gui: staging area)"
+msgstr "índice"
+
+#. "A successful merge results in the creation of a new commit representing 
the result of the merge."
+msgid "merge [noun]"
+msgstr "integração"
+
+#. "To bring the contents of another branch into the current branch."
+msgid "merge [verb]"
+msgstr "integrar"
+
+#. ""
+msgid "message"
+msgstr "mensagem"
+
+#. "Deletes all stale tracking branches under . These stale branches 
have already been removed from the remote repository referenced by , but 
are still locally available in 'remotes/'."
+msgid "prune"
+msgstr "podar"
+
+#. "Pulling a branch means to fetch it and merge it."
+msgid "pull"
+msgstr "puxar"
+
+#. "Pushing a branch means to get the branch's head ref from a remote 
repository, and ... (well, can someone please explain it for mere mortals?)"
+msgid "push"
+msgstr "publicar"
+
+#. ""
+msgid "redo"
+msgstr "refazer"
+
+#. "An other repository ('remote'). One might have a set of remotes whose 
branches one tracks."
+msgid "remote"
+msgstr "remoto"
+
+#. "A collection of refs (?) together with an object database containing all 
objects which are reachable from the refs... (oops, you've lost me here. Again, 
please an explanation for mere mortals?)"
+msgid "repository"
+msgstr "repositório"
+
+#. ""
+msgid "reset"
+msgstr "repor"
+
+#. ""
+msgid "revert"
+msgstr "reverter"
+
+#. "A particular state of files and directories which was stored in the object 
database."
+msgid "revision"
+msgstr "revisão"
+
+#. ""
+msgid "sign off"
+msgstr "assinar por baixo"
+
+#. ""
+msgid "staging area"
+msgstr "área de estágio"
+
+#. ""
+msgid "status"
+msgstr "estado"
+
+#. "A ref pointing to a tag or commit object"
+msgid "tag [noun]"
+msgstr "tag"
+
+#. ""
+msgid "tag [verb]"
+msgstr "criar tag"
+
+#. "A regular git 

Re: [PATCH v4 01/11] add fetch-pack --diag-url tests for some corner cases

[Junio C Hamano]

Torsten Bögershausen  writes:

> ssh itself does not use a password:
> ...
> Neither does Git.
> ...
> The user:password came in here:
> Commit 92722efec01f67a54b
> clone: do not use port number as dir name
>
> Actually, looking back, it may have been better to say
> git clone ssh://:@host:/path
> is illegal and simply die() out.

RFC2396, which updated RFC1738, discourages the use of :password
in "3.2.2 Server-based Naming Authority", for obvious reasons.

   Some URL schemes use the format "user:password" in the userinfo
   field.  This practice is NOT RECOMMENDED ...

and then this is marked as deprecated in RFC3986 "3.2.1. User
Information".

   Use of the format "user:password" in the userinfo field is
   deprecated.  Applications should not render as clear text any
   data after the first colon (":") character found within a
   userinfo subcomponent unless the data after the colon is the
   empty string (indicating no password).

However, at the parser level that _knows_ the syntax, you shouldn't
be unilaterally turning these "not recommended" and "deprecated" to
"forbidden".  It should be prepared to see ':' to its input, if only
to correctly recognize that as an attempt to express :password, in
order to be able to hide the data after the first colon when running
in verbose mode for example.

I'd recommend that the parser to allow :@, and
at least notice ':' that appears before the first '@' as having a
depreated form of .  After stripping :// from the
front, it is OK to assume that everything before the first '@' is
 (in RFC2396 lingo), and everything in  that is
before the first ':' is  when doing so.  

>>> ...  When you are constrained by the Common Internet
>>> Scheme Syntax, i.e.
>>>
>>> ://:@:/
>>>
>>> you cannot have arbitrary characters in these parts; within the user
>>> and password field, any ":", "@", or "/" must be encoded.
--
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 80/83] run-command: make dup_devnull() non static

[Johannes Schindelin]

Hi Chris,

On Fri, 6 May 2016, Christian Couder wrote:

> On Thu, May 5, 2016 at 10:07 PM, Johannes Sixt  wrote:
> > Am 05.05.2016 um 11:50 schrieb Christian Couder:
> >>
> >> On Mon, Apr 25, 2016 at 5:05 PM, Johannes Schindelin
> >>  wrote:
> >>>
> >>> Hi Chris,
> >>>
> >>> On Sun, 24 Apr 2016, Christian Couder wrote:
> >>>
>  diff --git a/run-command.c b/run-command.c
>  index 8c7115a..29d2bda 100644
>  --- a/run-command.c
>  +++ b/run-command.c
>  @@ -85,7 +85,7 @@ static inline void close_pair(int fd[2])
>    }
> 
>    #ifndef GIT_WINDOWS_NATIVE
>  -static inline void dup_devnull(int to)
>  +void dup_devnull(int to)
>    {
> >>>
> >>>
> >>> The #ifndef GIT_WINDOWS_NATIVE rings very, very loud alarm bells.
> >>
> >>
> >> Yeah, but I must say that I don't know what I should do about this.
> >> Do you have a suggestion? Should I try to implement the same function
> >> for Windows?

No, you should change the code that requires that ugly dup()ing so that it
can be configured to shut up.

> > No, just remove the #ifndef brackets. There is already code in
> > compat/mingw.c that treats the file name "/dev/null" specially.
> 
> Ok, I will do that in the same patch though the "#ifndef
> GIT_WINDOWS_NATIVE" was already there before.

The idea was that compat/mingw.c is *really* only for the MINGW version,
not for the MSVC version.

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 v16 0/7] config commit verbose

[Junio C Hamano]

SZEDER Gábor  writes:

> A while ago in a related thread Peff remarked about 'git commit's
> '--quiet' and '--verbose' options:
>
>I think that is a UX mistake, and we would not do
>it that way if designing from scratch. But we're stuck with it for
>historical reasons (I'd probably name "--verbose" as "--show-diff" or
>something if writing it today).
>
> http://thread.gmane.org/gmane.comp.version-control.git/289027/focus=289069
>
> Then I replied:
>
>However, that doesn't mean that we have to spread this badly chosen
>name from options to config variables, does it?  I think that if we
>are going to define a new config variable today, then it should be
>named properly, and it's better not to call it 'commit.verbose', but
>'commit.showDiff' or something.
>
> http://thread.gmane.org/gmane.comp.version-control.git/289027/focus=289303
>
> Any thoughts on this?  Before a poorly named config variable enters to
> the codebase and we'll have to maintain it "forever"...

My thoughts are --show-diff would probably be a UI mistake of a
different sort, if you are anticipating that the different kinds of
information to be shown in verbose modes would proliferate and that
you would want to give the user flexibility to pick and choose to
use some while not using some other among them.  You would end up
having --show-xyzzy --show-frotz --show-nitfol ... options.

I am not convinced that we would want such a degree of flexibility
in the first place, but even if we did, we'd be better off giving
that as "--verbose=diff,xyzzy,frotz...", I would think.

And commit.verbose that begins its life as a simple boolean, which
can be extended to become bool-or-string if needed, is better than
having commit.showDiff, commit.showXyzzy, commit.showFrotz, etc.
--
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: Found possible branch point, then "Use of uninitialized value $u in substitution/concatenation"

[Thierry Suzanne]

Sorry, can't help :(

On 6 May 2016 at 09:27, Christian Couder  wrote:
> Hi,
>
> On Thu, May 5, 2016 at 12:43 PM, Thierry Suzanne  
> wrote:
>>
>> Looking at svn.pm, I can see we're in find_parent_branch() which
>> outputs the message "Found possible branch point". It then calls
>> other_gs($new_url, $url, $branch_from, $r, $self->{ref_id});
>>
>> which itself calls:
>> Git::SVN->find_by_url($new_url, $url, $branch_from);
>>
>> which calls:
>> resolve_local_globs($u, $fetch, $globspec);
>>
>> and resolve_local_globs is where the error is thrown on line 100/101:
>> my $u = (::cmt_metadata("$refname"))[0];
>> $u =~ s!^\Q$url\E(/|$)!! or die
>>
>> I made a mistake in my command line for sure, so this might not be a
>> bug, I'm just emailing all this if someone think it could be handled
>> differently or a better error shown.
>
> Yeah, there is a die() calls above and also one on line 101, so it
> looks like $u could be checked between line 100 and line 101 and die()
> could be called if it is not properly defined.
>
> Could you send a patch to do something like that?
>
> 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

Re: [PATCH] mingw: introduce the 'core.hideDotFiles' setting

[Johannes Schindelin]

Hi Junio,

On Wed, 4 May 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > diff --git a/builtin/init-db.c b/builtin/init-db.c
> > index b2d8d40..c4269ac 100644
> > --- a/builtin/init-db.c
> > +++ b/builtin/init-db.c
> > @@ -370,6 +370,7 @@ int init_db(const char *template_dir, unsigned int 
> > flags)
> > check_repository_format();
> >  
> > reinit = create_default_files(template_dir);
> > +   mark_as_git_dir(get_git_dir());
> >  
> > create_object_directory();
> >  
> 
> I agree with the goal of the change, but I am having a hard time
> justifying this addition.  Primarily because I do not understand the
> need for this.
> 
> In order to be prepared to handle HIDE_DOTFILES_TRUE case,
> mingw_mkdir() needs to be taught about needs_hiding() and
> make_hidden() already.  Why isn't it sufficient to teach
> needs_hiding() to check with !strcmp(basename(path, ".git"))
> under HIDE_DOTFILES_DOTGITONLY?

The reason was that I wanted to avoid to compare a name unnecessarily when
I already had a code path that knew perfectly well that a given directory
is the .git/ directory.

I made the change. It was more painful than I expected, as two bugs were
uncovered, both introduced after the original patch by Erik.

First bug: basename() on Windows used to not remove the trailing slashes.
Since we adjusted it, to conform with the POSIX specs, the implicit
mkdir() in copy_templates() could replace the trailing slash by a NUL,
breaking the template copying (it truncated pretty much all paths). I did
not catch this earlier because basename() was only called with
HIDE_DOTFILES_TRUE, and that case was never thoroughly tested.

So I had to reroll a non-modifying basename(). It's not elegant to have
this, but it is better than strdup()ing each and every path, just to
determine whether the basename starts with a dot (or is equal to ".git")
or not.

Second bug: when we switched to override open()/fopen()/freopen() using
Windows' UTF-16 functions, we lost the ability to open hidden files
(internally, _wopen() uses CreateFile(), which allows the equivalent of
O_CREAT only if the attributes match any existing files' attributes
*exactly*, and there is no way to tell _wopen() that we want to create a
hidden file).

Again, this was only exercised with HIDE_DOTFILES_TRUE until the change
proposed by you: needs_hiding() now also triggers for .git *files* in
HIDE_DOTFILES_DOTGITONLY mode.

It worries me slightly that the new code is so different from the code
that was tried and tested through all those years (although admittedly it
is unlikely anybody ever ran with core.hideDotFiles = true, given above
findings). But I guess that cannot be helped. Not unless we reintroduce
those two bugs.

> I do not understand why core.hideDotFiles needs to be explicitly
> copied to the config file in the resulting repository, either.
> 
> Once you created a repository, Git won't unhide the hidden .git of
> that reposiotry, so the idea must be to propagate the setting to a
> new repository that will further be created, but wouldn't that get
> the "please hide" preference from the same place as we have got the
> preference to hide .git when the current repository was created
> anyway?

I had a look in the mail archives, and it looks as if I wanted to support
`git clone -c core.hideDotFiles...`. I introduced a new regression test
and verified that we no longer need to write that config setting
explicitly.

I will send out v2 as soon as the test suite passed (which is hopefully
30-45 minutes from now).

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

[PATCH v5 2/2] bisect: rewrite `check_term_format` shell function in C

[Pranit Bauva]

Reimplement the `check_term_format` shell function in C and add
a `--check-term-format` subcommand to `git bisect--helper` to call it
from git-bisect.sh

Using `--check-term-format` subcommand is a temporary measure to port
shell function to C so as to use the existing test suite. As more
functions are ported, this subcommand will loose its existence and will
be called by some other method/subcommand. For eg. In conversion of
write_terms() of git-bisect.sh, the subcommand will be removed and
instead check_term_format() will be called in its C implementation while
a new subcommand will be introduced for write_terms().

Helped-by: Johannes Schindelein 
Mentored-by: Lars Schneider 
Mentored-by: Christian Couder 
Signed-off-by: Pranit Bauva 
---
 builtin/bisect--helper.c | 57 +++-
 git-bisect.sh| 31 ++
 2 files changed, 58 insertions(+), 30 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index d8de651..cc50438 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -2,16 +2,65 @@
 #include "cache.h"
 #include "parse-options.h"
 #include "bisect.h"
+#include "refs.h"
 
 static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --next-all [--no-checkout]"),
+   N_("git bisect--helper --check-term-format  "),
NULL
 };
 
 enum subcommand {
-   NEXT_ALL = 1
+   NEXT_ALL = 1,
+   CHECK_TERM_FMT
 };
 
+/*
+ * To check whether the string `term` belongs to the set of strings
+ * included in the variable arguments.
+ */
+static int one_of(const char *term, ...)
+{
+   int res = 0;
+   va_list matches;
+   const char *match;
+
+   va_start(matches, term);
+   while (!res && (match=va_arg(matches, const char *)))
+   res = !strcmp(term, match);
+   va_end(matches);
+
+   return res;
+}
+
+static int check_term_format(const char *term, const char *orig_term)
+{
+   struct strbuf new_term = STRBUF_INIT;
+   strbuf_addf(_term, "refs/bisect/%s", term);
+
+   if (check_refname_format(new_term.buf, 0)) {
+   strbuf_release(_term);
+   return error(_("'%s' is not a valid term"), term);
+   }
+   strbuf_release(_term);
+
+   if (one_of(term, "help", "start", "skip", "next", "reset",
+   "visualize", "replay", "log", "run", NULL))
+   return error(_("can't use the builtin command '%s' as a term"), 
term);
+
+   /*
+* In theory, nothing prevents swapping completely good and bad,
+* but this situation could be confusing and hasn't been tested
+* enough. Forbid it for now.
+*/
+
+   if ((strcmp(orig_term, "bad") && one_of(term, "bad", "new", NULL)) ||
+(strcmp(orig_term, "good") && one_of(term, "good", "old", 
NULL)))
+   return error(_("can't change the meaning of the term '%s'"), 
term);
+
+   return 0;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
int subcommand = 0;
@@ -19,6 +68,8 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
struct option options[] = {
OPT_CMDMODE(0, "next-all", ,
 N_("perform 'git bisect next'"), NEXT_ALL),
+   OPT_CMDMODE(0, "check-term-format", ,
+N_("check format of the term"), CHECK_TERM_FMT),
OPT_BOOL(0, "no-checkout", _checkout,
 N_("update BISECT_HEAD instead of checking out the 
current commit")),
OPT_END()
@@ -33,6 +84,10 @@ int cmd_bisect__helper(int argc, const char **argv, const 
char *prefix)
switch (subcommand) {
case NEXT_ALL:
return bisect_next_all(prefix, no_checkout);
+   case CHECK_TERM_FMT:
+   if (argc != 2)
+   die(_("--check-term-format requires two arguments"));
+   return check_term_format(argv[0], argv[1]);
default:
die("BUG: unknown subcommand '%d'", subcommand);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index 5d1cb00..7d7965d 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -564,38 +564,11 @@ write_terms () {
then
die "$(gettext "please use two different terms")"
fi
-   check_term_format "$TERM_BAD" bad
-   check_term_format "$TERM_GOOD" good
+   git bisect--helper --check-term-format "$TERM_BAD" bad || exit
+   git bisect--helper --check-term-format "$TERM_GOOD" good || exit
printf '%s\n%s\n' "$TERM_BAD" "$TERM_GOOD" >"$GIT_DIR/BISECT_TERMS"
 }
 
-check_term_format () {
-   term=$1
-   git check-ref-format refs/bisect/"$term" ||
-   die "$(eval_gettext "'\$term' is not a valid term")"
-   case "$term" in
-   

[PATCH v5 0/2] bisect--helper: rewrite of check-term-format()

[Pranit Bauva]

A very interesting fact to note:
I made a mistake by dropping of the first argument 'term' in the function
one_of() and compiled and the test suite passed fully (which was pointed
out by Christian). This shows that the test coverage is incomplete. I am
currently writing tests to improve on that coverage. It will be included
separately.

Link to v4: http://thread.gmane.org/gmane.comp.version-control.git/293488

Changes wrt v4:
 * Stick with 'subcommand' in the commit message.
 * Rename enum as 'subcommand' to make it singular.
 * s/bug/BUG/g
 * Drop _() in the default of switch case
 * Use some suggestions for commit message no. 2 and also include why I am
   using subcommand.
 * Drop of the latter part in the comment of one_of()
 * Remove flag from check_term_format()
 * Use return error() instead of die()
 * Drop '\n' from the localization strings
 * Include localization for a missed string
 * A comment in function check_term_format() is re-wrapped to fit in 80 columns
 * Use 'term' instead of 'ref'
 * Reword the string in the default of switch case
 * I have kept the commit message same as previous for commit no. 1 except for
   addressing the minor nits.

Pranit Bauva (2):
  bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
  bisect: rewrite `check_term_format` shell function in C

 builtin/bisect--helper.c | 76 
 git-bisect.sh| 31 ++--
 2 files changed, 72 insertions(+), 35 deletions(-)

-- 
2.8.1

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

Re: [PATCH v2 1/4] rev-parse: fix some options when executed from subpath of main tree

[SZEDER Gábor]

Quoting Mike Rappazzo :


On Fri, Apr 29, 2016 at 9:50 AM, SZEDER Gábor  wrote:

Executing `git-rev-parse` with `--git-common-dir`, `--git-path `,
or `--shared-index-path` from the root of the main worktree results in
a relative path to the git dir.

When executed from a subdirectory of the main tree, however, it incorrectly
returns a path which starts 'sub/path/.git'.


This is not completely true, because '--git-path ...' returns a
relative path starting with '.git':

 $ git -C t/ rev-parse --git-dir --git-path objects --git-common-dir
 /home/szeder/src/git/.git
 .git/objects
 t/.git

It's still wrong, of course.


I'll try to reword this to make it indicate that the value isn't
always incorrect.


Not sure I understand your intention about rewording, in particular that
"isn't always incorrect" part.  Just to make sure there is no
misunderstanding let's have a look at the two broken cases:

   $ git -C t/ rev-parse --git-common-dir
   t/.git

Obviously wrong.
This is what you correctly described in the commit message as
"incorrectly returns a path which starts 'sub/path/.git'.

   $ git -C t/ rev-parse --git-path objects
   .git/objects

Wrong as well.  It would be correct if we were at the top of the working
tree, but we are not.  It must be relative to the directory where '-C t/'
brought us.
Your description in the commit message implies that '--git-path '
is wrong in the same way as '--git-common-dir' is, i.e. in this case
would also return the relative path starting with the subdirectory.
That is apparently not the case.

My point in the previous email was that both are wrong when executed in
a subdir of the working tree, but they are wrong in different ways.  And
they are always wrong when executed from a subdir of the working tree.
I would have described the current wrong behavior as:

   When executed from a subdirectory of the working tree, however,
   '--git-common-dir' incorrectly returns a path which starts
   'sub/path/.git', while '--git-path ' incorrectly returns a path
   relative to the top of the working tree.

(Still hasn't looked at shared index...)


Change this to return the
proper relative path to the git directory.


I think returning absolute paths would be better.  It is consistent
with the already properly working '--git-dir' option, which returns an
absolute path in this case.  Furthermore, both '--git-path ...' and
'--git-common-dir' already return absolute paths when run from a
subdirectory of the .git directory:

 $ git -C .git/refs rev-parse --git-dir --git-path objects --git-common-dir
 /home/szeder/src/git/.git
 /home/szeder/src/git/.git/objects
 /home/szeder/src/git/.git



I agree with this, but at one point Junio suggested that it should
return the relative path[1],


I wasn't aware of Junio's suggestion.

It shouldn't really matter in practice, because both the absolute and
relative paths will ultimately lead to the same place.  However, I
still think that for consistency's sake absolute paths would be better
when executed in a subdir of the working tree.


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

Re: [PATCH] git-cvsserver: fix duplicate words

[Marc Branchaud]

On 2016-05-06 08:09 AM, Li Peng wrote:

Fix duplicate words in comments.

Signed-off-by: Li Peng 
---
  git-cvsserver.perl | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 02c0445..392e59e 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -1156,7 +1156,7 @@ sub prepDirForOutput
  # FUTURE: This would more accurately emulate CVS by sending
  #   another copy of sticky after processing the files in that
  #   directory.  Or intermediate: perhaps send all sticky's for
-#   $seendirs after after processing all files.
+#   $seendirs after processing all files.
  }

  # update \n
@@ -2824,7 +2824,7 @@ sub statecleanup
  }

  # Return working directory CVS revision "1.X" out
-# of the the working directory "entries" state, for the given filename.
+# of the working directory "entries" state, for the given filename.
  # This is prefixed with a dash if the file is scheduled for removal
  # when it is committed.
  sub revparse
@@ -2935,7 +2935,7 @@ sub filecleanup
  return $filename;
  }

-# Remove prependdir from the path, so that is is relative to the directory
+# Remove prependdir from the path, so that is relative to the directory


This one is a typo -- it should be "it is".

Did you write a script to find these?  :)

(Also, I agree with Jeff that putting all of these changes into one 
patch would have been fine.)


M.

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

Re: [PATCH 80/83] run-command: make dup_devnull() non static

[Christian Couder]

On Thu, May 5, 2016 at 10:07 PM, Johannes Sixt  wrote:
> Am 05.05.2016 um 11:50 schrieb Christian Couder:
>>
>> On Mon, Apr 25, 2016 at 5:05 PM, Johannes Schindelin
>>  wrote:
>>>
>>> Hi Chris,
>>>
>>> On Sun, 24 Apr 2016, Christian Couder wrote:
>>>
 diff --git a/run-command.c b/run-command.c
 index 8c7115a..29d2bda 100644
 --- a/run-command.c
 +++ b/run-command.c
 @@ -85,7 +85,7 @@ static inline void close_pair(int fd[2])
   }

   #ifndef GIT_WINDOWS_NATIVE
 -static inline void dup_devnull(int to)
 +void dup_devnull(int to)
   {
>>>
>>>
>>> The #ifndef GIT_WINDOWS_NATIVE rings very, very loud alarm bells.
>>
>>
>> Yeah, but I must say that I don't know what I should do about this.
>> Do you have a suggestion? Should I try to implement the same function
>> for Windows?
>
> No, just remove the #ifndef brackets. There is already code in
> compat/mingw.c that treats the file name "/dev/null" specially.

Ok, I will do that in the same patch though the "#ifndef
GIT_WINDOWS_NATIVE" was already there before.

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

[PATCH v3 0/2] [PATCH v3 0/2] rev-parse: fix some options when executed from subpath of main tree

[Michael Rappazzo]

Differenced from v2[1]:
 - Rewrote the commits to just two; one that introduces tests, and 
   one which fixes the problem.

[1] http://thread.gmane.org/gmane.comp.version-control.git/292272

Michael Rappazzo (2):
  rev-parse tests: add tests executed from a subdirectory
  rev-parse: fix some options when executed from subpath of main tree

 builtin/rev-parse.c  | 19 ++-
 t/t1500-rev-parse.sh | 38 ++
 t/t1700-split-index.sh   | 17 +
 t/t2027-worktree-list.sh | 10 +-
 4 files changed, 78 insertions(+), 6 deletions(-)

-- 
2.8.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 v3 1/2] rev-parse tests: add tests executed from a subdirectory

[Michael Rappazzo]

t1500-rev-parse contains envrionment leaks (changing dir without
changing back, setting config variables, etc).  Add a test to clean this
up up so that future tests can be added without worry of any setting
from a previous test.

t2027-worktree-list has an incorrect expectation for --git-common-dir
which has been adjusted and marked to expect failure.

Some of the tests added have been marked to expect failure.  These
demonstrate a problem with the way that some options to git rev-parse
behave when executed from a subdirectory of the main worktree.

Signed-off-by: Michael Rappazzo 
---
 t/t1500-rev-parse.sh | 38 ++
 t/t1700-split-index.sh   | 17 +
 t/t2027-worktree-list.sh | 12 ++--
 3 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 48ee077..442ca46 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -36,6 +36,7 @@ test_rev_parse() {
 # label is-bare is-inside-git is-inside-work prefix git-dir
 
 ROOT=$(pwd)
+original_core_bare=$(git config core.bare)
 
 test_rev_parse toplevel false false true '' .git
 
@@ -84,4 +85,41 @@ test_rev_parse 'GIT_DIR=../repo.git, core.bare = true' true 
false false ''
 git config --unset core.bare
 test_rev_parse 'GIT_DIR=../repo.git, core.bare undefined' false false true ''
 
+test_expect_success 'cleanup from the previous tests' '
+   cd .. &&
+   rm -r work &&
+   mv repo.git .git &&
+   unset GIT_DIR &&
+   unset GIT_CONFIG &&
+   git config core.bare $original_core_bare
+'
+
+test_expect_success 'git-common-dir from worktree root' '
+   echo .git >expect &&
+   git rev-parse --git-common-dir >actual &&
+   test_cmp expect actual
+'
+
+test_expect_failure 'git-common-dir inside sub-dir' '
+   mkdir -p path/to/child &&
+   test_when_finished "rm -rf path" &&
+   echo "$(git -C path/to/child rev-parse --show-cdup).git" >expect &&
+   git -C path/to/child rev-parse --git-common-dir >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'git-path from worktree root' '
+   echo .git/objects >expect &&
+   git rev-parse --git-path objects >actual &&
+   test_cmp expect actual
+'
+
+test_expect_failure 'git-path inside sub-dir' '
+   mkdir -p path/to/child &&
+   test_when_finished "rm -rf path" &&
+   echo "$(git -C path/to/child rev-parse --show-cdup).git/objects" 
>expect &&
+   git -C path/to/child rev-parse --git-path objects >actual &&
+   test_cmp expect actual
+'
+
 test_done
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 8aef49f..8ca21bd 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -200,4 +200,21 @@ EOF
test_cmp expect actual
 '
 
+test_expect_failure 'rev-parse --shared-index-path' '
+   rm -rf .git &&
+   test_create_repo . &&
+   git update-index --split-index &&
+   ls -t .git/sharedindex* | tail -n 1 >expect &&
+   git rev-parse --shared-index-path >actual &&
+   test_cmp expect actual &&
+   mkdir work &&
+   test_when_finished "rm -rf work" &&
+   (
+   cd work &&
+   ls -t ../.git/sharedindex* | tail -n 1 >expect &&
+   git rev-parse --shared-index-path >actual &&
+   test_cmp expect actual
+   )
+'
+
 test_done
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 1b1b65a..53cc5d3 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -8,16 +8,24 @@ test_expect_success 'setup' '
test_commit init
 '
 
-test_expect_success 'rev-parse --git-common-dir on main worktree' '
+test_expect_failure 'rev-parse --git-common-dir on main worktree' '
git rev-parse --git-common-dir >actual &&
echo .git >expected &&
test_cmp expected actual &&
mkdir sub &&
git -C sub rev-parse --git-common-dir >actual2 &&
-   echo sub/.git >expected2 &&
+   echo ../.git >expected2 &&
test_cmp expected2 actual2
 '
 
+test_expect_failure 'rev-parse --git-path objects linked worktree' '
+   echo "$(git rev-parse 
--show-toplevel)/.git/worktrees/linked-tree/objects" >expect &&
+   test_when_finished "rm -rf linked-tree && git worktree prune" &&
+   git worktree add --detach linked-tree master &&
+   git -C linked-tree rev-parse --git-path objects >actual &&
+   test_cmp expect actual
+'
+
 test_expect_success '"list" all worktrees from main' '
echo "$(git rev-parse --show-toplevel) $(git rev-parse --short HEAD) 
[$(git symbolic-ref --short HEAD)]" >expect &&
test_when_finished "rm -rf here && git worktree prune" &&
-- 
2.8.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 v3 2/2] rev-parse: fix some options when executed from subpath of main tree

[Michael Rappazzo]

Executing `git-rev-parse` with `--git-common-dir`, `--git-path `,
or `--shared-index-path` from the root of the main worktree results in
a relative path to the git dir.

When executed from a subdirectory of the main tree, it can incorrectly
return a path which starts 'sub/path/.git'.  Change this to return the
proper relative path to the git directory.

Related tests marked to expect failure are updated to expect success

Signed-off-by: Michael Rappazzo 
---
 builtin/rev-parse.c  | 19 ++-
 t/t1500-rev-parse.sh |  4 ++--
 t/t1700-split-index.sh   |  2 +-
 t/t2027-worktree-list.sh |  4 ++--
 4 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index c961b74..1da2e10 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -564,10 +564,13 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
}
 
if (!strcmp(arg, "--git-path")) {
+   struct strbuf sb = STRBUF_INIT;
if (!argv[i + 1])
die("--git-path requires an argument");
-   puts(git_path("%s", argv[i + 1]));
-   i++;
+
+   puts(relative_path(xstrfmt("%s/%s", get_git_dir(), 
argv[++i]),
+   prefix, ));
+   strbuf_release();
continue;
}
if (as_is) {
@@ -787,8 +790,9 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
continue;
}
if (!strcmp(arg, "--git-common-dir")) {
-   const char *pfx = prefix ? prefix : "";
-   puts(prefix_filename(pfx, strlen(pfx), 
get_git_common_dir()));
+   struct strbuf sb = STRBUF_INIT;
+   puts(relative_path(get_git_common_dir(), 
prefix, ));
+   strbuf_release();
continue;
}
if (!strcmp(arg, "--is-inside-git-dir")) {
@@ -811,7 +815,12 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
die(_("Could not read the index"));
if (the_index.split_index) {
const unsigned char *sha1 = 
the_index.split_index->base_sha1;
-   puts(git_path("sharedindex.%s", 
sha1_to_hex(sha1)));
+   struct strbuf sb = STRBUF_INIT;
+
+   puts(relative_path(
+   xstrfmt("%s/sharedindex.%s", 
get_git_dir(), sha1_to_hex(sha1)),
+   prefix, ));
+   strbuf_release();
}
continue;
}
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 442ca46..cc89392 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -100,7 +100,7 @@ test_expect_success 'git-common-dir from worktree root' '
test_cmp expect actual
 '
 
-test_expect_failure 'git-common-dir inside sub-dir' '
+test_expect_success 'git-common-dir inside sub-dir' '
mkdir -p path/to/child &&
test_when_finished "rm -rf path" &&
echo "$(git -C path/to/child rev-parse --show-cdup).git" >expect &&
@@ -114,7 +114,7 @@ test_expect_success 'git-path from worktree root' '
test_cmp expect actual
 '
 
-test_expect_failure 'git-path inside sub-dir' '
+test_expect_success 'git-path inside sub-dir' '
mkdir -p path/to/child &&
test_when_finished "rm -rf path" &&
echo "$(git -C path/to/child rev-parse --show-cdup).git/objects" 
>expect &&
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 8ca21bd..d2d9e02 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -200,7 +200,7 @@ EOF
test_cmp expect actual
 '
 
-test_expect_failure 'rev-parse --shared-index-path' '
+test_expect_success 'rev-parse --shared-index-path' '
rm -rf .git &&
test_create_repo . &&
git update-index --split-index &&
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 53cc5d3..16eec6e 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -8,7 +8,7 @@ test_expect_success 'setup' '
test_commit init
 '
 
-test_expect_failure 'rev-parse --git-common-dir on main worktree' '
+test_expect_success 'rev-parse --git-common-dir on main worktree' '
git rev-parse --git-common-dir >actual &&
echo .git >expected &&
test_cmp expected actual &&
@@ -18,7 +18,7 @@ test_expect_failure 'rev-parse --git-common-dir on main 
worktree' '

wishlist; unify behavior while cloning non-bare repos over http to be in line with ssh/local

[Yaroslav Halchenko]

Dear Git Folks,

Originally this issue was mentioned in previous thread [1], and I have decided
to bring it into a separate thread.  ATM there is a dichotomy in git behavior
between cloning non-bare repos:  if I clone over ssh or just locally by
providing url without trailing /.git, git senses for /.git and works just fine
with ssh or local repositories, but fails for "dummy" http ones, the demo
script is here [2] which produces output listed below.  From which you can see
that  cloning using http URL to the repository without /.git fails (git version
2.8.1, Debian).  As it was noted in [1], concern could have been to not
traverse website since could lead to dangerous places.  But .git is under
originating url directory, as well as info/ or HEAD or any other object
accessed by git, so IMHO this concern is not a concern.

So do you think that cloning from  http urls could be adjusted so git senses
for presence of .git/ subdirectory if originating url is missing necessary
/info/refs? ?  That would make behavior uniform and help us and users in
many use-cases IMHO (in particular with relative paths to submodules as in
[1])

Thank you in advance, and please maintain CC

$> /tmp/democlone
Initiating repo
cloning via ssh: localhost:/tmp/repo1
Cloning into 'repo1-ssh'...
remote: Counting objects: 3, done.
remote: Total 3 (delta 0), reused 0 (delta 0)
Receiving objects: 100% (3/3), done.
Checking connectivity... done.
SUCCESS
cloning locally: /tmp/repo1
Cloning into 'repo1-local'...
done.
SUCCESS
starting local http server
Serving HTTP on 0.0.0.0 port 8080 ...
cloning via http: http://localhost:8080/repo1
Cloning into 'repo1-http-failed'...
127.0.0.1 - - [06/May/2016 09:13:41] code 404, message File not found
127.0.0.1 - - [06/May/2016 09:13:41] "GET 
/repo1/info/refs?service=git-upload-pack HTTP/1.1" 404 -
fatal: repository 'http://localhost:8080/repo1/' not found
FAILED
doing with /.git: http://localhost:8080/repo1/.git
Cloning into 'repo1-http'...
127.0.0.1 - - [06/May/2016 09:13:41] "GET 
/repo1/.git/info/refs?service=git-upload-pack HTTP/1.1" 200 -
127.0.0.1 - - [06/May/2016 09:13:41] "GET /repo1/.git/HEAD HTTP/1.1" 200 -
127.0.0.1 - - [06/May/2016 09:13:41] "GET 
/repo1/.git/objects/36/31fa81eb6422349035c915a5a11b177688f491 HTTP/1.1" 200 -
127.0.0.1 - - [06/May/2016 09:13:41] "GET 
/repo1/.git/objects/12/ddeb87cc045cd67063e95125fbeb014bd1d9b1 HTTP/1.1" 200 -
127.0.0.1 - - [06/May/2016 09:13:41] "GET 
/repo1/.git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 HTTP/1.1" 200 -
Checking connectivity... done.
SUCCESS



[1] problems serving non-bare repos with submodules over http
http://thread.gmane.org/gmane.comp.version-control.git/292032
[2] http://www.onerussian.com/tmp/democlone
-- 
Yaroslav O. Halchenko
Center for Open Neuroscience http://centerforopenneuroscience.org
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834   Fax: +1 (603) 646-1419
WWW:   http://www.linkedin.com/in/yarik
--
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: /* compiler workaround */ - what was the issue?

[Philip Oakley]

From: "Duy Nguyen" 

On Fri, May 6, 2016 at 4:41 AM, Junio C Hamano  wrote:

"Philip Oakley"  writes:


int saved_namelen = saved_namelen; /* compiler workaround */

Which then becomes an MSVC compile warning C4700: uninitialized local
variable.

I'm wondering what was the compiler workaround being referred to? i.e. 
why

does it need that tweak? There's no mention of the reason in the commit
message.


That was a fairly well-known workaround for GCC that issues a false
warning that variable is used before initialized.  I thought we
stopped using it several years ago in new code after doing a bulk
sanitizing


I guess that's 803a777 (cat-file: Fix an gcc -Wuninitialized warning -
2013-03-26) and more commits around that time. The split-index commit
is in 2014. I must have missed the trend.


(I think the new recommended workaround was to initialise
such a variable to the nil value like '0' for integers).


Yep. First Jeff removed the " = xxx" part from "xxx = xxx" then Ramsay
added the " = NULL" back. So we probably just do "int saved_namelen =
0;" in this case.
--

Thanks,

I'll try and work up a patch - probably next week as I'm away for the 
weekend.


--
Philip 


--
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] utf8: fix duplicate words of "the"

[Jeff King]

On Fri, May 06, 2016 at 08:31:33PM +0800, Li Peng wrote:

> Fix duplicate words of "the" in comment.

Obviously a good change, along with the other one of mine you found.

> ---
>  transport-helper.c | 2 +-
>  utf8.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

You seem to be breaking these up by file; was the change in
transport-helper supposed to be in a different patch?

IMHO it would be fine to just do all of these in a single patch. They're
different files, yes, but it's all conceptually the same change.

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

Re: [PATCH v2 1/4] rev-parse: fix some options when executed from subpath of main tree

[Mike Rappazzo]

On Fri, Apr 29, 2016 at 9:50 AM, SZEDER Gábor  wrote:
>> Executing `git-rev-parse` with `--git-common-dir`, `--git-path `,
>> or `--shared-index-path` from the root of the main worktree results in
>> a relative path to the git dir.
>>
>> When executed from a subdirectory of the main tree, however, it incorrectly
>> returns a path which starts 'sub/path/.git'.
>
> This is not completely true, because '--git-path ...' returns a
> relative path starting with '.git':
>
>   $ git -C t/ rev-parse --git-dir --git-path objects --git-common-dir
>   /home/szeder/src/git/.git
>   .git/objects
>   t/.git
>
> It's still wrong, of course.

I'll try to reword this to make it indicate that the value isn't
always incorrect.

>
>> Change this to return the
>> proper relative path to the git directory.
>
> I think returning absolute paths would be better.  It is consistent
> with the already properly working '--git-dir' option, which returns an
> absolute path in this case.  Furthermore, both '--git-path ...' and
> '--git-common-dir' already return absolute paths when run from a
> subdirectory of the .git directory:
>
>   $ git -C .git/refs rev-parse --git-dir --git-path objects --git-common-dir
>   /home/szeder/src/git/.git
>   /home/szeder/src/git/.git/objects
>   /home/szeder/src/git/.git
>

I agree with this, but at one point Junio suggested that it should
return the relative path[1],
so I went with that.  Maybe I could RFC a separate patch that changes
all of these to
absolute.

>> Signed-off-by: Michael Rappazzo 
>> ---
>>  builtin/rev-parse.c | 19 ++-
>>  1 file changed, 14 insertions(+), 5 deletions(-)
>
> This patch doesn't add any new tests, while subsequent patches of the
> series do nothing but add more tests.  Splitting up your changes this
> way doesn't add any value, it only increases the number of commits.  I
> think either:
>
>   - all those new tests could be added with this patch, or
>
>   - if you want to add the test separately, then add them before
> this patch and mark them with 'test_expect_failure' to clearly
> demonstrate what the series is about to fix, and flip them to
> 'test_expect_success' in this patch.
>
>   - An alternative way to split this series, following the "Make
> separate commits for logically separate changes" guideline, would
> be to fix and test these options in separate patches, i.e. fix and
> test '--git-path ...' in one patch, then fix and test
> '--git-common-dir' in the next, ...
>
>

Thanks, have a re-roll ready to go based on you input.  I went with
option 2 - add
tests with expect failure and fix them with the next.

[1] http://article.gmane.org/gmane.comp.version-control.git/290061
--
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] wildmatch: fix duplicate words of "the"

[Li Peng]

Fix duplicate words of "the" in comment.

Signed-off-by: Li Peng 
---
 wildmatch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/wildmatch.c b/wildmatch.c
index f91ba99..57c8765 100644
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -136,7 +136,7 @@ static int dowild(const uchar *p, const uchar *text, 
unsigned int flags)
/*
 * Try to advance faster when an asterisk is
 * followed by a literal. We know in this case
-* that the the string before the literal
+* that the string before the literal
 * must belong to "*".
 * If match_slash is false, do not look past
 * the first slash as it cannot belong to '*'.
-- 
1.8.3.1

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

[PATCH] utf8: fix duplicate words of "the"

[Li Peng]

Fix duplicate words of "the" in comment.

Signed-off-by: Li Peng 
---
 transport-helper.c | 2 +-
 utf8.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/transport-helper.c b/transport-helper.c
index b934183..13b7a57 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1152,7 +1152,7 @@ static void udt_close_if_finished(struct 
unidirectional_transfer *t)
 }
 
 /*
- * Tries to read read data from source into buffer. If buffer is full,
+ * Tries to read data from source into buffer. If buffer is full,
  * no data is read. Returns 0 on success, -1 on error.
  */
 static int udt_do_read(struct unidirectional_transfer *t)
diff --git a/utf8.h b/utf8.h
index 7930b44..6bbcf31 100644
--- a/utf8.h
+++ b/utf8.h
@@ -48,7 +48,7 @@ static inline char *reencode_string(const char *in,
 int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding);
 
 /*
- * Returns true if the the path would match ".git" after HFS case-folding.
+ * Returns true if the path would match ".git" after HFS case-folding.
  * The path should be NUL-terminated, but we will match variants of both 
".git\0"
  * and ".git/..." (but _not_ ".../.git"). This makes it suitable for both fsck
  * and verify_path().
-- 
1.8.3.1

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

[PATCH] transport-helper: fix duplicate words of "read"

[Li Peng]

Fix duplicate words of "read" in comment.

Signed-off-by: Li Peng 
---
 transport-helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/transport-helper.c b/transport-helper.c
index b934183..13b7a57 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1152,7 +1152,7 @@ static void udt_close_if_finished(struct 
unidirectional_transfer *t)
 }
 
 /*
- * Tries to read read data from source into buffer. If buffer is full,
+ * Tries to read data from source into buffer. If buffer is full,
  * no data is read. Returns 0 on success, -1 on error.
  */
 static int udt_do_read(struct unidirectional_transfer *t)
-- 
1.8.3.1

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