Re: [PATCH 13/21] rev-parse: add '--absolute-git-dir' option

[Jeff King]

On Mon, Apr 25, 2016 at 10:33:13PM -0400, Mike Rappazzo wrote:

> I propose that it might make more sense to use something like
> `--abs-path` to indicate
> that the result should include an absolute path (or we could also just spell 
> out
> `--absolute-path`).  That way we don't have to add additional options
> for any other type
> that might want an absolute path.
> 
> git rev-parse --git-dir --abs-path
> git rev-parse --git-common-dir --absolute-path
> 
> I do understand that this might be more work than is necessary for the
> completion series
> here.  Would it be unreasonable to suggest a partial implementation
> that, for now, only
> works with `--git-dir`?

I do like the concept of keeping "--absolute-path" orthogonal. The only
trick is that we need to either support it for all appropriate options,
or document which options it _does_ work with. Otherwise, we're going to
get bug reports when somebody tries "--absolute-path --git-common-dir".

It would be cleaner to provide a separate option to let people compose
the options, like:

  git rev-parse --git-dir | git rev-parse --realpath

but that's a lot less efficient.

> > +   if (gitdir) {
> > +   char 
> > absolute_path[PATH_MAX];
> > +   if (!realpath(gitdir, 
> > absolute_path))
> > +   die_errno(_("unable 
> > to get absolute path"));
> > +   puts(absolute_path);
> > +   continue;
> > +   }

I don't recall if this came up in earlier review, but I happened to
notice the use of realpath() here. We should be using our custom
real_path() instead. There are some platforms without realpath(), I
think, and our real_path() is not limited to the static PATH_MAX (which
is too small on some platforms).

-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 13/21] rev-parse: add '--absolute-git-dir' option

[Mike Rappazzo]

On Thu, Feb 25, 2016 at 5:54 PM SZEDER Gábor  wrote:
>
> Some scripts can benefit from not having to deal with the possibility
> of relative paths to the repository, but the output of 'git rev-parse
> --git-dir' can be a relative path.  Case in point: supporting 'git -C
> ' in our Bash completion script turned out to be considerably
> more difficult, error prone and required more subshells and git
> processes when we had to cope with a relative path to the .git
> directory.
>
> Help these use cases and teach 'git rev-parse' a new
> '--absolute-git-dir' option which always outputs a canonicalized
> absolute path to the .git directory, regardless of whether the path is
> discovered automatically or is specified via $GIT_DIR or 'git
> --git-dir='.
>
> Signed-off-by: SZEDER Gábor 
> ---
>  Documentation/git-rev-parse.txt |  4 
>  builtin/rev-parse.c | 29 +
>  t/t1500-rev-parse.sh| 17 ++---
>  3 files changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
> index b6c6326cdc7b..fb06e3118570 100644
> --- a/Documentation/git-rev-parse.txt
> +++ b/Documentation/git-rev-parse.txt
> @@ -216,6 +216,10 @@ If `$GIT_DIR` is not defined and the current directory
>  is not detected to lie in a Git repository or work tree
>  print a message to stderr and exit with nonzero status.
>
> +--absolute-git-dir::
> +   Like `--git-dir`, but its output is always the canonicalized
> +   absolute path.
> +
>  --git-common-dir::
> Show `$GIT_COMMON_DIR` if defined, else `$GIT_DIR`.
>

After working a little bit with rev-parse [1], I feel that this might
be better served as a
stand-alone option.  Consider that in addition to --git-dir, the
options --git-common-dir,
--git-path, and --git-shared-index produce relative paths.

I propose that it might make more sense to use something like
`--abs-path` to indicate
that the result should include an absolute path (or we could also just spell out
`--absolute-path`).  That way we don't have to add additional options
for any other type
that might want an absolute path.

git rev-parse --git-dir --abs-path
git rev-parse --git-common-dir --absolute-path

I do understand that this might be more work than is necessary for the
completion series
here.  Would it be unreasonable to suggest a partial implementation
that, for now, only
works with `--git-dir`?

> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index cf8487b3b95f..90a4dd6032c0 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -744,17 +744,30 @@ int cmd_rev_parse(int argc, const char **argv, const 
> char *prefix)
> putchar('\n');
> continue;
> }
> -   if (!strcmp(arg, "--git-dir")) {
> +   if (!strcmp(arg, "--git-dir") ||
> +   !strcmp(arg, "--absolute-git-dir")) {
> const char *gitdir = 
> getenv(GIT_DIR_ENVIRONMENT);
> char *cwd;
> int len;
> -   if (gitdir) {
> -   puts(gitdir);
> -   continue;
> -   }
> -   if (!prefix) {
> -   puts(".git");
> -   continue;
> +   if (arg[2] == 'g') {/* --git-dir */
> +   if (gitdir) {
> +   puts(gitdir);
> +   continue;
> +   }
> +   if (!prefix) {
> +   puts(".git");
> +   continue;
> +   }
> +   } else {/* --absolute-git-dir 
> */
> +   if (!gitdir && !prefix)
> +   gitdir = ".git";
> +   if (gitdir) {
> +   char absolute_path[PATH_MAX];
> +   if (!realpath(gitdir, 
> absolute_path))
> +   die_errno(_("unable 
> to get absolute path"));
> +   puts(absolute_path);
> +   continue;
> +   }
> }
> cwd = xgetcwd();
> len = strlen(cwd);
> diff --git 

肯德基大

[肯德基大]

你的老朋友邀你来Q群:343257759 
抢优惠券N�r��yb�X��ǧv�^�)޺{.n�+ا���ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf

Re: [PATCH 51/83] builtin/apply: make apply_patch() return -1 instead of die()ing

[Eric Sunshine]

On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder
 wrote:
> To libify `git apply` functionality we have to signal errors
> to the caller instead of die()ing.
>
> As a first step in this direction, let's make apply_patch() return
> -1 in case of errors instead of dying. For now its only caller
> apply_all_patches() will exit(1) when apply_patch() return -1.
>
> In a later patch, apply_all_patches() will return -1 too instead of
> exiting.
>
> Signed-off-by: Christian Couder 
> ---
> diff --git a/builtin/apply.c b/builtin/apply.c
> @@ -4522,6 +4522,14 @@ static int write_out_results(struct apply_state 
> *state, struct patch *list)
>  static int apply_patch(struct apply_state *state,
>int fd,
>const char *filename,
> @@ -4564,7 +4572,7 @@ static int apply_patch(struct apply_state *state,
> }
>
> if (!list && !skipped_patch)
> -   die(_("unrecognized input"));
> +   return error(_("unrecognized input"));
>
> if (state->whitespace_error && (state->ws_error_action == 
> die_on_ws_error))
> state->apply = 0;
> @@ -4575,19 +4583,17 @@ static int apply_patch(struct apply_state *state,
> hold_locked_index(state->lock_file, 1);
> }
>
> -   if (state->check_index) {
> -   if (read_cache() < 0)
> -   die(_("unable to read index file"));
> -   }
> +   if (state->check_index && read_cache() < 0)
> +   return error(_("unable to read index file"));
>
> if ((state->check || state->apply) &&
> check_patch_list(state, list) < 0 &&
> !state->apply_with_reject)
> -   exit(1);
> +   return -1;
>
> if (state->apply && write_out_results(state, list)) {
> if (state->apply_with_reject)
> -   exit(1);
> +   return -1;
> /* with --3way, we still need to write the index out */
> return 1;
> }

Are these new 'returns' leaking 'list', 'buf', and 'fn_table' which
otherwise get released at the end of the function?
--
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

[PATCHv2] clone: add `--shallow-submodules` flag

[Stefan Beller]

When creating a shallow clone of a repository with submodules, the depth
argument does not influence the submodules, i.e. the submodules are done
as non-shallow clones. It is unclear what the best default is for the
depth of submodules of a shallow clone, so we need to have the possibility
to do all kinds of combinations:

* shallow super project with shallow submodules
  e.g. build bots starting always from scratch. They want to transmit
  the least amount of network data as well as using the least amount
  of space on their hard drive.
* shallow super project with unshallow submodules
  e.g. The superproject is just there to track a collection of repositories
  and it is not important to have the relationship between the repositories
  intact. However the history of the individual submodules matter.
* unshallow super project with shallow submodules
  e.g. The superproject is the actual project and the submodule is a
  library which is rarely touched.

The new switch to select submodules to be shallow or unshallow supports
all of these three cases.

It is easy to transition from the first to the second case by just
unshallowing the submodules (`git submodule foreach git fetch
--unshallow`), but it is not possible to transition from the second to the
first case (as we would have already transmitted the non shallow over
the network). That is why we want to make the first case the default in
case of a shallow super project. This leads to the inconvenience in the
second case with the shallow super project and unshallow submodules,
as you need to pass `--no-shallow-submodules`.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---

 This is origin/sb/clone-shallow-passthru, with $pwd instead of $p
 in the tests.
 
 I did not rebase this on 85705cfb (Merge branch 'ss/clone-depth-single-doc',
 2016-01-20) or later, but worked on it with the base unchanged.

 Documentation/git-clone.txt | 13 +--
 builtin/clone.c |  7 
 t/t5614-clone-submodules.sh | 85 +
 3 files changed, 102 insertions(+), 3 deletions(-)
 create mode 100755 t/t5614-clone-submodules.sh

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 6db7b6d..e1a21b7 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -14,8 +14,8 @@ SYNOPSIS
  [-o ] [-b ] [-u ] [--reference ]
  [--dissociate] [--separate-git-dir ]
  [--depth ] [--[no-]single-branch]
- [--recursive | --recurse-submodules] [--jobs ] [--] 
- []
+ [--recursive | --recurse-submodules] [--[no-]shallow-submodules]
+ [--jobs ] [--]  []
 
 DESCRIPTION
 ---
@@ -190,7 +190,11 @@ objects from the source repository into a pack in the 
cloned repository.
 
 --depth ::
Create a 'shallow' clone with a history truncated to the
-   specified number of revisions.
+   specified number of revisions. Implies `--single-branch` unless
+   `--no-single-branch` is given to fetch the histories near the
+   tips of all branches. This implies `--shallow-submodules`. If
+   you want to have a shallow superproject clone, but full submodules,
+   also pass `--no-shallow-submodules`.
 
 --[no-]single-branch::
Clone only the history leading to the tip of a single branch,
@@ -214,6 +218,9 @@ objects from the source repository into a pack in the 
cloned repository.
repository does not have a worktree/checkout (i.e. if any of
`--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
 
+--[no-]shallow-submodules::
+   All submodules which are cloned will be shallow with a depth of 1.
+
 --separate-git-dir=::
Instead of placing the cloned repository where it is supposed
to be, place the cloned repository at the specified directory,
diff --git a/builtin/clone.c b/builtin/clone.c
index b004fb4..ecdf308 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -40,6 +40,7 @@ static const char * const builtin_clone_usage[] = {
 
 static int option_no_checkout, option_bare, option_mirror, 
option_single_branch = -1;
 static int option_local = -1, option_no_hardlinks, option_shared, 
option_recursive;
+static int option_shallow_submodules = -1;
 static char *option_template, *option_depth;
 static char *option_origin = NULL;
 static char *option_branch = NULL;
@@ -91,6 +92,8 @@ static struct option builtin_clone_options[] = {
N_("create a shallow clone of that depth")),
OPT_BOOL(0, "single-branch", _single_branch,
N_("clone only one branch, HEAD or --branch")),
+   OPT_BOOL(0, "shallow-submodules", _shallow_submodules,
+   N_("any cloned submodules will be shallow")),
OPT_STRING(0, "separate-git-dir", _git_dir, N_("gitdir"),
   N_("separate git dir from working tree")),
OPT_STRING_LIST('c', "config", _config, N_("key=value"),
@@ -727,6 +730,10 

Re: [PATCH] clone: add `--shallow-submodules` flag

[Stefan Beller]

On Mon, Apr 25, 2016 at 3:37 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> I agree. How about `currentdir`, `testdir` or `testtop` instead?
>> That is slightly longer than `D`, `here` or `top`, but is slightly more
>> informative. $TRASH would also work for me.
>
> I would not be happy to see a patch that adds yet another variable
> that is never used so far, like currentdir, testdir, or testtop.

D seems to be popular in 5510, the submodule crowd seems to prefer
$pwd though, see
t5526-fetch-submodules.sh
t7406-submodule-update.sh
t7407-submodule-foreach.s
t7400-submodule-basic.sh

as grep -r "\$pwd" put it.

>
> Among the ones I found that are already in use, $here is probably my
> favourite, because it does _not_ have to be set to the top; it just
> says "this is the directory my main focus is in".
>
> Having said that, personally, I find $D (as long as D is in capital)
> is distinctive and descriptive enough.

I rather go with pwd for now (it keeps the submodule stuff consistent),

Thanks,
Stefan
--
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] Update git-p4 to be compatible with git-lfs 1.2

[SZEDER Gábor]

Quoting Junio C Hamano :


SZEDER Gábor  writes:


You can have a look at these patches at

  https://github.com/szeder/git completion-test-multiple-bash-versions

and perhaps you could even adapt it to LFS and/or p4 somehow.


Plus if we want to be consistent we would
need to do the same for LFS 1.0, 1.2, and for pretty much every other
dependency...


I'm not sure we should be consistent in this case, at least not solely
for consistency's sake and not in git.git. Taking what I did for Bash
and doing it for different versions of LFS, p4, etc. could perhaps
keep the runtime under control, but t/Makefile would surely get out
of control rather quickly.  Putting these into a travis-ci matrix is
so much simpler, but the runtime makes it infeasible, of course.


I took a brief look of your branch, and I like its approach.  If I
understood your approach correctly, you:

 * Group selected tests in t/ as "these are bash related tests I
   care about" in t/Makefile;


Yes.


 * Add Travis test target to build Git with specific versions of
   bash, and run the above target instead of the full test to
   exercise the version of bash you are testing.


Not quite.

  * Add t/Makefile targets to run a Bash-related test script with a
specific Bash version, one target for each script-version pair,
and a target to run all Bash-related tests with all listed
Bash-versions.

  * Extend the travis-ci config so that, after building Git as usual
and running the full test suite as usual, it additionaly runs all
Bash-related tests will all listed Bash versions on Linux builds.


And I agree that the same can be done for LFS versions and P4
versions.  Only a handful tests in t/ are about these niches.


Luckily for me, running a test script with a specific Bash version is
as trivial as '/path/to/bash-vX.Y t9902-completion.sh'.  No
modifications to the test scripts or to lib-bash.sh were necessary.

OTOH, Git LFS and p4 tests, AFAICS, rely on git-lfs and p4 binaries
being available in $PATH, and the p4 tests need two binaries.  So
there is more work that has to be done, as we would need a way to
override those binaries found in $PATH, either through an environment
variable or a command line option.  Bonus points for a solution that
would work equally well with LFS, p4 and Bash: then perhaps we could
have a single unified block of Makefile metaprogramming, which could
generate  all those test targets from a list of dependency-specific
tests and a list of paths to different versions of that dependency.
Then it might even be suitable for inclusion in git.git.



I think the best we can do is to keep this out of git.git and let
(hope?) developers interested in a particular subsystem do this
"multiple version compatibility" tests as they see fit.



--
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 (Apr 2016, #07; Mon, 25)

[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 seventh batch of topics of this
cycle.

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

* ad/commit-have-m-option (2016-04-07) 2 commits
  (merged to 'next' on 2016-04-13 at 74088c2)
 + commit: do not ignore an empty message given by -m ''
 + commit: --amend -m '' silently fails to wipe message

 "git commit" misbehaved in a few minor ways when an empty message
 is given via -m '', all of which has been corrected.


* ad/cygwin-wants-rename (2015-08-07) 1 commit
  (merged to 'next' on 2016-04-19 at 1352ede)
 + config.mak.uname: Cygwin needs OBJECT_CREATION_USES_RENAMES

 On Cygwin, object creation uses the "create a temporary and then
 rename it to the final name" pattern, not "create a temporary,
 hardlink it to the final name and then unlink the temporary"
 pattern.

 This is necessary to use Git on Windows shared directories, and is
 already enabled for the MinGW and plain Windows builds.  It also
 has been used in Cygwin packaged versions of Git for quite a while.
 See http://thread.gmane.org/gmane.comp.version-control.git/291853
 ($gmane/275680, $gmane/291853).


* dt/pre-refs-backend (2016-04-10) 24 commits
  (merged to 'next' on 2016-04-13 at 0a8f9dd)
 + refs: on symref reflog expire, lock symref not referrent
 + refs: move resolve_ref_unsafe into common code
 + show_head_ref(): check the result of resolve_ref_namespace()
 + check_aliased_update(): check that dst_name is non-NULL
 + checkout_paths(): remove unneeded flag variable
 + cmd_merge(): remove unneeded flag variable
 + fsck_head_link(): remove unneeded flag variable
 + read_raw_ref(): change flags parameter to unsigned int
 + files-backend: inline resolve_ref_1() into resolve_ref_unsafe()
 + read_raw_ref(): manage own scratch space
 + files-backend: break out ref reading
 + resolve_ref_1(): eliminate local variable "bad_name"
 + resolve_ref_1(): reorder code
 + resolve_ref_1(): eliminate local variable
 + resolve_ref_unsafe(): ensure flags is always set
 + resolve_ref_unsafe(): use for loop to count up to MAXDEPTH
 + resolve_missing_loose_ref(): simplify semantics
 + t1430: improve test coverage of deletion of badly-named refs
 + t1430: test for-each-ref in the presence of badly-named refs
 + t1430: don't rely on symbolic-ref for creating broken symrefs
 + t1430: clean up broken refs/tags/shadow
 + t1430: test the output and error of some commands more carefully
 + refs: move for_each_*ref* functions into common code
 + refs: move head_ref{,_submodule} to the common code

 Code restructuring around the "refs" area to prepare for pluggable
 refs backends.


* en/merge-octopus-fix (2016-04-12) 2 commits
  (merged to 'next' on 2016-04-13 at 600b479)
 + merge-octopus: abort if index does not match HEAD
 + t6044: new merge testcases for when index doesn't match HEAD

 "merge-octopus" strategy did not ensure that the index is clean
 when merge begins.


* en/merge-trivial-fix (2016-04-12) 2 commits
  (merged to 'next' on 2016-04-13 at fb3ea86)
 + builtin/merge.c: fix a bug with trivial merges
 + t7605: add a testcase demonstrating a bug with trivial merges

 When "git merge" notices that the merge can be resolved purely at
 the tree level (without having to merge blobs) and the resulting
 tree happens to already exist in the object store, it forgot to
 update the index, which lead to an inconsistent state for later
 operations.


* ew/send-email-drop-data-dumper (2016-04-06) 1 commit
  (merged to 'next' on 2016-04-13 at 180266c)
 + send-email: do not load Data::Dumper

 Code clean-up.


* ew/send-email-readable-message-id (2016-04-06) 1 commit
  (merged to 'next' on 2016-04-13 at 422959a)
 + send-email: more meaningful Message-ID

 "git send-email" now uses a more readable timestamps when
 formulating a message ID.


* jc/http-socks5h (2016-04-10) 1 commit
  (merged to 'next' on 2016-04-13 at eb27afc)
 + http: differentiate socks5:// and socks5h://

 The socks5:// proxy support added back in 2.6.4 days was not aware
 that socks5h:// proxies behave differently.


* jc/rerere-multi (2016-04-06) 11 commits
  (merged to 'next' on 2016-04-13 at 3db2753)
 + rerere: adjust 'forget' to multi-variant world order
 + rerere: split code to call ll_merge() further
 + rerere: move code related to "forget" together
 + rerere: gc and clear
 + rerere: do use multiple variants
 + t4200: rerere a merge with two identical conflicts
 + rerere: allow multiple variants to exist
 + rerere: delay the recording of preimage
 + rerere: handle leftover rr-cache/$ID directory and postimage 

Re: [PATCH] clone: add `--shallow-submodules` flag

[Junio C Hamano]

Stefan Beller  writes:

> I agree. How about `currentdir`, `testdir` or `testtop` instead?
> That is slightly longer than `D`, `here` or `top`, but is slightly more
> informative. $TRASH would also work for me.

I would not be happy to see a patch that adds yet another variable
that is never used so far, like currentdir, testdir, or testtop.

Among the ones I found that are already in use, $here is probably my
favourite, because it does _not_ have to be set to the top; it just
says "this is the directory my main focus is in".

Having said that, personally, I find $D (as long as D is in capital)
is distinctive and descriptive enough.
--
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] remote.c: spell __attribute__ correctly

[Ramsay Jones]

On 25/04/16 22:50, Philip Oakley wrote:
> From: "Jeff King" 
>> On Mon, Apr 25, 2016 at 05:10:30PM -0400, Jeff King wrote:
>>
>>> It should be handled in git-compat-util.h, which is included by cache.h,
>>> which is included by remote.c.
>>>
>>> There we have:
>>>
>>>   #ifndef __GNUC__
>>>   #ifndef __attribute__
>>>   #define __attribute__(x)
>>>   #endif
>>>   #endif
>>>
>>> which should make it a noop on compilers which don't know about it. Is
>>> VS (or another file) setting __GNUC__?
>>
>> Of course it helps if we spell the name right...

Indeed! ;-)

Not that it matters, but the above #define in git-compat-util.h is not
the relevant definition - msvc will not see it. However, it does see
the #define on line 12 of compat/msvc.h. :-D

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/RFC 4/6] transport: add refspec list parameters to functions

[Stefan Beller]

On Mon, Apr 25, 2016 at 9:44 AM, David Turner  wrote:
> On Wed, 2016-04-20 at 16:57 -0400, Jeff King wrote:
>> On Wed, Apr 20, 2016 at 04:46:55PM -0400, David Turner wrote:
>>
>> > As you note, it appears that git-daemon does sort-of have support
>> > for
>> > extra args -- see parse_host_arg.  So it wouldn't be hard to add
>> > something here. Unfortunately, current versions of git die on
>> > unknown
>> > args.  So this change would not be backwards-compatible.  We could
>> > put
>> > a decider on it so that clients would only try it when explicitly
>> > enabled.  Or we could have clients try it with, and in the event of
>> > an
>> > error, retry without.  Neither is ideal, but both are possible.
>>
>> Right. This ends up being the same difficulty that the v2 protocol
>> encountered; how do you figure out what you can speak without
>> resorting
>> to expensive fallbacks, when do you flip the switch, do you remember
>> the
>> protocol you used last time with this server, etc.
>
> Right.
>
> [moved]
>> > If I read this code correctly, git-over-ssh will pass through
>> > arbitrary
>> > arguments.  So this should be trivial.
>>
>> It does if you are ssh-ing to a real shell-level account on the
>> server,
>> but if you are using git-shell or some other wrapper to restrict
>> clients
>> from running arbitrary commands, it will likely reject it.
>
> Oh, I see how I was mis-reading shell.c.  Oops.
> [/moved]
>
>
>> Which isn't to say it's necessarily a bad thing. Maybe the path
>> forward
>> instead of v2 is to shoe-horn this data into the pre-protocol
>> conversation, and go from there. The protocol accepts that "somehow"
>> it
>> got some extra data from the transport layer, and acts on its
>> uniformly.
>
> OK, so it seems like only HTTP (and non-git-shell-git://) allow backwar
> ds-compatible optional pre-protocol messages.  So we don't have good
> options; we only have bad ones.  We have to decide which particular
> kind of badness we're willing to accept, and to what degree we care
> about extensibility.  As I see it, the badness options are (in no
> particular order):
>
> 1. Nothing changes.
> 2. HTTP grows more extensions; other protocols stagnate.
> 3. HTTP grows extensions; other protocols get extensions but:
>a. only use them on explicit client configuration or
>b. try/fail/remember per-remote
> 4. A backwards-incompatible protocol v2 is introduced, which
>hits alternate endpoints (with the same a/b as above).  This is
>different from 3. in that protocol v2 is explicitly designed around
>a capabilities negotiation phase rather than unilateral client-side
>decisions.
> 5. Think of another way to make fetch performant with many refs, and
> defer the extension decision.

I'd prefer 2,3,4 over 1,5.

Speaking about 2,3,4:

Maybe we can do a mix of 2 and 4:

   1) HTTP grows more extensions; other protocols stagnate for now.
   2) Come up with a backwards-incompatible protocol v2, foccussed on
   capabilities negotiation phase, hitting alternative end points
   (non http only, or rather a subset of protocols only)
3) if HTTP sees the benefits of the native protocol v2, we may switch
HTTP, too

(in time order of execution. Each point is decoupled from the others and may
be done by different people at different times.)


>
> --
> 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 0/2] git-p4: support python3 in the tests

[Junio C Hamano]

Luke Diamand  writes:

> This patchset updates the git-p4 tests so that they work with
> either Python2 or Python3.
>
> Note that this does *not* fix git-p4 to work with Python3 - that's
> a much bigger challenge.

We use Python outside p4 tests (e.g. remote-svn test), and the way
they invoke the interpreter is to say "$PYTHON_PATH" and avoid
saying "python" which picks whatever random version of Python
interpreter happens to be the first on $PATH.  Shouldn't the tests
touched by this series be doing the same?


>
> Luke Diamand (2):
>   git-p4 tests: cd to testdir before running python
>   git-p4 tests: work with python3 as well as python2
>
>  t/lib-git-p4.sh| 7 ---
>  t/t9802-git-p4-filetype.sh | 6 +++---
>  2 files changed, 7 insertions(+), 6 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 47/83] builtin/apply: move applying patches into apply_all_patches()

[Stefan Beller]

On Sun, Apr 24, 2016 at 6:33 AM, Christian Couder
 wrote:
> Signed-off-by: Christian Couder 

Up to this patch, have a
Reviewed-by: Stefan Beller 
in case you want to split the series in here (as indicated in the
cover letter, this was the last
patch rerolled, the next patches are new and may need more discussion).

I had some nits, but they cleared up in later patches.

Thanks,
Stefan

> ---
>  builtin/apply.c | 128 
> ++--
>  1 file changed, 69 insertions(+), 59 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 94bf120..787426f 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -4720,13 +4720,79 @@ static void check_apply_state(struct apply_state 
> *state, int force_apply)
> state->unsafe_paths = 0;
>  }
>
> -int cmd_apply(int argc, const char **argv, const char *prefix_)
> +static int apply_all_patches(struct apply_state *state,
> +int argc,
> +const char **argv,
> +int options)
>  {
> int i;
> int errs = 0;
> +   int read_stdin = 1;
> +
> +   for (i = 0; i < argc; i++) {
> +   const char *arg = argv[i];
> +   int fd;
> +
> +   if (!strcmp(arg, "-")) {
> +   errs |= apply_patch(state, 0, "", options);
> +   read_stdin = 0;
> +   continue;
> +   } else if (0 < state->prefix_length)
> +   arg = prefix_filename(state->prefix,
> + state->prefix_length,
> + arg);
> +
> +   fd = open(arg, O_RDONLY);
> +   if (fd < 0)
> +   die_errno(_("can't open patch '%s'"), arg);
> +   read_stdin = 0;
> +   set_default_whitespace_mode(state);
> +   errs |= apply_patch(state, fd, arg, options);
> +   close(fd);
> +   }
> +   set_default_whitespace_mode(state);
> +   if (read_stdin)
> +   errs |= apply_patch(state, 0, "", options);
> +
> +   if (state->whitespace_error) {
> +   if (state->squelch_whitespace_errors &&
> +   state->squelch_whitespace_errors < 
> state->whitespace_error) {
> +   int squelched =
> +   state->whitespace_error - 
> state->squelch_whitespace_errors;
> +   warning(Q_("squelched %d whitespace error",
> +  "squelched %d whitespace errors",
> +  squelched),
> +   squelched);
> +   }
> +   if (state->ws_error_action == die_on_ws_error)
> +   die(Q_("%d line adds whitespace errors.",
> +  "%d lines add whitespace errors.",
> +  state->whitespace_error),
> +   state->whitespace_error);
> +   if (state->applied_after_fixing_ws && state->apply)
> +   warning("%d line%s applied after"
> +   " fixing whitespace errors.",
> +   state->applied_after_fixing_ws,
> +   state->applied_after_fixing_ws == 1 ? "" : 
> "s");
> +   else if (state->whitespace_error)
> +   warning(Q_("%d line adds whitespace errors.",
> +  "%d lines add whitespace errors.",
> +  state->whitespace_error),
> +   state->whitespace_error);
> +   }
> +
> +   if (state->update_index) {
> +   if (write_locked_index(_index, _file, COMMIT_LOCK))
> +   die(_("Unable to write new index file"));
> +   }
> +
> +   return !!errs;
> +}
> +
> +int cmd_apply(int argc, const char **argv, const char *prefix_)
> +{
> int force_apply = 0;
> int options = 0;
> -   int read_stdin = 1;
> struct apply_state state;
>
> struct option builtin_apply_options[] = {
> @@ -4805,61 +4871,5 @@ int cmd_apply(int argc, const char **argv, const char 
> *prefix_)
>
> check_apply_state(, force_apply);
>
> -   for (i = 0; i < argc; i++) {
> -   const char *arg = argv[i];
> -   int fd;
> -
> -   if (!strcmp(arg, "-")) {
> -   errs |= apply_patch(, 0, "", options);
> -   read_stdin = 0;
> -   continue;
> -   } else if (0 < state.prefix_length)
> -   arg = prefix_filename(state.prefix,
> - state.prefix_length,
> - arg);
> -
> -  

Re: [PATCH 33/83] builtin/apply: move 'root' global into 'struct apply_state'

[Stefan Beller]

>
> Eventually we want to have some sort of `init_apply_state` function or
> a define which has all the values, I guess?
>
> Compare for example to sliding_window.h, where
> we have
>
> struct sliding_view {
> ...
> struct strbuf buf;
> };
>
> #define SLIDING_VIEW_INIT(..., STRBUF_INIT }

Nevermind, just read patch
"builtin/apply: move 'state' init into init_apply_state()"
--
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] remote.c: spell __attribute__ correctly

[Philip Oakley]

From: "Jeff King" 

On Mon, Apr 25, 2016 at 05:10:30PM -0400, Jeff King wrote:


It should be handled in git-compat-util.h, which is included by cache.h,
which is included by remote.c.

There we have:

  #ifndef __GNUC__
  #ifndef __attribute__
  #define __attribute__(x)
  #endif
  #endif

which should make it a noop on compilers which don't know about it. Is
VS (or another file) setting __GNUC__?


Of course it helps if we spell the name right...

-- >8 --
Subject: remote.c: spell __attribute__ correctly

We want to tell the compiler that error_buf() uses
printf()-style arguments via the __attribute__ mechanism,
but the original commit (3a429d0), forgot the trailing "__".
This happens to work with real GNUC-compatible compilers
like gcc and clang, but confuses our fallback macro in
git-compat-util.h, which only matches the official name (and
thus the build fails on compilers like Visual Studio).

Reported-by: Philip Oakley 
Signed-off-by: Jeff King 
---
remote.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index 28fd676..ddc4f8f 100644
--- a/remote.c
+++ b/remote.c
@@ -1660,7 +1660,7 @@ int branch_merge_matches(struct branch *branch,
 return refname_match(branch->merge[i]->src, refname);
}

-__attribute((format (printf,2,3)))
+__attribute__((format (printf,2,3)))
static const char *error_buf(struct strbuf *err, const char *fmt, ...)
{
 if (err) {
--


Thanks for clarifying that (sorry about the crossed emails). The compile is 
now looking good.

I'm just left with some unresolved external symbol link errors now.

The same naming issue in compat/regex/regcomp.c, compat/regex/regexec.c, 
compat/regex/regex_internal.c and compat/regex/regex_internal.h  was 
probably what lead me astray...


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 33/83] builtin/apply: move 'root' global into 'struct apply_state'

[Stefan Beller]

On Sun, Apr 24, 2016 at 6:33 AM, Christian Couder
 wrote:
> Signed-off-by: Christian Couder 
> ---
>  builtin/apply.c | 82 
> ++---
>  1 file changed, 49 insertions(+), 33 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index fecdb66..209a1b4 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -73,6 +73,8 @@ struct apply_state {
>
> struct string_list limit_by_name;
> int has_include;
> +
> +   struct strbuf root;
>  };
>
>  static int newfd = -1;
> @@ -98,8 +100,6 @@ static enum ws_ignore {
>  } ws_ignore_action = ignore_ws_none;
>
>
> -static struct strbuf root = STRBUF_INIT;
> -
>  static void parse_whitespace_option(const char *option)
>  {
> if (!option) {
> @@ -489,7 +489,10 @@ static char *squash_slash(char *name)
> return name;
>  }
>
> -static char *find_name_gnu(const char *line, const char *def, int p_value)
> +static char *find_name_gnu(struct apply_state *state,
> +  const char *line,
> +  const char *def,
> +  int p_value)
>  {
> struct strbuf name = STRBUF_INIT;
> char *cp;
> @@ -513,8 +516,8 @@ static char *find_name_gnu(const char *line, const char 
> *def, int p_value)
> }
>
> strbuf_remove(, 0, cp - name.buf);
> -   if (root.len)
> -   strbuf_insert(, 0, root.buf, root.len);
> +   if (state->root.len)
> +   strbuf_insert(, 0, state->root.buf, state->root.len);
> return squash_slash(strbuf_detach(, NULL));
>  }
>
> @@ -677,8 +680,12 @@ static size_t diff_timestamp_len(const char *line, 
> size_t len)
> return line + len - end;
>  }
>
> -static char *find_name_common(const char *line, const char *def,
> - int p_value, const char *end, int terminate)
> +static char *find_name_common(struct apply_state *state,
> + const char *line,
> + const char *def,
> + int p_value,
> + const char *end,
> + int terminate)
>  {
> int len;
> const char *start = NULL;
> @@ -716,32 +723,39 @@ static char *find_name_common(const char *line, const 
> char *def,
> return squash_slash(xstrdup(def));
> }
>
> -   if (root.len) {
> -   char *ret = xstrfmt("%s%.*s", root.buf, len, start);
> +   if (state->root.len) {
> +   char *ret = xstrfmt("%s%.*s", state->root.buf, len, start);
> return squash_slash(ret);
> }
>
> return squash_slash(xmemdupz(start, len));
>  }
>
> -static char *find_name(const char *line, char *def, int p_value, int 
> terminate)
> +static char *find_name(struct apply_state *state,
> +  const char *line,
> +  char *def,
> +  int p_value,
> +  int terminate)
>  {
> if (*line == '"') {
> -   char *name = find_name_gnu(line, def, p_value);
> +   char *name = find_name_gnu(state, line, def, p_value);
> if (name)
> return name;
> }
>
> -   return find_name_common(line, def, p_value, NULL, terminate);
> +   return find_name_common(state, line, def, p_value, NULL, terminate);
>  }
>
> -static char *find_name_traditional(const char *line, char *def, int p_value)
> +static char *find_name_traditional(struct apply_state *state,
> +  const char *line,
> +  char *def,
> +  int p_value)
>  {
> size_t len;
> size_t date_len;
>
> if (*line == '"') {
> -   char *name = find_name_gnu(line, def, p_value);
> +   char *name = find_name_gnu(state, line, def, p_value);
> if (name)
> return name;
> }
> @@ -749,10 +763,10 @@ static char *find_name_traditional(const char *line, 
> char *def, int p_value)
> len = strchrnul(line, '\n') - line;
> date_len = diff_timestamp_len(line, len);
> if (!date_len)
> -   return find_name_common(line, def, p_value, NULL, TERM_TAB);
> +   return find_name_common(state, line, def, p_value, NULL, 
> TERM_TAB);
> len -= date_len;
>
> -   return find_name_common(line, def, p_value, line + len, 0);
> +   return find_name_common(state, line, def, p_value, line + len, 0);
>  }
>
>  static int count_slashes(const char *cp)
> @@ -777,7 +791,7 @@ static int guess_p_value(struct apply_state *state, const 
> char *nameline)
>
> if (is_dev_null(nameline))
> return -1;
> -   name = find_name_traditional(nameline, NULL, 0);
> +   name = find_name_traditional(state, 

Re: [PATCH v5b 00/17] port branch.c to use ref-filter's printing options

[Junio C Hamano]

Karthik Nayak  writes:

> This is part of unification of the commands 'git tag -l, git branch -l
> and git for-each-ref'. This ports over branch.c to use ref-filter's
> printing options.
>
> Initially posted here: $(gmane/279226). It was decided that this series
> would follow up after refactoring ref-filter parsing mechanism, which
> is now merged into master (9606218b32344c5c756f7c29349d3845ef60b80c).
>
> v1 can be found here: $(gmane/288342)
> v2 can be found here: $(gmane/288863)
> v3 can be found here: $(gmane/290299)
> v4 can be found here: $(gmane/291106)
>
> Changes in this version (v5b):
> 1. Added the first patch of the series which was missing in v5.

2. Rebased on top of 'master', which includes
   jk/branch-shortening-funny-symrefs.

> Interdiff:
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index c9a2e5b..6847ac3 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -288,9 +288,11 @@ static int calc_maxwidth(struct ref_array *refs, int 
> remote_bonus)
>
> skip_prefix(it->refname, "refs/heads/", );
> skip_prefix(it->refname, "refs/remotes/", );
> -   if (it->kind == FILTER_REFS_DETACHED_HEAD)
> -   w = strlen(get_head_description());
> -   else
> +   if (it->kind == FILTER_REFS_DETACHED_HEAD) {
> +   char *head_desc = get_head_description();
> +   w = strlen(head_desc);
> +   free(head_desc);
> +   } else
> w = utf8_strwidth(desc);

Presumably w is computed here to be used later for some kind of
alignment?  It is curious why we can assume that head_desc does not
need utf8_strwidth() here.
--
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 18/83] builtin/apply: move 'numstat' global into 'struct apply_state'

[Stefan Beller]

On Sun, Apr 24, 2016 at 6:33 AM, Christian Couder
 wrote:
> Signed-off-by: Christian Couder 
> ---
>  builtin/apply.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index d90948a..16d78f9 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -36,6 +36,9 @@ struct apply_state {
> /* --stat does just a diffstat, and doesn't actually apply */
> int diffstat;
>
> +   /* --numstat does numeric diffstat, and doesn't actually apply */
> +   int numstat;
> +
> /*
>  *  --check turns on checking that the working tree matches the
>  *files that are being modified, but doesn't apply the patch
> @@ -51,14 +54,12 @@ struct apply_state {
>  };
>
>  /*
> - *  --numstat does numeric diffstat, and doesn't actually apply
>   *  --index-info shows the old and new index info for paths if available.
>   */
>  static int newfd = -1;
>
>  static int state_p_value = 1;
>  static int p_value_known;
> -static int numstat;
>  static int summary;
>  static int apply = 1;
>  static int no_add;
> @@ -4500,7 +4501,7 @@ static int apply_patch(struct apply_state *state,
> if (state->diffstat)
> stat_patch_list(list);
>
> -   if (numstat)
> +   if (state->numstat)
> numstat_patch_list(list);
>
> if (summary)
> @@ -4598,7 +4599,7 @@ int cmd_apply(int argc, const char **argv, const char 
> *prefix_)
> N_("instead of applying the patch, output diffstat 
> for the input")),
> OPT_NOOP_NOARG(0, "allow-binary-replacement"),
> OPT_NOOP_NOARG(0, "binary"),
> -   OPT_BOOL(0, "numstat", ,
> +   OPT_BOOL(0, "numstat", ,
> N_("show number of added and deleted lines in decimal 
> notation")),
> OPT_BOOL(0, "summary", ,
> N_("instead of applying the patch, output a summary 
> for the input")),
> @@ -4675,7 +4676,7 @@ int cmd_apply(int argc, const char **argv, const char 
> *prefix_)
> }
> if (state.apply_with_reject)
> apply = state.apply_verbosely = 1;
> -   if (!force_apply && (state.diffstat || numstat || summary || 
> state.check || fake_ancestor))
> +   if (!force_apply && (state.diffstat || state.numstat || summary || 
> state.check || fake_ancestor))

Mental note: This patch is just doing a mechanical conversion, so it
is fine to check for many "state.FOOs" here.

However later we may want to move this out to a static oneliner like:

static int really_apply(state *s) {
  return s->diffstat || s->numstat || ...;
}

(with a better name obviously)


> apply = 0;
> if (state.check_index && is_not_gitdir)
> die(_("--index outside a repository"));
> --
> 2.8.1.300.g5fed0c0
>
> --
> 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: definition for _attribute() in remote.c

[Jeff King]

On Mon, Apr 25, 2016 at 10:34:27PM +0100, Philip Oakley wrote:

> It's not the __attribute__ definition (a Gnu C ism), rather its the
> __attribute variant, which has a definition in regex_internal.h, and is used
> in the regex code. It's that one that's used in remote.c that I can't fathom
> (i.e. how it worked in normally)
> 
> regex_internal.h#L160-164
> #ifdef __GNUC__
> # define __attribute(arg) __attribute__ (arg)
> #else
> # define __attribute(arg)
> #endif
> 
> thus when the compilation get to remote.c#L1662 it fails to find that
> definition.
> 
> Should that line use the gnu extension name?

Yeah, sorry, see my followup response. We don't use "__attribute" at
all ourselves. The reference you found is in compat code that we pulled
in (so it does its own thing entirely), and the one in remote.c is just
a typo/think-o that happened to work on gcc, et al.

-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: [RFC/PATCH] Ordering of remotes for fetch --all

[Jeff King]

On Mon, Apr 25, 2016 at 11:15:05PM +0200, Guido Martínez wrote:

> I run a server with several git mirrors, that are updated every hour. On
> that same server, users clone those projects and work on them. We use
> the local mirrors to reduce network load: the users can fetch from the
> mirror first (to get most of the objects with zero network cost) and
> then fetch the real remote (to make sure they're completely up to date).
> 
> I would like this to be configurable in each git working directory,
> so users can just configure the order they want and then just do "git
> remote update".
> 
> I'm aware one can get this behavior by editing .git/config and
> ordering the remotes as one wishes, but I find that very hacky and not
> scripting-friendly.

You can also define your own ordered groups, like:

  $ git config remotes.foo "one two three"
  $ git fetch foo 2>&1 | grep ^Fetching
  Fetching one
  Fetching two
  Fetching three

That's not _exactly_ the same, because you can't give a partial ordering
of one high-priority remote and then say "all the rest, in whatever
order you want", because there's no way to say "all the rest".

You _can_ say:

  git config remotes.foo "high-priority --all"

but the final "--all" will fetch from high-priority again. An
alternative feature would be to teach remotes.* groups to cull
duplicates, if that's not acceptable.

I don't have a strong opinion against your approach, though. Just
exploring alternatives.

-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: definition for _attribute() in remote.c

[Philip Oakley]

From: "Jeff King" 

On Mon, Apr 25, 2016 at 10:02:38PM +0100, Philip Oakley wrote:


I'm looking at getting Git for Windows to compile via Visual Studio
(https://github.com/git-for-windows/git/pull/256).

However the use of __attribute() in remote.c at L1662
(https://github.com/git-for-windows/git/blob/master/remote.c#L1662) has 
got
me confused in that I can't see how the regular definition of 
__attribute()

is #included in this case. A definition is given in
git\compat\regex\regex_internal.h but doesn't appear to be on remote.c's
include path.

The line was introduced by 3a429d0 (remote.c: report specific errors from
branch_get_upstream, 2015-05-21) which appears to be later than the 
previous

MSVC testers had looked at.


It should be handled in git-compat-util.h, which is included by cache.h,
which is included by remote.c.

There we have:

 #ifndef __GNUC__
 #ifndef __attribute__
 #define __attribute__(x)
 #endif
 #endif

which should make it a noop on compilers which don't know about it. Is
VS (or another file) setting __GNUC__?


It's not the __attribute__ definition (a Gnu C ism), rather its the 
__attribute variant, which has a definition in regex_internal.h, and is used 
in the regex code. It's that one that's used in remote.c that I can't fathom 
(i.e. how it worked in normally)


regex_internal.h#L160-164
#ifdef __GNUC__
# define __attribute(arg) __attribute__ (arg)
#else
# define __attribute(arg)
#endif

thus when the compilation get to remote.c#L1662 it fails to find that 
definition.


Should that line use the gnu extension name?

--
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: [RFC] How to pass Git config command line instructions to Submodule commands?

[Jeff King]

On Mon, Apr 25, 2016 at 01:59:03PM -0700, Jacob Keller wrote:

> >> However, I noticed that git config command line instructions such as
> >> "-c filter.lfs.smudge=" are not passed to Git submodule operations. Thus
> >> this does not work as expected:
> >>
> >> git -c filter.lfs.smudge= -c filter.lfs.required=false clone 
> >> --recursive  
> >
> > I have cc'd Jacob Keller, who authored origin/jk/submodule-c-credential,
> > which does work in that area (deciding which config option to pass down
> > into the submodule commands).
> >
> 
> This is a tricky question. The problem is that some configurations are
> obviously not intended to go into the submodules, but determining how
> is somewhat troublesome. There was some discussion on this previous
> thread when we added support for credential options to pass through.

Right. I think it may be reasonable to pass through filter.* in the
whitelist.  They are not activated without a matching .gitattributes
entry in the repository (and people would generally configure them in
their user-level ~/.gitconfig for that reason).

It does mean that somebody would be stuck who really wanted to run the
smudge filter in their local repo, but for some reason not in the
subrepos. I am trying to think of a case in which that might be
security-relevant if you didn't trust the sub-repos[1]. But I really
don't see it. The filter is arbitrary code, but that's specified by the
user; we're just feeding it possibly untrusted blobs.

-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: [RFC] How to pass Git config command line instructions to Submodule commands?

[Jeff King]

On Mon, Apr 25, 2016 at 05:24:50PM -0400, Jeff King wrote:

> It does mean that somebody would be stuck who really wanted to run the
> smudge filter in their local repo, but for some reason not in the
> subrepos. I am trying to think of a case in which that might be
> security-relevant if you didn't trust the sub-repos[1]. But I really
> don't see it. The filter is arbitrary code, but that's specified by the
> user; we're just feeding it possibly untrusted blobs.

I forgot my [1], which was going to be: I wonder if there are any
interesting things you can do by feeding git-lfs untrusted content
(e.g., convincing it to hit arbitrary URLs). But I don't think so. The
URL is derived from the remote, and the LFS pointer files just contain
hashes.

That's all orthogonal to this thread anyway, though. People using LFS
generally have the config in ~/.gitconfig, so they run it for all repos,
trusted and untrusted.

-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] clone: add `--shallow-submodules` flag

[Stefan Beller]

On Mon, Apr 25, 2016 at 2:04 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>>   This replaces origin/sb/clone-shallow-passthru.
>> @@ -190,7 +190,11 @@ objects from the source repository into a pack in the 
>> cloned repository.
>>
>>  --depth ::
>>   Create a 'shallow' clone with a history truncated to the
>> - specified number of revisions.
>> + specified number of revisions. Implies `--single-branch` unless
>> + `--no-single-branch` is given to fetch the histories near the
>> + tips of all branches. This implies `--shallow-submodules`. If
>> + you want to have a shallow superproject clone, but full submodules,
>> + also pass `--no-shallow-submodules`.
>
> This is not wrong per-se but the early half of the new text seems to
> come from 28a1b569 (docs: clarify that passing --depth to git-clone
> implies --single-branch, 2016-01-06), which was merged to 'master'
> some time ago.
>
> I've resolved the conflicts coming from the duplicates, so no need
> to resend, but the resulting history would become misleading.  I am
> undecided if I should rebasing this on top of 85705cfb (Merge branch
> 'ss/clone-depth-single-doc', 2016-01-20) or later.

I could reroll with another variable name on top of 85705cfb?

>
>> diff --git a/t/t5614-clone-submodules.sh b/t/t5614-clone-submodules.sh
>> new file mode 100755
>> index 000..825f511
>> --- /dev/null
>> +++ b/t/t5614-clone-submodules.sh
>> @@ -0,0 +1,85 @@
>> +#!/bin/sh
>> +
>> +test_description='Test shallow cloning of repos with submodules'
>> +
>> +. ./test-lib.sh
>> +
>> +p=$(pwd)
>
> A single-letter lower-case global that needs to stay constant for
> the entire file looks like a ticking time bomb screaming to be
> broken by future updates.

I agree. How about `currentdir`, `testdir` or `testtop` instead?
That is slightly longer than `D`, `here` or `top`, but is slightly more
informative. $TRASH would also work for me.

Talking about time bombs: I see there are many tests in single files
(e.g. 7412 has ~80 tests IIRC and adding a test in there I often spend
more time figuring out the current state of the testing directory rather than
writing the test.

So I'd rather see more testing files with fewer tests rather than a few
files with all the tests for one topic. (e.g. these tests could have gone
into the clone tests, or the basic submodule tests, but instead I chose
a new file to test this.)


> I see $D used for this purpose in many
> scripts, $here and $top in some, and $TRASH in yet some others.
> Perhaps $D may be more appropriate if we wanted to keep this
> ultra-short-and-cryptic while mimicking existing ones.

I was not keen on being ultrashort, rather I was toying around
to drop the patch introducing the --no-local option.

>
>> +test_expect_success 'setup' '
>> + git checkout -b master &&
>> + test_commit commit1 &&
>> + test_commit commit2 &&
>> + mkdir sub &&
>> + (
>> + cd sub &&
>> + git init &&
>> + test_commit subcommit1 &&
>> + test_commit subcommit2 &&
>> + test_commit subcommit3
>> + ) &&
>> + git submodule add "file://$p/sub" sub &&
>> + git commit -m "add submodule"
>> +'
>> +
>> +test_expect_success 'nonshallow clone implies nonshallow submodule' '
>> + test_when_finished "rm -rf super_clone" &&
>> + git clone --recurse-submodules "file://$p/." super_clone &&
>> + (
>> + cd super_clone &&
>> + git log --oneline >lines &&
>> + test_line_count = 3 lines
>> + ) &&
>> + (
>> + cd super_clone/sub &&
>> + git log --oneline >lines &&
>> + test_line_count = 3 lines
>> + )
>> +'
>> +
>> +test_expect_success 'shallow clone implies shallow submodule' '
>> + test_when_finished "rm -rf super_clone" &&
>> + git clone --recurse-submodules --depth 2 "file://$p/." super_clone &&
>> + (
>> + cd super_clone &&
>> + git log --oneline >lines &&
>> + test_line_count = 2 lines
>> + ) &&
>> + (
>> + cd super_clone/sub &&
>> + git log --oneline >lines &&
>> + test_line_count = 1 lines
>> + )
>> +'
>> +
>> +test_expect_success 'shallow clone with non shallow submodule' '
>> + test_when_finished "rm -rf super_clone" &&
>> + git clone --recurse-submodules --depth 2 --no-shallow-submodules 
>> "file://$p/." super_clone &&
>> + (
>> + cd super_clone &&
>> + git log --oneline >lines &&
>> + test_line_count = 2 lines
>> + ) &&
>> + (
>> + cd super_clone/sub &&
>> + git log --oneline >lines &&
>> + test_line_count = 3 lines
>> + )
>> +'
>> +
>> +test_expect_success 'non shallow clone with shallow submodule' '
>> + test_when_finished "rm -rf super_clone" &&
>> + git clone --recurse-submodules --no-local 

[PATCH v3] fast-import: implement unpack limit

[Eric Wong]

With many incremental imports, small packs become highly
inefficient due to the need to readdir scan and load many
indices to locate even a single object.  Frequent repacking and
consolidation may be prohibitively expensive in terms of disk
I/O, especially in large repositories where the initial packs
were aggressively optimized and marked with .keep files.

In those cases, users may be better served with loose objects
and relying on "git gc --auto".

This changes the default behavior of fast-import for small
imports found in test cases, so adjustments to t9300 were
necessary.

Signed-off-by: Eric Wong 
---
  v2 changes: implemented as a git-config directive
  v3 changes:
  * honor --quiet for fast-import as -q for unpack-objects
  * minor style adjustment when calling loosen_small_pack

 Documentation/config.txt|  9 +++
 Documentation/git-fast-import.txt   |  2 ++
 fast-import.c   | 32 +
 t/t9300-fast-import.sh  |  2 ++
 t/t9302-fast-import-unpack-limit.sh | 48 +
 5 files changed, 93 insertions(+)
 create mode 100755 t/t9302-fast-import-unpack-limit.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 42d2b50..3d8bc97 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1154,6 +1154,15 @@ difftool..cmd::
 difftool.prompt::
Prompt before each invocation of the diff tool.
 
+fastimport.unpackLimit::
+   If the number of objects imported by linkgit:git-fast-import[1]
+   is below this limit, then the objects will be unpacked into
+   loose object files.  However if the number of imported objects
+   equals or exceeds this limit then the pack will be stored as a
+   pack.  Storing the pack from a fast-import can make the import
+   operation complete faster, especially on slow filesystems.  If
+   not set, the value of `transfer.unpackLimit` is used instead.
+
 fetch.recurseSubmodules::
This option can be either set to a boolean value or to 'on-demand'.
Setting it to a boolean changes the behavior of fetch and pull to
diff --git a/Documentation/git-fast-import.txt 
b/Documentation/git-fast-import.txt
index 66910aa..644df99 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -136,6 +136,8 @@ Performance and Compression Tuning
Maximum size of each output packfile.
The default is unlimited.
 
+fastimport.unpackLimit::
+   See linkgit:git-config[1]
 
 Performance
 ---
diff --git a/fast-import.c b/fast-import.c
index 9fc7093..4fb464c 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -166,6 +166,7 @@ Format of STDIN stream:
 #include "quote.h"
 #include "exec_cmd.h"
 #include "dir.h"
+#include "run-command.h"
 
 #define PACK_ID_BITS 16
 #define MAX_PACK_ID ((1<pack_fd, 0, SEEK_SET) < 0)
+   die_errno("Failed seeking to start of '%s'", p->pack_name);
+
+   unpack.in = p->pack_fd;
+   unpack.git_cmd = 1;
+   unpack.stdout_to_stderr = 1;
+   argv_array_push(, "unpack-objects");
+   if (!show_stats)
+   argv_array_push(, "-q");
+
+   return run_command();
+}
+
 static void end_packfile(void)
 {
static int running;
@@ -972,6 +991,12 @@ static void end_packfile(void)
fixup_pack_header_footer(pack_data->pack_fd, pack_data->sha1,
pack_data->pack_name, object_count,
cur_pack_sha1, pack_size);
+
+   if (object_count <= unpack_limit) {
+   if (!loosen_small_pack(pack_data))
+   goto discard_pack;
+   }
+
close(pack_data->pack_fd);
idx_name = keep_pack(create_index());
 
@@ -1002,6 +1027,7 @@ static void end_packfile(void)
pack_id++;
}
else {
+discard_pack:
close(pack_data->pack_fd);
unlink_or_warn(pack_data->pack_name);
}
@@ -3317,6 +3343,7 @@ static void parse_option(const char *option)
 static void git_pack_config(void)
 {
int indexversion_value;
+   int limit;
unsigned long packsizelimit_value;
 
if (!git_config_get_ulong("pack.depth", _depth)) {
@@ -3341,6 +3368,11 @@ static void git_pack_config(void)
if (!git_config_get_ulong("pack.packsizelimit", _value))
   

[PATCH] remote.c: spell __attribute__ correctly

[Jeff King]

On Mon, Apr 25, 2016 at 05:10:30PM -0400, Jeff King wrote:

> It should be handled in git-compat-util.h, which is included by cache.h,
> which is included by remote.c.
> 
> There we have:
> 
>   #ifndef __GNUC__
>   #ifndef __attribute__
>   #define __attribute__(x)
>   #endif
>   #endif
> 
> which should make it a noop on compilers which don't know about it. Is
> VS (or another file) setting __GNUC__?

Of course it helps if we spell the name right...

-- >8 --
Subject: remote.c: spell __attribute__ correctly

We want to tell the compiler that error_buf() uses
printf()-style arguments via the __attribute__ mechanism,
but the original commit (3a429d0), forgot the trailing "__".
This happens to work with real GNUC-compatible compilers
like gcc and clang, but confuses our fallback macro in
git-compat-util.h, which only matches the official name (and
thus the build fails on compilers like Visual Studio).

Reported-by: Philip Oakley 
Signed-off-by: Jeff King 
---
 remote.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index 28fd676..ddc4f8f 100644
--- a/remote.c
+++ b/remote.c
@@ -1660,7 +1660,7 @@ int branch_merge_matches(struct branch *branch,
return refname_match(branch->merge[i]->src, refname);
 }
 
-__attribute((format (printf,2,3)))
+__attribute__((format (printf,2,3)))
 static const char *error_buf(struct strbuf *err, const char *fmt, ...)
 {
if (err) {
-- 
2.8.1.562.gc7e1b3c

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

[RFC/PATCH] remote: add a fetching priority to each remote

[Guido Martínez]

Add an int that allows for a way of setting a fetch order for remotes,
mainly for the use case of "git remote update" which updates every
remote.

This way, users can set local mirrors of repositories first to quickly
get a bunch of objects, and later the upstream repo to make sure that
they pulled the latest commit.

Signed-off-by: Guido Martínez 
---
 Documentation/config.txt |  5 +
 builtin/fetch.c  |  2 +-
 remote.c | 43 +++
 remote.h |  2 ++
 4 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 42d2b50..5ca199c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2489,6 +2489,11 @@ remote..fetch::
The default set of "refspec" for linkgit:git-fetch[1]. See
linkgit:git-fetch[1].
 
+remote..fetchprio::
+   Set a priority for fetching this remote, to allow you to set
+   a custom order when doing "git fetch --all" (thus also when
+   running "git remote update"). Default value is 50.
+
 remote..push::
The default set of "refspec" for linkgit:git-push[1]. See
linkgit:git-push[1].
diff --git a/builtin/fetch.c b/builtin/fetch.c
index f8455bd..44f42bf 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1187,7 +1187,7 @@ int cmd_fetch(int argc, const char **argv, const char 
*prefix)
die(_("fetch --all does not take a repository 
argument"));
else if (argc > 1)
die(_("fetch --all does not make sense with refspecs"));
-   (void) for_each_remote(get_one_remote_for_fetch, );
+   (void) for_each_sorted_remote(get_one_remote_for_fetch, );
result = fetch_multiple();
} else if (argc == 0) {
/* No arguments -- use default remote */
diff --git a/remote.c b/remote.c
index 28fd676..3b9f20a 100644
--- a/remote.c
+++ b/remote.c
@@ -168,6 +168,7 @@ static struct remote *make_remote(const char *name, int len)
ALLOC_GROW(remotes, remotes_nr + 1, remotes_alloc);
remotes[remotes_nr++] = ret;
ret->name = xstrndup(name, len);
+   ret->fetch_prio = 50;
 
hashmap_entry_init(ret, lookup_entry.hash);
replaced = hashmap_put(_hash, ret);
@@ -375,6 +376,8 @@ static int handle_config(const char *key, const char 
*value, void *cb)
remote->mirror = git_config_bool(key, value);
else if (!strcmp(subkey, "skipdefaultupdate"))
remote->skip_default_update = git_config_bool(key, value);
+   else if (!strcmp(subkey, "fetchprio"))
+   remote->fetch_prio = git_config_int(key, value);
else if (!strcmp(subkey, "skipfetchall"))
remote->skip_default_update = git_config_bool(key, value);
else if (!strcmp(subkey, "prune"))
@@ -719,12 +722,12 @@ int remote_is_configured(struct remote *remote)
return remote && remote->origin;
 }
 
-int for_each_remote(each_remote_fn fn, void *priv)
+static int for_each_remote_do(struct remote **rlist, int len,
+ each_remote_fn fn, void *priv)
 {
int i, result = 0;
-   read_config();
-   for (i = 0; i < remotes_nr && !result; i++) {
-   struct remote *r = remotes[i];
+   for (i = 0; i < len && !result; i++) {
+   struct remote *r = rlist[i];
if (!r)
continue;
if (!r->fetch)
@@ -738,6 +741,38 @@ int for_each_remote(each_remote_fn fn, void *priv)
return result;
 }
 
+int for_each_remote(each_remote_fn fn, void *priv)
+{
+   read_config();
+   return for_each_remote_do(remotes, remotes_nr, fn, priv);
+}
+
+int compare_fetch_prio(const void *p, const void *q)
+{
+   const struct remote *pp = *(struct remote**)p;
+   const struct remote *qq = *(struct remote**)q;
+
+   return pp->fetch_prio - qq->fetch_prio;
+}
+
+int for_each_sorted_remote(each_remote_fn fn, void *priv)
+{
+   struct remote **sr;
+   int i, rc;
+
+   read_config();
+
+   sr = xmalloc(sizeof (struct remote*) * remotes_nr);
+   for (i = 0; i < remotes_nr; i++)
+   sr[i] = remotes[i];
+
+   qsort(sr, remotes_nr, sizeof (struct remote*), compare_fetch_prio);
+
+   rc = for_each_remote_do(sr, remotes_nr, fn, priv);
+   free(sr);
+   return rc;
+}
+
 static void handle_duplicate(struct ref *ref1, struct ref *ref2)
 {
if (strcmp(ref1->name, ref2->name)) {
diff --git a/remote.h b/remote.h
index c21fd37..09b48e4 100644
--- a/remote.h
+++ b/remote.h
@@ -47,6 +47,7 @@ struct remote {
int skip_default_update;
int mirror;
int prune;
+   int fetch_prio;
 
const char *receivepack;
const char *uploadpack;
@@ -64,6 +65,7 @@ int remote_is_configured(struct remote *remote);
 
 typedef int each_remote_fn(struct remote *remote, 

[RFC/PATCH] Ordering of remotes for fetch --all

[Guido Martínez]

Hi all,

I run a server with several git mirrors, that are updated every hour. On
that same server, users clone those projects and work on them. We use
the local mirrors to reduce network load: the users can fetch from the
mirror first (to get most of the objects with zero network cost) and
then fetch the real remote (to make sure they're completely up to date).

I would like this to be configurable in each git working directory,
so users can just configure the order they want and then just do "git
remote update".

I'm aware one can get this behavior by editing .git/config and
ordering the remotes as one wishes, but I find that very hacky and not
scripting-friendly.

This patch introduces a fetch priority for each remote, at a default of
50 and modifiable via git config. This new order will only matter when
doing fetch --all.

Do you think this is a useful feature? Hopefully you don't consider this
as just noise :)

(As a side note: for ordering the remotes a stable sort would be best,
to have the least impact possible on current behavior. I believe
git_qsort is stable but a confirmation would be nice.)

Thanks!
Guido

Guido Martínez (1):
  remote: add a fetching priority to each remote

 Documentation/config.txt |  5 +
 builtin/fetch.c  |  2 +-
 remote.c | 43 +++
 remote.h |  2 ++
 4 files changed, 47 insertions(+), 5 deletions(-)

-- 
2.8.1.281.g0994585

--
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: definition for _attribute() in remote.c

[Jeff King]

On Mon, Apr 25, 2016 at 10:02:38PM +0100, Philip Oakley wrote:

> I'm looking at getting Git for Windows to compile via Visual Studio
> (https://github.com/git-for-windows/git/pull/256).
> 
> However the use of __attribute() in remote.c at L1662
> (https://github.com/git-for-windows/git/blob/master/remote.c#L1662) has got
> me confused in that I can't see how the regular definition of __attribute()
> is #included in this case. A definition is given in
> git\compat\regex\regex_internal.h but doesn't appear to be on remote.c's
> include path.
> 
> The line was introduced by 3a429d0 (remote.c: report specific errors from
> branch_get_upstream, 2015-05-21) which appears to be later than the previous
> MSVC testers had looked at.

It should be handled in git-compat-util.h, which is included by cache.h,
which is included by remote.c.

There we have:

  #ifndef __GNUC__
  #ifndef __attribute__
  #define __attribute__(x)
  #endif
  #endif

which should make it a noop on compilers which don't know about it. Is
VS (or another file) setting __GNUC__?

-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] clone: add `--shallow-submodules` flag

[Junio C Hamano]

Stefan Beller  writes:

>   This replaces origin/sb/clone-shallow-passthru.
> @@ -190,7 +190,11 @@ objects from the source repository into a pack in the 
> cloned repository.
>  
>  --depth ::
>   Create a 'shallow' clone with a history truncated to the
> - specified number of revisions.
> + specified number of revisions. Implies `--single-branch` unless
> + `--no-single-branch` is given to fetch the histories near the
> + tips of all branches. This implies `--shallow-submodules`. If
> + you want to have a shallow superproject clone, but full submodules,
> + also pass `--no-shallow-submodules`.

This is not wrong per-se but the early half of the new text seems to
come from 28a1b569 (docs: clarify that passing --depth to git-clone
implies --single-branch, 2016-01-06), which was merged to 'master'
some time ago.

I've resolved the conflicts coming from the duplicates, so no need
to resend, but the resulting history would become misleading.  I am
undecided if I should rebasing this on top of 85705cfb (Merge branch
'ss/clone-depth-single-doc', 2016-01-20) or later.

> diff --git a/t/t5614-clone-submodules.sh b/t/t5614-clone-submodules.sh
> new file mode 100755
> index 000..825f511
> --- /dev/null
> +++ b/t/t5614-clone-submodules.sh
> @@ -0,0 +1,85 @@
> +#!/bin/sh
> +
> +test_description='Test shallow cloning of repos with submodules'
> +
> +. ./test-lib.sh
> +
> +p=$(pwd)

A single-letter lower-case global that needs to stay constant for
the entire file looks like a ticking time bomb screaming to be
broken by future updates.  I see $D used for this purpose in many
scripts, $here and $top in some, and $TRASH in yet some others.
Perhaps $D may be more appropriate if we wanted to keep this
ultra-short-and-cryptic while mimicking existing ones.

> +test_expect_success 'setup' '
> + git checkout -b master &&
> + test_commit commit1 &&
> + test_commit commit2 &&
> + mkdir sub &&
> + (
> + cd sub &&
> + git init &&
> + test_commit subcommit1 &&
> + test_commit subcommit2 &&
> + test_commit subcommit3
> + ) &&
> + git submodule add "file://$p/sub" sub &&
> + git commit -m "add submodule"
> +'
> +
> +test_expect_success 'nonshallow clone implies nonshallow submodule' '
> + test_when_finished "rm -rf super_clone" &&
> + git clone --recurse-submodules "file://$p/." super_clone &&
> + (
> + cd super_clone &&
> + git log --oneline >lines &&
> + test_line_count = 3 lines
> + ) &&
> + (
> + cd super_clone/sub &&
> + git log --oneline >lines &&
> + test_line_count = 3 lines
> + )
> +'
> +
> +test_expect_success 'shallow clone implies shallow submodule' '
> + test_when_finished "rm -rf super_clone" &&
> + git clone --recurse-submodules --depth 2 "file://$p/." super_clone &&
> + (
> + cd super_clone &&
> + git log --oneline >lines &&
> + test_line_count = 2 lines
> + ) &&
> + (
> + cd super_clone/sub &&
> + git log --oneline >lines &&
> + test_line_count = 1 lines
> + )
> +'
> +
> +test_expect_success 'shallow clone with non shallow submodule' '
> + test_when_finished "rm -rf super_clone" &&
> + git clone --recurse-submodules --depth 2 --no-shallow-submodules 
> "file://$p/." super_clone &&
> + (
> + cd super_clone &&
> + git log --oneline >lines &&
> + test_line_count = 2 lines
> + ) &&
> + (
> + cd super_clone/sub &&
> + git log --oneline >lines &&
> + test_line_count = 3 lines
> + )
> +'
> +
> +test_expect_success 'non shallow clone with shallow submodule' '
> + test_when_finished "rm -rf super_clone" &&
> + git clone --recurse-submodules --no-local --shallow-submodules 
> "file://$p/." super_clone &&
> + (
> + cd super_clone &&
> + git log --oneline >lines &&
> + test_line_count = 3 lines
> + ) &&
> + (
> + cd super_clone/sub &&
> + git log --oneline >lines &&
> + test_line_count = 1 lines
> + )
> +'
> +
> +test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

definition for _attribute() in remote.c

[Philip Oakley]

Hi,

I'm looking at getting Git for Windows to compile via Visual Studio 
(https://github.com/git-for-windows/git/pull/256).


However the use of __attribute() in remote.c at L1662 
(https://github.com/git-for-windows/git/blob/master/remote.c#L1662) has got 
me confused in that I can't see how the regular definition of __attribute() 
is #included in this case. A definition is given in 
git\compat\regex\regex_internal.h but doesn't appear to be on remote.c's 
include path.


The line was introduced by 3a429d0 (remote.c: report specific errors from 
branch_get_upstream, 2015-05-21) which appears to be later than the previous 
MSVC testers had looked at.


Any guidance on what is happening in this case would be welcomed as this is 
the last compile error I'm seeing.


--

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: [RFC] How to pass Git config command line instructions to Submodule commands?

[Jacob Keller]

On Mon, Apr 25, 2016 at 10:02 AM, Stefan Beller  wrote:
> On Mon, Apr 25, 2016 at 3:39 AM, Lars Schneider
>  wrote:
>> Hi,
>>
>> a few folks from the Git LFS project and I try to make cloning of 
>> repositories
>> with a lot of LFS files faster.
>>
>> The core problem is that Git LFS uses a Git smudge filter to replace LFS
>> pointers with the actual file content. Right now, a smudge filter can only
>> be executed on an individual file which makes the operation slow for many
>> files [1].
>>
>> We solved this issue by temporarily disabling the smudge filter for the clone
>> command via Git config (optimized in 1a8630 [2]):
>>
>> git -c filter.lfs.smudge= -c filter.lfs.required=false clone  
>>
>> Afterwards Git LFS runs a special command to download and replace all LFS
>> content in bulk [3]. This works great for LFS repositories.
>>
>> However, I noticed that git config command line instructions such as
>> "-c filter.lfs.smudge=" are not passed to Git submodule operations. Thus
>> this does not work as expected:
>>
>> git -c filter.lfs.smudge= -c filter.lfs.required=false clone --recursive 
>>  
>
> I have cc'd Jacob Keller, who authored origin/jk/submodule-c-credential,
> which does work in that area (deciding which config option to pass down
> into the submodule commands).
>

This is a tricky question. The problem is that some configurations are
obviously not intended to go into the submodules, but determining how
is somewhat troublesome. There was some discussion on this previous
thread when we added support for credential options to pass through.

Thanks,
Jake
--
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 06/15] index-helper: add --detach

[David Turner]

On Wed, 2016-04-20 at 06:50 +0700, Duy Nguyen wrote:
> On Wed, Apr 20, 2016 at 6:28 AM, David Turner <
> dtur...@twopensource.com> wrote:
> > @@ -317,6 +320,8 @@ int main(int argc, char **argv)
> > if (fd < 0)
> > die_errno(_("could not set up index-helper
> > socket"));
> > 
> > +   if (detach && daemonize())
> > +   die_errno(_("unable to detach"));
> 
> At the least, I think we need to redirect both stdout and stderr to a
> file, so we can catch errors. The watchman patch uses warning() to
> report errors, for example. And there is always a chance of die().
> 
> Then we need to report the errors back. I faced the same problem with
> daemonizing git-gc, but I'm not sure if we can do exactly the same
> here like in commit 329e6e8 (gc: save log from daemonized gc --auto
> and print it next time - 2015-09-19)

On reflection, I decided not to do a complicated system for replaying
warnings.  Here are my reasons why:

1. A user will not be expecting to see warnings from previous git
commands.  

2. It's not super-important that users see most of these warnings.  GC
is different because it's critical to the health of a repository so it
really matters that users learn about issues.  

3. It involves many complications to the (presently very simple)
protocol. We would need to distinguish between messages, warnings, and
previous-session warnings.

4. There are only a few cases where errors will happen, and none seem
to be exciting to clients.

Instead, I'll just log errors.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] l10n updates for maint branch (2.8.2)

[Junio C Hamano]

Jiang Xin  writes:

> Please pull this update to the maint branch.  It should have been merged to
> Git 2.8.0, but I was busy these weeks and forgot to check my private mailbox.

Thanks, will 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: [PATCH v2] hooks: Add ability to specify where the hook directory is

[Junio C Hamano]

Ævar Arnfjörð Bjarmason   writes:

> +core.hooksPath::
> + By default Git will look for your hooks in the
> + '$GIT_DIR/hooks' directory. Set this to different path,
> + e.g. '/etc/git/hooks', and Git will try to find your hooks in
> + that directory, e.g. '/etc/git/hooks/pre-receive' instead of
> + in '$GIT_DIR/hooks/pre-receive'.
> ++
> +The path can either be absolute or relative. In the latter case see
> +the discussion in the "DESCRIPTION" section of linkgit:githooks[5]
> +about what the relative path will be relative to.

... which does not seem to appear there, it seems?

>  DESCRIPTION
>  ---
>  
> -Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
> -trigger action at certain points. Hooks that don't have the executable
> -bit set are ignored.
> +Hooks are programs you can place in a hooks directory to trigger action
> +at certain points. Hooks that don't have the executable bit set are
> +ignored.
> +
> +By default the hooks directory is `$GIT_DIR/hooks`, but that can be
> +changed via the `core.hooksPath` configuration variable (see
> +linkgit:git-config[1]).

The section talks about what the cwd of the process that runs the
hook is, but it is not clear at all from these three lines in
core.hooksPath description above how the cwd of the process is
related with the directory the relative path will be relative to.

Unless the readers know that the implementation makes sure that the
process chdir'ed to that final directory before calling find_hook(),
that is.  And I do not think you want to assume that knowledge on
the side of the readers.

> diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh
> new file mode 100755
> index 000..31461aa
> --- /dev/null
> +++ b/t/t1350-config-hooks-path.sh
> @@ -0,0 +1,33 @@
> +#!/bin/sh
> +
> +test_description='Test the core.hooksPath configuration variable'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'set up a pre-commit hook in core.hooksPath' '
> + mkdir -p .git/custom-hooks .git/hooks &&
> + write_script .git/custom-hooks/pre-commit < +printf "%s" "." >>.git/PRE-COMMIT-HOOK-WAS-CALLED
> +EOF

Because there is no funny interpolation going on, it would help
readers to signal that fact by quoting the end-of-here-text marker.
And it makes the entire test block reads nicer if you indent the
body of hte here-text by prefixing the end-of-here-text marker with
a dash, i.e.

write_script .git/custom-hooks/pre-commit <<-\EOF &&
printf "%s" "." >>.git/PRE-COMMIT-HOOK-WAS-CALLED
EOF

> + cat >.git/hooks/pre-commit < + write_script .git/hooks/pre-commit &&
> +printf "%s" "SHOULD NOT BE CALLED" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
> +EOF
> + chmod +x .git/custom-hooks/pre-commit

You didn't want cat and chmod here?

> +'
> +
> +test_expect_success 'Check that various forms of specifying core.hooksPath 
> work' '
> + test_commit no_custom_hook &&
> + git config core.hooksPath .git/custom-hooks &&
> + test_commit have_custom_hook &&
> + git config core.hooksPath .git/custom-hooks/ &&
> + test_commit have_custom_hook_trailing_slash &&
> + git config core.hooksPath "$PWD/.git/custom-hooks" &&
> + test_commit have_custom_hook_abs_path &&
> + git config core.hooksPath "$PWD/.git/custom-hooks/" &&
> + test_commit have_custom_hook_abs_path_trailing_slash &&
> + printf "%s" "" >.git/PRE-COMMIT-HOOK-WAS-CALLED.expect &&
> + test_cmp .git/PRE-COMMIT-HOOK-WAS-CALLED.expect 
> .git/PRE-COMMIT-HOOK-WAS-CALLED
> +'
> +
> +test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1] travis-ci: build documentation

[Junio C Hamano]

Matthieu Moy  writes:

> Actually, this may also be a motivation to move anything non-trivial out
> of .travic.yml and to start using separate scripts (to avoid writting
> shell within a Yaml syntax).

Excellent suggestion.  I do not mind if we added a new directory at
the top-level of the tree to hold helper scripts for CI (perhaps
call it ci/ not travis/ so that people may later add helper scripts
for other CI systems of their liking if they wanted to).
--
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 v7 07/10] convert: unify the "auto" handling of CRLF

[Junio C Hamano]

tbo...@web.de writes:

> From: Torsten Bögershausen 
>
> Before this change,
> $ echo "* text=auto" >.gitattributes
> $ echo "* eol=crlf" >>.gitattributes
>
> would have the same effect as
> $ echo "* text" >.gitattributes
> $ git config core.eol crlf
>
> Since the 'eol' attribute had higher priority than 'text=auto', this may
> corrupt binary files and is not what users expect to happen.

Is the last "Since ..." continuation of the above two, i.e. "with
the current code, the first one (unfortunately) means the same as
the second one, BECAUSE ..."?  If so, s/Since/since/ (I needed to
read it twice to guess that is probably what you meant).

The above is a very good justification of the entire series,
illustrating why this is worth doing.

> Make the 'eol' attribute to obey 'text=auto', and now

s/to obey/obey/ (make X do Y, not make X to do Y); I'd probably
further do 's/obey/honor/' or even 's/obey/work better with/'

> $ echo "* text=auto" >.gitattributes
> $ echo "* eol=crlf" >>.gitattributes
> behaves the same as
> $ echo "* text=auto" >.gitattributes
> $ git config core.eol crlf
>
> In other words,
> $ echo "* text=auto eol=crlf" >.gitattributes
> has the same effect as
> $ git config core.autocrlf true
> and
> $ echo "* text=auto eol=lf" >.gitattributes
> has the same effect as
> $ git config core.autocrlf input

It is kind of disappointing that the none of the above block of
configuration examples does not quite say what the reader really
wants to know directly.  I am guessing that the point is

now text=auto does autodetection even when eol=crlf is
specified, and files that are declared to be non-text will not
get clobbered with eol=crlf conversion

right?  If that is the case, replacing everything below "In other
words" with these three lines would make the text much easier to
grok, I would think.

> Signed-off-by: Torsten Bögershausen 
> ---
>  Documentation/config.txt| 14 +++---
>  Documentation/gitattributes.txt | 15 +--
>  convert.c   | 42 
> +
>  convert.h   |  3 ++-
>  t/t0025-crlf-auto.sh|  4 ++--
>  t/t0027-auto-crlf.sh| 32 +++
>  t/t6038-merge-text-auto.sh  | 23 ++
>  7 files changed, 73 insertions(+), 60 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 4a27ad4..9caf6ae 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -389,13 +389,13 @@ file with mixed line endings would be reported by the 
> `core.safecrlf`
>  mechanism.
>  
>  core.autocrlf::
> - Setting this variable to "true" is almost the same as setting
> - the `text` attribute to "auto" on all files except that text
> - files are not guaranteed to be normalized: files that contain
> - `CRLF` in the repository will not be touched.  Use this
> - setting if you want to have `CRLF` line endings in your
> - working directory even though the repository does not have
> - normalized line endings.  This variable can be set to 'input',
> + Setting this variable to "true" is the same as setting
> + the `text` attribute to "auto" on all files and core.eol to "crlf".
> + Set to true if you want to have `CRLF` line endings in your
> + working directory and the repository has LF line endings.
> + Text files are guaranteed not to be normalized: files that contain
> + `CRLF` in the repository will not be touched.

This is not a new problem but the last sentence and a half look
bad.  Telling readers "X is not guaranteed to happen" is not all
that useful--telling them what happens is.  Also the use of colon
there is probably ungrammatical.

Set to true if you want to have CRLF line endings in your
working directory and LF line endings in the repository.
Text files that contain CRLF in the repository will not get
their end-of-line converted.

perhaps?

> diff --git a/convert.h b/convert.h
> index ccf436b..81b6cdf 100644
> --- a/convert.h
> +++ b/convert.h
> @@ -7,7 +7,8 @@
>  enum safe_crlf {
>   SAFE_CRLF_FALSE = 0,
>   SAFE_CRLF_FAIL = 1,
> - SAFE_CRLF_WARN = 2
> + SAFE_CRLF_WARN = 2,
> + SAFE_CRLF_RENORMALIZE = 4

Are these bits in a word?  If not where did 3 go?

> diff --git a/convert.c b/convert.c
> index 24ab095..3782172 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -227,7 +227,9 @@ static enum eol output_eol(enum crlf_action crlf_action)
>   return EOL_LF;
>   case CRLF_UNDEFINED:
>   case CRLF_AUTO_CRLF:
> + return EOL_CRLF;

Hmph, do we want UNDEFINED case to return EOL_CRLF here?

>   case CRLF_AUTO_INPUT:
> + return EOL_LF;
>   case CRLF_TEXT:
>   case CRLF_AUTO:
>   /* fall through */

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message 

Re: [PATCH v1] travis-ci: build documentation

[Matthieu Moy]

Junio C Hamano  writes:

> Junio C Hamano  writes:
>
>> Lars Schneider  writes:
>>
 There also are existing instances of "useless ;" that would want to
 be cleaned up regardless of portability issues.
>>> Unfortunately it seems to be required. Travis CI generates a shell script
>>> out of the yml file and I think they don't respect newlines or something...
>>
>> If they squash all the lines into a single long line before
>> executing, these semicolons do indeed become necessary (we have to
>> write a logical single line shell script in our Makefiles with ';',
>> and I'd imagine Travis's scriptlets are done similarly).

Actually, it's not Travis, but just the Yaml syntax:

  http://docs.ansible.com/ansible/YAMLSyntax.html
  
  Values can span multiple lines using | or >. Spanning multiple lines
  using a | will include the newlines. Using a > will ignore newlines;
  it’s used to make what would otherwise be a very long line easier to
  read and edit

So, a simpler solution would be to apply something like this:

--- a/.travis.yml
+++ b/.travis.yml
@@ -33,7 +33,7 @@ env:
 - GIT_SKIP_TESTS="t9810 t9816"
 
 before_install:
-  - >
+  - |
 case "${TRAVIS_OS_NAME:-linux}" in
 linux)
   mkdir --parents custom/p4
@@ -81,7 +81,7 @@ before_script: make --jobs=2
 script: make --quiet test
 
 after_failure:
-  - >
+  - |
 : '<-- Click here to see detailed test output! 
 
';  

 for TEST_EXIT in t/test-results/*.exit;
 do

(untested)

Actually, this may also be a motivation to move anything non-trivial out
of .travic.yml and to start using separate scripts (to avoid writting
shell within a Yaml syntax).

> ... but the above does not quite explain it.  The newlines are
> mostly honoured as logical end-of-line in existing .travis.yml e.g.
> we do not see a semicolon before "pushd".
>
> case "${TRAVIS_OS_NAME:-linux}" in
> linux)
>   mkdir --parents custom/p4
>   pushd custom/p4

I'm tempted to think that these lines create directories pushd/ and
custom/p4/, but that doesn't quite explain it either. There's more magic
somewhere ...

> Puzzled...

Likewise.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 v7 01/10] t0027: Make commit_chk_wrnNNO() reliable

[Junio C Hamano]

tbo...@web.de writes:

> From: Torsten Bögershausen 
>
> When the content of a commited file is unchanged and the attributes are 
> changed,
> Git may not detect that the next commit must treat the file as changed.
> This happens when lstat() doesn't detect a change, since neither inode,
> mtime nor size are changed.
>
> Add a singe "Z" character to change the file size (and content).

singLe (no need to reroll for this nit; locally fixed already).



> When the files are compared later in checkout_files(), the "Z" is removed
> before the comparison.
>
> Signed-off-by: Torsten Bögershausen 
> ---
>  Change since v6b:
>  - Remove 2 sparse warnings, thanks Ramsay
>
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> index f33962b..9fe539b 100755
> --- a/t/t0027-auto-crlf.sh
> +++ b/t/t0027-auto-crlf.sh
> @@ -12,7 +12,7 @@ fi
>  
>  compare_files () {
>   tr '\015\000' QN <"$1" >"$1".expect &&
> - tr '\015\000' QN <"$2" >"$2".actual &&
> + tr '\015\000' QN <"$2" | tr -d 'Z' >"$2".actual &&
>   test_cmp "$1".expect "$2".actual &&
>   rm "$1".expect "$2".actual
>  }
> @@ -114,6 +114,7 @@ commit_chk_wrnNNO () {
>   do
>   fname=${pfx}_$f.txt &&
>   cp $f $fname &&
> + printf Z >>"$fname" &&
>   git -c core.autocrlf=$crlf add $fname 2>/dev/null &&
>   git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname 
> >"${pfx}_$f.err" 2>&1
>   done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] hooks: Add ability to specify where the hook directory is

[Johannes Sixt]

Am 24.04.2016 um 23:18 schrieb Ævar Arnfjörð Bjarmason:

+test_expect_success 'set up a pre-commit hook in core.hooksPath' '
+   mkdir -p .git/custom-hooks .git/hooks &&
+   write_script .git/custom-hooks/pre-commit <>.git/PRE-COMMIT-HOOK-WAS-CALLED
+EOF
+   cat >.git/hooks/pre-commit <>.git/PRE-COMMIT-HOOK-WAS-CALLED
+EOF
+   chmod +x .git/custom-hooks/pre-commit


Here I see a half-baked attempt to use write_script. Once you've fixed 
that, we have a pre-commit hook in the regular hook directory. 
Obviously, the hook is expected not to be called...



+'
+
+test_expect_success 'Check that various forms of specifying core.hooksPath 
work' '
+   test_commit no_custom_hook &&


... but at this point, it *will* be called...


+   git config core.hooksPath .git/custom-hooks &&
+   test_commit have_custom_hook &&
+   git config core.hooksPath .git/custom-hooks/ &&
+   test_commit have_custom_hook_trailing_slash &&
+   git config core.hooksPath "$PWD/.git/custom-hooks" &&
+   test_commit have_custom_hook_abs_path &&
+   git config core.hooksPath "$PWD/.git/custom-hooks/" &&
+   test_commit have_custom_hook_abs_path_trailing_slash &&
+   printf "%s" "" >.git/PRE-COMMIT-HOOK-WAS-CALLED.expect &&


... and this expectation is wrong.


+   test_cmp .git/PRE-COMMIT-HOOK-WAS-CALLED.expect 
.git/PRE-COMMIT-HOOK-WAS-CALLED
+'
+
+test_done



I feel a bit uneasy that expected and actual files are not POSIXly 
correct text files, i.e., the LF at the end is missing...


-- Hannes

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

Re: [PATCH v1] travis-ci: build documentation

[Junio C Hamano]

Junio C Hamano  writes:

> Lars Schneider  writes:
>
>>> There also are existing instances of "useless ;" that would want to
>>> be cleaned up regardless of portability issues.
>> Unfortunately it seems to be required. Travis CI generates a shell script
>> out of the yml file and I think they don't respect newlines or something...
>
> If they squash all the lines into a single long line before
> executing, these semicolons do indeed become necessary (we have to
> write a logical single line shell script in our Makefiles with ';',
> and I'd imagine Travis's scriptlets are done similarly).
>
> Thanks.

... but the above does not quite explain it.  The newlines are
mostly honoured as logical end-of-line in existing .travis.yml e.g.
we do not see a semicolon before "pushd".

case "${TRAVIS_OS_NAME:-linux}" in
linux)
  mkdir --parents custom/p4
  pushd custom/p4
...
esac;
echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)";
...

everything outside the big "case/esac" seems to have ';' in the
current incarnation of the script.

Puzzled...
--
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 09/83] builtin/apply: move 'check' global into 'struct apply_state'

[Stefan Beller]

On Sun, Apr 24, 2016 at 6:33 AM, Christian Couder
 wrote:
> Signed-off-by: Christian Couder 
> ---
>  builtin/apply.c | 16 +---
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index ad81210..6c628f6 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -25,12 +25,15 @@ struct apply_state {
> const char *prefix;
> int prefix_length;
>
> +   /*
> +*  --check turns on checking that the working tree matches the
> +*files that are being modified, but doesn't apply the patch

This is true, but at this part of the file/code we rather want to know what
`check` does, instead of what the command line option --check does.
(They are 2 different things, though one leading to the other one?) How about:

/*
 * Only check the files to be modified, but do not modify the files.
 */


>  /*
> - *  --check turns on checking that the working tree matches the
> - *files that are being modified, but doesn't apply the patch

Oh I see it was moved from here. Not sure if we want to rename
comments along the way or just keep it in this series.
--
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 05/83] builtin/apply: extract line_by_line_fuzzy_match() from match_fragment()

[Stefan Beller]

> @@ -2251,7 +2319,7 @@ static int match_fragment(struct image *img,
>   int match_beginning, int match_end)
>  {
> int i;
> -   char *fixed_buf, *buf, *orig, *target;
> +   char *fixed_buf, *orig, *target;
> struct strbuf fixed;
> size_t fixed_len, postlen;
> int preimage_limit;
> @@ -2312,6 +2380,7 @@ static int match_fragment(struct image *img,
>  * There must be one non-blank context line that match
>  * a line before the end of img.
>  */
> +   char *buf;

patches 1-4 looking good, with no comment from me. Here is the first spot to
comment on.

It's not clear why we need to declare buf here? Oh wait it is. It's just
moved from the start of the function. But why do it in this patch?
It seems unrelated to the general intent of the patch. No need to reroll
for this nit alone, it just took me a while to figure out it was an unrelated
thing.


> char *buf_end;
>
> buf = preimage->buf;
--
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] http: Support sending custom HTTP headers

[Junio C Hamano]

Johannes Schindelin  writes:

> To make communication for `git fetch`, `git ls-remote` and friends extra
> secure, we introduce a way to send custom HTTP headers with all
> requests.

I think an ability to send custom headers may be a good addition and
have no problem with it, but I tend to agree with Shawn that its log
message that advertises it as if it has anything to do with security
is probably a bad idea in both ways (i.e. it isn't very secure, and
the usefulness of the feature is not limited to security).

> This allows us, for example, to send an extra token that the server
> tests for. The server could use this token e.g. to ensure that only
> certain operations or refs are allowed, or allow the token to be used
> only once.
>
> This feature can be used like this:
>
>   git -c http.extraheader='Secret: sssh!' fetch $URL $REF
>
> Signed-off-by: Johannes Schindelin 


> Published-As: https://github.com/dscho/git/releases/tag/extra-http-headers-v1

Move this after "---".

> ---

This obviously needs documentation updates and tests, no?

>  http-push.c   | 10 +-
>  http.c| 28 +---
>  http.h|  1 +
>  remote-curl.c |  4 ++--
>  4 files changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/http-push.c b/http-push.c
> index bd60668..04eef17 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -211,7 +211,7 @@ static void curl_setup_http(CURL *curl, const char *url,
>  static struct curl_slist *get_dav_token_headers(struct remote_lock *lock, 
> enum dav_header_flag options)
>  {
>   struct strbuf buf = STRBUF_INIT;
> - struct curl_slist *dav_headers = NULL;
> + struct curl_slist *dav_headers = http_get_default_headers();
>  
>   if (options & DAV_HEADER_IF) {
>   strbuf_addf(, "If: (<%s>)", lock->token);
> @@ -417,7 +417,7 @@ static void start_put(struct transfer_request *request)
>  static void start_move(struct transfer_request *request)
>  {
>   struct active_request_slot *slot;
> - struct curl_slist *dav_headers = NULL;
> + struct curl_slist *dav_headers = http_get_default_headers();
>  
>   slot = get_active_slot();
>   slot->callback_func = process_response;
> @@ -845,7 +845,7 @@ static struct remote_lock *lock_remote(const char *path, 
> long timeout)
>   char *ep;
>   char timeout_header[25];
>   struct remote_lock *lock = NULL;
> - struct curl_slist *dav_headers = NULL;
> + struct curl_slist *dav_headers = http_get_default_headers();
>   struct xml_ctx ctx;
>   char *escaped;
>  
> @@ -1126,7 +1126,7 @@ static void remote_ls(const char *path, int flags,
>   struct slot_results results;
>   struct strbuf in_buffer = STRBUF_INIT;
>   struct buffer out_buffer = { STRBUF_INIT, 0 };
> - struct curl_slist *dav_headers = NULL;
> + struct curl_slist *dav_headers = http_get_default_headers();
>   struct xml_ctx ctx;
>   struct remote_ls_ctx ls;
>  
> @@ -1204,7 +1204,7 @@ static int locking_available(void)
>   struct slot_results results;
>   struct strbuf in_buffer = STRBUF_INIT;
>   struct buffer out_buffer = { STRBUF_INIT, 0 };
> - struct curl_slist *dav_headers = NULL;
> + struct curl_slist *dav_headers = http_get_default_headers();
>   struct xml_ctx ctx;
>   int lock_flags = 0;
>   char *escaped;
> diff --git a/http.c b/http.c
> index 4304b80..02d7147 100644
> --- a/http.c
> +++ b/http.c
> @@ -114,6 +114,7 @@ static unsigned long http_auth_methods = CURLAUTH_ANY;
>  
>  static struct curl_slist *pragma_header;
>  static struct curl_slist *no_pragma_header;
> +static struct curl_slist *extra_http_headers;
>  
>  static struct active_request_slot *active_queue_head;
>  
> @@ -323,6 +324,12 @@ static int http_options(const char *var, const char 
> *value, void *cb)
>  #endif
>   }
>  
> + if (!strcmp("http.extraheader", var)) {
> + extra_http_headers =
> + curl_slist_append(extra_http_headers, value);
> + return 0;
> + }
> +
>   /* Fall back on the default ones */
>   return git_default_config(var, value, cb);
>  }
> @@ -678,8 +685,10 @@ void http_init(struct remote *remote, const char *url, 
> int proactive_auth)
>   if (remote)
>   var_override(_proxy_authmethod, 
> remote->http_proxy_authmethod);
>  
> - pragma_header = curl_slist_append(pragma_header, "Pragma: no-cache");
> - no_pragma_header = curl_slist_append(no_pragma_header, "Pragma:");
> + pragma_header = curl_slist_append(http_get_default_headers(),
> + "Pragma: no-cache");
> + no_pragma_header = curl_slist_append(http_get_default_headers(),
> + "Pragma:");
>  
>  #ifdef USE_CURL_MULTI
>   {
> @@ -765,6 +774,9 @@ void http_cleanup(void)
>  #endif
>   curl_global_cleanup();
>  
> + curl_slist_free_all(extra_http_headers);
> + extra_http_headers = NULL;
> +
>   

Re: [PATCH v14 3/6] t0040-parse-options: improve test coverage

[Pranit Bauva]

On Wed, Apr 13, 2016 at 10:57 PM, Eric Sunshine  wrote:
> On Wed, Apr 13, 2016 at 4:59 AM, Pranit Bauva  wrote:
>> On Wed, Apr 13, 2016 at 10:56 AM, Eric Sunshine  
>> wrote:
>>> On Tue, Apr 12, 2016 at 7:02 PM, Pranit Bauva  
>>> wrote:
 +test_expect_success '--no-quiet sets quiet to 0' '
 +   test-parse-options --no-quiet >output 2>output.err &&
>>>
>>> Meh, as implemented, this isn't a very interesting test, is it?
>>> 'quiet' started at 0, so all this shows is that --no-quiet didn't
>>> disturb the 0. To really test that it resets it to 0, you'd want:
>>>
>>> test-parse-options --quiet --no-quiet >... 2>... &&
>>>
 +   test_must_be_empty output.err &&
 +   test_cmp expect output
 +'
  test_done
>>
>> This is to test whether the 0 of quiet remains 0 if --no-quiet is
>> included. This test "defines" the current behavior. Then when I change
>> OPT_COUNTUP(), this test will ensure that this behavior is not
>> interrupted as promised by the commit message of that patch[1]. I
>> guess this also describe why I choose to include these tests between
>> 2/5 and 3/5 rather than 3/5 and 4/5. And also see the extended
>> discussion[2] for this. If I do a re-roll then I include `--quiet`
>> before `--no-quiet`
>
> Each of these patches should have a single conceptual purpose. It
> seems, from the above explanation, that you're mixing and mismatching
> bits of such changes between patches.
>
> * The two new tests for --no-verbose and --no-quiet should be together
> and check that they correctly reverse --verbose and --quiet,
> respectively.
>
> * The test you describe above which ensures that --no-quiet leaves
> 'quiet' at 0 should be bundled with the change that might break that
> behavior, namely, the OPT__COUNTUP() change.

I am planning to re-roll this.
So, I am just confirming whether I understood properly.

 * I will add the tests for check for '-q --no-quiet' instead of just
'--no-quiet' sets to 0 and '-v --no-verbose' sets to 0 in the patch
which improves test coverage which will be before the OPT_COUNTUP()
change.

 * I will then add the test for '--no-quiet' sets to 0 in the separate
patch after OPT_COUNTUP() change.

Is there something else or something different that you are suggesting?
--
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 0/3] Improvements to githooks.txt documentation

[Junio C Hamano]

Ævar Arnfjörð Bjarmason   writes:

> This includes minor grammar edits pointed out by Eric Sunshine + the
> one v2 patch I sent out in response to comments by Jacob Keller.
>
> I thought it was less confusing to just send out a whole v3 series
> than ask Junio to piece together v1..v3 of various patches.

Yup, that greatly reduces the chance of screw-up on my end and is
very much appreciated.

>
> Ævar Arnfjörð Bjarmason (3):
>   githooks.txt: Improve the intro section
>   githooks.txt: Amend dangerous advice about 'update' hook ACL
>   githooks.txt: Minor improvements to the grammar & phrasing
>
>  Documentation/git-init.txt |  6 +++-
>  Documentation/githooks.txt | 72 
> +++---
>  2 files changed, 47 insertions(+), 31 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] githooks.txt: Minor improvements to the grammar & phrasing

[Junio C Hamano]

Ævar Arnfjörð Bjarmason   writes:

> -This hook is invoked by 'git am' script.  It takes a single
> +This hook is invoked by 'git am'.  It takes a single

Good, as it does not matter to the readers that "am" happens to be
implemented as a script.

>  parameter, the name of the file that holds the proposed commit
> -log message.  Exiting with non-zero status causes
> -'git am' to abort before applying the patch.
> +log message.  Exiting with non-zero causes 'git am' to abort
> +before applying the patch.

I am not sure why you dropped "status" from here.  The result is
harder to grok, at least to me.  Perhaps "with a non-zero status"?

>  The hook is allowed to edit the message file in place, and can
> -be used to normalize the message into some project standard
> -format (if the project has one). It can also be used to refuse
> -the commit after inspecting the message file.
> +be used to e.g. normalize the message into some project standard
> +format. It can also be used to refuse the commit after inspecting
> +the message file.

OK.  Or we can even drop "e.g." and the result still reads perfectly
fine.

>  This hook is invoked by 'git commit', and can be bypassed
> -with `--no-verify` option.  It takes no parameter, and is
> +with the `--no-verify` option.  It takes no parameters, and is
>  invoked before obtaining the proposed commit log message and
> -making a commit.  Exiting with non-zero status from this script
> -causes the 'git commit' to abort.
> +making a commit.  Exiting with a non-zero status from this script
> +causes the 'git commit' command to abort before creating a commit.

Good.  And you kept "status" here, which is doubly 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

[PATCH] clone: add `--shallow-submodules` flag

[Stefan Beller]

When creating a shallow clone of a repository with submodules, the depth
argument does not influence the submodules, i.e. the submodules are done
as non-shallow clones. It is unclear what the best default is for the
depth of submodules of a shallow clone, so we need to have the possibility
to do all kinds of combinations:

* shallow super project with shallow submodules
  e.g. build bots starting always from scratch. They want to transmit
  the least amount of network data as well as using the least amount
  of space on their hard drive.
* shallow super project with unshallow submodules
  e.g. The superproject is just there to track a collection of repositories
  and it is not important to have the relationship between the repositories
  intact. However the history of the individual submodules matter.
* unshallow super project with shallow submodules
  e.g. The superproject is the actual project and the submodule is a
  library which is rarely touched.

The new switch to select submodules to be shallow or unshallow supports
all of these three cases.

It is easy to transition from the first to the second case by just
unshallowing the submodules (`git submodule foreach git fetch
--unshallow`), but it is not possible to transition from the second to the
first case (as we would have already transmitted the non shallow over
the network). That is why we want to make the first case the default in
case of a shallow super project. This leads to the inconvenience in the
second case with the shallow super project and unshallow submodules,
as you need to pass `--no-shallow-submodules`.

Signed-off-by: Stefan Beller 
---

  This replaces origin/sb/clone-shallow-passthru.
  
  Thanks Jacob and Lars for review!
  
Changes compared to origin/sb/clone-shallow-passthru:

 * Dropped the first patch (pass --[no-]local down to submodules).
   This was only used to ease testing in patch 3, but we can use the
   file protocol explicitely for now. Instead of a --no-local option Lars
   rather wants to see a more universal solution for specifying protocol
   issues (local/non local is just one of them, we may want to clone
   the submodules with http, while the parent project uses ssh. That option
   may go into the .gitmodules file as well as a "preferred protocol" option?)
 * squashed patches 1 & 2 together (2 was implementing the feature and 3 was
   testing only, so having just one should do as well.)
 * By squashing there is no need to reword the commit message of the 3rd patch.
 * improved documentation slightly, fixed typo in commit message.
 * deepended the history in the test cases to easier see the outcome, i.e.
   git clone --depth 2 --recurse-submodules implies a submodule depth of 1.
   
Thanks,
Stefan

 Documentation/git-clone.txt | 13 +--
 builtin/clone.c |  7 
 t/t5614-clone-submodules.sh | 85 +
 3 files changed, 102 insertions(+), 3 deletions(-)
 create mode 100755 t/t5614-clone-submodules.sh

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 6db7b6d..e1a21b7 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -14,8 +14,8 @@ SYNOPSIS
  [-o ] [-b ] [-u ] [--reference ]
  [--dissociate] [--separate-git-dir ]
  [--depth ] [--[no-]single-branch]
- [--recursive | --recurse-submodules] [--jobs ] [--] 
- []
+ [--recursive | --recurse-submodules] [--[no-]shallow-submodules]
+ [--jobs ] [--]  []
 
 DESCRIPTION
 ---
@@ -190,7 +190,11 @@ objects from the source repository into a pack in the 
cloned repository.
 
 --depth ::
Create a 'shallow' clone with a history truncated to the
-   specified number of revisions.
+   specified number of revisions. Implies `--single-branch` unless
+   `--no-single-branch` is given to fetch the histories near the
+   tips of all branches. This implies `--shallow-submodules`. If
+   you want to have a shallow superproject clone, but full submodules,
+   also pass `--no-shallow-submodules`.
 
 --[no-]single-branch::
Clone only the history leading to the tip of a single branch,
@@ -214,6 +218,9 @@ objects from the source repository into a pack in the 
cloned repository.
repository does not have a worktree/checkout (i.e. if any of
`--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
 
+--[no-]shallow-submodules::
+   All submodules which are cloned will be shallow with a depth of 1.
+
 --separate-git-dir=::
Instead of placing the cloned repository where it is supposed
to be, place the cloned repository at the specified directory,
diff --git a/builtin/clone.c b/builtin/clone.c
index b004fb4..ecdf308 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -40,6 +40,7 @@ static const char * const builtin_clone_usage[] = {
 
 static int option_no_checkout, option_bare, option_mirror, 
option_single_branch = -1;
 static int 

Re: [PATCH 2/3] githooks.txt: Amend dangerous advice about 'update' hook ACL

[Junio C Hamano]

Jacob Keller  writes:

>> -Another use suggested on the mailing list is to use this hook to
>> -implement access control which is finer grained than the one
>> -based on filesystem group.
>> +Another use for this hook to implement access control which is finer
>> +grained than the one based on filesystem group. Note that if the user
>> +pushing has a normal login shell on the machine receiving the push
>> +implementing access control like this can be trivially bypassed by
>> +just using not executing the hook. In those cases consider using
>
> "by just using not executing the hook."
>
> This grammar doesn't make sense. It doesn't quite match what you said
> in the commit message either.
>
>> +e.g. linkgit:git-shell[1] as the login shell to restrict the user's
>> +access.

While there is nothing technically wrong in what it says, I wonder
if it is worth to state the obvious.  If one can bypass update hook,
one can bypass any other hook, so the information does not even
belong here.

Instead of saying "acl can be implemented on top of update hook, but
not quite because you can bypass it", it may be more useful to say
"in an environment that restricts the users' access only to git
commands over the wire, this hook can be used to access control
without relying on filesystem ownership and group membership",
perhaps?

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

Re: [PATCH 1/3] githooks.txt: Improve the intro section

[Junio C Hamano]

Ævar Arnfjörð Bjarmason   writes:

> Change the documentation so that:
>
>  * We don't talk about "little scripts". Hooks can be as big as you
>want, and don't have to be scripts, just call them "programs".
>
>  * We note what happens with chdir() before a hook is called, nothing
>documented this explicitly, but the current behavior is
>predictable. It helps a lot to know what directory these hooks will
>be executed from.
>
>  * We don't make claims about the example hooks which may not be true
>depending on the configuration of 'init.templateDir'. Clarify that
>we're talking about the default settings of git-init in those cases,
>and move some of this documentation into git-init's documentation
>about the default templates.
>
>  * We briefly note in the intro that hooks can get their arguments in
>various different ways, and that how exactly is described below for
>each hook.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
>  Documentation/git-init.txt |  6 +-
>  Documentation/githooks.txt | 32 
>  2 files changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
> index 8174d27..cf37926 100644
> --- a/Documentation/git-init.txt
> +++ b/Documentation/git-init.txt
> @@ -130,7 +130,11 @@ The template directory will be one of the following (in 
> order):
>   - the default template directory: `/usr/share/git-core/templates`.
>  
>  The default template directory includes some directory structure, suggested
> -"exclude patterns" (see linkgit:gitignore[5]), and sample hook files (see 
> linkgit:githooks[5]).
> +"exclude patterns" (see linkgit:gitignore[5]), and example hook files.
> +
> +The example are all disabled by default. To enable a hook, rename it

"sample hooks are all disabled" would be better; if for some unknown
reason you are trying to avoid "sample hooks", "examples are all
disabled" may be acceptable.

> +by removing its `.sample` suffix. See linkgit:githooks[5] for more
> +info on hook execution.

Makes a first-time reader wonder if I am allowed to ignore the
sample and write my own from scratch, or if the only thing that is
allowed is to enable what is shipped with .sample suffix.

I wonder it would become less confusing if we placed even _less_
stress on these sample hooks; I find that saying we ship sample
hooks that are disabled is probably more confusing.

We do not ship any hook (hence nothing is enabled by definition);
there are a handful of sample files that you can use when adding
your own hook by either referencing them or taking them as-is, and
the latter can be done by removing .sample suffix, which is merely a
special case convenience.


> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index a2f59b1..2f3caf7 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -13,18 +13,26 @@ $GIT_DIR/hooks/*
>  DESCRIPTION
>  ---
>  
> -Hooks are little scripts you can place in `$GIT_DIR/hooks`
> -directory to trigger action at certain points.  When
> -'git init' is run, a handful of example hooks are copied into the
> -`hooks` directory of the new repository, but by default they are
> -all disabled.  To enable a hook, rename it by removing its `.sample`
> -suffix.
> -
> -NOTE: It is also a requirement for a given hook to be executable.
> -However - in a freshly initialized repository - the `.sample` files are
> -executable by default.
> -
> -This document describes the currently defined hooks.
> +Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
> +trigger action at certain points. Hooks that don't have the executable
> +bit set are ignored.

The last sentence is POSIXPERM only, though.

> +When a hook is called in a non-bare repository the working directory
> +is guaranteed to be the root of the working tree, in a bare repository
> +the working directory will be the path to the repository. I.e. hooks
> +don't need to worry about the user's current working directory.

This sentence took me two reads until I mentally substituted "the
working directory" with "its working diretory", to realize that you
are talking about the cwd of the process that runs the hook.

While "is guaranteed" may be technically correct and we have no
intention to change it, it sounds unnecessarily strong.

When a hook is invoked, it is run at the root of the working
tree in a non-bare repository, or in the $GIT_DIR in a bare
repository.

perhaps?

> +Hooks can get their arguments via the environment, command-line
> +arguments, and stdin. See the documentation for each below hook for
> +details.

"each below hook" sounds somewhat ungrammatical.
--
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 03/21] t/test-lib-functions.sh: generalize test_cmp_rev

[Junio C Hamano]

Stephan Beyer  writes:

> Hi,
>
> On 04/15/2016 10:00 PM, Junio C Hamano wrote:
>> Stephan Beyer  writes:
>> 
>>> test_cmp_rev() took exactly two parameters, the expected revision
>>> and the revision to test. This commit generalizes this function
>>> such that it takes any number of at least two revisions: the
>>> expected one and a list of actual ones. The function returns true
>>> if and only if at least one actual revision coincides with the
>>> expected revision.
>> 
>> There may be cases where you want to find the expected one among
>> various things you actually have (which is what the above talks
>> about; it is like "list-what-I-actually-got | grep what-i-want"),
>> but an equally useful use case would be "I would get only one
>> outcome from test, I anticipate one of these things, all of which is
>> OK, but I cannot dictate which one of them should come out" (it is
>> like "list-what-I-can-accept | grep what-I-actually-got").
>
> I see that these are strictly speaking (slightly) different semantics
> but in the end it boils down to be the same, or am I missing anything?
>
>> I am not enthused by the new test that implements the "match one
>> against multi" check only in one way among these possible two to
>> squat on a very generic name, test_cmp_rev.
>> 
>> The above _may_ appear a non-issue until you realize one thing that
>> is there to help those who debug the tests, which is ...
>> 
>>> While at it, the side effect of generating two (temporary) files
>>> is removed.
>> 
>> That is not strictly a side effect.  test_cmp allows you to see what
>> was expected and what you actually had when the test failed (we
>> always compare expect with actual and not the other way around, so
>> that "diff -u expect actual" would show how the actual behaviour
>> diverted from our expectation in a natural way).
>
> I was referring to *generating the files* as a side effect. I did not
> even think about the fact that "diff" in the original code does not only
> return an exit code but that it also generates output that can be used
> as "helpful diagnostic information" (referring to Eric Sunshine's mail
> here). I was not aware that the Git tests should -- besides testing --
> already include "tools" for easier debugging in case of a failure... So
> dropping this information was not intentional.
>
>> Something with the semantics of these two:
>> 
>>  test_revs_have_expected () {
>>  expect=$1
>>  shift
>>  git rev-parse "$@" | grep -e "$expect" >/dev/null && return
>>  echo >&2 "The expected '$1' is not found in:"
>> printf >&2 " '%s'\n", "$@"
>> return 1
>>  }
>> 
>>  test_rev_among_expected () {
>>  actual=$1
>> shift
>>  git rev-parse "$@" | grep -e "$actual" >/dev/null && return
>>  echo >&2 "'$1' is not among expected ones:"
>> printf >&2 " '%s'\n", "$@"
>> return 1
>>  }
>> 
>> might be more appropriate.
>
> Ah! That's what I meant above. The code is copy besides variable
> naming and the output "title". Such code duplication for the sake of
> "easier debugging" in case of a failure?
>
> Also I wonder if test authors in the future would really know *which*
> one is the right one to use.

I saw that even you were originally confused about it ;-).

In your proposed log message, you talk about "the expected, and list
of actual ones", which can only mean "there may be multiple answers
from the command (e.g. "merge-base --all") and we only require that
one of the answers is the expected one", which is why among the two
necessary functions I listed "test_revs_have_expected" above first,
but I think most (if not all) of the invocations of the multi-match
form in your patch actually wanted "test_rev_among_expected"
variant, i.e. "there will be one answer from the command, but there
are multiple acceptable answers, all of them valid".

I do not think test authors who understands the reason why we always
say "test_cmp actual expect" and not the other way around will share
the same confusion (and now you were explained and understood why
"diff" in the original was given the expected and actual result in
that order, you no longer are confused wrt this).

And no, this is not "for the sake of easier debugging in case of a
failure".  It is about knowing what you are doing--are you going to
have multiple answers and making sure one right one appears in it,
or are you going to have one answer and allowing any one of multiple
valid ones?  These two are quite different things and it helps the
readers of the test to know which one is being used.
--
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 49/83] builtin/apply: move 'lock_file' global into 'struct apply_state'

[Junio C Hamano]

Eric Sunshine  writes:

>> +   /*
>> +* Since lockfile.c keeps a linked list of all created
>> +* lock_file structures, it isn't safe to free(lock_file).
>> +*/
>> +   struct lock_file *lock_file;
>
> Is there ever a time when lock_file is removed from the list (such as
> upon successful write of the index), in which case it would be safe to
> free() this?

I do not think you need to think about "free"ing.

Even if the libified version of the apply internal can be called
multiple times to process multiple patch inputs, there is no need to
run multiple instances of it yet.  And a lockfile, after the caller
finishes interacting with one file using it by calling commit or
rollback, can be reused to interact with another file.

So the cleanest would be to:

 * make the caller of apply API responsible for preparing a static
   or (allocating and leaking) lockfile instance,

 * make it pass a pointer to the lockfile to the apply API so that
   it can store it in apply_state, and

 * have the caller use apply API feeding one patch at a time in a
   loop, allowing the same lockfile instance used for each
   "invocation".

I sounded as if I were advocating non-reentrant implementation in
the introductory paragraph, but that is not the intention.  If you
want to do N threads, you would prepare N lockfile instances, give
them to N apply API instances to be stored in N apply_state
instances, and you would have N parallel applications, if you wanted
to.
--
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] string_list: use string-list API in unsorted_string_list_lookup()

[Stefan Beller]

On Mon, Apr 25, 2016 at 10:40 AM, Ralf Thielow  wrote:
> Using the string-list API in function unsorted_string_list_lookup()
> makes the code more readable.
>
> Signed-off-by: Ralf Thielow 
> ---
> Changes since v1:
> - remove extra curly braces

Reviewed-by: Stefan Beller 

>
>  string-list.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/string-list.c b/string-list.c
> index 2a32a3f..62d2084 100644
> --- a/string-list.c
> +++ b/string-list.c
> @@ -231,12 +231,12 @@ void string_list_sort(struct string_list *list)
>  struct string_list_item *unsorted_string_list_lookup(struct string_list 
> *list,
>  const char *string)
>  {
> -   int i;
> +   struct string_list_item *item;
> compare_strings_fn cmp = list->cmp ? list->cmp : strcmp;
>
> -   for (i = 0; i < list->nr; i++)
> -   if (!cmp(string, list->items[i].string))
> -   return list->items + i;
> +   for_each_string_list_item(item, list)
> +   if (!cmp(string, item->string))
> +   return item;
> return NULL;
>  }
>
> --
> 2.8.1.430.g7c694c5
>
--
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: RFC: Supporting .git/hooks/$NAME.d/* && /etc/git/hooks/$NAME.d/*

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> The reason for supporting the *.d directories was that I spotted a lot
> of hooks people had hacked up at work using the pee(1) command[1] to
> run sequences of other unrelated hook commands.

IIRC, we wanted to do this several years ago but after discussion
decided that we didn't want to have this in the core, because we
didn't want to hardcode the policy on interaction among multiple
hooks.

You can easily resolve the ordering of hooks--just declare that they
are executed sequentially in strcmp() order of filenames and users
will know to prefix them with fixed-number-of-digits to force their
desired ordering without complaining.

What is harder and the core part cannot unilaterally dictate is what
should happen after seeing a failure/rejection from a hook.  Some
hooks among the remainder would not want to be even called.  Some
others do want to be called but want to learn that the previous
hooks already have decided to fail/reject the operation.  There may
even be some others that cannot be moved to earlier part of the hook
chain for other external constraints (e.g. side effect of some
previous hook is part of its input), but would want to override the
previous decision to reject and let the operation pass.

I am happy to see that the idea brought back alive again, but I
think we prefer this start its life clearly marked as "highly
experimental and subject to change", then invite interested and
brave users who tolerate backward incompatible changes to
experiment, in order to allow us to gauge what the right semantics
and flexibility the users would want.  One way to do so may be an
opt-in configuration variable e.g. "experimental.multiHooks";
another may be to implement the logic as a pair of scripts (one for
the command line argument variant, the other for stdin variant) and
ship them in contrib/.

The latter approach (i.e. scripting) might be easier for people to
experiment and tweak, and in the olden days that would certainly be
the approach would would have taken, but I am not too afraid of
appearing uninviting to casual scripters anymore these days, so...

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

Re: [PATCH 3/3] clone: add t5614 to test cloning submodules with shallowness involved

[Stefan Beller]

On Tue, Apr 12, 2016 at 11:52 PM, Jacob Keller  wrote:
> On Tue, Apr 12, 2016 at 4:48 PM, Stefan Beller  wrote:
>> There are some inherent issues with shallow clones and submodules, such
>> as having not having a commit available the superproject may point to
>> in the submodule due to being shallow. Use the new file t5614 to document
>> and test expectations in this area.
>>
>
> The description seems to imply that there will be
> "test_expect_failure" tests to indicate what needs to be improved...
> Maybe I am just mis-reading it?

That commit message certainly reads like that. I'll tone it down to
just say it's testing
the shallowness.

Lars wrote:
>> +test_expect_success 'shallow clone implies shallow submodule' '
>> + test_when_finished "rm -rf super_clone" &&
>> + git clone --recurse-submodules --no-local --depth 1 . super_clone &&
>
> You could add another commit to the super repo and then clone with "--depth 
> 2".
> The super repo would then contain 2 lines and the submodule still just 1.
> This would make it more obvious that shallow submodules always have a depth 
> of 1.

ok will fix.
--
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] string_list: use string-list API in unsorted_string_list_lookup()

[Ralf Thielow]

Using the string-list API in function unsorted_string_list_lookup()
makes the code more readable.

Signed-off-by: Ralf Thielow 
---
Changes since v1:
- remove extra curly braces

 string-list.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/string-list.c b/string-list.c
index 2a32a3f..62d2084 100644
--- a/string-list.c
+++ b/string-list.c
@@ -231,12 +231,12 @@ void string_list_sort(struct string_list *list)
 struct string_list_item *unsorted_string_list_lookup(struct string_list *list,
 const char *string)
 {
-   int i;
+   struct string_list_item *item;
compare_strings_fn cmp = list->cmp ? list->cmp : strcmp;
 
-   for (i = 0; i < list->nr; i++)
-   if (!cmp(string, list->items[i].string))
-   return list->items + i;
+   for_each_string_list_item(item, list)
+   if (!cmp(string, item->string))
+   return item;
return NULL;
 }
 
-- 
2.8.1.430.g7c694c5

--
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] string_list: use string-list API in unsorted_string_list_lookup()

[Ralf Thielow]

Johannes Schindelin  writes:

>>  
>> -for (i = 0; i < list->nr; i++)
>> -if (!cmp(string, list->items[i].string))
>> -return list->items + i;
>> +for_each_string_list_item(item, list) {
>> +if (!cmp(string, item->string))
>> +return item;
>> +}
>>  return NULL;
>
> If you drop the extra curly braces (thereby matching Git's coding style as
> an additional bonus), at least the patch won't increase the number of
> lines.
>

Oops. I'll send a new version of the patch without these
extra curly braces.

Thanks!

> In any case, I like the simplification of the code.
>
> 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 1/3] clone: add `--shallow-submodules` flag

[Stefan Beller]

On Mon, Apr 25, 2016 at 4:50 AM, Lars Schneider
 wrote:
>> first case (as we wouldd have already transmitted the non shallow over
> s/wouldd/would/

will be fixed in a reroll

>> --depth ::
>>   Create a 'shallow' clone with a history truncated to the
>> - specified number of revisions.
>> + specified number of revisions. Implies `--single-branch` unless
>> + `--no-single-branch` is given to fetch the histories near the
>> + tips of all branches. This implies `--shallow-submodules`. If
>> + you want to have a shallow clone, but full submodules, also pass
> Can we make this more explicit. Attention, bikeshedding:
> If you want to have a shallow parent clone, but full submodules...

Good point, but with s/parent/superproject/ ? (What is a "parent clone"?
It is not defined in the glossary, so let's not make up more confusing words
for Git here :)

>
>
>> + `--no-shallow-submodules`.
>>
>> --[no-]single-branch::
>>   Clone only the history leading to the tip of a single branch,
>> @@ -214,6 +218,9 @@ objects from the source repository into a pack in the 
>> cloned repository.
>>   repository does not have a worktree/checkout (i.e. if any of
>>   `--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
>>
>> +--shallow-submodules::
> Should that be "--[no-]shallow-submodules" ?

will be fixed in a reroll

>
>
>> + All submodules which are cloned, will be shallow.
> Maybe you could extend it with "... will be shallow with a depth of 1." ?

done

>
>
>> +
>> --separate-git-dir=::
>>   Instead of placing the cloned repository where it is supposed
>>   to be, place the cloned repository at the specified directory,
>> diff --git a/builtin/clone.c b/builtin/clone.c
>> index b004fb4..ecdf308 100644
>> --- a/builtin/clone.c
>> +++ b/builtin/clone.c
>> @@ -40,6 +40,7 @@ static const char * const builtin_clone_usage[] = {
>>
>> static int option_no_checkout, option_bare, option_mirror, 
>> option_single_branch = -1;
>> static int option_local = -1, option_no_hardlinks, option_shared, 
>> option_recursive;
>> +static int option_shallow_submodules = -1;
>> static char *option_template, *option_depth;
>> static char *option_origin = NULL;
>> static char *option_branch = NULL;
>> @@ -91,6 +92,8 @@ static struct option builtin_clone_options[] = {
>>   N_("create a shallow clone of that depth")),
>>   OPT_BOOL(0, "single-branch", _single_branch,
>>   N_("clone only one branch, HEAD or --branch")),
>> + OPT_BOOL(0, "shallow-submodules", _shallow_submodules,
>> + N_("any cloned submodules will be shallow")),
> I am not familiar with this code but I assume the "no-" prefix flips the bool?

Giving the --no-option stores an explicit 0 (it is initialized as -1),
and passing --option stores a 1.

>> @@ -727,6 +730,10 @@ static int checkout(void)
>>   struct argv_array args = ARGV_ARRAY_INIT;
>>   argv_array_pushl(, "submodule", "update", "--init", 
>> "--recursive", NULL);
>>
>> + if (option_shallow_submodules == 1
>> + || (option_shallow_submodules == -1 && option_depth))
>> + argv_array_push(, "--depth=1");
>> +

which explains this here as:

If  --shallow-submodules was given
  || (neither --[no-]shallow-submodules was given, but --depth
was given,
i.e. depth implies --shallow-submodule only if no explicit
choice was made
by the user.

>>   if (max_jobs != -1)
>>   argv_array_pushf(, "--jobs=%d", max_jobs);
>>
>> --
>> 2.7.0.rc0.42.g8e9204f.dirty
>>
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Update git-p4 to be compatible with git-lfs 1.2

[Junio C Hamano]

SZEDER Gábor  writes:

> You can have a look at these patches at
>
>   https://github.com/szeder/git completion-test-multiple-bash-versions
>
> and perhaps you could even adapt it to LFS and/or p4 somehow.
>
>> Plus if we want to be consistent we would
>> need to do the same for LFS 1.0, 1.2, and for pretty much every other
>> dependency...  
>
> I'm not sure we should be consistent in this case, at least not solely
> for consistency's sake and not in git.git. Taking what I did for Bash
> and doing it for different versions of LFS, p4, etc. could perhaps
> keep the runtime under control, but t/Makefile would surely get out
> of control rather quickly.  Putting these into a travis-ci matrix is
> so much simpler, but the runtime makes it infeasible, of course.

I took a brief look of your branch, and I like its approach.  If I
understood your approach correctly, you:

 * Group selected tests in t/ as "these are bash related tests I
   care about" in t/Makefile;

 * Add Travis test target to build Git with specific versions of
   bash, and run the above target instead of the full test to
   exercise the version of bash you are testing.

And I agree that the same can be done for LFS versions and P4
versions.  Only a handful tests in t/ are about these niches.

> I think the best we can do is to keep this out of git.git and let
> (hope?) developers interested in a particular subsystem do this
> "multiple version compatibility" tests as they see fit.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] submodule clone: pass along `local` option

[Stefan Beller]

On Mon, Apr 25, 2016 at 5:18 AM, Lars Schneider
 wrote:
>> @@ -140,6 +141,10 @@ static int clone_submodule(const char *path, const char 
>> *gitdir, const char *url
>>   argv_array_pushl(, "--reference", reference, NULL);
>>   if (gitdir && *gitdir)
>>   argv_array_pushl(, "--separate-git-dir", gitdir, NULL);
>> + if (local == 1)
> bikeshedding: Why do you use == 1 here and !local below? I would either 
> compare both against
> integers or none ("if (local)" should work here, too, no?). Or is this a Git 
> coding guideline
> that I am not yet aware of?

"if (local)" doesn't work as it includes -1 as well.
What I want to express here is:

switch (local) {
case 1: argv_array_push(, "--local"); break; /* local == 1
did that */
case 0: argv_array_push(, "--no-local"); break; /* !local*/
case -1: default: /* pass nothing */
}

This seems to be more expressive now, so I'll replace with that?

>> @@ -180,6 +186,8 @@ static int module_clone(int argc, const char **argv, 
>> const char *prefix)
>>   OPT_STRING(0, "depth", ,
>>  N_("string"),
>>  N_("depth for shallow clones")),
>> + OPT_BOOL(0, "local", ,
>> +  N_("to clone from a local repository")),
> TBH I think "local" could be misleading here. How about 
> "--pass-transfer-protocol-on-to-submodules"
> or something? If I understand this option correctly then this could be useful 
> for other cases besides
> "local". Imagine you clone a repo via HTTPS that has references to SSH 
> submodules on the same
> server. If you don't have a proper SSH setup then the submodule clone will 
> fail.

Good point. We just converted an internal repository to use relative
path for submodules instead of
absolute paths and that broke the auth (they used different protocols,
which we were unaware of)

However  "--pass-transfer-protocol-on-to-submodules" would mean more
than just "--[no-]local,
but also passing on the protocol prefix (which can be http://,
https://, ssh://, git:// or more),
so it would enlarge the scope of this series a lot. (Original purpose
of this patch was to allow
not using the explicit file:// protocol in the test for the next
patch. But I see passing on the protocol
as a new issue worth resolving :)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1] travis-ci: build documentation

[Jeff King]

On Mon, Apr 25, 2016 at 10:35:24AM +0200, Lars Schneider wrote:

> > This does slow down the normal test results for linux/gcc, though. I
> > don't know very much about Travis, but is it possible to break out the
> > documentation build into its own test, with a separate build status from
> > the other runs?
> 
> Great idea. This is how it looks.
> 
> All jobs of a run:
> https://travis-ci.org/larsxschneider/git/builds/125506781
> 
> The documentation job:
> https://travis-ci.org/larsxschneider/git/jobs/125506786

Very cool. Thanks!

-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] http: Support sending custom HTTP headers

[Jeff King]

On Mon, Apr 25, 2016 at 03:13:08PM +0200, Johannes Schindelin wrote:

> diff --git a/http.c b/http.c
> index 4304b80..02d7147 100644
> --- a/http.c
> +++ b/http.c
> @@ -114,6 +114,7 @@ static unsigned long http_auth_methods = CURLAUTH_ANY;
>  
>  static struct curl_slist *pragma_header;
>  static struct curl_slist *no_pragma_header;
> +static struct curl_slist *extra_http_headers;
>  
>  static struct active_request_slot *active_queue_head;
>  
> @@ -323,6 +324,12 @@ static int http_options(const char *var, const char 
> *value, void *cb)
>  #endif
>   }
>  
> + if (!strcmp("http.extraheader", var)) {
> + extra_http_headers =
> + curl_slist_append(extra_http_headers, value);
> + return 0;
> + }

I wondered if this would trigger for "http.*.extraheader", too. And it
should, as that is all handled in the caller of http_options. Good.

> @@ -678,8 +685,10 @@ void http_init(struct remote *remote, const char *url, 
> int proactive_auth)
>   if (remote)
>   var_override(_proxy_authmethod, 
> remote->http_proxy_authmethod);
>  
> - pragma_header = curl_slist_append(pragma_header, "Pragma: no-cache");
> - no_pragma_header = curl_slist_append(no_pragma_header, "Pragma:");
> + pragma_header = curl_slist_append(http_get_default_headers(),
> + "Pragma: no-cache");
> + no_pragma_header = curl_slist_append(http_get_default_headers(),
> + "Pragma:");

This looked wrong to me at first, because we are appending to the
default header list in each case. But the secret sauce is that calling
http_get_default_headers() actually creates a _new_ list that is a copy
of the default headers (and the caller can do what they will with it,
and must free it).

I think that's really the only sane way to do it because of curl's
interfaces. But maybe it is worth a comment either here, or along with
http_get_default_headers(), or both.

-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: [RFC] How to pass Git config command line instructions to Submodule commands?

[Stefan Beller]

On Mon, Apr 25, 2016 at 3:39 AM, Lars Schneider
 wrote:
> Hi,
>
> a few folks from the Git LFS project and I try to make cloning of repositories
> with a lot of LFS files faster.
>
> The core problem is that Git LFS uses a Git smudge filter to replace LFS
> pointers with the actual file content. Right now, a smudge filter can only
> be executed on an individual file which makes the operation slow for many
> files [1].
>
> We solved this issue by temporarily disabling the smudge filter for the clone
> command via Git config (optimized in 1a8630 [2]):
>
> git -c filter.lfs.smudge= -c filter.lfs.required=false clone  
>
> Afterwards Git LFS runs a special command to download and replace all LFS
> content in bulk [3]. This works great for LFS repositories.
>
> However, I noticed that git config command line instructions such as
> "-c filter.lfs.smudge=" are not passed to Git submodule operations. Thus
> this does not work as expected:
>
> git -c filter.lfs.smudge= -c filter.lfs.required=false clone --recursive 
>  

I have cc'd Jacob Keller, who authored origin/jk/submodule-c-credential,
which does work in that area (deciding which config option to pass down
into the submodule commands).

>
> I tried to work around that by copying the relevant pieced from the Git
> Submodule command [4] and applying the command line Git config
> manually (look closely at the modified checkout command):
>
> git -c filter.lfs.smudge= -c filter.lfs.required=false clone $@
> if [[ -z $2 ]]; then
> CLONE_PATH=$(basename ${1%.git});
> else
> CLONE_PATH=$2;
> fi
> pushd "$CLONE_PATH"
> git submodule init
> wt_prefix=$(git rev-parse --show-prefix)
> git submodule--helper list --prefix "$wt_prefix" | {
> while read mode sha1 stage sm_path
> do
> name=$(git submodule--helper name "$sm_path") || exit
> url=$(git config submodule."$name".url)
> if ! test -d "$sm_path"/.git && ! test -f "$sm_path"/.git
> then
> git submodule--helper clone --prefix "$wt_prefix" --path 
> "$sm_path" --name "$name" --url "$url"

The init and then clone should be covered by
"git submodule--helper update-clone", which may be better named as
"list-and-clone-if-necessary", then you get parallel cloning for free
as well. ;)

> pushd "$sm_path"
> git -c filter.lfs.smudge= -c 
> filter.lfs.required=false checkout -q $sha1 || exit
> git-lfs pull || exit
> popd
> fi
> done
> }
> popd
>
> Do you see an easier way to pass command line Git config instructions to the
> underlaying Git Submodule commands? If not, do you think a patch adding this
> would be worth working on?

I would build on top of origin/jk/submodule-c-credential at least, and using
"git submodule--helper update-clone" (origin/sb/submodule-parallel-update)

>
> I also started a discussion about that on the Git LFS issue page [5].

Unrelated to this, but about LFS:
Currently it is only possible to store the big blobs at a $VENDOR hosting site.
How would you backup you whole repository including all binariers?

I think we should add an option to store one blob in a dedicated ref
(refs/lfs/$blobsha1 or such). That way you can make a backup of the
repository including all large files using "git clone --mirror" and
you don't have
to rely on the $VENDOR hosting your files.

Thanks,
Stefan

>
> Thanks,
> Lars
>
>
> [1] https://github.com/github/git-lfs/issues/931
> [2] https://github.com/git/git/commit/1a8630dc3b1cc6f1361a4e5d94630133c24c97d9
> [3] https://developer.atlassian.com/blog/2016/04/git-lfs-12-clone-faster/
> [4] 
> https://github.com/git/git/blob/6a6636270fbaf74609cd3e1bd207dd2c420d640a/git-submodule.sh#L686-L731
> [5] https://github.com/github/git-lfs/issues/1172
>
--
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 v7 07/10] convert: unify the "auto" handling of CRLF

[tboegi]

From: Torsten Bögershausen 

Before this change,
$ echo "* text=auto" >.gitattributes
$ echo "* eol=crlf" >>.gitattributes

would have the same effect as
$ echo "* text" >.gitattributes
$ git config core.eol crlf

Since the 'eol' attribute had higher priority than 'text=auto', this may
corrupt binary files and is not what users expect to happen.

Make the 'eol' attribute to obey 'text=auto', and now
$ echo "* text=auto" >.gitattributes
$ echo "* eol=crlf" >>.gitattributes
behaves the same as
$ echo "* text=auto" >.gitattributes
$ git config core.eol crlf

In other words,
$ echo "* text=auto eol=crlf" >.gitattributes
has the same effect as
$ git config core.autocrlf true

and
$ echo "* text=auto eol=lf" >.gitattributes
has the same effect as
$ git config core.autocrlf input

Signed-off-by: Torsten Bögershausen 
---
 Documentation/config.txt| 14 +++---
 Documentation/gitattributes.txt | 15 +--
 convert.c   | 42 +
 convert.h   |  3 ++-
 t/t0025-crlf-auto.sh|  4 ++--
 t/t0027-auto-crlf.sh| 32 +++
 t/t6038-merge-text-auto.sh  | 23 ++
 7 files changed, 73 insertions(+), 60 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 4a27ad4..9caf6ae 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -389,13 +389,13 @@ file with mixed line endings would be reported by the 
`core.safecrlf`
 mechanism.
 
 core.autocrlf::
-   Setting this variable to "true" is almost the same as setting
-   the `text` attribute to "auto" on all files except that text
-   files are not guaranteed to be normalized: files that contain
-   `CRLF` in the repository will not be touched.  Use this
-   setting if you want to have `CRLF` line endings in your
-   working directory even though the repository does not have
-   normalized line endings.  This variable can be set to 'input',
+   Setting this variable to "true" is the same as setting
+   the `text` attribute to "auto" on all files and core.eol to "crlf".
+   Set to true if you want to have `CRLF` line endings in your
+   working directory and the repository has LF line endings.
+   Text files are guaranteed not to be normalized: files that contain
+   `CRLF` in the repository will not be touched.
+   This variable can be set to 'input',
in which case no output conversion is performed.
 
 core.symlinks::
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index e3b1de8..d7a124b 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -115,6 +115,7 @@ text file is normalized, its line endings are converted to 
LF in the
 repository.  To control what line ending style is used in the working
 directory, use the `eol` attribute for a single file and the
 `core.eol` configuration variable for all text files.
+Note that `core.autocrlf` overrides `core.eol`
 
 Set::
 
@@ -130,8 +131,9 @@ Unset::
 Set to string value "auto"::
 
When `text` is set to "auto", the path is marked for automatic
-   end-of-line normalization.  If Git decides that the content is
-   text, its line endings are normalized to LF on checkin.
+   end-of-line conversion.  If Git decides that the content is
+   text, its line endings are converted to LF on checkin.
+   When the file has been commited with CRLF, no conversion is done.
 
 Unspecified::
 
@@ -146,7 +148,7 @@ unspecified.
 ^
 
 This attribute sets a specific line-ending style to be used in the
-working directory.  It enables end-of-line normalization without any
+working directory.  It enables end-of-line conversion without any
 content checks, effectively setting the `text` attribute.
 
 Set to string value "crlf"::
@@ -186,9 +188,10 @@ the working directory, and prevent .jpg files from being 
normalized
 regardless of their content.
 
 
+*   text=auto
 *.txt  text
-*.vcproj   eol=crlf
-*.sh   eol=lf
+*.vcproj   text eol=crlf
+*.sh   text eol=lf
 *.jpg  -text
 
 
@@ -198,7 +201,7 @@ normalization in Git.
 
 If you simply want to have CRLF line endings in your working directory
 regardless of the repository you are working with, you can set the
-config variable "core.autocrlf" without changing any attributes.
+config variable "core.autocrlf" without using any attributes.
 
 
 [core]
diff --git a/convert.c b/convert.c
index 24ab095..3782172 100644
--- a/convert.c
+++ b/convert.c
@@ -227,7 +227,9 @@ static enum eol output_eol(enum crlf_action crlf_action)
return EOL_LF;
case CRLF_UNDEFINED:
case CRLF_AUTO_CRLF:
+   return EOL_CRLF;
case CRLF_AUTO_INPUT:
+   return EOL_LF;

Re: [PATCH v1] travis-ci: build documentation

[Junio C Hamano]

Lars Schneider  writes:

>> There also are existing instances of "useless ;" that would want to
>> be cleaned up regardless of portability issues.
> Unfortunately it seems to be required. Travis CI generates a shell script
> out of the yml file and I think they don't respect newlines or something...

If they squash all the lines into a single long line before
executing, these semicolons do indeed become necessary (we have to
write a logical single line shell script in our Makefiles with ';',
and I'd imagine Travis's scriptlets are done similarly).

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

[PATCH v7 08/10] convert.c: more safer crlf handling with text attribute

[tboegi]

From: Torsten Bögershausen 

This patch extends the work done in commit c480539:
"Make it work also for un-normalized repositories". Make sure that CRLF
can be converted round trip, or don't convert them at all.

The old handling would treat a file as unchanged after checkout,
as long as it is not touched in the work tree and mtime matches the value
recorded in the index.
When the mtime is changed in the working tree, or the inode is changed,
the file is reported as modified.

The following sequence is now handled reproducable:
$ git init
$ printf "line1\r\n" >file.bat
$ git add file.bat
$ git commit -m "Add file with CRLF" file.bat
$ echo "*.bat text eol=crlf" >.gitattributes
$ git commit -m "bat files should have CRLF"
$ git status
 # nothing to commit, working directory clean
$ git push 
$ printf "newline\r\n" >>file.bat
$ mv file.bat file.sav
$ git checkout file.bat
$ git status
 #modified:   file.bat

The new handling makes sure that after running "git reset --hard".
"git status" reports the working tree as clean regarding CRLF conversion.
It makes sure that the Git-internal eol conversion is
doing roundtrip. A user can still write an external smudge/clean filter
outside Git, which doesn't do a roundtrip and the working directory is
not clean.

The functionality of has_cr_in_index() is turned into has_crlf_in_index(),
and the function is integrated into would_convert_crlf_at_commit().

Check for CRLF in the index instead of CR, the bit CONVERT_STAT_BITS_ANY_CR
is no longer used and removed, as well as "lonecr" in struct text_stat.

Rewrite check_safe_crlf() in convert.c to simulate checkin-checkout,
to detect whether any line endings are converted.

Add a warning, similar to the CRLF-LF replacement, when a file is commited,
and after the next checkout the line endings are not what they should be.

Modify the lf_to_crlf_filter:
Files with LF are converted into CRLF, file with CRLF are not changed.
Files with mixed line endings are not converted, the filter fails, and Git
falls back to the non-streaming handling, see write_entry().

Signed-off-by: Torsten Bögershausen 
---
 Documentation/gitattributes.txt |  19 ++--
 convert.c   | 235 +---
 t/t0025-crlf-auto.sh|   8 +-
 t/t0027-auto-crlf.sh|  92 
 4 files changed, 213 insertions(+), 141 deletions(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index d7a124b..836461d 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -110,7 +110,7 @@ repository upon 'git add' and 'git commit'.
 `text`
 ^^
 
-This attribute enables and controls end-of-line normalization.  When a
+This attribute enables and controls end-of-line conversion.  When a
 text file is normalized, its line endings are converted to LF in the
 repository.  To control what line ending style is used in the working
 directory, use the `eol` attribute for a single file and the
@@ -120,8 +120,11 @@ Note that `core.autocrlf` overrides `core.eol`
 Set::
 
Setting the `text` attribute on a path enables end-of-line
-   normalization and marks the path as a text file.  End-of-line
+   conversion and marks the path as a text file.  End-of-line
conversion takes place without guessing the content type.
+   Files that have been commited with CRLF before the text attribute
+   is set and commited are not normalized. No end-of-line conversion
+   is done at checkout or checkin.
 
 Unset::
 
@@ -131,9 +134,9 @@ Unset::
 Set to string value "auto"::
 
When `text` is set to "auto", the path is marked for automatic
-   end-of-line conversion.  If Git decides that the content is
-   text, its line endings are converted to LF on checkin.
-   When the file has been commited with CRLF, no conversion is done.
+   end-of-line normalization.  If Git decides that the content is
+   text, and the path has no CRLF in the index,
+   its line endings are converted to LF on checkin.
 
 Unspecified::
 
@@ -148,8 +151,10 @@ unspecified.
 ^
 
 This attribute sets a specific line-ending style to be used in the
-working directory.  It enables end-of-line conversion without any
-content checks, effectively setting the `text` attribute.
+working directory.  It sets the `text` attribute, unless `text=auto`
+is specified.
+When the file had been commited with CRLF in the index, no conversion
+is done at checkout or commit.
 
 Set to string value "crlf"::
 
diff --git a/convert.c b/convert.c
index 3782172..3d36c45 100644
--- a/convert.c
+++ b/convert.c
@@ -17,7 +17,8 @@
 #define CONVERT_STAT_BITS_TXT_LF0x1
 #define CONVERT_STAT_BITS_TXT_CRLF  0x2
 #define CONVERT_STAT_BITS_BIN   0x4
-#define CONVERT_STAT_BITS_ANY_CR0x8
+
+#define CONVERT_STAT_BITS_MIXED (CONVERT_STAT_BITS_TXT_LF | 
CONVERT_STAT_BITS_TXT_CRLF)
 
 enum crlf_action {
CRLF_UNDEFINED,
@@ 

[PATCH v7 09/10] t6038; use crlf on all platforms

[tboegi]

From: Torsten Bögershausen 

t6038 uses different code, dependig if NATIVE_CRLF is set ot not. When
the native line endings are LF, merge.renormalize is not tested very well.
Change the test to always use CRLF by setting core.eol=crlf.
After doing so, the test fails:

rm '.gitattributes'
rm 'control_file'
rm 'file'
rm 'inert_file'
HEAD is now at 0d9ffb6 add line from b
error: addinfo_cache failed for path 'file'
file: unmerged (cbd69ec7cd12dd0989e853923867d94c8519aa52)
file: unmerged (ad55e240aeb42e0d9a0e18d6d8b02dd82ee3e527)
file: unmerged (99b633103c15c20cebebf821133ab526b0ff90b2)
fatal: git write-tree failed to write a tree
Merging:
0d9ffb6 add line from b
virtual a
found 1 common ancestor:
1c56df1 Initial
Auto-merging file
not ok 4 - Merge addition of text=auto

This will be addressed in the next commit.

Signed-off-by: Torsten Bögershausen 
---
 t/t6038-merge-text-auto.sh | 37 +++--
 1 file changed, 11 insertions(+), 26 deletions(-)

diff --git a/t/t6038-merge-text-auto.sh b/t/t6038-merge-text-auto.sh
index 33b77ee..0108ead 100755
--- a/t/t6038-merge-text-auto.sh
+++ b/t/t6038-merge-text-auto.sh
@@ -25,6 +25,7 @@ compare_files () {
 
 test_expect_success setup '
git config core.autocrlf false &&
+   git config core.eol crlf &&
 
echo first line | append_cr >file &&
echo first line >control_file &&
@@ -79,10 +80,8 @@ test_expect_success 'Merge after setting text=auto' '
same line
EOF
 
-   if test_have_prereq NATIVE_CRLF; then
-   append_cr expected.temp &&
-   mv expected.temp expected
-   fi &&
+   append_cr expected.temp &&
+   mv expected.temp expected &&
git config merge.renormalize true &&
git rm -fr . &&
rm -f .gitattributes &&
@@ -97,10 +96,8 @@ test_expect_success 'Merge addition of text=auto' '
same line
EOF
 
-   if test_have_prereq NATIVE_CRLF; then
-   append_cr expected.temp &&
-   mv expected.temp expected
-   fi &&
+   append_cr expected.temp &&
+   mv expected.temp expected &&
git config merge.renormalize true &&
git rm -fr . &&
rm -f .gitattributes &&
@@ -111,15 +108,9 @@ test_expect_success 'Merge addition of text=auto' '
 
 test_expect_success 'Detect CRLF/LF conflict after setting text=auto' '
echo "<<<" >expected &&
-   if test_have_prereq NATIVE_CRLF; then
-   echo first line | append_cr >>expected &&
-   echo same line | append_cr >>expected &&
-   echo === | append_cr >>expected
-   else
-   echo first line >>expected &&
-   echo same line >>expected &&
-   echo === >>expected
-   fi &&
+   echo first line | append_cr >>expected &&
+   echo same line | append_cr >>expected &&
+   echo === | append_cr >>expected &&
echo first line | append_cr >>expected &&
echo same line | append_cr >>expected &&
echo ">>>" >>expected &&
@@ -135,15 +126,9 @@ test_expect_success 'Detect LF/CRLF conflict from addition 
of text=auto' '
echo "<<<" >expected &&
echo first line | append_cr >>expected &&
echo same line | append_cr >>expected &&
-   if test_have_prereq NATIVE_CRLF; then
-   echo === | append_cr >>expected &&
-   echo first line | append_cr >>expected &&
-   echo same line | append_cr >>expected
-   else
-   echo === >>expected &&
-   echo first line >>expected &&
-   echo same line >>expected
-   fi &&
+   echo === | append_cr >>expected &&
+   echo first line | append_cr >>expected &&
+   echo same line | append_cr >>expected &&
echo ">>>" >>expected &&
git config merge.renormalize false &&
rm -f .gitattributes &&
-- 
2.0.0.rc1.6318.g0c2c796

--
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 v7 05/10] read-cache: factor out get_sha1_from_index() helper

[tboegi]

From: Torsten Bögershausen 

Factor out the retrieval of the sha1 for a given path in
read_blob_data_from_index() into the function get_sha1_from_index().

This will be used in the next commit, when convert.c can do the
analyze for "text=auto" without slurping the whole blob into memory
at once.

Add a wrapper definition get_sha1_from_cache().

Signed-off-by: Torsten Bögershausen 
---
 cache.h  |  3 +++
 read-cache.c | 29 ++---
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/cache.h b/cache.h
index b829410..bd1210a 100644
--- a/cache.h
+++ b/cache.h
@@ -379,6 +379,7 @@ extern void free_name_hash(struct index_state *istate);
 #define unmerge_cache_entry_at(at) unmerge_index_entry_at(_index, at)
 #define unmerge_cache(pathspec) unmerge_index(_index, pathspec)
 #define read_blob_data_from_cache(path, sz) 
read_blob_data_from_index(_index, (path), (sz))
+#define get_sha1_from_cache(path)  get_sha1_from_index (_index, (path))
 #endif
 
 enum object_type {
@@ -1008,6 +1009,8 @@ static inline void *read_sha1_file(const unsigned char 
*sha1, enum object_type *
return read_sha1_file_extended(sha1, type, size, LOOKUP_REPLACE_OBJECT);
 }
 
+const unsigned char *get_sha1_from_index(struct index_state *istate, const 
char *path);
+
 /*
  * This internal function is only declared here for the benefit of
  * lookup_replace_object().  Please do not call it directly.
diff --git a/read-cache.c b/read-cache.c
index d9fb78b..a3ef967 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2263,13 +2263,27 @@ int index_name_is_other(const struct index_state 
*istate, const char *name,
 
 void *read_blob_data_from_index(struct index_state *istate, const char *path, 
unsigned long *size)
 {
-   int pos, len;
+   const unsigned char *sha1;
unsigned long sz;
enum object_type type;
void *data;
 
-   len = strlen(path);
-   pos = index_name_pos(istate, path, len);
+   sha1 = get_sha1_from_index(istate, path);
+   if (!sha1)
+   return NULL;
+   data = read_sha1_file(sha1, , );
+   if (!data || type != OBJ_BLOB) {
+   free(data);
+   return NULL;
+   }
+   if (size)
+   *size = sz;
+   return data;
+}
+
+const unsigned char *get_sha1_from_index(struct index_state *istate, const 
char *path)
+{
+   int pos = index_name_pos(istate, path, strlen(path));
if (pos < 0) {
/*
 * We might be in the middle of a merge, in which
@@ -2285,14 +2299,7 @@ void *read_blob_data_from_index(struct index_state 
*istate, const char *path, un
}
if (pos < 0)
return NULL;
-   data = read_sha1_file(istate->cache[pos]->sha1, , );
-   if (!data || type != OBJ_BLOB) {
-   free(data);
-   return NULL;
-   }
-   if (size)
-   *size = sz;
-   return data;
+   return (istate->cache[pos]->sha1);
 }
 
 void stat_validity_clear(struct stat_validity *sv)
-- 
2.0.0.rc1.6318.g0c2c796

--
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 v7 10/10] ce_compare_data() did not respect conversion

[tboegi]

From: Torsten Bögershausen 

We define the working tree file is clean if either:

  * the result of running convert_to_git() on the working tree
contents matches what is in the index (because that would mean
doing another "git add" on the path is a no-op); OR

  * the result of running convert_to_working_tree() on the content
in the index matches what is in the working tree (because that
would mean doing another "git checkout -f" on the path is a
no-op).

Add an extra check in ce_compare_data() in read_cache.c.

Helped-by: Junio C Hamano 
Signed-off-by: Torsten Bögershausen 
---
 read-cache.c   | 61 ++
 t/t6038-merge-text-auto.sh | 14 +--
 2 files changed, 68 insertions(+), 7 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index a3ef967..48c4b31 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -156,17 +156,78 @@ void fill_stat_cache_info(struct cache_entry *ce, struct 
stat *st)
ce_mark_uptodate(ce);
 }
 
+/*
+ * Compare the data in buf with the data in the file pointed by fd and
+ * return 0 if they are identical, and non-zero if they differ.
+ */
+static int compare_with_fd(const char *input, ssize_t len, int fd)
+{
+   for (;;) {
+   char buf[1024 * 16];
+   ssize_t chunk_len, read_len;
+
+   chunk_len = sizeof(buf) < len ? sizeof(buf) : len;
+   read_len = xread(fd, buf, chunk_len ? chunk_len : 1);
+
+   if (!read_len)
+   /* EOF on the working tree file */
+   return !len ? 0 : -1;
+
+   if (!len)
+   /* we expected there is nothing left */
+   return -1;
+
+   if (memcmp(buf, input, read_len))
+   return -1;
+   input += read_len;
+   len -= read_len;
+   }
+}
+
 static int ce_compare_data(const struct cache_entry *ce, struct stat *st)
 {
int match = -1;
int fd = open(ce->name, O_RDONLY);
 
+   /*
+* Would another "git add" on the path change what is in the
+* index for the path?
+*/
if (fd >= 0) {
unsigned char sha1[20];
if (!index_fd(sha1, fd, st, OBJ_BLOB, ce->name, 0))
match = hashcmp(sha1, ce->sha1);
/* index_fd() closed the file descriptor already */
}
+   if (!match)
+   return match;
+
+   /*
+* Would another "git checkout -f" out of the index change
+* what is in the working tree file?
+*/
+   fd = open(ce->name, O_RDONLY);
+   if (fd >= 0) {
+   enum object_type type;
+   unsigned long size_long;
+   void *data = read_sha1_file(ce->sha1, , _long);
+
+   if (type == OBJ_BLOB) {
+   struct strbuf worktree = STRBUF_INIT;
+   if (convert_to_working_tree(ce->name, data,
+   size_long,
+   )) {
+   size_t size;
+   free(data);
+   data = strbuf_detach(, );
+   size_long = size;
+   }
+   if (!compare_with_fd(data, size_long, fd))
+   match = 0;
+   }
+   free(data);
+   close(fd);
+   }
return match;
 }
 
diff --git a/t/t6038-merge-text-auto.sh b/t/t6038-merge-text-auto.sh
index 0108ead..565daf3 100755
--- a/t/t6038-merge-text-auto.sh
+++ b/t/t6038-merge-text-auto.sh
@@ -108,9 +108,9 @@ test_expect_success 'Merge addition of text=auto' '
 
 test_expect_success 'Detect CRLF/LF conflict after setting text=auto' '
echo "<<<" >expected &&
-   echo first line | append_cr >>expected &&
-   echo same line | append_cr >>expected &&
-   echo === | append_cr >>expected &&
+   echo first line >>expected &&
+   echo same line >>expected &&
+   echo === >>expected &&
echo first line | append_cr >>expected &&
echo same line | append_cr >>expected &&
echo ">>>" >>expected &&
@@ -121,14 +121,13 @@ test_expect_success 'Detect CRLF/LF conflict after 
setting text=auto' '
fuzz_conflict file >file.fuzzy &&
compare_files expected file.fuzzy
 '
-
 test_expect_success 'Detect LF/CRLF conflict from addition of text=auto' '
echo "<<<" >expected &&
echo first line | append_cr >>expected &&
echo same line | append_cr >>expected &&
-   echo === | append_cr >>expected &&
-   echo first line | append_cr >>expected &&
-   echo same line | append_cr >>expected &&
+   echo ===  >>expected &&
+   echo first line >>expected &&
+   echo 

[PATCH v7 01/10] t0027: Make commit_chk_wrnNNO() reliable

[tboegi]

From: Torsten Bögershausen 

When the content of a commited file is unchanged and the attributes are changed,
Git may not detect that the next commit must treat the file as changed.
This happens when lstat() doesn't detect a change, since neither inode,
mtime nor size are changed.

Add a singe "Z" character to change the file size (and content).
When the files are compared later in checkout_files(), the "Z" is removed
before the comparison.

Signed-off-by: Torsten Bögershausen 
---
 Change since v6b:
 - Remove 2 sparse warnings, thanks Ramsay

diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index f33962b..9fe539b 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -12,7 +12,7 @@ fi
 
 compare_files () {
tr '\015\000' QN <"$1" >"$1".expect &&
-   tr '\015\000' QN <"$2" >"$2".actual &&
+   tr '\015\000' QN <"$2" | tr -d 'Z' >"$2".actual &&
test_cmp "$1".expect "$2".actual &&
rm "$1".expect "$2".actual
 }
@@ -114,6 +114,7 @@ commit_chk_wrnNNO () {
do
fname=${pfx}_$f.txt &&
cp $f $fname &&
+   printf Z >>"$fname" &&
git -c core.autocrlf=$crlf add $fname 2>/dev/null &&
git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname 
>"${pfx}_$f.err" 2>&1
done
-- 
2.0.0.rc1.6318.g0c2c796

--
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 v7 02/10] convert: allow core.autocrlf=input and core.eol=crlf

[tboegi]

From: Torsten Bögershausen 

Even though the configuration parser errors out when core.autocrlf
is set to 'input' when core.eol is set to 'crlf', there is no need
to do so, because the core.autocrlf setting trumps core.eol.

Allow all combinations of core.crlf and core.eol and document
that core.autocrlf overrides core.eol.

Signed-off-by: Torsten Bögershausen 
---
 Documentation/config.txt | 6 +++---
 config.c | 4 
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2cd6bdd..4a27ad4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -337,9 +337,9 @@ core.quotePath::
 
 core.eol::
Sets the line ending type to use in the working directory for
-   files that have the `text` property set.  Alternatives are
-   'lf', 'crlf' and 'native', which uses the platform's native
-   line ending.  The default value is `native`.  See
+   files that have the `text` property set when core.autocrlf is false.
+   Alternatives are 'lf', 'crlf' and 'native', which uses the platform's
+   native line ending.  The default value is `native`.  See
linkgit:gitattributes[5] for more information on end-of-line
conversion.
 
diff --git a/config.c b/config.c
index 9ba40bc..a6adc8b 100644
--- a/config.c
+++ b/config.c
@@ -803,8 +803,6 @@ static int git_default_core_config(const char *var, const 
char *value)
 
if (!strcmp(var, "core.autocrlf")) {
if (value && !strcasecmp(value, "input")) {
-   if (core_eol == EOL_CRLF)
-   return error("core.autocrlf=input conflicts 
with core.eol=crlf");
auto_crlf = AUTO_CRLF_INPUT;
return 0;
}
@@ -830,8 +828,6 @@ static int git_default_core_config(const char *var, const 
char *value)
core_eol = EOL_NATIVE;
else
core_eol = EOL_UNSET;
-   if (core_eol == EOL_CRLF && auto_crlf == AUTO_CRLF_INPUT)
-   return error("core.autocrlf=input conflicts with 
core.eol=crlf");
return 0;
}
 
-- 
2.0.0.rc1.6318.g0c2c796

--
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 v7 06/10] convert.c: stream and early out

[tboegi]

From: Torsten Bögershausen 

When statistics are done for the autocrlf handling, the search in
the content can be stopped, if e.g
- a search for binary is done, and a NUL character is found
- a search for CRLF is done, and the first CRLF is found.

Similar when statistics for binary vs non-binary are gathered:
Whenever a lone CR or NUL is found, the search can be aborted.

When checking out files in "auto" mode, any file that has a "lone CR"
or a CRLF will not be converted, so the search can be aborted early.

Add the new bit, CONVERT_STAT_BITS_ANY_CR,
which is set for either lone CR or CRLF.

Many binary files have a NUL very early (within the first few bytes,
latest within the first 1..2K).
It is often not necessary to load the whole content of a file or blob
into memory.

Use a streaming handling for blobs and files in the worktree.

Signed-off-by: Torsten Bögershausen 
---
 convert.c | 159 --
 1 file changed, 103 insertions(+), 56 deletions(-)

diff --git a/convert.c b/convert.c
index b1614bf..24ab095 100644
--- a/convert.c
+++ b/convert.c
@@ -3,6 +3,7 @@
 #include "run-command.h"
 #include "quote.h"
 #include "sigchain.h"
+#include "streaming.h"
 
 /*
  * convert.c - convert a file when checking it out and checking it in.
@@ -13,10 +14,10 @@
  * translation when the "text" attribute or "auto_crlf" option is set.
  */
 
-/* Stat bits: When BIN is set, the txt bits are unset */
 #define CONVERT_STAT_BITS_TXT_LF0x1
 #define CONVERT_STAT_BITS_TXT_CRLF  0x2
 #define CONVERT_STAT_BITS_BIN   0x4
+#define CONVERT_STAT_BITS_ANY_CR0x8
 
 enum crlf_action {
CRLF_UNDEFINED,
@@ -31,30 +32,36 @@ enum crlf_action {
 
 struct text_stat {
/* NUL, CR, LF and CRLF counts */
-   unsigned nul, lonecr, lonelf, crlf;
+   unsigned stat_bits, lonecr, lonelf, crlf;
 
/* These are just approximations! */
unsigned printable, nonprintable;
 };
 
-static void gather_stats(const char *buf, unsigned long size, struct text_stat 
*stats)
+static void do_gather_stats(const char *buf, unsigned long size,
+   struct text_stat *stats, unsigned earlyout)
 {
unsigned long i;
 
-   memset(stats, 0, sizeof(*stats));
-
+   if (!buf || !size)
+   return;
for (i = 0; i < size; i++) {
unsigned char c = buf[i];
if (c == '\r') {
+   stats->stat_bits |= CONVERT_STAT_BITS_ANY_CR;
if (i+1 < size && buf[i+1] == '\n') {
stats->crlf++;
i++;
-   } else
+   stats->stat_bits |= CONVERT_STAT_BITS_TXT_CRLF;
+   } else {
stats->lonecr++;
+   stats->stat_bits |= CONVERT_STAT_BITS_BIN;
+   }
continue;
}
if (c == '\n') {
stats->lonelf++;
+   stats->stat_bits |= CONVERT_STAT_BITS_TXT_LF;
continue;
}
if (c == 127)
@@ -67,7 +74,7 @@ static void gather_stats(const char *buf, unsigned long size, 
struct text_stat *
stats->printable++;
break;
case 0:
-   stats->nul++;
+   stats->stat_bits |= CONVERT_STAT_BITS_BIN;
/* fall through */
default:
stats->nonprintable++;
@@ -75,6 +82,8 @@ static void gather_stats(const char *buf, unsigned long size, 
struct text_stat *
}
else
stats->printable++;
+   if (stats->stat_bits & earlyout)
+   break; /* We found what we have been searching for */
}
 
/* If file ends with EOF then don't count this EOF as non-printable. */
@@ -86,41 +95,62 @@ static void gather_stats(const char *buf, unsigned long 
size, struct text_stat *
  * The same heuristics as diff.c::mmfile_is_binary()
  * We treat files with bare CR as binary
  */
-static int convert_is_binary(unsigned long size, const struct text_stat *stats)
+static void convert_nonprintable(struct text_stat *stats)
 {
-   if (stats->lonecr)
-   return 1;
-   if (stats->nul)
-   return 1;
if ((stats->printable >> 7) < stats->nonprintable)
-   return 1;
-   return 0;
+   stats->stat_bits |= CONVERT_STAT_BITS_BIN;
+}
+
+static void gather_stats(const char *buf, unsigned long size,
+struct text_stat *stats, unsigned earlyout)
+{
+   memset(stats, 0, sizeof(*stats));
+   do_gather_stats(buf, size, stats, earlyout);
+   

[PATCH v7 04/10] convert.c: ident + core.autocrlf didn't work

[tboegi]

From: Torsten Bögershausen 

When the ident attributes is set, get_stream_filter() did not obey
core.autocrlf=true, and the file was checked out with LF.

Change the rule when a streaming filter can be used:
- if an external filter is specified, don't use a stream filter.
- if the worktree eol is CRLF and "auto" is active, don't use a stream filter.
- Otherwise the stream filter can be used.

Add test cases in t0027.

Signed-off-by: Torsten Bögershausen 
---
 convert.c| 19 +++
 t/t0027-auto-crlf.sh |  2 +-
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/convert.c b/convert.c
index f524b8d..b1614bf 100644
--- a/convert.c
+++ b/convert.c
@@ -1380,27 +1380,22 @@ static struct stream_filter *ident_filter(const 
unsigned char *sha1)
 struct stream_filter *get_stream_filter(const char *path, const unsigned char 
*sha1)
 {
struct conv_attrs ca;
-   enum crlf_action crlf_action;
struct stream_filter *filter = NULL;
 
convert_attrs(, path);
-
if (ca.drv && (ca.drv->smudge || ca.drv->clean))
-   return filter;
+   return NULL;
+
+   if (ca.crlf_action == CRLF_AUTO || ca.crlf_action == CRLF_AUTO_CRLF)
+   return NULL;
 
if (ca.ident)
filter = ident_filter(sha1);
 
-   crlf_action = ca.crlf_action;
-
-   if ((crlf_action == CRLF_BINARY) ||
-   crlf_action == CRLF_AUTO_INPUT ||
-   (crlf_action == CRLF_TEXT_INPUT))
-   filter = cascade_filter(filter, _filter_singleton);
-
-   else if (output_eol(crlf_action) == EOL_CRLF &&
-!(crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_CRLF))
+   if (output_eol(ca.crlf_action) == EOL_CRLF)
filter = cascade_filter(filter, lf_to_crlf_filter());
+   else
+   filter = cascade_filter(filter, _filter_singleton);
 
return filter;
 }
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index fd5e326..9372589 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -493,7 +493,7 @@ fi
 export CRLF_MIX_LF_CR MIX NL
 
 # Same handling with and without ident
-for id in ""
+for id in "" ident
 do
for ceol in lf crlf native
do
-- 
2.0.0.rc1.6318.g0c2c796

--
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 v7 03/10] t0027: test cases for combined attributes

[tboegi]

From: Torsten Bögershausen 

Add more test cases for the not normalized files ("NNO"). The
"text" attribute is most important, use it as the first parameter.
"ident", if set, is the second paramater followed by the eol
attribute.  The eol attribute overrides core.autocrlf, which
overrides core.eol.
indent is not yet used, this will be done in the next commit.

Use loops to test more combinations of attributes, like
"* text eol=crlf" or especially "*text=auto eol=crlf".

Signed-off-by: Torsten Bögershausen 
---
 t/t0027-auto-crlf.sh | 298 ++-
 1 file changed, 129 insertions(+), 169 deletions(-)

diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 9fe539b..fd5e326 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -52,14 +52,17 @@ create_gitattributes () {
 create_NNO_files () {
for crlf in false true input
do
-   for attr in "" auto text -text lf crlf
+   for attr in "" auto text -text
do
-   pfx=NNO_${crlf}_attr_${attr} &&
-   cp CRLF_mix_LF ${pfx}_LF.txt &&
-   cp CRLF_mix_LF ${pfx}_CRLF.txt &&
-   cp CRLF_mix_LF ${pfx}_CRLF_mix_LF.txt &&
-   cp CRLF_mix_LF ${pfx}_LF_mix_CR.txt &&
-   cp CRLF_mix_LF ${pfx}_CRLF_nul.txt
+   for aeol in "" lf crlf
+   do
+   pfx=NNO_attr_${attr}_aeol_${aeol}_${crlf}
+   cp CRLF_mix_LF ${pfx}_LF.txt &&
+   cp CRLF_mix_LF ${pfx}_CRLF.txt &&
+   cp CRLF_mix_LF ${pfx}_CRLF_mix_LF.txt &&
+   cp CRLF_mix_LF ${pfx}_LF_mix_CR.txt &&
+   cp CRLF_mix_LF ${pfx}_CRLF_nul.txt
+   done
done
done
 }
@@ -100,16 +103,17 @@ commit_check_warn () {
 }
 
 commit_chk_wrnNNO () {
-   crlf=$1
-   attr=$2
-   lfwarn=$3
-   crlfwarn=$4
-   lfmixcrlf=$5
-   lfmixcr=$6
-   crlfnul=$7
-   pfx=NNO_${crlf}_attr_${attr}
+   attr=$1 ; shift
+   aeol=$1 ; shift
+   crlf=$1 ; shift
+   lfwarn=$1 ; shift
+   crlfwarn=$1 ; shift
+   lfmixcrlf=$1 ; shift
+   lfmixcr=$1 ; shift
+   crlfnul=$1 ; shift
+   pfx=NNO_attr_${attr}_aeol_${aeol}_${crlf}
#Commit files on top of existing file
-   create_gitattributes "$attr" &&
+   create_gitattributes "$attr" $aeol &&
for f in LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
do
fname=${pfx}_$f.txt &&
@@ -122,19 +126,19 @@ commit_chk_wrnNNO () {
test_expect_success "commit NNO files crlf=$crlf attr=$attr LF" '
check_warning "$lfwarn" ${pfx}_LF.err
'
-   test_expect_success "commit NNO files crlf=$crlf attr=$attr CRLF" '
+   test_expect_success "commit NNO files attr=$attr aeol=$aeol crlf=$crlf 
CRLF" '
check_warning "$crlfwarn" ${pfx}_CRLF.err
'
 
-   test_expect_success "commit NNO files crlf=$crlf attr=$attr 
CRLF_mix_LF" '
+   test_expect_success "commit NNO files attr=$attr aeol=$aeol crlf=$crlf 
CRLF_mix_LF" '
check_warning "$lfmixcrlf" ${pfx}_CRLF_mix_LF.err
'
 
-   test_expect_success "commit NNO files crlf=$crlf attr=$attr LF_mix_cr" '
+   test_expect_success "commit NNO files attr=$attr aeol=$aeol crlf=$crlf 
LF_mix_cr" '
check_warning "$lfmixcr" ${pfx}_LF_mix_CR.err
'
 
-   test_expect_success "commit NNO files crlf=$crlf attr=$attr CRLF_nul" '
+   test_expect_success "commit NNO files attr=$attr aeol=$aeol crlf=$crlf 
CRLF_nul" '
check_warning "$crlfnul" ${pfx}_CRLF_nul.err
'
 }
@@ -163,6 +167,7 @@ stats_ascii () {
 
 # contruct the attr/ returned by git ls-files --eol
 # Take none (=empty), one or two args
+# convert.c: eol=XX overrides text=auto
 attr_ascii () {
case $1,$2 in
-text,*)   echo "-text" ;;
@@ -170,8 +175,8 @@ attr_ascii () {
text,lf)   echo "text eol=lf" ;;
text,crlf) echo "text eol=crlf" ;;
auto,) echo "text=auto" ;;
-   auto,lf)   echo "text=auto eol=lf" ;;
-   auto,crlf) echo "text=auto eol=crlf" ;;
+   auto,lf)   echo "text eol=lf" ;;
+   auto,crlf) echo "text eol=crlf" ;;
lf,)   echo "text eol=lf" ;;
crlf,) echo "text eol=crlf" ;;
,) echo "" ;;
@@ -196,28 +201,29 @@ check_files_in_repo () {
 }
 
 check_in_repo_NNO () {
-   crlf=$1
-   attr=$2
-   lfname=$3
-   crlfname=$4
-   lfmixcrlf=$5
-   lfmixcr=$6
-   crlfnul=$7
-   pfx=NNO_${crlf}_attr_${attr}_
-   test_expect_success "compare_files $lfname ${pfx}LF.txt" '
-   compare_files $lfname ${pfx}LF.txt
+   attr=$1 ; shift
+   aeol=$1 ; shift
+   crlf=$1 ; shift

Re: [PATCH v7 19/33] refs: allow log-only updates

[David Turner]

On Thu, 2016-04-21 at 16:17 +0200, Michael Haggerty wrote:
> On 03/01/2016 01:52 AM, David Turner wrote:
> > 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 
> > ---
> >  refs/files-backend.c | 15 ++-
> >  refs/refs-internal.h |  7 +++
> >  2 files changed, 17 insertions(+), 5 deletions(-)
> > 
> > diff --git a/refs/files-backend.c b/refs/files-backend.c
> > index 1f565cb..189b86e 100644
> > --- a/refs/files-backend.c
> > +++ b/refs/files-backend.c
> > @@ -2702,7 +2702,7 @@ static int commit_ref_update(struct ref_lock
> > *lock,
> > }
> > }
> > }
> > -   if (commit_ref(lock)) {
> > +   if (!(flags & REF_LOG_ONLY) && commit_ref(lock)) {
> > error("Couldn't set %s", lock->ref_name);
> > unlock_ref(lock);
> > return -1;
> > @@ -3056,7 +3056,8 @@ static int files_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));
> >  
> > @@ -3086,7 +3087,9 @@ static int files_transaction_commit(struct
> > ref_transaction *transaction,
> > update->flags |= REF_NEEDS_COMMIT;
> > }
> > }
> > -   if (!(update->flags & REF_NEEDS_COMMIT)) {
> > +
> > +   if (!(update->flags & REF_LOG_ONLY) &&
> > +   !(update->flags & REF_NEEDS_COMMIT)) {
> 
> I was just going over this series again, and I think this hunk is
> incorrect. If REF_LOG_ONLY, we created and opened the lockfile. And
> we
> didn't call write_ref_to_logfile(), so the lockfile is still open.
> That
> means that we want to call close_ref() here to free up the file
> descriptor. (Note that close_ref() closes the lockfile but doesn't
> release the lock. That is done further down by unlock_ref().)
> 
> So I think this hunk should be omitted.
> 
> I realize that this patch series is obsolete, so there is no need to
> re-submit. I just wanted to get a sanity check as I implement a new
> version of this patch that I'm not misunderstanding something.

I think your logic seems sound, but if you're going to change this,
please make sure tests still pass -- as you know, this area is
something of a minefield.
--
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: How to have EOL=LF and keep binary files auto-detection?

[Torsten Bögershausen]

On 25.04.16 16:11, Kirill Likhodedov wrote:
> Hi, 
> 
> I wonder if it is possible both to have LFs in all and only text files in 
> working trees, and keep Git’s binary files auto-detection?
> 
> To be more precise:
> * we want all text files to be checked out in LF; 
> * we don’t want force people to set “core.autocrlf” to false, preferring to 
> keep this configuration in .gitattributes; 
> * we obviously don’t want binary files to be touched by eol-normalization; 
> * we also don’t want to declare all possible patterns of binary files - Git 
> is good enough in detecting them automatically.
> 
> However, I’ve found no way to do so.
> 
> If I declare `* eol=lf` in .gitattributes, it makes Git treat all files as 
> text and thus convert CRLF to LF even in binary files. It is consistent with 
> man, but a bit surprising to have e.g. a zip or png file modified in this way.
> 
> One could expect `* text=auto eol=lf` to work the way we want, but 
> unfortunately it doesn’t work either: “eol=lf” forces “text” on all files.
> 
> Thanks a lot for your help!
> -- Kirill.

The short answer: Git doesn't currently do that.
The closest you can get, is to use
echo "* text=auto" >.gitattributes
and
git config core.eol lf
git config core.autocrlf false.

The longer answer is, that I am working on a patch to allow just
the combination of "* text=auto eol=lf" to work as you want it.

Which platform do you use ?
And (out of curiosity, why do you want text files with LF ?)

If you are willing to compile and install Git yourself,
you can use the branch here:
https://github.com/tboegi/git/commits/160421_0706_reliable_t0027_allow_TC_combined_ident_CRLF_v7

Feedback is welcome, if it works as expected.




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

Re: [PATCH/RFC 4/6] transport: add refspec list parameters to functions

[David Turner]

On Wed, 2016-04-20 at 16:57 -0400, Jeff King wrote:
> On Wed, Apr 20, 2016 at 04:46:55PM -0400, David Turner wrote:
> 
> > As you note, it appears that git-daemon does sort-of have support
> > for
> > extra args -- see parse_host_arg.  So it wouldn't be hard to add
> > something here. Unfortunately, current versions of git die on
> > unknown
> > args.  So this change would not be backwards-compatible.  We could
> > put
> > a decider on it so that clients would only try it when explicitly
> > enabled.  Or we could have clients try it with, and in the event of
> > an
> > error, retry without.  Neither is ideal, but both are possible.
> 
> Right. This ends up being the same difficulty that the v2 protocol
> encountered; how do you figure out what you can speak without
> resorting
> to expensive fallbacks, when do you flip the switch, do you remember
> the
> protocol you used last time with this server, etc.

Right.

[moved]
> > If I read this code correctly, git-over-ssh will pass through
> > arbitrary
> > arguments.  So this should be trivial.
> 
> It does if you are ssh-ing to a real shell-level account on the
> server,
> but if you are using git-shell or some other wrapper to restrict
> clients
> from running arbitrary commands, it will likely reject it.

Oh, I see how I was mis-reading shell.c.  Oops.
[/moved]


> Which isn't to say it's necessarily a bad thing. Maybe the path
> forward
> instead of v2 is to shoe-horn this data into the pre-protocol
> conversation, and go from there. The protocol accepts that "somehow"
> it
> got some extra data from the transport layer, and acts on its
> uniformly.

OK, so it seems like only HTTP (and non-git-shell-git://) allow backwar
ds-compatible optional pre-protocol messages.  So we don't have good
options; we only have bad ones.  We have to decide which particular
kind of badness we're willing to accept, and to what degree we care
about extensibility.  As I see it, the badness options are (in no
particular order):

1. Nothing changes.
2. HTTP grows more extensions; other protocols stagnate.
3. HTTP grows extensions; other protocols get extensions but:
   a. only use them on explicit client configuration or
   b. try/fail/remember per-remote
4. A backwards-incompatible protocol v2 is introduced, which
   hits alternate endpoints (with the same a/b as above).  This is
   different from 3. in that protocol v2 is explicitly designed around
   a capabilities negotiation phase rather than unilateral client-side
   decisions.
5. Think of another way to make fetch performant with many refs, and
defer the extension decision.

--
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] Update git-p4 to be compatible with git-lfs 1.2

[SZEDER Gábor]

> > * One option on the Travis front would be to just test one combination
> > of the 1.1 build - e.g. linux + clang + 1.1, so you'll stay within the
> > 5 parallel builds while also having some coverage on lfs 1.1.
> TBH I still think testing an outdated Git LFS version does not justify
> +10 extra minutes of computing.

I agree that checking compatibility with an older Git LFS version
doesn't worth the extra 10 minutes.  However, since Git LFS is only
involved in two test scripts and for the rest of the test suite it
doesn't matter at all, doing a full build and running the whole test
suite for the sole sake of a different Git LFS version is definitely
unnecessary.

The Bash completion and prompt scripts are in a similar situation:
there are only two test scripts involved and the rest of the test
suite couldn't care less.  However, we definitely want to support
older Bash versions as well, all the way back to v3.0, and there are a
few commits fixing breakages reported by users of old Bash versions.

As I somehow grew fond of those Bash scripts over the years, I put
together a couple of patches allowing me to say 'cd t && make -j4
full-bash-test', which runs the completion and prompt tests with
multiple Bash versions.  For the seven major and minor releases
including and after v3.0 it usually takes less than 8 seconds.  As far
as runtime goes, I think that's well worth it.

You can have a look at these patches at

  https://github.com/szeder/git completion-test-multiple-bash-versions

and perhaps you could even adapt it to LFS and/or p4 somehow.

> Plus if we want to be consistent we would
> need to do the same for LFS 1.0, 1.2, and for pretty much every other
> dependency...  

I'm not sure we should be consistent in this case, at least not solely
for consistency's sake and not in git.git. Taking what I did for Bash
and doing it for different versions of LFS, p4, etc. could perhaps
keep the runtime under control, but t/Makefile would surely get out
of control rather quickly.  Putting these into a travis-ci matrix is
so much simpler, but the runtime makes it infeasible, of course.

I think the best we can do is to keep this out of git.git and let
(hope?) developers interested in a particular subsystem do this
"multiple version compatibility" tests as they see fit.


--
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 81/83] apply: roll back index in case of error

[Johannes Schindelin]

Hi Chris,

On Sun, 24 Apr 2016, Christian Couder wrote:

> @@ -4734,16 +4737,22 @@ int apply_all_patches(struct apply_state *state,
>   read_stdin = 0;
>   set_default_whitespace_mode(state);
>   res = apply_patch(state, fd, arg, options);
> - if (res < 0)
> + if (res < 0) {
> + if (state->lock_file)
> + rollback_lock_file(state->lock_file);
>   return -1;
> + }
>   errs |= res;
>   close(fd);

In case of error, this leaves fd open, which in the end will prevent the
"patch" file, and hence the "rebase-apply/" directory from being removed
on Windows. This triggered a failure of t4014 here (and possibly more, but
it took me quite a while to track this down, what with builtin/am.c's
am_destroy() not bothering at all to check the return value of
remove_dir_recursively(), resulting in the error to be caught only much,
much later).

Could you please review all open()/close() and fopen()/fclose() calls in
your patch series, to make sure that there are no mistakes? A passing test
suite does not really make me confident here, as our code coverage is not
quite 100%.

Thanks,
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] http: Support sending custom HTTP headers

[Shawn Pearce]

On Mon, Apr 25, 2016 at 6:13 AM, Johannes Schindelin
 wrote:
> To make communication for `git fetch`, `git ls-remote` and friends extra
> secure, we introduce a way to send custom HTTP headers with all
> requests.

Hmm. Its not Apr 1 2016. So I guess you are serious. :)

> This allows us, for example, to send an extra token that the server
> tests for. The server could use this token e.g. to ensure that only
> certain operations or refs are allowed, or allow the token to be used
> only once.
>
> This feature can be used like this:
>
> git -c http.extraheader='Secret: sssh!' fetch $URL $REF

Its not very secure to be adding secure data to the command line, e.g.
on Linux you can see that data in /proc.
--
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 v5b 11/17] ref-filter: introduce symref_atom_parser() and refname_atom_parser()

[Karthik Nayak]

Using refname_atom_parser_internal(), introduce symref_atom_parser() and
refname_atom_parser() which will parse the atoms %(symref) and
%(refname) respectively. Store the parsed information into the
'used_atom' structure based on the modifiers used along with the atoms.

Now the '%(symref)' atom supports the ':strip' atom modifier. Update the
Documentation and tests to reflect this.

Helped-by: Jeff King 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  5 +++
 ref-filter.c   | 78 ++
 t/t6300-for-each-ref.sh|  9 +
 3 files changed, 59 insertions(+), 33 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 5c85f27..e0c0a12 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -163,6 +163,11 @@ if::
the value between the %(if:...) and %(then) atoms with the
given string.
 
+symref::
+   The ref which the given symbolic ref refers to. If not a
+   symbolic ref, nothing is printed. Respects the `:short` and
+   `:strip` options in the same way as `refname` above.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/ref-filter.c b/ref-filter.c
index 778fe02..933f8ca 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -176,6 +176,16 @@ static void objectname_atom_parser(struct used_atom *atom, 
const char *arg)
die(_("unrecognized %%(objectname) argument: %s"), arg);
 }
 
+static void symref_atom_parser(struct used_atom *atom, const char *arg)
+{
+   return refname_atom_parser_internal(>u.refname, arg, atom->name);
+}
+
+static void refname_atom_parser(struct used_atom *atom, const char *arg)
+{
+   return refname_atom_parser_internal(>u.refname, arg, atom->name);
+}
+
 static align_type parse_align_position(const char *s)
 {
if (!strcmp(s, "right"))
@@ -244,7 +254,7 @@ static struct {
cmp_type cmp_type;
void (*parser)(struct used_atom *atom, const char *arg);
 } valid_atom[] = {
-   { "refname" },
+   { "refname" , FIELD_STR, refname_atom_parser },
{ "objecttype" },
{ "objectsize", FIELD_ULONG },
{ "objectname", FIELD_STR, objectname_atom_parser },
@@ -273,7 +283,7 @@ static struct {
{ "contents", FIELD_STR, contents_atom_parser },
{ "upstream", FIELD_STR, remote_ref_atom_parser },
{ "push", FIELD_STR, remote_ref_atom_parser },
-   { "symref" },
+   { "symref", FIELD_STR, symref_atom_parser },
{ "flag" },
{ "HEAD" },
{ "color", FIELD_STR, color_atom_parser },
@@ -1058,21 +1068,16 @@ static inline char *copy_advance(char *dst, const char 
*src)
return dst;
 }
 
-static const char *strip_ref_components(const char *refname, const char 
*nr_arg)
+static const char *strip_ref_components(const char *refname, unsigned int len)
 {
-   char *end;
-   long nr = strtol(nr_arg, , 10);
-   long remaining = nr;
+   long remaining = len;
const char *start = refname;
 
-   if (nr < 1 || *end != '\0')
-   die(_(":strip= requires a positive integer argument"));
-
while (remaining) {
switch (*start++) {
case '\0':
-   die(_("ref '%s' does not have %ld components to 
:strip"),
-   refname, nr);
+   die(_("ref '%s' does not have %ud components to 
:strip"),
+   refname, len);
case '/':
remaining--;
break;
@@ -1081,6 +1086,16 @@ static const char *strip_ref_components(const char 
*refname, const char *nr_arg)
return start;
 }
 
+static const char *show_ref(struct refname_atom *atom, const char *refname)
+{
+   if (atom->option == R_SHORT)
+   return shorten_unambiguous_ref(refname, warn_ambiguous_refs);
+   else if (atom->option == R_STRIP)
+   return strip_ref_components(refname, atom->strip);
+   else
+   return refname;
+}
+
 static void fill_remote_ref_details(struct used_atom *atom, const char 
*refname,
struct branch *branch, const char **s)
 {
@@ -1153,6 +1168,21 @@ char *get_head_description(void)
return strbuf_detach(, NULL);
 }
 
+static const char *get_symref(struct used_atom *atom, struct ref_array_item 
*ref)
+{
+   if (!ref->symref)
+   return "";
+   else
+   return show_ref(>u.refname, ref->symref);
+}
+
+static const char *get_refname(struct used_atom *atom, struct ref_array_item 
*ref)
+{
+   if (ref->kind & FILTER_REFS_DETACHED_HEAD)
+   return get_head_description();
+   return show_ref(>u.refname, 

[PATCH v5b 09/17] ref-filter: make "%(symref)" atom work with the ':short' modifier

[Karthik Nayak]

The "%(symref)" atom doesn't work when used with the ':short' modifier
because we strictly match only 'symref' for setting the 'need_symref'
indicator. Fix this by using comparing with valid_atom rather than used_atom.

Add tests for %(symref) and %(symref:short) while we're here.

Helped-by: Junio C Hamano 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c|  2 +-
 t/t6300-for-each-ref.sh | 24 
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/ref-filter.c b/ref-filter.c
index b898bcd..083cc27 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -338,7 +338,7 @@ int parse_ref_filter_atom(const char *atom, const char *ep)
valid_atom[i].parser(_atom[at], arg);
if (*atom == '*')
need_tagged = 1;
-   if (!strcmp(used_atom[at].name, "symref"))
+   if (!strcmp(valid_atom[i].name, "symref"))
need_symref = 1;
return at;
 }
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 2c5f177..b06ea1c 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -38,6 +38,7 @@ test_atom() {
case "$1" in
head) ref=refs/heads/master ;;
 tag) ref=refs/tags/testtag ;;
+sym) ref=refs/heads/sym ;;
   *) ref=$1 ;;
esac
printf '%s\n' "$3" >expected
@@ -565,4 +566,27 @@ test_expect_success 'Verify sort with multiple keys' '
refs/tags/bogo refs/tags/master > actual &&
test_cmp expected actual
 '
+
+test_expect_success 'Add symbolic ref for the following tests' '
+   git symbolic-ref refs/heads/sym refs/heads/master
+'
+
+cat >expected < actual &&
+   test_cmp expected actual
+'
+
+cat >expected < actual &&
+   test_cmp expected actual
+'
+
 test_done
-- 
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 v5b 02/17] ref-filter: include reference to 'used_atom' within 'atom_value'

[Karthik Nayak]

Ensure that each 'atom_value' has a reference to its corresponding
'used_atom'. This let's us use values within 'used_atom' in the
'handler' function.

Hence we can get the %(align) atom's parameters directly from the
'used_atom' therefore removing the necessity of passing %(align) atom's
parameters to 'atom_value'.

This also acts as a preparatory patch for the upcoming patch where we
introduce %(if:equals=) and %(if:notequals=).

Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 41e73f0..12e646c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -230,11 +230,9 @@ struct ref_formatting_state {
 
 struct atom_value {
const char *s;
-   union {
-   struct align align;
-   } u;
void (*handler)(struct atom_value *atomv, struct ref_formatting_state 
*state);
unsigned long ul; /* used for sorting when not FIELD_STR */
+   struct used_atom *atom;
 };
 
 /*
@@ -370,7 +368,7 @@ static void align_atom_handler(struct atom_value *atomv, 
struct ref_formatting_s
push_stack_element(>stack);
new = state->stack;
new->at_end = end_align_handler;
-   new->at_end_data = >u.align;
+   new->at_end_data = >atom->u.align;
 }
 
 static void if_then_else_handler(struct ref_formatting_stack **stack)
@@ -1069,6 +1067,7 @@ static void populate_value(struct ref_array_item *ref)
struct branch *branch = NULL;
 
v->handler = append_atom;
+   v->atom = atom;
 
if (*name == '*') {
deref = 1;
@@ -1133,7 +1132,6 @@ static void populate_value(struct ref_array_item *ref)
v->s = " ";
continue;
} else if (starts_with(name, "align")) {
-   v->u.align = atom->u.align;
v->handler = align_atom_handler;
continue;
} else if (!strcmp(name, "end")) {
-- 
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 v5b 13/17] ref-filter: add `:dir` and `:base` options for ref printing atoms

[Karthik Nayak]

Add the options `:dir` and `:base` to all ref printing ('%(refname)',
'%(symref)', '%(push)' and '%(upstream)') atoms. The `:dir` option gives
the directory (the part after $GIT_DIR/) of the ref without the
refname. The `:base` option gives the base directory of the given
ref (i.e. the directory following $GIT_DIR/refs/).

Add tests and documentation for the same.

Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt | 34 +++---
 ref-filter.c   | 29 +
 t/t6300-for-each-ref.sh| 24 
 3 files changed, 68 insertions(+), 19 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 3eaf770..4a5e9ea 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -96,7 +96,9 @@ refname::
slash-separated path components from the front of the refname
(e.g., `%(refname:strip=2)` turns `refs/tags/foo` into `foo`.
`` must be a positive integer.  If a displayed ref has fewer
-   components than ``, the command aborts with an error.
+   components than ``, the command aborts with an error. For the base
+   directory of the ref (i.e. foo in refs/foo/bar/boz) append
+   `:base`. For the entire directory path append `:dir`.
 
 objecttype::
The type of the object (`blob`, `tree`, `commit`, `tag`).
@@ -114,22 +116,23 @@ objectname::
 
 upstream::
The name of a local ref which can be considered ``upstream''
-   from the displayed ref. Respects `:short` and `:strip` in the
-   same way as `refname` above.  Additionally respects `:track`
-   to show "[ahead N, behind M]" and `:trackshort` to show the
-   terse version: ">" (ahead), "<" (behind), "<>" (ahead and
-   behind), or "=" (in sync). `:track` also prints "[gone]"
-   whenever unknown upstream ref is encountered. Append
-   `:track,nobracket` to show tracking information without
-   brackets (i.e "ahead N, behind M").  Has no effect if the ref
-   does not have tracking information associated with it.
+   from the displayed ref. Respects `:short`, `:strip`, `:base`
+   and `:dir` in the same way as `refname` above.  Additionally
+   respects `:track` to show "[ahead N, behind M]" and
+   `:trackshort` to show the terse version: ">" (ahead), "<"
+   (behind), "<>" (ahead and behind), or "=" (in sync). `:track`
+   also prints "[gone]" whenever unknown upstream ref is
+   encountered. Append `:track,nobracket` to show tracking
+   information without brackets (i.e "ahead N, behind M").  Has
+   no effect if the ref does not have tracking information
+   associated with it.
 
 push::
The name of a local ref which represents the `@{push}`
location for the displayed ref. Respects `:short`, `:strip`,
-   `:track`, and `:trackshort` options as `upstream`
-   does. Produces an empty string if no `@{push}` ref is
-   configured.
+   `:track`, `:trackshort`, `:base` and `:dir` options as
+   `upstream` does. Produces an empty string if no `@{push}` ref
+   is configured.
 
 HEAD::
'*' if HEAD matches current ref (the checked out branch), ' '
@@ -166,8 +169,9 @@ if::
 
 symref::
The ref which the given symbolic ref refers to. If not a
-   symbolic ref, nothing is printed. Respects the `:short` and
-   `:strip` options in the same way as `refname` above.
+   symbolic ref, nothing is printed. Respects the `:short`,
+   `:strip`, `:base` and `:dir` options in the same way as
+   `refname` above.
 
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
diff --git a/ref-filter.c b/ref-filter.c
index 3b42aab..97977a5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -31,7 +31,7 @@ struct if_then_else {
 };
 
 struct refname_atom {
-   enum { R_NORMAL, R_SHORT, R_STRIP } option;
+   enum { R_BASE, R_DIR, R_NORMAL, R_SHORT, R_STRIP } option;
unsigned int strip;
 };
 
@@ -93,7 +93,11 @@ static void refname_atom_parser_internal(struct refname_atom 
*atom,
atom->option = R_STRIP;
if (strtoul_ui(arg, 10, >strip) || atom->strip <= 0)
die(_("positive value expected refname:strip=%s"), arg);
-   }   else
+   } else if (!strcmp(arg, "dir"))
+   atom->option = R_DIR;
+   else if (!strcmp(arg, "base"))
+   atom->option = R_BASE;
+   else
die(_("unrecognized %%(%s) argument: %s"), name, arg);
 }
 
@@ -252,7 +256,6 @@ static void if_atom_parser(struct used_atom *atom, const 
char *arg)
die(_("unrecognized %%(if) argument: %s"), arg);
 }
 
-
 static struct {
const char *name;
cmp_type cmp_type;
@@ -1096,7 +1099,25 @@ static const char 

[PATCH v5b 05/17] ref-filter: move get_head_description() from branch.c

[Karthik Nayak]

Move the implementation of get_head_description() from branch.c to
ref-filter.  This gives a description of the HEAD ref if called. This
is used as the refname for the HEAD ref whenever the
FILTER_REFS_DETACHED_HEAD option is used. Make it public because we
need it to calculate the length of the HEAD refs description in
branch.c:calc_maxwidth() when we port branch.c to use ref-filter
APIs.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/branch.c | 31 ---
 ref-filter.c | 38 --
 ref-filter.h |  2 ++
 3 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 0adba62..d467840 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -361,37 +361,6 @@ static void add_verbose_info(struct strbuf *out, struct 
ref_array_item *item,
strbuf_release();
 }
 
-static char *get_head_description(void)
-{
-   struct strbuf desc = STRBUF_INIT;
-   struct wt_status_state state;
-   memset(, 0, sizeof(state));
-   wt_status_get_state(, 1);
-   if (state.rebase_in_progress ||
-   state.rebase_interactive_in_progress)
-   strbuf_addf(, _("(no branch, rebasing %s)"),
-   state.branch);
-   else if (state.bisect_in_progress)
-   strbuf_addf(, _("(no branch, bisect started on %s)"),
-   state.branch);
-   else if (state.detached_from) {
-   /* TRANSLATORS: make sure these match _("HEAD detached at ")
-  and _("HEAD detached from ") in wt-status.c */
-   if (state.detached_at)
-   strbuf_addf(, _("(HEAD detached at %s)"),
-   state.detached_from);
-   else
-   strbuf_addf(, _("(HEAD detached from %s)"),
-   state.detached_from);
-   }
-   else
-   strbuf_addstr(, _("(no branch)"));
-   free(state.branch);
-   free(state.onto);
-   free(state.detached_from);
-   return strbuf_detach(, NULL);
-}
-
 static void format_and_print_ref_item(struct ref_array_item *item, int 
maxwidth,
  struct ref_filter *filter, const char 
*remote_prefix)
 {
diff --git a/ref-filter.c b/ref-filter.c
index 7d3af1c..fcb3353 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -13,6 +13,7 @@
 #include "utf8.h"
 #include "git-compat-util.h"
 #include "version.h"
+#include "wt-status.h"
 
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
@@ -1077,6 +1078,37 @@ static void fill_remote_ref_details(struct used_atom 
*atom, const char *refname,
*s = refname;
 }
 
+char *get_head_description(void)
+{
+   struct strbuf desc = STRBUF_INIT;
+   struct wt_status_state state;
+   memset(, 0, sizeof(state));
+   wt_status_get_state(, 1);
+   if (state.rebase_in_progress ||
+   state.rebase_interactive_in_progress)
+   strbuf_addf(, _("(no branch, rebasing %s)"),
+   state.branch);
+   else if (state.bisect_in_progress)
+   strbuf_addf(, _("(no branch, bisect started on %s)"),
+   state.branch);
+   else if (state.detached_from) {
+   /* TRANSLATORS: make sure these match _("HEAD detached at ")
+  and _("HEAD detached from ") in wt-status.c */
+   if (state.detached_at)
+   strbuf_addf(, _("(HEAD detached at %s)"),
+   state.detached_from);
+   else
+   strbuf_addf(, _("(HEAD detached from %s)"),
+   state.detached_from);
+   }
+   else
+   strbuf_addstr(, _("(no branch)"));
+   free(state.branch);
+   free(state.onto);
+   free(state.detached_from);
+   return strbuf_detach(, NULL);
+}
+
 /*
  * Parse the object referred by ref, and grab needed value.
  */
@@ -1116,9 +1148,11 @@ static void populate_value(struct ref_array_item *ref)
name++;
}
 
-   if (starts_with(name, "refname"))
+   if (starts_with(name, "refname")) {
refname = ref->refname;
-   else if (starts_with(name, "symref"))
+   if (ref->kind & FILTER_REFS_DETACHED_HEAD)
+   refname = get_head_description();
+   } else if (starts_with(name, "symref"))
refname = ref->symref ? ref->symref : "";
else if (starts_with(name, "upstream")) {
const char *branch_name;
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..4aea594 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -106,5 +106,7 @@ int 

[PATCH v5b 12/17] ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()

[Karthik Nayak]

Use the recently introduced refname_atom_parser_internal() within
remote_ref_atom_parser(), this provides a common base for all the ref
printing atoms, allowing %(upstream) and %(push) to also use the
':strip' option.

The atoms '%(push)' and '%(upstream)' will retain the ':track' and
':trackshort' atom modifiers to themselves as they have no meaning in
context to the '%(refname)' and '%(symref)' atoms.

Update the documentation and tests to reflect the same.

Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt | 27 ++-
 ref-filter.c   | 26 +++---
 t/t6300-for-each-ref.sh|  2 ++
 3 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index e0c0a12..3eaf770 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -114,21 +114,22 @@ objectname::
 
 upstream::
The name of a local ref which can be considered ``upstream''
-   from the displayed ref. Respects `:short` in the same way as
-   `refname` above.  Additionally respects `:track` to show
-   "[ahead N, behind M]" and `:trackshort` to show the terse
-   version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
-   or "=" (in sync). `:track` also prints "[gone]" whenever
-   unknown upstream ref is encountered. Append `:track,nobracket`
-   to show tracking information without brackets (i.e "ahead N,
-   behind M").  Has no effect if the ref does not have tracking
-   information associated with it.
+   from the displayed ref. Respects `:short` and `:strip` in the
+   same way as `refname` above.  Additionally respects `:track`
+   to show "[ahead N, behind M]" and `:trackshort` to show the
+   terse version: ">" (ahead), "<" (behind), "<>" (ahead and
+   behind), or "=" (in sync). `:track` also prints "[gone]"
+   whenever unknown upstream ref is encountered. Append
+   `:track,nobracket` to show tracking information without
+   brackets (i.e "ahead N, behind M").  Has no effect if the ref
+   does not have tracking information associated with it.
 
 push::
-   The name of a local ref which represents the `@{push}` location
-   for the displayed ref. Respects `:short`, `:track`, and
-   `:trackshort` options as `upstream` does. Produces an empty
-   string if no `@{push}` ref is configured.
+   The name of a local ref which represents the `@{push}`
+   location for the displayed ref. Respects `:short`, `:strip`,
+   `:track`, and `:trackshort` options as `upstream`
+   does. Produces an empty string if no `@{push}` ref is
+   configured.
 
 HEAD::
'*' if HEAD matches current ref (the checked out branch), ' '
diff --git a/ref-filter.c b/ref-filter.c
index 933f8ca..3b42aab 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -52,7 +52,8 @@ static struct used_atom {
char color[COLOR_MAXLEN];
struct align align;
struct {
-   enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT } 
option;
+   enum { RR_REF, RR_TRACK, RR_TRACKSHORT } option;
+   struct refname_atom refname;
unsigned int nobracket: 1;
} remote_ref;
struct {
@@ -102,7 +103,9 @@ static void remote_ref_atom_parser(struct used_atom *atom, 
const char *arg)
int i;
 
if (!arg) {
-   atom->u.remote_ref.option = RR_NORMAL;
+   atom->u.remote_ref.option = RR_REF;
+   refname_atom_parser_internal(>u.remote_ref.refname,
+arg, atom->name);
return;
}
 
@@ -112,16 +115,17 @@ static void remote_ref_atom_parser(struct used_atom 
*atom, const char *arg)
for (i = 0; i < params.nr; i++) {
const char *s = params.items[i].string;
 
-   if (!strcmp(s, "short"))
-   atom->u.remote_ref.option = RR_SHORTEN;
-   else if (!strcmp(s, "track"))
+   if (!strcmp(s, "track"))
atom->u.remote_ref.option = RR_TRACK;
else if (!strcmp(s, "trackshort"))
atom->u.remote_ref.option = RR_TRACKSHORT;
else if (!strcmp(s, "nobracket"))
atom->u.remote_ref.nobracket = 1;
-   else
-   die(_("unrecognized format: %%(%s)"), atom->name);
+   else {
+   atom->u.remote_ref.option = RR_REF;
+   
refname_atom_parser_internal(>u.remote_ref.refname,
+arg, atom->name);
+   }
}
 
string_list_clear(, 0);
@@ -1100,8 +1104,8 @@ static void fill_remote_ref_details(struct used_atom 

[PATCH v5b 08/17] ref-filter: add support for %(upstream:track,nobracket)

[Karthik Nayak]

Add support for %(upstream:track,nobracket) which will print the
tracking information without the brackets (i.e. "ahead N, behind M").
This is needed when we port branch.c to use ref-filter's printing APIs.

Add test and documentation for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  8 +++--
 ref-filter.c   | 67 +-
 t/t6300-for-each-ref.sh|  2 ++
 3 files changed, 51 insertions(+), 26 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index f1e46a7..5c85f27 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -118,9 +118,11 @@ upstream::
`refname` above.  Additionally respects `:track` to show
"[ahead N, behind M]" and `:trackshort` to show the terse
version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
-   or "=" (in sync).  Has no effect if the ref does not have
-   tracking information associated with it. `:track` also prints
-   "[gone]" whenever unknown upstream ref is encountered.
+   or "=" (in sync). `:track` also prints "[gone]" whenever
+   unknown upstream ref is encountered. Append `:track,nobracket`
+   to show tracking information without brackets (i.e "ahead N,
+   behind M").  Has no effect if the ref does not have tracking
+   information associated with it.
 
 push::
The name of a local ref which represents the `@{push}` location
diff --git a/ref-filter.c b/ref-filter.c
index 3bb6923..b898bcd 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -46,8 +46,10 @@ static struct used_atom {
union {
char color[COLOR_MAXLEN];
struct align align;
-   enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT }
-   remote_ref;
+   struct {
+   enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT } 
option;
+   unsigned int nobracket: 1;
+   } remote_ref;
struct {
enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, 
C_SUB } option;
unsigned int nlines;
@@ -75,16 +77,33 @@ static void color_atom_parser(struct used_atom *atom, const 
char *color_value)
 
 static void remote_ref_atom_parser(struct used_atom *atom, const char *arg)
 {
-   if (!arg)
-   atom->u.remote_ref = RR_NORMAL;
-   else if (!strcmp(arg, "short"))
-   atom->u.remote_ref = RR_SHORTEN;
-   else if (!strcmp(arg, "track"))
-   atom->u.remote_ref = RR_TRACK;
-   else if (!strcmp(arg, "trackshort"))
-   atom->u.remote_ref = RR_TRACKSHORT;
-   else
-   die(_("unrecognized format: %%(%s)"), atom->name);
+   struct string_list params = STRING_LIST_INIT_DUP;
+   int i;
+
+   if (!arg) {
+   atom->u.remote_ref.option = RR_NORMAL;
+   return;
+   }
+
+   atom->u.remote_ref.nobracket = 0;
+   string_list_split(, arg, ',', -1);
+
+   for (i = 0; i < params.nr; i++) {
+   const char *s = params.items[i].string;
+
+   if (!strcmp(s, "short"))
+   atom->u.remote_ref.option = RR_SHORTEN;
+   else if (!strcmp(s, "track"))
+   atom->u.remote_ref.option = RR_TRACK;
+   else if (!strcmp(s, "trackshort"))
+   atom->u.remote_ref.option = RR_TRACKSHORT;
+   else if (!strcmp(s, "nobracket"))
+   atom->u.remote_ref.nobracket = 1;
+   else
+   die(_("unrecognized format: %%(%s)"), atom->name);
+   }
+
+   string_list_clear(, 0);
 }
 
 static void body_atom_parser(struct used_atom *atom, const char *arg)
@@ -1045,25 +1064,27 @@ static void fill_remote_ref_details(struct used_atom 
*atom, const char *refname,
struct branch *branch, const char **s)
 {
int num_ours, num_theirs;
-   if (atom->u.remote_ref == RR_SHORTEN)
+   if (atom->u.remote_ref.option == RR_SHORTEN)
*s = shorten_unambiguous_ref(refname, warn_ambiguous_refs);
-   else if (atom->u.remote_ref == RR_TRACK) {
+   else if (atom->u.remote_ref.option == RR_TRACK) {
if (stat_tracking_info(branch, _ours,
   _theirs, NULL)) {
-   *s = "[gone]";
-   return;
-   }
-
-   if (!num_ours && !num_theirs)
+   *s = xstrdup("gone");
+   } else if (!num_ours && !num_theirs)
*s = "";
else if (!num_ours)
-   *s = xstrfmt("[behind %d]", num_theirs);
+ 

[PATCH v5b 16/17] branch: use ref-filter printing APIs

[Karthik Nayak]

Port branch.c to use ref-filter APIs for printing. This clears out
most of the code used in branch.c for printing and replaces them with
calls made to the ref-filter library.

Introduce build_format() which gets the format required for printing
of refs. Make amendments to print_ref_list() to reflect these changes.

Change calc_maxwidth() to also account for the length of HEAD ref, by
calling ref-filter:get_head_discription().

Also change the test in t6040 to reflect the changes.

Before this patch, all cross-prefix symrefs weren't shortened. Since
we're using ref-filter APIs, we shorten all symrefs by default. We also
allow the user to change the format if needed with the introduction of
the '--format' option in the next patch.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/branch.c | 234 ++-
 t/t3203-branch-output.sh |   2 +-
 t/t6040-tracking-info.sh |   2 +-
 3 files changed, 70 insertions(+), 168 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index ecd7ffc..07068d2 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -36,12 +36,12 @@ static unsigned char head_sha1[20];
 
 static int branch_use_color = -1;
 static char branch_colors[][COLOR_MAXLEN] = {
-   GIT_COLOR_RESET,
-   GIT_COLOR_NORMAL,   /* PLAIN */
-   GIT_COLOR_RED,  /* REMOTE */
-   GIT_COLOR_NORMAL,   /* LOCAL */
-   GIT_COLOR_GREEN,/* CURRENT */
-   GIT_COLOR_BLUE, /* UPSTREAM */
+   "%(color:reset)",
+   "%(color:reset)",   /* PLAIN */
+   "%(color:red)", /* REMOTE */
+   "%(color:reset)",   /* LOCAL */
+   "%(color:green)",   /* CURRENT */
+   "%(color:blue)",/* UPSTREAM */
 };
 enum color_branch {
BRANCH_COLOR_RESET = 0,
@@ -277,162 +277,6 @@ static int delete_branches(int argc, const char **argv, 
int force, int kinds,
return(ret);
 }
 
-static void fill_tracking_info(struct strbuf *stat, const char *branch_name,
-   int show_upstream_ref)
-{
-   int ours, theirs;
-   char *ref = NULL;
-   struct branch *branch = branch_get(branch_name);
-   const char *upstream;
-   struct strbuf fancy = STRBUF_INIT;
-   int upstream_is_gone = 0;
-   int added_decoration = 1;
-
-   if (stat_tracking_info(branch, , , ) < 0) {
-   if (!upstream)
-   return;
-   upstream_is_gone = 1;
-   }
-
-   if (show_upstream_ref) {
-   ref = shorten_unambiguous_ref(upstream, 0);
-   if (want_color(branch_use_color))
-   strbuf_addf(, "%s%s%s",
-   branch_get_color(BRANCH_COLOR_UPSTREAM),
-   ref, 
branch_get_color(BRANCH_COLOR_RESET));
-   else
-   strbuf_addstr(, ref);
-   }
-
-   if (upstream_is_gone) {
-   if (show_upstream_ref)
-   strbuf_addf(stat, _("[%s: gone]"), fancy.buf);
-   else
-   added_decoration = 0;
-   } else if (!ours && !theirs) {
-   if (show_upstream_ref)
-   strbuf_addf(stat, _("[%s]"), fancy.buf);
-   else
-   added_decoration = 0;
-   } else if (!ours) {
-   if (show_upstream_ref)
-   strbuf_addf(stat, _("[%s: behind %d]"), fancy.buf, 
theirs);
-   else
-   strbuf_addf(stat, _("[behind %d]"), theirs);
-
-   } else if (!theirs) {
-   if (show_upstream_ref)
-   strbuf_addf(stat, _("[%s: ahead %d]"), fancy.buf, ours);
-   else
-   strbuf_addf(stat, _("[ahead %d]"), ours);
-   } else {
-   if (show_upstream_ref)
-   strbuf_addf(stat, _("[%s: ahead %d, behind %d]"),
-   fancy.buf, ours, theirs);
-   else
-   strbuf_addf(stat, _("[ahead %d, behind %d]"),
-   ours, theirs);
-   }
-   strbuf_release();
-   if (added_decoration)
-   strbuf_addch(stat, ' ');
-   free(ref);
-}
-
-static void add_verbose_info(struct strbuf *out, struct ref_array_item *item,
-struct ref_filter *filter, const char *refname)
-{
-   struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;
-   const char *sub = _("  invalid ref ");
-   struct commit *commit = item->commit;
-
-   if (!parse_commit(commit)) {
-   pp_commit_easy(CMIT_FMT_ONELINE, commit, );
-   sub = subject.buf;
-   }
-
-   if (item->kind == FILTER_REFS_BRANCHES)
-   fill_tracking_info(, refname, filter->verbose > 1);
-
-  

[PATCH v5b 10/17] ref-filter: introduce refname_atom_parser_internal()

[Karthik Nayak]

Since there are multiple atoms which print refs ('%(refname)',
'%(symref)', '%(push)', '%upstream'), it makes sense to have a common
ground for parsing them. This would allow us to share implementations of
the atom modifiers between these atoms.

Introduce refname_atom_parser_internal() to act as a common parsing
function for ref printing atoms. This would eventually be used to
introduce refname_atom_parser() and symref_atom_parser() and also be
internally used in remote_ref_atom_parser().

Helped-by: Jeff King 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/ref-filter.c b/ref-filter.c
index 083cc27..778fe02 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -30,6 +30,11 @@ struct if_then_else {
condition_satisfied : 1;
 };
 
+struct refname_atom {
+   enum { R_NORMAL, R_SHORT, R_STRIP } option;
+   unsigned int strip;
+};
+
 /*
  * An atom is a valid field atom listed below, possibly prefixed with
  * a "*" to denote deref_tag().
@@ -62,6 +67,7 @@ static struct used_atom {
enum { O_FULL, O_LENGTH, O_SHORT } option;
unsigned int length;
} objectname;
+   struct refname_atom refname;
} u;
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
@@ -75,6 +81,21 @@ static void color_atom_parser(struct used_atom *atom, const 
char *color_value)
die(_("unrecognized color: %%(color:%s)"), color_value);
 }
 
+static void refname_atom_parser_internal(struct refname_atom *atom,
+const char *arg, const char *name)
+{
+   if (!arg)
+   atom->option = R_NORMAL;
+   else if (!strcmp(arg, "short"))
+   atom->option = R_SHORT;
+   else if (skip_prefix(arg, "strip=", )) {
+   atom->option = R_STRIP;
+   if (strtoul_ui(arg, 10, >strip) || atom->strip <= 0)
+   die(_("positive value expected refname:strip=%s"), arg);
+   }   else
+   die(_("unrecognized %%(%s) argument: %s"), name, arg);
+}
+
 static void remote_ref_atom_parser(struct used_atom *atom, const char *arg)
 {
struct string_list params = STRING_LIST_INIT_DUP;
-- 
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 v5b 17/17] branch: implement '--format' option

[Karthik Nayak]

Implement the '--format' option provided by 'ref-filter'. This lets the
user list branches as per desired format similar to the implementation
in 'git for-each-ref'.

Add tests and documentation for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-branch.txt |  7 ++-
 builtin/branch.c | 14 +-
 t/t3203-branch-output.sh | 14 ++
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 4a7037f..8af132f 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -12,7 +12,7 @@ SYNOPSIS
[--list] [-v [--abbrev= | --no-abbrev]]
[--column[=] | --no-column]
[(--merged | --no-merged | --contains) []] [--sort=]
-   [--points-at ] [...]
+   [--points-at ] [--format=] [...]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f]  
[]
 'git branch' (--set-upstream-to= | -u ) []
 'git branch' --unset-upstream []
@@ -246,6 +246,11 @@ start-point is either a local or remote-tracking branch.
 --points-at ::
Only list branches of the given object.
 
+--format ::
+   A string that interpolates `%(fieldname)` from the object
+   pointed at by a ref being shown.  The format is the same as
+   that of linkgit:git-for-each-ref[1].
+
 Examples
 
 
diff --git a/builtin/branch.c b/builtin/branch.c
index 07068d2..6847ac3 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -28,6 +28,7 @@ static const char * const builtin_branch_usage[] = {
N_("git branch [] [-r] (-d | -D) ..."),
N_("git branch [] (-m | -M) [] "),
N_("git branch [] [-r | -a] [--points-at]"),
+   N_("git branch [] [-r | -a] [--format]"),
NULL
 };
 
@@ -340,14 +341,14 @@ static char *build_format(struct ref_filter *filter, int 
maxwidth, const char *r
return strbuf_detach(, NULL);
 }
 
-static void print_ref_list(struct ref_filter *filter, struct ref_sorting 
*sorting)
+static void print_ref_list(struct ref_filter *filter, struct ref_sorting 
*sorting, const char *format)
 {
int i;
struct ref_array array;
int maxwidth = 0;
const char *remote_prefix = "";
struct strbuf out = STRBUF_INIT;
-   char *format;
+   char *to_free = NULL;
 
/*
 * If we are listing more than just remote branches,
@@ -364,7 +365,8 @@ static void print_ref_list(struct ref_filter *filter, 
struct ref_sorting *sortin
if (filter->verbose)
maxwidth = calc_maxwidth(, strlen(remote_prefix));
 
-   format = build_format(filter, maxwidth, remote_prefix);
+   if (!format)
+   format = to_free = build_format(filter, maxwidth, 
remote_prefix);
verify_ref_format(format);
 
/*
@@ -392,7 +394,7 @@ static void print_ref_list(struct ref_filter *filter, 
struct ref_sorting *sortin
}
 
ref_array_clear();
-   free(format);
+   free(to_free);
 }
 
 static void rename_branch(const char *oldname, const char *newname, int force)
@@ -491,6 +493,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
enum branch_track track;
struct ref_filter filter;
static struct ref_sorting *sorting = NULL, **sorting_tail = 
+   const char *format = NULL;
 
struct option options[] = {
OPT_GROUP(N_("Generic options")),
@@ -531,6 +534,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
OPTION_CALLBACK, 0, "points-at", _at, 
N_("object"),
N_("print only branches of the object"), 0, 
parse_opt_object_name
},
+   OPT_STRING(  0 , "format", , N_("format"), N_("format to 
use for the output")),
OPT_END(),
};
 
@@ -591,7 +595,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)
filter.kind |= FILTER_REFS_DETACHED_HEAD;
filter.name_patterns = argv;
-   print_ref_list(, sorting);
+   print_ref_list(, sorting, format);
print_columns(, colopts, NULL);
string_list_clear(, 0);
return 0;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 980c732..d8edaf2 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -196,4 +196,18 @@ test_expect_success 'local-branch symrefs shortened 
properly' '
test_cmp expect actual
 '
 
+test_expect_success 'git branch --format option' '
+   cat >expect <<-\EOF &&
+   Refname is (HEAD detached from fromtag)
+   Refname is refs/heads/ambiguous
+   Refname is refs/heads/branch-one
+   Refname is refs/heads/branch-two
+   Refname 

[PATCH v5b 15/17] branch, tag: use porcelain output

[Karthik Nayak]

Call ref-filter's setup_ref_filter_porcelain_msg() to enable
translated messages for the %(upstream:tack) atom. Although branch.c
doesn't currently use ref-filter's printing API's, this will ensure
that when it does in the future patches, we do not need to worry about
translation.

Written-by: Matthieu Moy 
Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 builtin/branch.c | 2 ++
 builtin/tag.c| 2 ++
 2 files changed, 4 insertions(+)

diff --git a/builtin/branch.c b/builtin/branch.c
index d467840..ecd7ffc 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -632,6 +632,8 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
OPT_END(),
};
 
+   setup_ref_filter_porcelain_msg();
+
memset(, 0, sizeof(filter));
filter.kind = FILTER_REFS_BRANCHES;
filter.abbrev = -1;
diff --git a/builtin/tag.c b/builtin/tag.c
index 528a1ba..3b51be1 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -379,6 +379,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_END()
};
 
+   setup_ref_filter_porcelain_msg();
+
git_config(git_tag_config, sorting_tail);
 
memset(, 0, sizeof(opt));
-- 
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 v5b 14/17] ref-filter: allow porcelain to translate messages in the output

[Karthik Nayak]

Introduce setup_ref_filter_porcelain_msg() so that the messages used in
the atom %(upstream:track) can be translated if needed. This is needed
as we port branch.c to use ref-filter's printing API's.

Written-by: Matthieu Moy 
Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 28 
 ref-filter.h |  2 ++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 97977a5..74c4869 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -15,6 +15,26 @@
 #include "version.h"
 #include "wt-status.h"
 
+static struct ref_msg {
+   const char *gone;
+   const char *ahead;
+   const char *behind;
+   const char *ahead_behind;
+} msgs = {
+   "gone",
+   "ahead %d",
+   "behind %d",
+   "ahead %d, behind %d"
+};
+
+void setup_ref_filter_porcelain_msg(void)
+{
+   msgs.gone = _("gone");
+   msgs.ahead = _("ahead %d");
+   msgs.behind = _("behind %d");
+   msgs.ahead_behind = _("ahead %d, behind %d");
+}
+
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
 struct align {
@@ -1130,15 +1150,15 @@ static void fill_remote_ref_details(struct used_atom 
*atom, const char *refname,
else if (atom->u.remote_ref.option == RR_TRACK) {
if (stat_tracking_info(branch, _ours,
   _theirs, NULL)) {
-   *s = xstrdup("gone");
+   *s = xstrdup(msgs.gone);
} else if (!num_ours && !num_theirs)
*s = "";
else if (!num_ours)
-   *s = xstrfmt("behind %d", num_theirs);
+   *s = xstrfmt(msgs.behind, num_theirs);
else if (!num_theirs)
-   *s = xstrfmt("ahead %d", num_ours);
+   *s = xstrfmt(msgs.ahead, num_ours);
else
-   *s = xstrfmt("ahead %d, behind %d",
+   *s = xstrfmt(msgs.ahead_behind,
 num_ours, num_theirs);
if (!atom->u.remote_ref.nobracket && *s[0]) {
const char *to_free = *s;
diff --git a/ref-filter.h b/ref-filter.h
index 0014b92..da17145 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -111,5 +111,7 @@ struct ref_sorting *ref_default_sorting(void);
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
unset);
 /*  Get the current HEAD's description */
 char *get_head_description(void);
+/*  Set up translated strings in the output. */
+void setup_ref_filter_porcelain_msg(void);
 
 #endif /*  REF_FILTER_H  */
-- 
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 v5b 01/17] ref-filter: implement %(if), %(then), and %(else) atoms

[Karthik Nayak]

Implement %(if), %(then) and %(else) atoms. Used as
%(if)...%(then)...%(end) or %(if)...%(then)...%(else)...%(end). If the
format string between %(if) and %(then) expands to an empty string, or
to only whitespaces, then the whole %(if)...%(end) expands to the string
following %(then). Otherwise, it expands to the string following
%(else), if any. Nesting of this construct is possible.

This is in preparation for porting over `git branch -l` to use
ref-filter APIs for printing.

Add Documentation and tests regarding the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  45 +++--
 ref-filter.c   | 133 +++--
 t/t6302-for-each-ref-filter.sh |  76 +
 3 files changed, 243 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index c52578b..473b6bf 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -141,10 +141,17 @@ align::
"width=" and/or "position=" prefixes may be omitted, and bare
 and  used instead.  For instance,
`%(align:,)`. If the contents length is more
-   than the width then no alignment is performed. If used with
-   '--quote' everything in between %(align:...) and %(end) is
-   quoted, but if nested then only the topmost level performs
-   quoting.
+   than the width then no alignment is performed.
+
+if::
+   Used as %(if)...%(then)...(%end) or
+   %(if)...%(then)...%(else)...%(end).  If there is an atom with
+   value or string literal after the %(if) then everything after
+   the %(then) is printed, else if the %(else) atom is used, then
+   everything after %(else) is printed. We ignore space when
+   evaluating the string before %(then), this is useful when we
+   use the %(HEAD) atom which prints either "*" or " " and we
+   want to apply the 'if' condition only on the 'HEAD' ref.
 
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
@@ -181,6 +188,20 @@ As a special case for the date-type fields, you may 
specify a format for
 the date by adding `:` followed by date format name (see the
 values the `--date` option to linkgit::git-rev-list[1] takes).
 
+Some atoms like %(align) and %(if) always require a matching %(end).
+We call them "opening atoms" and sometimes denote them as %($open).
+
+When a scripting language specific quoting is in effect (i.e. one of
+`--shell`, `--perl`, `--python`, `--tcl` is used), except for opening
+atoms, replacement from every %(atom) is quoted when and only when it
+appears at the top-level (that is, when it appears outside
+%($open)...%(end)).
+
+When a scripting language specific quoting is in effect, everything
+between a top-level opening atom and its matching %(end) is evaluated
+according to the semantics of the opening atom and its result is
+quoted.
+
 
 EXAMPLES
 
@@ -268,6 +289,22 @@ eval=`git for-each-ref --shell --format="$fmt" \
 eval "$eval"
 
 
+
+An example to show the usage of %(if)...%(then)...%(else)...%(end).
+This prefixes the current branch with a star.
+
+
+git for-each-ref --format="%(if)%(HEAD)%(then)* %(else)  
%(end)%(refname:short)" refs/heads/
+
+
+
+An example to show the usage of %(if)...%(then)...%(end).
+This prints the authorname, if present.
+
+
+git for-each-ref --format="%(refname)%(if)%(authorname)%(then) 
%(color:red)Authored by: %(authorname)%(end)"
+
+
 SEE ALSO
 
 linkgit:git-show-ref[1]
diff --git a/ref-filter.c b/ref-filter.c
index bc551a7..41e73f0 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -21,6 +21,12 @@ struct align {
unsigned int width;
 };
 
+struct if_then_else {
+   unsigned int then_atom_seen : 1,
+   else_atom_seen : 1,
+   condition_satisfied : 1;
+};
+
 /*
  * An atom is a valid field atom listed below, possibly prefixed with
  * a "*" to denote deref_tag().
@@ -203,6 +209,9 @@ static struct {
{ "color", FIELD_STR, color_atom_parser },
{ "align", FIELD_STR, align_atom_parser },
{ "end" },
+   { "if" },
+   { "then" },
+   { "else" },
 };
 
 #define REF_FORMATTING_STATE_INIT  { 0, NULL }
@@ -210,7 +219,7 @@ static struct {
 struct ref_formatting_stack {
struct ref_formatting_stack *prev;
struct strbuf output;
-   void (*at_end)(struct ref_formatting_stack *stack);
+   void (*at_end)(struct ref_formatting_stack **stack);
void *at_end_data;
 };
 
@@ -343,13 +352,14 @@ static void pop_stack_element(struct ref_formatting_stack 
**stack)
*stack = prev;
 }
 
-static void end_align_handler(struct ref_formatting_stack 

[PATCH v5b 03/17] ref-filter: implement %(if:equals=) and %(if:notequals=)

[Karthik Nayak]

Implement %(if:equals=) wherein the if condition is only
satisfied if the value obtained between the %(if:...) and %(then) atom
is the same as the given ''.

Similarly, implement (if:notequals=) wherein the if condition
is only satisfied if the value obtained between the %(if:...) and
%(then) atom is differnt from the given ''.

This is done by introducing 'if_atom_parser()' which parses the given
%(if) atom and then stores the data in used_atom which is later passed
on to the used_atom of the %(then) atom, so that it can do the required
comparisons.

Add tests and Documentation for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  3 +++
 ref-filter.c   | 43 +-
 t/t6302-for-each-ref-filter.sh | 18 
 3 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 473b6bf..3e7db10 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -152,6 +152,9 @@ if::
evaluating the string before %(then), this is useful when we
use the %(HEAD) atom which prints either "*" or " " and we
want to apply the 'if' condition only on the 'HEAD' ref.
+   Append ":equals=" or ":notequals=" to compare
+   the value between the %(if:...) and %(then) atoms with the
+   given string.
 
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
diff --git a/ref-filter.c b/ref-filter.c
index 12e646c..857a8b5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -22,6 +22,8 @@ struct align {
 };
 
 struct if_then_else {
+   const char *if_equals,
+   *not_equals;
unsigned int then_atom_seen : 1,
else_atom_seen : 1,
condition_satisfied : 1;
@@ -49,6 +51,10 @@ static struct used_atom {
enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, 
C_SUB } option;
unsigned int nlines;
} contents;
+   struct {
+   const char *if_equals,
+   *not_equals;
+   } if_then_else;
enum { O_FULL, O_SHORT } objectname;
} u;
 } *used_atom;
@@ -169,6 +175,19 @@ static void align_atom_parser(struct used_atom *atom, 
const char *arg)
string_list_clear(, 0);
 }
 
+static void if_atom_parser(struct used_atom *atom, const char *arg)
+{
+   if (!arg)
+   return;
+   else if (skip_prefix(arg, "equals=", >u.if_then_else.if_equals))
+;
+   else if (skip_prefix(arg, "notequals=", 
>u.if_then_else.not_equals))
+   ;
+   else
+   die(_("unrecognized %%(if) argument: %s"), arg);
+}
+
+
 static struct {
const char *name;
cmp_type cmp_type;
@@ -209,7 +228,7 @@ static struct {
{ "color", FIELD_STR, color_atom_parser },
{ "align", FIELD_STR, align_atom_parser },
{ "end" },
-   { "if" },
+   { "if", FIELD_STR, if_atom_parser },
{ "then" },
{ "else" },
 };
@@ -410,6 +429,9 @@ static void if_atom_handler(struct atom_value *atomv, 
struct ref_formatting_stat
struct ref_formatting_stack *new;
struct if_then_else *if_then_else = xcalloc(sizeof(struct 
if_then_else), 1);
 
+   if_then_else->if_equals = atomv->atom->u.if_then_else.if_equals;
+   if_then_else->not_equals = atomv->atom->u.if_then_else.not_equals;
+
push_stack_element(>stack);
new = state->stack;
new->at_end = if_then_else_handler;
@@ -441,10 +463,17 @@ static void then_atom_handler(struct atom_value *atomv, 
struct ref_formatting_st
die(_("format: %%(then) atom used after %%(else)"));
if_then_else->then_atom_seen = 1;
/*
-* If there exists non-empty string between the 'if' and
-* 'then' atom then the 'if' condition is satisfied.
+* If the 'equals' or 'notequals' attribute is used then
+* perform the required comparison. If not, only non-empty
+* strings satisfy the 'if' condition.
 */
-   if (cur->output.len && !is_empty(cur->output.buf))
+   if (if_then_else->if_equals) {
+   if (!strcmp(if_then_else->if_equals, cur->output.buf))
+   if_then_else->condition_satisfied = 1;
+   } else  if (if_then_else->not_equals) {
+   if (strcmp(if_then_else->not_equals, cur->output.buf))
+   if_then_else->condition_satisfied = 1;
+   } else if (cur->output.len && !is_empty(cur->output.buf))
if_then_else->condition_satisfied = 1;
strbuf_reset(>output);
 }
@@ -1137,7 +1166,11 @@ static void 

[PATCH v5b 07/17] ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams

[Karthik Nayak]

Borrowing from branch.c's implementation print "[gone]" whenever an
unknown upstream ref is encountered instead of just ignoring it.

This makes sure that when branch.c is ported over to using ref-filter
APIs for printing, this feature is not lost.

Make changes to t/t6300-for-each-ref.sh and
Documentation/git-for-each-ref.txt to reflect this change.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Helped-by : Jacob Keller 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt | 3 ++-
 ref-filter.c   | 4 +++-
 t/t6300-for-each-ref.sh| 2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 5370fe5..f1e46a7 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -119,7 +119,8 @@ upstream::
"[ahead N, behind M]" and `:trackshort` to show the terse
version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
or "=" (in sync).  Has no effect if the ref does not have
-   tracking information associated with it.
+   tracking information associated with it. `:track` also prints
+   "[gone]" whenever unknown upstream ref is encountered.
 
 push::
The name of a local ref which represents the `@{push}` location
diff --git a/ref-filter.c b/ref-filter.c
index da2b9ee..3bb6923 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1049,8 +1049,10 @@ static void fill_remote_ref_details(struct used_atom 
*atom, const char *refname,
*s = shorten_unambiguous_ref(refname, warn_ambiguous_refs);
else if (atom->u.remote_ref == RR_TRACK) {
if (stat_tracking_info(branch, _ours,
-  _theirs, NULL))
+  _theirs, NULL)) {
+   *s = "[gone]";
return;
+   }
 
if (!num_ours && !num_theirs)
*s = "";
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 2be0a3f..a92b36f 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -382,7 +382,7 @@ test_expect_success 'Check that :track[short] cannot be 
used with other atoms' '
 
 test_expect_success 'Check that :track[short] works when upstream is invalid' '
cat >expected <<-\EOF &&
-
+   [gone]
 
EOF
test_when_finished "git config branch.master.merge refs/heads/master" &&
-- 
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 v5b 06/17] ref-filter: introduce format_ref_array_item()

[Karthik Nayak]

To allow column display, we will need to first render the output in a
string list to allow print_columns() to compute the proper size of
each column before starting the actual output. Introduce the function
format_ref_array_item() that does the formatting of a ref_array_item
to an strbuf.

show_ref_array_item() is kept as a convenience wrapper around it which
obtains the strbuf and prints it the standard output.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Signed-off-by: Karthik Nayak 
---
 ref-filter.c | 16 
 ref-filter.h |  3 +++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index fcb3353..da2b9ee 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1813,10 +1813,10 @@ static void append_literal(const char *cp, const char 
*ep, struct ref_formatting
}
 }
 
-void show_ref_array_item(struct ref_array_item *info, const char *format, int 
quote_style)
+void format_ref_array_item(struct ref_array_item *info, const char *format,
+  int quote_style, struct strbuf *final_buf)
 {
const char *cp, *sp, *ep;
-   struct strbuf *final_buf;
struct ref_formatting_state state = REF_FORMATTING_STATE_INIT;
 
state.quote_style = quote_style;
@@ -1846,9 +1846,17 @@ void show_ref_array_item(struct ref_array_item *info, 
const char *format, int qu
}
if (state.stack->prev)
die(_("format: %%(end) atom missing"));
-   final_buf = >output;
-   fwrite(final_buf->buf, 1, final_buf->len, stdout);
+   strbuf_addbuf(final_buf, >output);
pop_stack_element();
+}
+
+void show_ref_array_item(struct ref_array_item *info, const char *format, int 
quote_style)
+{
+   struct strbuf final_buf = STRBUF_INIT;
+
+   format_ref_array_item(info, format, quote_style, _buf);
+   fwrite(final_buf.buf, 1, final_buf.len, stdout);
+   strbuf_release(_buf);
putchar('\n');
 }
 
diff --git a/ref-filter.h b/ref-filter.h
index 4aea594..0014b92 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -98,6 +98,9 @@ int parse_ref_filter_atom(const char *atom, const char *ep);
 int verify_ref_format(const char *format);
 /*  Sort the given ref_array as per the ref_sorting provided */
 void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
+/*  Based on the given format and quote_style, fill the strbuf */
+void format_ref_array_item(struct ref_array_item *info, const char *format,
+  int quote_style, struct strbuf *final_buf);
 /*  Print the ref using the given format and quote_style */
 void show_ref_array_item(struct ref_array_item *info, const char *format, int 
quote_style);
 /*  Callback function for parsing the sort option */
-- 
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 v5b 04/17] ref-filter: modify "%(objectname:short)" to take length

[Karthik Nayak]

Add support for %(objectname:short=) which would print the
abbreviated unique objectname of given length. When no length is
specified, the length is 'DEFAULT_ABBREV'. The minimum length is
'MINIMUM_ABBREV'. The length may be exceeded to ensure that the provided
object name is unique.

Add tests and documentation for the same.

Mentored-by: Christian Couder 
Mentored-by: Matthieu Moy 
Helped-by: Jacob Keller 
Signed-off-by: Karthik Nayak 
---
 Documentation/git-for-each-ref.txt |  4 
 ref-filter.c   | 25 +++--
 t/t6300-for-each-ref.sh| 10 ++
 3 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 3e7db10..5370fe5 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -107,6 +107,10 @@ objectsize::
 objectname::
The object name (aka SHA-1).
For a non-ambiguous abbreviation of the object name append `:short`.
+   For an abbreviation of the object name with desired length append
+   `:short=`, where the minimum length is MINIMUM_ABBREV. The
+   length may be exceeded to ensure unique object names.
+
 
 upstream::
The name of a local ref which can be considered ``upstream''
diff --git a/ref-filter.c b/ref-filter.c
index 857a8b5..7d3af1c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -55,7 +55,10 @@ static struct used_atom {
const char *if_equals,
*not_equals;
} if_then_else;
-   enum { O_FULL, O_SHORT } objectname;
+   struct {
+   enum { O_FULL, O_LENGTH, O_SHORT } option;
+   unsigned int length;
+   } objectname;
} u;
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
@@ -118,10 +121,17 @@ static void contents_atom_parser(struct used_atom *atom, 
const char *arg)
 static void objectname_atom_parser(struct used_atom *atom, const char *arg)
 {
if (!arg)
-   atom->u.objectname = O_FULL;
+   atom->u.objectname.option = O_FULL;
else if (!strcmp(arg, "short"))
-   atom->u.objectname = O_SHORT;
-   else
+   atom->u.objectname.option = O_SHORT;
+   else if (skip_prefix(arg, "short=", )) {
+   atom->u.objectname.option = O_LENGTH;
+   if (strtoul_ui(arg, 10, >u.objectname.length) ||
+   atom->u.objectname.length == 0)
+   die(_("positive value expected objectname:short=%s"), 
arg);
+   if (atom->u.objectname.length < MINIMUM_ABBREV)
+   atom->u.objectname.length = MINIMUM_ABBREV;
+   } else
die(_("unrecognized %%(objectname) argument: %s"), arg);
 }
 
@@ -591,12 +601,15 @@ static int grab_objectname(const char *name, const 
unsigned char *sha1,
   struct atom_value *v, struct used_atom *atom)
 {
if (starts_with(name, "objectname")) {
-   if (atom->u.objectname == O_SHORT) {
+   if (atom->u.objectname.option == O_SHORT) {
v->s = xstrdup(find_unique_abbrev(sha1, 
DEFAULT_ABBREV));
return 1;
-   } else if (atom->u.objectname == O_FULL) {
+   } else if (atom->u.objectname.option == O_FULL) {
v->s = xstrdup(sha1_to_hex(sha1));
return 1;
+   } else if (atom->u.objectname.option == O_LENGTH) {
+   v->s = xstrdup(find_unique_abbrev(sha1, 
atom->u.objectname.length));
+   return 1;
} else
die("BUG: unknown %%(objectname) option");
}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 19a2823..2be0a3f 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -60,6 +60,8 @@ test_atom head objecttype commit
 test_atom head objectsize 171
 test_atom head objectname $(git rev-parse refs/heads/master)
 test_atom head objectname:short $(git rev-parse --short refs/heads/master)
+test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
+test_atom head objectname:short=10 $(git rev-parse --short=10 
refs/heads/master)
 test_atom head tree $(git rev-parse refs/heads/master^{tree})
 test_atom head parent ''
 test_atom head numparent 0
@@ -99,6 +101,8 @@ test_atom tag objecttype tag
 test_atom tag objectsize 154
 test_atom tag objectname $(git rev-parse refs/tags/testtag)
 test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)
+test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
+test_atom head objectname:short=10 $(git rev-parse --short=10 
refs/heads/master)
 test_atom tag tree ''
 test_atom tag 

Re: [PATCH 00/83] libify apply and use lib in am

[Johannes Schindelin]

Hi Chris,

On Sun, 24 Apr 2016, Christian Couder wrote:

> On Sun, Apr 24, 2016 at 6:27 PM, Christian Couder
>  wrote:
> > On Sun, Apr 24, 2016 at 5:23 PM, Ramsay Jones
> >  wrote:
> >>
> >>
> >> On 24/04/16 14:33, Christian Couder wrote:
> >>> This is a patch series about libifying `git apply` functionality, and
> >>> using this libified functionality in `git am`, so that no 'git apply'
> >>> process is spawn anymore. This makes `git am` significantly faster, so
> >>> `git rebase`, when it uses the am backend, is also significantly
> >>> faster.
> >>
> >> I just tried to git-am these patches, but patch #78 did not make it
> >> to the list.
> >
> > That's strange because gmail tells me it has been sent, and it made it
> > to chrisc...@tuxfamily.org.
> 
> Instead of waiting for the patch to appear on the list, you might want
> to use branch libify-apply-use-in-am25 in my GitHub repo:
> 
> https://github.com/chriscool/git/commits/libify-apply-use-in-am25

Thanks for this. In particular with longer patch series, I find mail-based
patch submissions *really* cumbersome, not only on the submitter's side
but also on the consumers' side.

I wonder, however, why you use numbers in the branch name to version
things? I thought Git allowed for more advanced versioning than that...

:-)

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 80/83] run-command: make dup_devnull() non static

[Johannes Schindelin]

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.

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 83/83] builtin/am: use apply api in run_apply()

[Johannes Schindelin]

Hi Chris,

On Sun, 24 Apr 2016, Christian Couder wrote:

> [...]
>   /*
>* If we are allowed to fall back on 3-way merge, don't give false
>* errors during the initial attempt.
>*/
> +
>   if (state->threeway && !index_file) {
> - cp.no_stdout = 1;
> - cp.no_stderr = 1;
> + save_stdout_fd = dup(1);
> + dup_devnull(1);
> + save_stderr_fd = dup(2);
> + dup_devnull(2);

I wonder. It should be possible to teach the apply function to be quiet by
default, yes? That would be more elegant than dup()ing back and forth.

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