Re: [PATCH v7 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Jeff King]

On Fri, Feb 05, 2016 at 11:29:06PM +0200, Dan Aloni wrote:

> diff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh
> new file mode 100755
> index ..f2b33881e46b
> --- /dev/null
> +++ b/t/t9904-per-repo-email.sh

Is t9904 the right place for this? Usually t99xx is for very separate
components.

This is sort-of about "commit", which would put it in the t75xx range.
But in some ways, it is even more fundamental than that. We don't seem
to have a lot of tests for ident stuff. The closest is the strict ident
stuff in t0007.

> +reprepare () {
> + git reset --hard initial
> +}

Do we need this reprepare stuff at all now? The tests don't care which
commit we're at when they start.

> +test_expect_success setup '
> + # Initial repo state
> + echo "Initial" >foo &&
> + git add foo &&
> + git commit -m foo &&
> + git tag initial &&

A shorter way of saying this is "test_commit foo".

I almost thought we could get rid of this part entirely; the commit
tests don't care. But we do still need _a_ commit for the clone test,
since we want to make sure a reflog is written. It would be nice to push
it down there, but our test environment doesn't allow creating commits,
because of of useConfigOnly. So it's probably fine to leave it here.

Technically, the final "commit" test does make a commit for us to push,
but we do generally try to avoid unnecessary dependencies between the
individual tests.

So all together, maybe:

diff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh
index f2b3388..5694b84 100755
--- a/t/t9904-per-repo-email.sh
+++ b/t/t9904-per-repo-email.sh
@@ -7,48 +7,31 @@ test_description='per-repo forced setting of email address'
 
 . ./test-lib.sh
 
-reprepare () {
-   git reset --hard initial
-}
-
-test_expect_success setup '
-   # Initial repo state
-   echo "Initial" >foo &&
-   git add foo &&
-   git commit -m foo &&
-   git tag initial &&
-
-   # Setup a likely user.useConfigOnly use case
+test_expect_success 'setup a likely user.useConfigOnly use case' '
+   # we want to make sure a reflog is written, since that needs
+   # a non-strict ident. So be sure we have an actual commit.
+   test_commit foo &&
+
sane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&
sane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL &&
git config user.name "test" &&
-   git config --global user.useConfigOnly true &&
-
-   reprepare
+   git config --global user.useConfigOnly true
 '
 
 test_expect_success 'fails committing if clone email is not set' '
-   test_when_finished reprepare &&
-
test_must_fail git commit --allow-empty -m msg
 '
 
 test_expect_success 'fails committing if clone email is not set, but EMAIL 
set' '
-   test_when_finished reprepare &&
-
test_must_fail env EMAIL=t...@fail.com git commit --allow-empty -m msg
 '
 
 test_expect_success 'succeeds committing if clone email is set' '
-   test_when_finished reprepare &&
-
test_config user.email "t...@ok.com" &&
git commit --allow-empty -m msg
 '
 
 test_expect_success 'succeeds cloning if global email is not set' '
-   test_when_finished reprepare &&
-
git clone . clone
 '
 
--
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-curl: don't fall back to Basic auth if we haven't tried Negotiate

[Junio C Hamano]

"brian m. carlson"  writes:

> On Fri, Feb 05, 2016 at 01:02:58PM -0800, Junio C Hamano wrote:
>> Hmph, so documenting that :@
>> as a supported way might be an ugly-looking solution to the original
>> problem.  A less ugly-looking solution might be a boolean that can
>> be set per URL (we already have urlmatch-config infrastructure to
>> help us do so) to tell us to pass the empty credential to lubCurl,
>> bypassing the step to ask the user for password that we do not use.
>> 
>> The end-result of either of these solution would strictly be better
>> than the patch we discussed in that the end user will not have to
>> interact with the prompt at all, right?
>
> Yes, that's true.  I'll try to come up with a patch this weekend that
> implements that (maybe remote.forceAuth = true or somesuch).

Thanks.

I think the configuration should live inside http.* namespace, as
there are already things like http[.].sslCert and friends.

I do not have a good suggestion on the name of the leaf-level
variable.  ForceAuth sounds as if you are forcing authentication
even when the other side does not require it, though.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v8 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

It used to be that:

   git config --global user.email "(none)"

was a viable way for people to force themselves to set user.email in
each repository.  This was helpful for people with more than one
email address, targeting different email addresses for different
clones, as it barred git from creating a commit unless the user.email
config was set in the per-repo config to the correct email address.

A recent change, 19ce497c (ident: keep a flag for bogus
default_email, 2015-12-10), however, declared that an explicitly
configured user.email is not bogus, no matter what its value is, so
this hack no longer works.

Provide the same functionality by adding a new configuration
variable user.useConfigOnly; when this variable is set, the
user must explicitly set user.email configuration.

Signed-off-by: Junio C Hamano 
Helped-by: Eric Sunshine 
Signed-off-by: Jeff King 
Signed-off-by: Dan Aloni 
---
 Documentation/config.txt  | 10 ++
 ident.c   | 16 
 t/t7517-per-repo-email.sh | 39 +++
 3 files changed, 65 insertions(+)
 create mode 100755 t/t7517-per-repo-email.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 02bcde6bb596..e153c7c1ec35 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2821,6 +2821,16 @@ user.name::
Can be overridden by the 'GIT_AUTHOR_NAME' and 'GIT_COMMITTER_NAME'
environment variables.  See linkgit:git-commit-tree[1].
 
+user.useConfigOnly::
+   Instruct Git to avoid trying to guess defaults for 'user.email'
+   and 'user.name', and instead retrieve the values only from the
+   configuration. For example, if you have multiple email addresses
+   and would like to use a different one for each repository, then
+   with this configuration option set to `true` in the global config
+   along with a name, Git will prompt you to set up an email upon
+   making new commits in a newly cloned repository.
+   Defaults to `false`.
+
 user.signingKey::
If linkgit:git-tag[1] or linkgit:git-commit[1] is not selecting the
key you want it to automatically when creating a signed tag or
diff --git a/ident.c b/ident.c
index f3a431f738cc..6e125821f056 100644
--- a/ident.c
+++ b/ident.c
@@ -13,11 +13,14 @@ static struct strbuf git_default_date = STRBUF_INIT;
 static int default_email_is_bogus;
 static int default_name_is_bogus;
 
+static int ident_use_config_only;
+
 #define IDENT_NAME_GIVEN 01
 #define IDENT_MAIL_GIVEN 02
 #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
 static int committer_ident_explicitly_given;
 static int author_ident_explicitly_given;
+static int ident_config_given;
 
 #ifdef NO_GECOS_IN_PWENT
 #define get_gecos(ignored) "&"
@@ -354,6 +357,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect name (got '%s')", 
name);
}
+   if (strict && ident_use_config_only
+   && !(ident_config_given & IDENT_NAME_GIVEN))
+   die("user.useConfigOnly set but no name given");
}
if (!*name) {
struct passwd *pw;
@@ -373,6 +379,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect email address (got '%s')", 
email);
}
+   if (strict && ident_use_config_only
+   && !(ident_config_given & IDENT_MAIL_GIVEN))
+   die("user.useConfigOnly set but no mail given");
}
 
strbuf_reset();
@@ -446,6 +455,11 @@ int author_ident_sufficiently_given(void)
 
 int git_ident_config(const char *var, const char *value, void *data)
 {
+   if (!strcmp(var, "user.useconfigonly")) {
+   ident_use_config_only = git_config_bool(var, value);
+   return 0;
+   }
+
if (!strcmp(var, "user.name")) {
if (!value)
return config_error_nonbool(var);
@@ -453,6 +467,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_name, value);
committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
author_ident_explicitly_given |= IDENT_NAME_GIVEN;
+   ident_config_given |= IDENT_NAME_GIVEN;
return 0;
}
 
@@ -463,6 +478,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_email, value);
committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
+   ident_config_given |= 

Re: no luck with colors for branch names in gitk yet

[Britton Kerin]

On Fri, Feb 5, 2016 at 12:25 PM, Philip Oakley  wrote:
> From: "Britton Kerin" 
>>
>> Someone suggested using color.branch.upstream, I tried like this and
>> variants
>>
>> [color "branch"]
>>  local = red bold
>>  upstream = red bold
>>
>> Doesn't seem to matter what I put in for upstream, including invalid
>> colors, gitk just ignores it and does the dark green for local
>> branches
>> --
>
> Alternate, try
> https://github.com/oumu/mintty-color-schemes/blob/master/base16-mintty/base16-default.minttyrc
> (or any of the other colour schemes) and copy them into your .minttyrc file
> (works for me on g4w : git version 2.7.0.windows.1 )

I'm on linux so I think mintty is not an option.  Also, I'm a little
surprised in affects the rendering of branch tags in gitk, I would
have thought that would be an X or window system thing.

Britton
--
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 v8 1/2] fmt_ident: refactor strictness checks

[Dan Aloni]

From: Jeff King 

This function has evolved quite a bit over time, and as a
result, the logic for "is this an OK ident" has been
sprinkled throughout. This ends up with a lot of redundant
conditionals, like checking want_name repeatedly. Worse,
we want to know in many cases whether we are using the
"default" ident, and we do so by comparing directly to the
global strbuf, which violates the abstraction of the
ident_default_* functions.

Let's reorganize the function into a hierarchy of
conditionals to handle similar cases together. The only
case that doesn't just work naturally for this is that of an
empty name, where our advice is different based on whether
we came from ident_default_name() or not. We can use a
simple flag to cover this case.

Signed-off-by: Jeff King 
---
 ident.c | 46 --
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/ident.c b/ident.c
index 3da555634290..f3a431f738cc 100644
--- a/ident.c
+++ b/ident.c
@@ -345,32 +345,34 @@ const char *fmt_ident(const char *name, const char *email,
int want_date = !(flag & IDENT_NO_DATE);
int want_name = !(flag & IDENT_NO_NAME);
 
-   if (want_name && !name)
-   name = ident_default_name();
-   if (!email)
-   email = ident_default_email();
-
-   if (want_name && !*name) {
-   struct passwd *pw;
-
-   if (strict) {
-   if (name == git_default_name.buf)
+   if (want_name) {
+   int using_default = 0;
+   if (!name) {
+   name = ident_default_name();
+   using_default = 1;
+   if (strict && default_name_is_bogus) {
fputs(env_hint, stderr);
-   die("empty ident name (for <%s>) not allowed", email);
+   die("unable to auto-detect name (got '%s')", 
name);
+   }
+   }
+   if (!*name) {
+   struct passwd *pw;
+   if (strict) {
+   if (using_default)
+   fputs(env_hint, stderr);
+   die("empty ident name (for <%s>) not allowed", 
email);
+   }
+   pw = xgetpwuid_self(NULL);
+   name = pw->pw_name;
}
-   pw = xgetpwuid_self(NULL);
-   name = pw->pw_name;
-   }
-
-   if (want_name && strict &&
-   name == git_default_name.buf && default_name_is_bogus) {
-   fputs(env_hint, stderr);
-   die("unable to auto-detect name (got '%s')", name);
}
 
-   if (strict && email == git_default_email.buf && default_email_is_bogus) 
{
-   fputs(env_hint, stderr);
-   die("unable to auto-detect email address (got '%s')", email);
+   if (!email) {
+   email = ident_default_email();
+   if (strict && default_email_is_bogus) {
+   fputs(env_hint, stderr);
+   die("unable to auto-detect email address (got '%s')", 
email);
+   }
}
 
strbuf_reset();
-- 
2.5.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 v8] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

Changes between v7 -> v8:

* Proofing fixes suggestions by Eric.
* Test script cleanup by Jeff.
* Renumbered test script.

v7: http://article.gmane.org/gmane.comp.version-control.git/285636
--
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] On the --depth argument when fetching with submodules

[Stefan Beller]

Currently when cloning a project, including submodules, the --depth argument
is passed on recursively, i.e. when cloning with "--depth 2", both the
superproject as well as the submodule will have a depth of 2.  It is not
garantueed that the commits as specified by the superproject are included
in these 2 commits of the submodule.

Illustration:
(superproject with depth 2, so A would have more parents, not shown)

superproject/master: A <- B
/  \
submodule/master:  C <- D <- E <- F <- G

(Current behavior is to fetch G and F)

The submodule is referenced at C and E from the two superproject commits.
So it is a reasonable expectation to have C and E included when fetching
the superproject with depth of 2.

So to fetch the correct submodule commits, we need to
* traverse the superproject and list all submodule commits.
* fetch these submodule commits (C and E) by sha1
* in case of shallow clones, I'd propose to have the submodules be shallowed
  with depth 1, i.e. to fetch C and E and no parents thereof
* we need to think of a way to preserve the commits in the submodule for
  later use (i.e. gc must not delete those commits)

For the later I propose to use a ref in the submodule (refs/meta/superproject ?)
which will be an evil merge of all relevant commits to be preserved for the
superproject. It is an evil merge commit as we do not need to store any worktree
at all, but we only care about the parents relationship preventing the gc to
collect data required from the superproject. The parents should contain all
branches or in case of disjoint shallow histories (like in the example above)
all the relevant commits.

Using these ideas we can go a step further in submodules and not need branches,
but only detached heads, which are directed from the superproject.

As for the implementation of these ideas, whenever you commit in the
superproject
with modification to a submodule, the superproject would need to modify the
refs/meta/superproject submodule branch to make sure the submodule is
safe against
garbage collection.

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

[PATCH v9] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

Changes between v8 -> v9:

* Rebased and tested on v2.7.1.
* Made a small correction suggested by Junio in the documentation for
  the new option: s/upon/before

v8: http://article.gmane.org/gmane.comp.version-control.git/285646
--
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 v9 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

It used to be that:

   git config --global user.email "(none)"

was a viable way for people to force themselves to set user.email in
each repository.  This was helpful for people with more than one
email address, targeting different email addresses for different
clones, as it barred git from creating a commit unless the user.email
config was set in the per-repo config to the correct email address.

A recent change, 19ce497c (ident: keep a flag for bogus
default_email, 2015-12-10), however, declared that an explicitly
configured user.email is not bogus, no matter what its value is, so
this hack no longer works.

Provide the same functionality by adding a new configuration
variable user.useConfigOnly; when this variable is set, the
user must explicitly set user.email configuration.

Signed-off-by: Junio C Hamano 
Helped-by: Eric Sunshine 
Signed-off-by: Jeff King 
Signed-off-by: Dan Aloni 
---
 Documentation/config.txt  | 10 ++
 ident.c   | 16 
 t/t7517-per-repo-email.sh | 39 +++
 3 files changed, 65 insertions(+)
 create mode 100755 t/t7517-per-repo-email.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 02bcde6bb596..cfb7d0e7652d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2821,6 +2821,16 @@ user.name::
Can be overridden by the 'GIT_AUTHOR_NAME' and 'GIT_COMMITTER_NAME'
environment variables.  See linkgit:git-commit-tree[1].
 
+user.useConfigOnly::
+   Instruct Git to avoid trying to guess defaults for 'user.email'
+   and 'user.name', and instead retrieve the values only from the
+   configuration. For example, if you have multiple email addresses
+   and would like to use a different one for each repository, then
+   with this configuration option set to `true` in the global config
+   along with a name, Git will prompt you to set up an email before
+   making new commits in a newly cloned repository.
+   Defaults to `false`.
+
 user.signingKey::
If linkgit:git-tag[1] or linkgit:git-commit[1] is not selecting the
key you want it to automatically when creating a signed tag or
diff --git a/ident.c b/ident.c
index f3a431f738cc..6e125821f056 100644
--- a/ident.c
+++ b/ident.c
@@ -13,11 +13,14 @@ static struct strbuf git_default_date = STRBUF_INIT;
 static int default_email_is_bogus;
 static int default_name_is_bogus;
 
+static int ident_use_config_only;
+
 #define IDENT_NAME_GIVEN 01
 #define IDENT_MAIL_GIVEN 02
 #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
 static int committer_ident_explicitly_given;
 static int author_ident_explicitly_given;
+static int ident_config_given;
 
 #ifdef NO_GECOS_IN_PWENT
 #define get_gecos(ignored) "&"
@@ -354,6 +357,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect name (got '%s')", 
name);
}
+   if (strict && ident_use_config_only
+   && !(ident_config_given & IDENT_NAME_GIVEN))
+   die("user.useConfigOnly set but no name given");
}
if (!*name) {
struct passwd *pw;
@@ -373,6 +379,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect email address (got '%s')", 
email);
}
+   if (strict && ident_use_config_only
+   && !(ident_config_given & IDENT_MAIL_GIVEN))
+   die("user.useConfigOnly set but no mail given");
}
 
strbuf_reset();
@@ -446,6 +455,11 @@ int author_ident_sufficiently_given(void)
 
 int git_ident_config(const char *var, const char *value, void *data)
 {
+   if (!strcmp(var, "user.useconfigonly")) {
+   ident_use_config_only = git_config_bool(var, value);
+   return 0;
+   }
+
if (!strcmp(var, "user.name")) {
if (!value)
return config_error_nonbool(var);
@@ -453,6 +467,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_name, value);
committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
author_ident_explicitly_given |= IDENT_NAME_GIVEN;
+   ident_config_given |= IDENT_NAME_GIVEN;
return 0;
}
 
@@ -463,6 +478,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_email, value);
committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
+   ident_config_given |= 

[PATCH v9 1/2] fmt_ident: refactor strictness checks

[Dan Aloni]

From: Jeff King 

This function has evolved quite a bit over time, and as a
result, the logic for "is this an OK ident" has been
sprinkled throughout. This ends up with a lot of redundant
conditionals, like checking want_name repeatedly. Worse,
we want to know in many cases whether we are using the
"default" ident, and we do so by comparing directly to the
global strbuf, which violates the abstraction of the
ident_default_* functions.

Let's reorganize the function into a hierarchy of
conditionals to handle similar cases together. The only
case that doesn't just work naturally for this is that of an
empty name, where our advice is different based on whether
we came from ident_default_name() or not. We can use a
simple flag to cover this case.

Signed-off-by: Jeff King 
---
 ident.c | 46 --
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/ident.c b/ident.c
index 3da555634290..f3a431f738cc 100644
--- a/ident.c
+++ b/ident.c
@@ -345,32 +345,34 @@ const char *fmt_ident(const char *name, const char *email,
int want_date = !(flag & IDENT_NO_DATE);
int want_name = !(flag & IDENT_NO_NAME);
 
-   if (want_name && !name)
-   name = ident_default_name();
-   if (!email)
-   email = ident_default_email();
-
-   if (want_name && !*name) {
-   struct passwd *pw;
-
-   if (strict) {
-   if (name == git_default_name.buf)
+   if (want_name) {
+   int using_default = 0;
+   if (!name) {
+   name = ident_default_name();
+   using_default = 1;
+   if (strict && default_name_is_bogus) {
fputs(env_hint, stderr);
-   die("empty ident name (for <%s>) not allowed", email);
+   die("unable to auto-detect name (got '%s')", 
name);
+   }
+   }
+   if (!*name) {
+   struct passwd *pw;
+   if (strict) {
+   if (using_default)
+   fputs(env_hint, stderr);
+   die("empty ident name (for <%s>) not allowed", 
email);
+   }
+   pw = xgetpwuid_self(NULL);
+   name = pw->pw_name;
}
-   pw = xgetpwuid_self(NULL);
-   name = pw->pw_name;
-   }
-
-   if (want_name && strict &&
-   name == git_default_name.buf && default_name_is_bogus) {
-   fputs(env_hint, stderr);
-   die("unable to auto-detect name (got '%s')", name);
}
 
-   if (strict && email == git_default_email.buf && default_email_is_bogus) 
{
-   fputs(env_hint, stderr);
-   die("unable to auto-detect email address (got '%s')", email);
+   if (!email) {
+   email = ident_default_email();
+   if (strict && default_email_is_bogus) {
+   fputs(env_hint, stderr);
+   die("unable to auto-detect email address (got '%s')", 
email);
+   }
}
 
strbuf_reset();
-- 
2.5.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

make notes show up in gitk graph view?

[Britton Kerin]

I'd like to be able to mark dysfunctional stuff that ended up in the
repo but we don't want to delete with DONT_USE or something, and have
it show up like tags do, but not have to be unique.

If git notes don't work for this purpose maybe something else does?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 7/7] convert.c: simplify text_stat

[tboegi]

From: Torsten Bögershausen 

Simplify the statistics:
lonecr counts the CR which is not followed by a LF,
lonelf counts the LF which is not preceded by a CR,
crlf counts CRLF combinations.
This simplifies the evaluation of the statistics.

Signed-off-by: Torsten Bögershausen 
---
 convert.c | 47 ++-
 1 file changed, 22 insertions(+), 25 deletions(-)

diff --git a/convert.c b/convert.c
index e4e2877..18af685 100644
--- a/convert.c
+++ b/convert.c
@@ -31,7 +31,7 @@ enum crlf_action {
 
 struct text_stat {
/* NUL, CR, LF and CRLF counts */
-   unsigned nul, cr, lf, crlf;
+   unsigned nul, lonecr, lonelf, crlf;
 
/* These are just approximations! */
unsigned printable, nonprintable;
@@ -46,13 +46,15 @@ static void gather_stats(const char *buf, unsigned long 
size, struct text_stat *
for (i = 0; i < size; i++) {
unsigned char c = buf[i];
if (c == '\r') {
-   stats->cr++;
-   if (i+1 < size && buf[i+1] == '\n')
+   if (i+1 < size && buf[i+1] == '\n') {
stats->crlf++;
+   i++;
+   } else
+   stats->lonecr++;
continue;
}
if (c == '\n') {
-   stats->lf++;
+   stats->lonelf++;
continue;
}
if (c == 127)
@@ -86,7 +88,7 @@ static void gather_stats(const char *buf, unsigned long size, 
struct text_stat *
  */
 static int convert_is_binary(unsigned long size, const struct text_stat *stats)
 {
-   if (stats->cr != stats->crlf)
+   if (stats->lonecr)
return 1;
if (stats->nul)
return 1;
@@ -98,19 +100,18 @@ static int convert_is_binary(unsigned long size, const 
struct text_stat *stats)
 static unsigned int gather_convert_stats(const char *data, unsigned long size)
 {
struct text_stat stats;
+   int ret = 0;
if (!data || !size)
return 0;
gather_stats(data, size, );
if (convert_is_binary(size, ))
-   return CONVERT_STAT_BITS_BIN;
-   else if (stats.crlf && stats.crlf == stats.lf)
-   return CONVERT_STAT_BITS_TXT_CRLF;
-   else if (stats.crlf && stats.lf)
-   return CONVERT_STAT_BITS_TXT_CRLF | CONVERT_STAT_BITS_TXT_LF;
-   else if (stats.lf)
-   return CONVERT_STAT_BITS_TXT_LF;
-   else
-   return 0;
+   ret |= CONVERT_STAT_BITS_BIN;
+   if (stats.crlf)
+   ret |= CONVERT_STAT_BITS_TXT_CRLF;
+   if (stats.lonelf)
+   ret |=  CONVERT_STAT_BITS_TXT_LF;
+
+   return ret;
 }
 
 static const char *gather_convert_stats_ascii(const char *data, unsigned long 
size)
@@ -207,7 +208,7 @@ static void check_safe_crlf(const char *path, enum 
crlf_action crlf_action,
 * CRLFs would be added by checkout:
 * check if we have "naked" LFs
 */
-   if (stats->lf != stats->crlf) {
+   if (stats->lonelf) {
if (checksafe == SAFE_CRLF_WARN)
warning("LF will be replaced by CRLF in 
%s.\nThe file will have its original line endings in your working directory.", 
path);
else /* i.e. SAFE_CRLF_FAIL */
@@ -266,8 +267,8 @@ static int crlf_to_git(const char *path, const char *src, 
size_t len,
 
check_safe_crlf(path, crlf_action, , checksafe);
 
-   /* Optimization: No CR? Nothing to convert, regardless. */
-   if (!stats.cr)
+   /* Optimization: No CRLF? Nothing to convert, regardless. */
+   if (!stats.crlf)
return 0;
 
/*
@@ -314,19 +315,15 @@ static int crlf_to_worktree(const char *path, const char 
*src, size_t len,
 
gather_stats(src, len, );
 
-   /* No LF? Nothing to convert, regardless. */
-   if (!stats.lf)
-   return 0;
-
-   /* Was it already in CRLF format? */
-   if (stats.lf == stats.crlf)
+   /* No "naked" LF? Nothing to convert, regardless. */
+   if (!stats.lonelf)
return 0;
 
if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || 
crlf_action == CRLF_AUTO_CRLF) {
if (crlf_action == CRLF_AUTO_INPUT || crlf_action == 
CRLF_AUTO_CRLF) {
/* If we have any CR or CRLF line endings, we do not 
touch it */
/* This is the new safer autocrlf-handling */
-   if (stats.cr > 0 || stats.crlf > 0)
+   if (stats.lonecr || stats.crlf )
return 0;
}
 
@@ -338,7 +335,7 @@ static int crlf_to_worktree(const char *path, const char 
*src, size_t len,
if 

[PATCH v3 1/7] t0027: Add tests for get_stream_filter()

[tboegi]

From: Torsten Bögershausen 

When a filter is configured, a different code-path is used in convert.c
and entry.c via get_stream_filter(), but there are no test cases yet.

Add tests for the filter API by configuring the ident filter.
The result of the SHA1 conversion is not checked, this is already
done in other TC.

Add a parameter to checkout_files() in t0027.
While changing the signature, add another parameter for the eol= attribute.
This is currently unused, tests for e.g.
"* text=auto eol=lf" will be added in a separate commit.

Signed-off-by: Torsten Bögershausen 
---
Changes since v2:
1/7 t0027 uses ident instead of i (the empty "" is still there)
better ident of case..esac (thanks for the patience)
(And again the question, if there is an official init.el, please)
4/7 don't change the logic which stream filter to use
7/7 had been broken (wrong bufferlen) , should be OK now


t/t0027-auto-crlf.sh | 281 ++-
 1 file changed, 146 insertions(+), 135 deletions(-)

diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 504e5a0..fc4c628 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -21,38 +21,32 @@ compare_ws_file () {
pfx=$1
exp=$2.expect
act=$pfx.actual.$3
-   tr '\015\000' QN <"$2" >"$exp" &&
-   tr '\015\000' QN <"$3" >"$act" &&
-   test_cmp $exp $act &&
-   rm $exp $act
+   tr '\015\000abcdef0123456789' QN0 <"$2" >"$exp" &&
+   tr '\015\000abcdef0123456789' QN0 <"$3" >"$act" &&
+   test_cmp "$exp" "$act" &&
+   rm "$exp" "$act"
 }
 
 create_gitattributes () {
-   attr=$1
-   case "$attr" in
-   auto)
-   echo "*.txt text=auto" >.gitattributes
-   ;;
-   text)
-   echo "*.txt text" >.gitattributes
-   ;;
-   -text)
-   echo "*.txt -text" >.gitattributes
-   ;;
-   crlf)
-   echo "*.txt eol=crlf" >.gitattributes
-   ;;
-   lf)
-   echo "*.txt eol=lf" >.gitattributes
-   ;;
-   "")
-   echo >.gitattributes
-   ;;
-   *)
-   echo >&2 invalid attribute: $attr
-   exit 1
-   ;;
-   esac
+   {
+   while test "$#" != 0
+   do
+   case "$1" in
+   auto)echo '*.txt text=auto' ;;
+   ident) echo '*.txt ident' ;;
+   text)echo '*.txt text' ;;
+   -text) echo '*.txt -text' ;;
+   crlf)  echo '*.txt eol=crlf' ;;
+   lf)echo '*.txt eol=lf' ;;
+   "") ;;
+   *)
+   echo >&2 invalid attribute: "$1"
+   exit 1
+   ;;
+   esac &&
+   shift
+   done
+   } >.gitattributes
 }
 
 create_NNO_files () {
@@ -208,28 +202,30 @@ check_in_repo_NNO () {
 }
 
 checkout_files () {
-   eol=$1
-   crlf=$2
-   attr=$3
-   lfname=$4
-   crlfname=$5
-   lfmixcrlf=$6
-   lfmixcr=$7
-   crlfnul=$8
-   create_gitattributes $attr &&
+   attr=$1 ; shift
+   ident=$1; shift
+   aeol=$1 ; shift
+   crlf=$1 ; shift
+   ceol=$1 ; shift
+   lfname=$1 ; shift
+   crlfname=$1 ; shift
+   lfmixcrlf=$1 ; shift
+   lfmixcr=$1 ; shift
+   crlfnul=$1 ; shift
+   create_gitattributes "$attr" "$ident" &&
git config core.autocrlf $crlf &&
-   pfx=eol_${eol}_crlf_${crlf}_attr_${attr}_ &&
+   pfx=eol_${ceol}_crlf_${crlf}_attr_${attr}_ &&
for f in LF CRLF LF_mix_CR CRLF_mix_LF LF_nul
do
rm crlf_false_attr__$f.txt &&
-   if test -z "$eol"; then
+   if test -z "$ceol"; then
git checkout crlf_false_attr__$f.txt
else
-   git -c core.eol=$eol checkout crlf_false_attr__$f.txt
+   git -c core.eol=$ceol checkout crlf_false_attr__$f.txt
fi
done
 
-   test_expect_success "ls-files --eol $lfname ${pfx}LF.txt" '
+   test_expect_success "ls-files --eol attr=$attr $ident $aeol 
core.autocrlf=$crlf core.eol=$ceol" '
test_when_finished "rm expect actual" &&
sort <<-EOF >expect &&
i/crlf w/$(stats_ascii $crlfname) crlf_false_attr__CRLF.txt
@@ -244,19 +240,19 @@ checkout_files () {
sort >actual &&
test_cmp expect actual
'
-   test_expect_success "checkout core.eol=$eol core.autocrlf=$crlf 
gitattributes=$attr file=LF" "
+   test_expect_success "checkout $ident $attr $aeol core.autocrlf=$crlf 

[PATCH v3 6/7] convert.c: refactor crlf_action

[tboegi]

From: Torsten Bögershausen 

Refactor the determination and usage of crlf_action.
Today, when no attributes are set on a file,
crlf_action is set to CRLF_GUESS, and later, CRLF_GUESS is used as an
indication that core.autocrlf is not false and that some automatic eol
conversion is needed.
Make more clear, what is what, by defining:

- CRLF_UNDEFINED : No attributes set. Temparally used, until core.autocrlf
   and core.eol is evaluated and one of CRLF_BINARY,
   CRLF_AUTO_INPUT or CRLF_AUTO_CRLF is selected
- CRLF_BINARY: No processing of line endings.
- CRLF_TEXT  : attribute "text" is set, line endings are processed.
- CRLF_TEXT_INPUT: attribute "input" or "eol=lf" is set. This implies text.
- CRLF_TEXT_CRLF : attribute "eol=crlf" is set. This implies text.
- CRLF_AUTO  : attribute "auto" is set.
- CRLF_AUTO_INPUT: No attributes, core.autocrlf=input
- CRLF_AUTO_CRLF : No attributes, core.autocrlf=true

Signed-off-by: Torsten Bögershausen 
---
 convert.c | 75 ++-
 1 file changed, 45 insertions(+), 30 deletions(-)

diff --git a/convert.c b/convert.c
index e9c9448..e4e2877 100644
--- a/convert.c
+++ b/convert.c
@@ -19,12 +19,14 @@
 #define CONVERT_STAT_BITS_BIN   0x4
 
 enum crlf_action {
-   CRLF_GUESS = -1,
-   CRLF_BINARY = 0,
+   CRLF_UNDEFINED,
+   CRLF_BINARY,
CRLF_TEXT,
-   CRLF_INPUT,
-   CRLF_CRLF,
-   CRLF_AUTO
+   CRLF_TEXT_INPUT,
+   CRLF_TEXT_CRLF,
+   CRLF_AUTO,
+   CRLF_AUTO_INPUT,
+   CRLF_AUTO_CRLF
 };
 
 struct text_stat {
@@ -167,18 +169,19 @@ static enum eol output_eol(enum crlf_action crlf_action)
switch (crlf_action) {
case CRLF_BINARY:
return EOL_UNSET;
-   case CRLF_CRLF:
+   case CRLF_TEXT_CRLF:
return EOL_CRLF;
-   case CRLF_INPUT:
+   case CRLF_TEXT_INPUT:
return EOL_LF;
-   case CRLF_GUESS:
-   if (!auto_crlf)
-   return EOL_UNSET;
-   /* fall through */
+   case CRLF_UNDEFINED:
+   case CRLF_AUTO_CRLF:
+   case CRLF_AUTO_INPUT:
case CRLF_TEXT:
case CRLF_AUTO:
+   /* fall through */
return text_eol_is_crlf() ? EOL_CRLF : EOL_LF;
}
+   warning("Illegal crlf_action %d\n", (int)crlf_action);
return core_eol;
 }
 
@@ -247,11 +250,11 @@ static int crlf_to_git(const char *path, const char *src, 
size_t len,
 
gather_stats(src, len, );
 
-   if (crlf_action == CRLF_AUTO || crlf_action == CRLF_GUESS) {
+   if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || 
crlf_action == CRLF_AUTO_CRLF) {
if (convert_is_binary(len, ))
return 0;
 
-   if (crlf_action == CRLF_GUESS) {
+   if (crlf_action == CRLF_AUTO_INPUT || crlf_action == 
CRLF_AUTO_CRLF) {
/*
 * If the file in the index has any CR in it, do not 
convert.
 * This is the new safer autocrlf handling.
@@ -278,7 +281,7 @@ static int crlf_to_git(const char *path, const char *src, 
size_t len,
if (strbuf_avail(buf) + buf->len < len)
strbuf_grow(buf, len - buf->len);
dst = buf->buf;
-   if (crlf_action == CRLF_AUTO || crlf_action == CRLF_GUESS) {
+   if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || 
crlf_action == CRLF_AUTO_CRLF) {
/*
 * If we guessed, we already know we rejected a file with
 * lone CR, and we can strip a CR without looking at what
@@ -319,8 +322,8 @@ static int crlf_to_worktree(const char *path, const char 
*src, size_t len,
if (stats.lf == stats.crlf)
return 0;
 
-   if (crlf_action == CRLF_AUTO || crlf_action == CRLF_GUESS) {
-   if (crlf_action == CRLF_GUESS) {
+   if (crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_INPUT || 
crlf_action == CRLF_AUTO_CRLF) {
+   if (crlf_action == CRLF_AUTO_INPUT || crlf_action == 
CRLF_AUTO_CRLF) {
/* If we have any CR or CRLF line endings, we do not 
touch it */
/* This is the new safer autocrlf-handling */
if (stats.cr > 0 || stats.crlf > 0)
@@ -708,16 +711,16 @@ static enum crlf_action git_path_check_crlf(struct 
git_attr_check *check)
const char *value = check->value;
 
if (ATTR_TRUE(value))
-   return CRLF_TEXT;
+   return text_eol_is_crlf() ? CRLF_TEXT_CRLF : CRLF_TEXT_INPUT;
else if (ATTR_FALSE(value))
return CRLF_BINARY;
else if (ATTR_UNSET(value))
;
else if (!strcmp(value, "input"))
-   return CRLF_INPUT;
+   return CRLF_TEXT_INPUT;
else if (!strcmp(value, "auto"))
  

[PATCH v3 5/7] convert: auto_crlf=false and no attributes set: same as binary

[tboegi]

From: Torsten Bögershausen 

When core.autocrlf is set to false, and no attributes are set, the file
is treated as binary.
Simplify the logic and remove duplicated code when dealing with
(crlf_action == CRLF_GUESS && auto_crlf == AUTO_CRLF_FALSE) by setting
crlf_action=CRLF_BINARY already in convert_attrs().

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

diff --git a/convert.c b/convert.c
index 557e574..e9c9448 100644
--- a/convert.c
+++ b/convert.c
@@ -235,7 +235,6 @@ static int crlf_to_git(const char *path, const char *src, 
size_t len,
char *dst;
 
if (crlf_action == CRLF_BINARY ||
-   (crlf_action == CRLF_GUESS && auto_crlf == AUTO_CRLF_FALSE) ||
(src && !len))
return 0;
 
@@ -798,6 +797,8 @@ static void convert_attrs(struct conv_attrs *ca, const char 
*path)
ca->crlf_action = CRLF_GUESS;
ca->ident = 0;
}
+   if (ca->crlf_action == CRLF_GUESS && auto_crlf == AUTO_CRLF_FALSE)
+   ca->crlf_action = CRLF_BINARY;
 }
 
 int would_convert_to_git_filter_fd(const char *path)
@@ -1382,8 +1383,7 @@ struct stream_filter *get_stream_filter(const char *path, 
const unsigned char *s
 
crlf_action = ca.crlf_action;
 
-   if ((crlf_action == CRLF_BINARY) || (crlf_action == CRLF_INPUT) ||
-   (crlf_action == CRLF_GUESS && auto_crlf == AUTO_CRLF_FALSE))
+   if ((crlf_action == CRLF_BINARY) || (crlf_action == CRLF_INPUT))
filter = cascade_filter(filter, _filter_singleton);
 
else if (output_eol(crlf_action) == EOL_CRLF &&
-- 
2.7.0.303.g2c4f448.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

[PATCH v3 4/7] convert.c: use text_eol_is_crlf()

[tboegi]

From: Torsten Bögershausen 

Add a helper function to find out, which line endings
text files should get at checkout, depending on
core.autocrlf and core.eol

Signed-off-by: Torsten Bögershausen 
---
 convert.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/convert.c b/convert.c
index d0c8c66..557e574 100644
--- a/convert.c
+++ b/convert.c
@@ -149,6 +149,19 @@ const char *get_wt_convert_stats_ascii(const char *path)
return ret;
 }
 
+static int text_eol_is_crlf(void)
+{
+   if (auto_crlf == AUTO_CRLF_TRUE)
+   return 1;
+   else if (auto_crlf == AUTO_CRLF_INPUT)
+   return 0;
+   if (core_eol == EOL_CRLF)
+   return 1;
+   if (core_eol == EOL_UNSET && EOL_NATIVE == EOL_CRLF)
+   return 1;
+   return 0;
+}
+
 static enum eol output_eol(enum crlf_action crlf_action)
 {
switch (crlf_action) {
@@ -164,12 +177,7 @@ static enum eol output_eol(enum crlf_action crlf_action)
/* fall through */
case CRLF_TEXT:
case CRLF_AUTO:
-   if (auto_crlf == AUTO_CRLF_TRUE)
-   return EOL_CRLF;
-   else if (auto_crlf == AUTO_CRLF_INPUT)
-   return EOL_LF;
-   else if (core_eol == EOL_UNSET)
-   return EOL_NATIVE;
+   return text_eol_is_crlf() ? EOL_CRLF : EOL_LF;
}
return core_eol;
 }
-- 
2.7.0.303.g2c4f448.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

[PATCH v3 2/7] convert.c: remove unused parameter 'path'

[tboegi]

From: Torsten Bögershausen 

Some functions get a parameter path, but don't use it.
Remove the unused parameter.

Signed-off-by: Torsten Bögershausen 
---
 convert.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/convert.c b/convert.c
index 4bb4ec1..a24c2a2 100644
--- a/convert.c
+++ b/convert.c
@@ -696,7 +696,7 @@ static int ident_to_worktree(const char *path, const char 
*src, size_t len,
return 1;
 }
 
-static enum crlf_action git_path_check_crlf(const char *path, struct 
git_attr_check *check)
+static enum crlf_action git_path_check_crlf(struct git_attr_check *check)
 {
const char *value = check->value;
 
@@ -713,7 +713,7 @@ static enum crlf_action git_path_check_crlf(const char 
*path, struct git_attr_ch
return CRLF_GUESS;
 }
 
-static enum eol git_path_check_eol(const char *path, struct git_attr_check 
*check)
+static enum eol git_path_check_eol(struct git_attr_check *check)
 {
const char *value = check->value;
 
@@ -726,8 +726,7 @@ static enum eol git_path_check_eol(const char *path, struct 
git_attr_check *chec
return EOL_UNSET;
 }
 
-static struct convert_driver *git_path_check_convert(const char *path,
-struct git_attr_check *check)
+static struct convert_driver *git_path_check_convert(struct git_attr_check 
*check)
 {
const char *value = check->value;
struct convert_driver *drv;
@@ -740,7 +739,7 @@ static struct convert_driver *git_path_check_convert(const 
char *path,
return NULL;
 }
 
-static int git_path_check_ident(const char *path, struct git_attr_check *check)
+static int git_path_check_ident(struct git_attr_check *check)
 {
const char *value = check->value;
 
@@ -783,12 +782,12 @@ static void convert_attrs(struct conv_attrs *ca, const 
char *path)
}
 
if (!git_check_attr(path, NUM_CONV_ATTRS, ccheck)) {
-   ca->crlf_action = git_path_check_crlf(path, ccheck + 4);
+   ca->crlf_action = git_path_check_crlf( ccheck + 4);
if (ca->crlf_action == CRLF_GUESS)
-   ca->crlf_action = git_path_check_crlf(path, ccheck + 0);
-   ca->ident = git_path_check_ident(path, ccheck + 1);
-   ca->drv = git_path_check_convert(path, ccheck + 2);
-   ca->eol_attr = git_path_check_eol(path, ccheck + 3);
+   ca->crlf_action = git_path_check_crlf(ccheck + 0);
+   ca->ident = git_path_check_ident(ccheck + 1);
+   ca->drv = git_path_check_convert(ccheck + 2);
+   ca->eol_attr = git_path_check_eol(ccheck + 3);
} else {
ca->drv = NULL;
ca->crlf_action = CRLF_GUESS;
-- 
2.7.0.303.g2c4f448.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

[PATCH v3 3/7] convert.c: Remove input_crlf_action()

[tboegi]

From: Torsten Bögershausen 

Integrate the code of input_crlf_action() into convert_attrs(),
so that ca.crlf_action is always valid after calling convert_attrs().
Keep a copy of crlf_action in attr_action, this is needed for
get_convert_attr_ascii().

Remove eol_attr from struct conv_attrs, as it is now used temporally.

Signed-off-by: Torsten Bögershausen 
---
 convert.c | 37 ++---
 1 file changed, 14 insertions(+), 23 deletions(-)

diff --git a/convert.c b/convert.c
index a24c2a2..d0c8c66 100644
--- a/convert.c
+++ b/convert.c
@@ -746,21 +746,10 @@ static int git_path_check_ident(struct git_attr_check 
*check)
return !!ATTR_TRUE(value);
 }
 
-static enum crlf_action input_crlf_action(enum crlf_action text_attr, enum eol 
eol_attr)
-{
-   if (text_attr == CRLF_BINARY)
-   return CRLF_BINARY;
-   if (eol_attr == EOL_LF)
-   return CRLF_INPUT;
-   if (eol_attr == EOL_CRLF)
-   return CRLF_CRLF;
-   return text_attr;
-}
-
 struct conv_attrs {
struct convert_driver *drv;
-   enum crlf_action crlf_action;
-   enum eol eol_attr;
+   enum crlf_action attr_action; /* What attr says */
+   enum crlf_action crlf_action; /* When no attr is set, use core.autocrlf 
*/
int ident;
 };
 
@@ -782,16 +771,23 @@ static void convert_attrs(struct conv_attrs *ca, const 
char *path)
}
 
if (!git_check_attr(path, NUM_CONV_ATTRS, ccheck)) {
-   ca->crlf_action = git_path_check_crlf( ccheck + 4);
+   enum eol eol_attr;
+   ca->crlf_action = git_path_check_crlf(ccheck + 4);
if (ca->crlf_action == CRLF_GUESS)
ca->crlf_action = git_path_check_crlf(ccheck + 0);
+   ca->attr_action = ca->crlf_action;
ca->ident = git_path_check_ident(ccheck + 1);
ca->drv = git_path_check_convert(ccheck + 2);
-   ca->eol_attr = git_path_check_eol(ccheck + 3);
+   if (ca->crlf_action == CRLF_BINARY)
+   return;
+   eol_attr = git_path_check_eol(ccheck + 3);
+   if (eol_attr == EOL_LF)
+   ca->crlf_action = CRLF_INPUT;
+   else if (eol_attr == EOL_CRLF)
+   ca->crlf_action = CRLF_CRLF;
} else {
ca->drv = NULL;
ca->crlf_action = CRLF_GUESS;
-   ca->eol_attr = EOL_UNSET;
ca->ident = 0;
}
 }
@@ -818,11 +814,9 @@ int would_convert_to_git_filter_fd(const char *path)
 const char *get_convert_attr_ascii(const char *path)
 {
struct conv_attrs ca;
-   enum crlf_action crlf_action;
 
convert_attrs(, path);
-   crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr);
-   switch (crlf_action) {
+   switch (ca.attr_action) {
case CRLF_GUESS:
return "";
case CRLF_BINARY:
@@ -861,7 +855,6 @@ int convert_to_git(const char *path, const char *src, 
size_t len,
src = dst->buf;
len = dst->len;
}
-   ca.crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr);
ret |= crlf_to_git(path, src, len, dst, ca.crlf_action, checksafe);
if (ret && dst) {
src = dst->buf;
@@ -882,7 +875,6 @@ void convert_to_git_filter_fd(const char *path, int fd, 
struct strbuf *dst,
if (!apply_filter(path, NULL, 0, fd, dst, ca.drv->clean))
die("%s: clean filter '%s' failed", path, ca.drv->name);
 
-   ca.crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr);
crlf_to_git(path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);
ident_to_git(path, dst->buf, dst->len, dst, ca.ident);
 }
@@ -912,7 +904,6 @@ static int convert_to_working_tree_internal(const char 
*path, const char *src,
 * is a smudge filter.  The filter might expect CRLFs.
 */
if (filter || !normalizing) {
-   ca.crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr);
ret |= crlf_to_worktree(path, src, len, dst, ca.crlf_action);
if (ret) {
src = dst->buf;
@@ -1381,7 +1372,7 @@ struct stream_filter *get_stream_filter(const char *path, 
const unsigned char *s
if (ca.ident)
filter = ident_filter(sha1);
 
-   crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr);
+   crlf_action = ca.crlf_action;
 
if ((crlf_action == CRLF_BINARY) || (crlf_action == CRLF_INPUT) ||
(crlf_action == CRLF_GUESS && auto_crlf == AUTO_CRLF_FALSE))
-- 
2.7.0.303.g2c4f448.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

git clean without ignored files

[Robert Dailey]

I noticed that `git clean` does not handle a specific scenario. I have
the following types of untracked entities in my working copy:

* Untracked files in tracked directories (non-recursive; sibling files
are tracked)
* Untracked files in untracked directories (recursive)
* Ignored files meeting the same criteria above

My goal is to clean the first 2 points, but not the 3rd. Basically, I
want all untracked files that show up in `git status` to be removed,
but not ignored files or directories.

If I run this command:

$ git clean -nd

I get a list of all the untracked files but oddly I get a few
directories in the list that indirectly ignored via my .gitignore.
Basically, the directory itself isn't ignored by any pattern in my
.gitignore but its entire contents (recursively) is ignored, due to
patterns matching the files themselves.

I think `clean` sees this as an untracked directory, because it's not
specifically matching a pattern in my .gitignore. Whereas git status
is smart enough to not display it because it knows that every single
file contained in there, regardless of depth, matches an ignore
pattern.

As a workaround, the following command accomplishes what I want:

$ git ls-files --others --exclude-standard | sed 's/.*/"&"/' | xargs rm -rfv

Is there a reason why `git clean -nd` file listing doesn't match more
closely with what I see in `git status` regarding untracked files and
directories?

Note I am aware git does not directly track directories. So when I
refer to tracked/untracked directories, I'm actually referring to that
directories contents recursively. If recursively 100 files exist, but
only 99 are ignored, then I consider that root directory to be
"untracked" but not ignored, because 1 file still shows up in `git
status`.
--
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

gitk view documentation? tooltips?

[Britton Kerin]

I guess I found the view documentation in git-log and git-rev-list man pages

For some reason my brain also slightly resists permanently learning
what some of the arrows, search, find fields etc at the top level do.

Might tooltips for all this stuff be helpful?
--
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] On the --depth argument when fetching with submodules

[Fredrik Gustafsson]

On Fri, Feb 05, 2016 at 04:05:01PM -0800, Junio C Hamano wrote:
> Stefan Beller  writes:
> 
> > Currently when cloning a project, including submodules, the --depth argument
> > is passed on recursively, i.e. when cloning with "--depth 2", both the
> > superproject as well as the submodule will have a depth of 2.  It is not
> > garantueed that the commits as specified by the superproject are included
> > in these 2 commits of the submodule.
> >
> > Illustration:
> > (superproject with depth 2, so A would have more parents, not shown)
> >
> > superproject/master: A <- B
> > /  \
> > submodule/master:  C <- D <- E <- F <- G
> >
> > (Current behavior is to fetch G and F)
> 

The --depth option to git submodule is something I've wondered for some
time if it was correct to implement it or not. It's clearly not a
complete feature yet and it has some weaknesses, the most obvious one
stated above.

The reason for implementing --depth in submodules was for people having
huge (or many) submodules, that they aren't interested in developing in,
but need to download to  build their project. So they want to save time
and bandwidth.

My first thought was to implement depth from the sha1 the superproject
refered sha1. However, when implementing this, git didn't support
fetching a sha1 from a remote repository for security reasons (you
should never be allowed to fetch a commit that is not reachable from a
branch or tag).

Waiting for this to be supported (an (expensive) check could be done if
the sha1 asked for exists in any branch or tag), the --depth was added
and it's a guessing game on how deep you should fetch.
-- 
Fredrik Gustafsson

phone: +46 733-608274
e-mail: iv...@iveqy.com
website: http://www.iveqy.com
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 3/3] ident: cleanup wrt ident's source

[Jeff King]

On Fri, Feb 05, 2016 at 11:05:19AM -0800, Junio C Hamano wrote:

> Dan Aloni  writes:
> 
> > This change condenses the variables that tells where we got the user's
> > ident into single enum, instead of a collection of booleans.
> >
> > In addtion, also have {committer,author}_ident_sufficiently_given
> > directly probe the environment and the afformentioned enum instead of
> > relying on git_{committer,author}_info to do so.
> >
> > Signed-off-by: Dan Aloni 
> > Helped-by: Jeff King 
> > Helped-by: Junio C Hamano 
> > ---
> >  ident.c | 126 
> > 
> >  1 file changed, 80 insertions(+), 46 deletions(-)
> 
> Peff what do you think?  I am asking you because personally I do not
> find this particularly easier to read than the original, but since
> you stared at the code around here recently much longer than I did
> when doing the 1/3, I thought you may be a better judge than I am.

I'm not sure it is really worth it unless we are going to expose this to
the user, and let them say "I am OK with IDENT_SOURCE_GUESSED, but not
IDENT_SOURCE_GUESSED_BOGUS" or similar.

Without that, I think it is probably just making things a bit more
brittle.

-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

Clarification on the git+ssh and ssh+git schemes

[Carlos Martín Nieto]

Hello gits,

git supports using git+ssh:// and ssh+git:// instead of ssh:// or the 
rsync-style format. The first two are however not documented in the git-clone 
manage as acceptable protocols (which is what I think of as the canonical 
source for what you can use). There are tests to make sure these are supported, 
but even the commit that allows for this (c05186cc; Support git+ssh:// and 
ssh+git:// URL) makes it pretty clear it’s not something that’s considered 
sensible.

But in either case, if we’re going to support it, it should be documented. If 
we don’t want to support it, then we should delete the references to these 
formats along with the tests for this.

I’m happy to write a patch going in either direction, but I’d like some input 
from the community as to what we want to do.

Cheers,
   cmn

--
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: changing colors in the tree view in gitk

[Philip Oakley]

From: "Britton Kerin" 

I upgraded from 2.5 to 2.7 and the branch names went from a light
green to dark green, the names of the tags are hard to read now.

Is it possible to configure the branch name color in the tree view?
--

Which Operating System is this on? and which Git version.?


For the Git for Windows, the Mintty window colours can be adjusted. e.g. 
https://code.google.com/archive/p/mintty/wikis/Tips.wiki


Though I just changed my config setting for the awkward to read items (for 
me it was color.branch.upstream=green !)

--

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] remote-curl: don't fall back to Basic auth if we haven't tried Negotiate

[Junio C Hamano]

Dmitry Vilkov  writes:

> 2016-02-03 2:29 GMT+03:00 brian m. carlson :
>> I'm unclear in what case you'd need to have a username and password
>> combination with GSS-Negotiate.  Kerberos doesn't use your password,
>> although you need some indication of a username (valid or not) to get
>> libcurl to do authentication.
>>
>> Are you basically using a bare URL (without a username component) and
>> waiting for git to prompt you for the username, so that it will then
>> enable authentication?  If so, this patch looks fine for that, although
>> I'd expand on the commit message.  If not, could you provide an example
>> of what you're trying to do?
>
> You are right, we are using a bare URL (without a username component).
> With username encoded in URL everything works just fine. But it's
> generally wrong to pass creds in URL (in my opinion) and security
> policy of my employer prohibits doing such thing.
>
> Anyway, as you said libcurl needs valid (not NULL) username/password
> to do GSS-Negotiate, so there is nothing wrong if I set empty
> username/password combination when git prompts for creds. 

OK, as Brian said, that use case would need to be in the log
message, at least.  I am curious, though, if you can give just a
random string to username, or at least that must match what the
underlying authentication mechanism uses.

Brian, I can see how this would work in that use case, but I haven't
convinced myself that the change would not affect other existing use
cases that are supported--do you think of any that would negatively
affect the user expeerience?

> Even more,
> there is no other way to let libcurl to use GSS-Negotiate without
> username in URL.

Asking lubcurl expert about that might not be a bad idea; Cc'ed
Daniel Stenberg.

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

Re: [PATCH 2/8] pack-objects: produce a stable pack when --skip is given

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 417c830..c58a9cb 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -2709,6 +2709,11 @@ int cmd_pack_objects(int argc, const char **argv, 
> const char *prefix)
>   if (get_oid_hex(skip_hash_hex, _hash))
>   die(_("%s is not SHA-1"), skip_hash_hex);
>   }
> +
> + /*
> +  * Parallel delta search can't produce stable packs.
> +  */
> + delta_search_threads = 1;
>   }
>  
>   argv_array_push(, "pack-objects");

A multi-threaded packing is _one_ source of regenerating the same
pack for the same set of objects, but we shouldn't be tying our
hands by promising it will forever be the _only_ source of it by
doing things like this.  We may want to dynamically tweak the
packing behaviour depending on the load of the minute and such for
example.

This is an indication that the approach this series takes is taking
us in a wrong direction.

I think a more sensible approach for "resuming" is to attack cloning
first.  Take a reasonable baseline snapshot periodically (depending
on the activity level of the project, the interval may span between
12 hours to 2 weeks and you would want to make it configurable) to
create a bundle, teach "clone" to check the bundle first and perform
a resumable and bulk transfer for the stable part of the history
(e.g. up to the latest tag or a branch that does not rewind, the set
of refs to use as the stable anchors you would want to make
configurable), and then fill the gap between that baseline snapshot
and up-to-date state by doing another round of "git fetch" and then
you'd have solved the most serious problem already.


--
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: [PATCHv8 6/9] fetching submodules: respect `submodule.fetchJobs` config option

[Stefan Beller]

On Thu, Feb 4, 2016 at 7:29 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> diff --git a/builtin/fetch.c b/builtin/fetch.c
>> index 586840d..5aa1c2d 100644
>> --- a/builtin/fetch.c
>> +++ b/builtin/fetch.c
>> @@ -37,7 +37,7 @@ static int prune = -1; /* unspecified */
>>  static int all, append, dry_run, force, keep, multiple, update_head_ok, 
>> verbosity;
>>  static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
>>  static int tags = TAGS_DEFAULT, unshallow, update_shallow;
>> -static int max_children = 1;
>> +static int max_children = -1;
>
> This makes sense as you would later be doing "If left unspecified,
> i.e. -1, then fall back to blah" ...

Right,
max_children is passed into fetch_populated_submodules, which
should handle that

>
> g> diff --git a/submodule-config.c b/submodule-config.c
>> index 2841c5e..339b59d 100644
>> --- a/submodule-config.c
>> +++ b/submodule-config.c
>> @@ -32,6 +32,7 @@ enum lookup_type {
>>
>>  static struct submodule_cache cache;
>>  static int is_cache_init;
>> +static unsigned long parallel_jobs = -1;
>
> ... but I do not think this does.  For one thing, you would not be
> doing "If parallel_jobs < -1, then that is unspecified yet" for the
> unsigned long variable, and for another, I do not think you would be
> behaving differently for the first time (i.e. "unspecified yet" case).
>
> I think you want to initialize this to 1, if your "not configured at
> all" default is supposed to be 1.

Please read on in the patch, such that you find

@@ -751,6 +751,11 @@ int fetch_populated_submodules(
(This is where the max_children from builtin/fetch is passed into,
named max_parallel_jobs now)

+   if (max_parallel_jobs < 0)
+   max_parallel_jobs = config_parallel_submodules();
+   if (max_parallel_jobs < 0)
+   max_parallel_jobs = 1;
+

So if we don't get the config option from builtin/fetch, we ask for
config_parallel_submodules(), which is what you were seeing above
initialized as -1, too.  And if that is still -1, we default to 1 there.

So from a design perspective, you're rather saying we want to move part of
this logic into submodule-config.c, such that
config_parallel_submodules never returns -1,
but either 1 as the default or the actual configuration?
Then the code in fetch_populated_submodules, would be as simple as

if (max_parallel_jobs < 0)
max_parallel_jobs = config_parallel_submodules();
// done , as config_parallel_submodules(); gives the last
authoritive answer.

Well looking at submodule-config.c this might be easier, too.

Ok.

>
>> @@ -233,6 +234,13 @@ static int parse_generic_submodule_config(const char 
>> *key,
>> const char *value,
>> struct parse_config_parameter *me)
>>  {
>> + if (!strcmp(key, "fetchjobs")) {
>> + if (!git_parse_ulong(value, _jobs)) {
>> + warning(_("Error parsing submodule.fetchJobs; 
>> Defaulting to 1."));
>> + parallel_jobs = 1;
>
> Hmph, this is not a die() because...?  Not a rhetorical question.

Because this config option doesn't alter the state of the repository.
After the fact you should not be able to tell how much parallel operations
were going on.

(As opposed to other options which alter the state of the repository)

I'll change it to die(...), though it sounds overly strict to me in this case.
But I guess consistency beats overstrictness here.

>
>> +unsigned long config_parallel_submodules(void)
>> +{
>> + return parallel_jobs;
>> +}
>
> It is not a crime to make this return "int", as the code that
> eventually uses it will assign it to a variable that will be
> passed to run_processes_parallel() that takes an "int".

ok

>
> In fact, returning "int" would be preferrable.  You are causing
> truncation somewhere in the callchain anyway.  If the truncation
> bothers you, this function or immediately after calling
> git_parse_ulong() would be a good place to do a range check.  The
> variable parallel_jobs has to stay "unsigned long" in any case as
> you are calling git_parse_ulong().

ok, that should be the best place.

>
>> diff --git a/submodule.c b/submodule.c
>> index b83939c..eb7d54b 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -751,6 +751,11 @@ int fetch_populated_submodules(const struct argv_array 
>> *options,
>>   argv_array_push(, "--recurse-submodules-default");
>>   /* default value, "--submodule-prefix" and its value are added later */
>>
>> + if (max_parallel_jobs < 0)
>> + max_parallel_jobs = config_parallel_submodules();
>> + if (max_parallel_jobs < 0)
>> + max_parallel_jobs = 1;
>> +
>>   calculate_changed_submodule_paths();
>>   run_processes_parallel(max_parallel_jobs,
>>  get_next_submodule,
--
To unsubscribe from this list: send the line "unsubscribe git" in
the 

Re: [PATCH v6 3/3] ident: cleanup wrt ident's source

[Junio C Hamano]

Dan Aloni  writes:

> This change condenses the variables that tells where we got the user's
> ident into single enum, instead of a collection of booleans.
>
> In addtion, also have {committer,author}_ident_sufficiently_given
> directly probe the environment and the afformentioned enum instead of
> relying on git_{committer,author}_info to do so.
>
> Signed-off-by: Dan Aloni 
> Helped-by: Jeff King 
> Helped-by: Junio C Hamano 
> ---
>  ident.c | 126 
> 
>  1 file changed, 80 insertions(+), 46 deletions(-)

Peff what do you think?  I am asking you because personally I do not
find this particularly easier to read than the original, but since
you stared at the code around here recently much longer than I did
when doing the 1/3, I thought you may be a better judge than I am.

> diff --git a/ident.c b/ident.c
> index 9bd6ac69bfe8..9bb05912b59a 100644
> --- a/ident.c
> +++ b/ident.c
> @@ -10,17 +10,19 @@
>  static struct strbuf git_default_name = STRBUF_INIT;
>  static struct strbuf git_default_email = STRBUF_INIT;
>  static struct strbuf git_default_date = STRBUF_INIT;
> -static int default_email_is_bogus;
> -static int default_name_is_bogus;
> +
> +enum ident_source {
> + IDENT_SOURCE_UNKNOWN = 0,
> + IDENT_SOURCE_CONFIG,
> + IDENT_SOURCE_ENVIRONMENT,
> + IDENT_SOURCE_GUESSED,
> + IDENT_SOURCE_GUESSED_BOGUS
> +};
>  
>  static int ident_use_config_only;
>  
> -#define IDENT_NAME_GIVEN 01
> -#define IDENT_MAIL_GIVEN 02
> -#define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
> -static int committer_ident_explicitly_given;
> -static int author_ident_explicitly_given;
> -static int ident_config_given;
> +static enum ident_source source_of_default_email = IDENT_SOURCE_UNKNOWN;
> +static enum ident_source source_of_default_name = IDENT_SOURCE_UNKNOWN;
>  
>  #ifdef NO_GECOS_IN_PWENT
>  #define get_gecos(ignored) "&"
> @@ -28,7 +30,7 @@ static int ident_config_given;
>  #define get_gecos(struct_passwd) ((struct_passwd)->pw_gecos)
>  #endif
>  
> -static struct passwd *xgetpwuid_self(int *is_bogus)
> +static struct passwd *xgetpwuid_self(enum ident_source *source)
>  {
>   struct passwd *pw;
>  
> @@ -41,9 +43,13 @@ static struct passwd *xgetpwuid_self(int *is_bogus)
>   fallback.pw_gecos = "Unknown";
>  #endif
>   pw = 
> - if (is_bogus)
> - *is_bogus = 1;
> + if (source)
> + *source = IDENT_SOURCE_GUESSED_BOGUS;
> + } else {
> + if (source)
> + *source = IDENT_SOURCE_GUESSED;
>   }
> +
>   return pw;
>  }
>  
> @@ -120,26 +126,26 @@ static int canonical_name(const char *host, struct 
> strbuf *out)
>   return status;
>  }
>  
> -static void add_domainname(struct strbuf *out, int *is_bogus)
> +static void add_domainname(struct strbuf *out, enum ident_source *source)
>  {
>   char buf[1024];
>  
>   if (gethostname(buf, sizeof(buf))) {
>   warning("cannot get host name: %s", strerror(errno));
>   strbuf_addstr(out, "(none)");
> - *is_bogus = 1;
> + *source = IDENT_SOURCE_GUESSED_BOGUS;
>   return;
>   }
>   if (strchr(buf, '.'))
>   strbuf_addstr(out, buf);
>   else if (canonical_name(buf, out) < 0) {
>   strbuf_addf(out, "%s.(none)", buf);
> - *is_bogus = 1;
> + *source = IDENT_SOURCE_GUESSED_BOGUS;
>   }
>  }
>  
>  static void copy_email(const struct passwd *pw, struct strbuf *email,
> -int *is_bogus)
> +enum ident_source *source)
>  {
>   /*
>* Make up a fake email address
> @@ -150,13 +156,13 @@ static void copy_email(const struct passwd *pw, struct 
> strbuf *email,
>  
>   if (!add_mailname_host(email))
>   return; /* read from "/etc/mailname" (Debian) */
> - add_domainname(email, is_bogus);
> + add_domainname(email, source);
>  }
>  
>  const char *ident_default_name(void)
>  {
>   if (!git_default_name.len) {
> - copy_gecos(xgetpwuid_self(_name_is_bogus), 
> _default_name);
> + copy_gecos(xgetpwuid_self(_of_default_name), 
> _default_name);
>   strbuf_trim(_default_name);
>   }
>   return git_default_name.buf;
> @@ -169,11 +175,12 @@ const char *ident_default_email(void)
>  
>   if (email && email[0]) {
>   strbuf_addstr(_default_email, email);
> - committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
> - author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
> - } else
> - copy_email(xgetpwuid_self(_email_is_bogus),
> -_default_email, _email_is_bogus);
> + source_of_default_email = IDENT_SOURCE_ENVIRONMENT;
> + 

changing colors in the tree view in gitk

[Britton Kerin]

I upgraded from 2.5 to 2.7 and the branch names went from a light
green to dark green, the names of the tags are hard to read now.

Is it possible to configure the branch name color in the tree view?
--
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 v6 2/3] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Jeff King]

On Fri, Feb 05, 2016 at 09:42:27AM +0200, Dan Aloni wrote:

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 02bcde6bb596..25cf7ce4e83a 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2821,6 +2821,15 @@ user.name::
>   Can be overridden by the 'GIT_AUTHOR_NAME' and 'GIT_COMMITTER_NAME'
>   environment variables.  See linkgit:git-commit-tree[1].
>  
> +user.useConfigOnly::
> + This instruct Git to avoid trying to guess defaults for 'user.email'

s/instruct/instructs/

> + and 'user.name' other than strictly from environment or config.

I find mention of the environment a bit ambiguous. Given our discussion,
I'm sure you mean $GIT_AUTHOR_EMAIL, etc, and not $EMAIL. But I don't
think that is clear to somebody who has not been looking at this patch
series.

I actually think we could simply say "other than strictly from the
config", as people don't generally use $GIT_* themselves (rather, they
get used mostly for inter-process communication, so at most script
authors need to know about them).

> + If you have multiple email addresses that you would like to set
> + up per repository, you may want to set this to 'true' in the global

I parsed this sentence as "multiple addresses per repository". Maybe:

  If you have multiple email addresses and would like to use a different
  one for each repository, you may...

would be more clear?

> +test_description='per-repo forced setting of email address'
> +
> +. ./test-lib.sh
> +
> +prepare () {
> + # Have a non-empty repository
> + rm -fr .git
> + git init
> + echo "Initial" >foo &&
> + git add foo &&
> + git commit -m foo &&
> +
> + # Setup a likely user.useConfigOnly use case
> + sane_unset GIT_AUTHOR_NAME &&
> + sane_unset GIT_AUTHOR_EMAIL &&
> + test_unconfig --global user.name &&
> + test_unconfig --global user.email &&
> + test_config user.name "test" &&
> + test_unconfig user.email &&
> + test_config_global user.useConfigOnly true
> +}
> +
> +about_to_commit () {
> + echo "Second" >>foo &&
> + git add foo
> +}
> +
> +test_expect_success 'fails committing if clone email is not set' '
> + prepare && about_to_commit &&
> +
> + test_must_fail git commit -m msg
> +'

The flow of this test script is a bit different than what we usually
write. Typically we have some early test_expect_success blocks do setup
for the whole script, and then progress through a sequence (and we rely
on the test harness to do things like "git init").

IOW, most of your "prepare" would go in the first block, and then the
rest of the tests rely on it.

The only thing I really see that needs to be repeated for each test is
setting up the "about to commit" scenario. But you can simply use
"commit --allow-empty" so that the tests work no matter what state the
previous test left us in. We care about the ident, not what gets
committed.

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

[PATCH v1] config: add '--sources' option to print the source of a config value

[larsxschneider]

From: Lars Schneider 

If config values are queried using 'git config' (e.g. via '--list' flag
or the '--get*' flags) then it is sometimes hard to find the
configuration file where the values were defined.

Teach 'git config' the '--sources' option to print the source
configuration file for every printed value.

Based-on-patch-by: Jeff King 
Signed-off-by: Lars Schneider 
---
 builtin/config.c   | 42 
 cache.h|  1 +
 config.c   |  7 ++
 t/t1300-repo-config.sh | 65 ++
 4 files changed, 115 insertions(+)

diff --git a/builtin/config.c b/builtin/config.c
index adc7727..f5dc79c 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -3,6 +3,7 @@
 #include "color.h"
 #include "parse-options.h"
 #include "urlmatch.h"
+#include "quote.h"
 
 static const char *const builtin_config_usage[] = {
N_("git config []"),
@@ -27,6 +28,7 @@ static int actions, types;
 static const char *get_color_slot, *get_colorbool_slot;
 static int end_null;
 static int respect_includes = -1;
+static int show_sources;
 
 #define ACTION_GET (1<<0)
 #define ACTION_GET_ALL (1<<1)
@@ -81,6 +83,7 @@ static struct option builtin_config_options[] = {
OPT_BOOL('z', "null", _null, N_("terminate values with NUL byte")),
OPT_BOOL(0, "name-only", _values, N_("show variable names only")),
OPT_BOOL(0, "includes", _includes, N_("respect include 
directives on lookup")),
+   OPT_BOOL(0, "sources", _sources, N_("show source filenames of 
config")),
OPT_END(),
 };
 
@@ -91,8 +94,34 @@ static void check_argc(int argc, int min, int max) {
usage_with_options(builtin_config_usage, builtin_config_options);
 }
 
+/* output to either fp or buf; only one should be non-NULL */
+static void show_config_source(struct strbuf *buf, FILE *fp)
+{
+   const char *fn = current_config_filename();
+   if (!fn)
+   return;
+
+   char term = '\t';
+   if (!end_null)
+   quote_c_style(fn, buf, fp, 0);
+   else {
+   term = '\0';
+   if (fp)
+   fprintf(fp, "%s", fn);
+   else
+   strbuf_addstr(buf, fn);
+   }
+
+   if (fp)
+   fputc(term, fp);
+   else
+   strbuf_addch(buf, term);
+}
+
 static int show_all_config(const char *key_, const char *value_, void *cb)
 {
+   if (show_sources)
+   show_config_source(NULL, stdout);
if (!omit_values && value_)
printf("%s%c%s%c", key_, delim, value_, term);
else
@@ -108,6 +137,8 @@ struct strbuf_list {
 
 static int format_config(struct strbuf *buf, const char *key_, const char 
*value_)
 {
+   if (show_sources)
+   show_config_source(buf, NULL);
if (show_keys)
strbuf_addstr(buf, key_);
if (!omit_values) {
@@ -538,6 +569,17 @@ int cmd_config(int argc, const char **argv, const char 
*prefix)
error("--name-only is only applicable to --list or 
--get-regexp");
usage_with_options(builtin_config_usage, 
builtin_config_options);
}
+
+   const int is_query_action = actions & (
+   ACTION_GET|ACTION_GET_ALL|ACTION_GET_REGEXP|ACTION_LIST|
+   ACTION_GET_COLOR|ACTION_GET_COLORBOOL|ACTION_GET_URLMATCH
+   );
+
+   if (show_sources && !is_query_action) {
+   error("--sources is only applicable to --list or --get-* 
actions");
+   usage_with_options(builtin_config_usage, 
builtin_config_options);
+   }
+
if (actions == ACTION_LIST) {
check_argc(argc, 0, 0);
if (git_config_with_options(show_all_config, NULL,
diff --git a/cache.h b/cache.h
index c63fcc1..c5111ea 100644
--- a/cache.h
+++ b/cache.h
@@ -1525,6 +1525,7 @@ extern const char *get_log_output_encoding(void);
 extern const char *get_commit_output_encoding(void);
 
 extern int git_config_parse_parameter(const char *, config_fn_t fn, void 
*data);
+extern const char *current_config_filename(void);
 
 struct config_include_data {
int depth;
diff --git a/config.c b/config.c
index 86a5eb2..b437002 100644
--- a/config.c
+++ b/config.c
@@ -2385,3 +2385,10 @@ int parse_config_key(const char *var,
 
return 0;
 }
+
+const char *current_config_filename(void)
+{
+   if (cf && cf->name)
+   return cf->name;
+   return NULL;
+}
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 52678e7..2444d8a 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1201,4 +1201,69 @@ test_expect_success POSIXPERM,PERL 'preserves existing 
permissions' '
  "die q(badrename) if ((stat(q(.git/config)))[2] & 0) != 0600"
 '
 
+test_expect_success '--sources' '
+   >.git/config &&
+   >"$HOME"/.gitconfig &&
+   

Re: Clarification on the git+ssh and ssh+git schemes

[Jeff King]

On Fri, Feb 05, 2016 at 09:33:06AM -0800, Carlos Martín Nieto wrote:

> git supports using git+ssh:// and ssh+git:// instead of ssh:// or the
> rsync-style format. The first two are however not documented in the
> git-clone manage as acceptable protocols (which is what I think of as
> the canonical source for what you can use). There are tests to make
> sure these are supported, but even the commit that allows for this
> (c05186cc; Support git+ssh:// and ssh+git:// URL) makes it pretty
> clear it’s not something that’s considered sensible.

Hrm. I tried to find more discussion on the list, but I couldn't find
any mention of git+ssh, nor of that patch. I wonder if there is a hole
in my archive, or if they were done off-list for some reason.

Anyway...

> But in either case, if we’re going to support it, it should be
> documented. If we don’t want to support it, then we should delete the
> references to these formats along with the tests for this.

Whether they are stupid or not (and I agree that they are), we cannot
just rip them out now without warning. And given that they are probably
not costing us a lot in maintenance burden to keep, I'd guess it is less
effort to simply leave them in place.

I suspect they were not really documented because nobody wanted to
encourage their use. I don't think it would be wrong to document that
they exist and are deprecated, though.

> I’m happy to write a patch going in either direction, but I’d like
> some input from the community as to what we want to do.

I imagine your ulterior motive is also figuring out whether libgit2
needs to support them?

-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 v6 2/3] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Eric Sunshine]

On Fri, Feb 5, 2016 at 2:18 PM, Jeff King  wrote:
> On Fri, Feb 05, 2016 at 09:42:27AM +0200, Dan Aloni wrote:
>> +prepare () {
>> + # Have a non-empty repository
>> + rm -fr .git
>> + git init
>> + echo "Initial" >foo &&
>> + git add foo &&
>> + git commit -m foo &&
>> +
>> + # Setup a likely user.useConfigOnly use case
>> + sane_unset GIT_AUTHOR_NAME &&
>> + sane_unset GIT_AUTHOR_EMAIL &&
>> + test_unconfig --global user.name &&
>> + test_unconfig --global user.email &&
>> + test_config user.name "test" &&
>> + test_unconfig user.email &&
>> + test_config_global user.useConfigOnly true
>> +}
>
> The flow of this test script is a bit different than what we usually
> write. Typically we have some early test_expect_success blocks do setup
> for the whole script, and then progress through a sequence (and we rely
> on the test harness to do things like "git init").
>
> IOW, most of your "prepare" would go in the first block, and then the
> rest of the tests rely on it.
>
> The only thing I really see that needs to be repeated for each test is
> setting up the "about to commit" scenario. But you can simply use
> "commit --allow-empty" so that the tests work no matter what state the
> previous test left us in. We care about the ident, not what gets
> committed.

I was going to make all the same suggestions, so thanks. One thing to
add is that the &&-chain is broken in the prepare() 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

Re: [PATCH v6 2/3] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Junio C Hamano]

Dan Aloni  writes:

> +user.useConfigOnly::
> + This instruct Git to avoid trying to guess defaults for 'user.email'
> + and 'user.name' other than strictly from environment or config.

OK.

> + If you have multiple email addresses that you would like to set
> + up per repository, you may want to set this to 'true' in the global
> + config, and then Git would prompt you to set user.email separately,
> + in each of the cloned repositories.

The first sentence mentioned both name and email, but here the
example is only about email.  A first time reader might be led into
thinking this is only about email and not name, but I am assuming
that is not the intention (i.e. this is merely showing just one use
case).

> + Defaults to `false`.
> +
>  user.signingKey::
>   If linkgit:git-tag[1] or linkgit:git-commit[1] is not selecting the
>   key you want it to automatically when creating a signed tag or
> diff --git a/ident.c b/ident.c
> index f3a431f738cc..9bd6ac69bfe8 100644
> --- a/ident.c
> +++ b/ident.c
> @@ -13,11 +13,14 @@ static struct strbuf git_default_date = STRBUF_INIT;
>  static int default_email_is_bogus;
>  static int default_name_is_bogus;
>  
> +static int ident_use_config_only;
> +
>  #define IDENT_NAME_GIVEN 01
>  #define IDENT_MAIL_GIVEN 02
>  #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
>  static int committer_ident_explicitly_given;
>  static int author_ident_explicitly_given;
> +static int ident_config_given;
>  
>  #ifdef NO_GECOS_IN_PWENT
>  #define get_gecos(ignored) "&"
> @@ -354,6 +357,9 @@ const char *fmt_ident(const char *name, const char *email,
>   fputs(env_hint, stderr);
>   die("unable to auto-detect name (got '%s')", 
> name);
>   }
> + if (strict && ident_use_config_only &&
> + !(ident_config_given & IDENT_NAME_GIVEN))
> + die("user.useConfigOnly set but no name given");
>   }
>   if (!*name) {
>   struct passwd *pw;
> @@ -373,6 +379,9 @@ const char *fmt_ident(const char *name, const char *email,
>   fputs(env_hint, stderr);
>   die("unable to auto-detect email address (got '%s')", 
> email);
>   }
> + if (strict && ident_use_config_only
> + && !(ident_config_given & IDENT_MAIL_GIVEN))
> + die("user.useConfigOnly set but no mail given");

I can read the split expression either with && hanging at the end of
line or && leading the next line just fine, but you'd want to be
consistent especially when you are writing two almost identical
things.

> diff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh
> new file mode 100755
> index ..0430f2e38434
> --- /dev/null
> +++ b/t/t9904-per-repo-email.sh
> @@ -0,0 +1,58 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2016 Dan Aloni
> +#
> +
> +test_description='per-repo forced setting of email address'
> +
> +. ./test-lib.sh
> +
> +prepare () {
> + # Have a non-empty repository
> + rm -fr .git
> + git init

Hmm, do we do something drastic like this in any of our existing
tests?

When your test script starts by dot-sourcing test-lib.sh, you will
be given an initial repository with an empty history, so I'd expect
that you would be able to use that without calling "prepare" over
and over again.  The usual convention is to do this kind of setup
to establish a reasonable baseline in the first test titled 'setup'.
I think the part up to where you unset the environment variables (by
the way, don't you need to unset GIT_COMMITTER_* variables, too?)
belongs to the 'setup' that is done only once at the beginning of
this script.

Each of your tests will make changes to the state by setting or
unsetting configuration variables and possibly making commits, that
would affect the state of the repository that will be used by the
next and subsequent tests.  test_when_finished helper can register
the clean-up procedure to revert these possible state changes, and
you can further avoid code duplication by calling that same clean-up
procedure at the end of the setup test.

So if this followed the style of a typical existing test, we would
probably do something along these lines:

reprepare () {
git reset --hard initial &&
echo Second >foo &&
git add foo
}

test_expect_success setup '
echo Initial >foo &&
git add foo &&
git commit -m foo &&
git tag initial &&

sane_unset GIT_AUTHOR_NAME GIT_COMMITTER_NAME ... &&
git config --global user.name test &&
git config --global user.useConfigOnly true &&
reprepare
'

test_expect_success 'fail without email anywhere' '

Re: Clarification on the git+ssh and ssh+git schemes

[Linus Torvalds]

On Fri, Feb 5, 2016 at 11:30 AM, Jeff King  wrote:
>
> I suspect they were not really documented because nobody wanted to
> encourage their use. I don't think it would be wrong to document that
> they exist and are deprecated, though.

They exist because some people seemed to think that people shouldn't
use "ssh://" since they thought that only ssh should use that.

Which is obviously bullshit, since by that logic all the other formats
should have that idiotic "git+" format too ("git+https", anybody?). It
doesn't actually help anything, and it only pushes somebodys broken
agenda.

So there was a push for that silly thing by a couple of people, but it
was always wrong. Don't even document it.

Leave it in the source code as an option, and maybe add a comment
about "This is stupid, but we support it for hysterical raisins".

Don't add it to any real documentation. Not even as deprecated. That
just validates it further.

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

[PATCH v4 00/20] refs backend

[David Turner]

Changes to this version:
re-rolled on top of pu as-of 9db66d9f1aa.

Bug fixes include:
For submodules: memory leaks; segfault on bad config. (thanks to Peff)
In symref splitting: check that would always succeed (thanks to Peff)
A bogus double-declaration of a var (thanks to Ramsay Jones)
Two memory leaks (thanks to Thomas Gummerer)
An unused var (thanks to Duy Nguyen)

Other improvements are:
Strings prepped for 18n (thanks to Duy Nguyen)
Cleaner submodule handling (thanks to Peff)
Whitelisting instead of blacklisting in git-new-workdir (thanks to
  Thomas Gummerer)
Allow older gits to recognize lmdb-backend git repos (thanks to Duy Nguyen)
Tab completion and cleaner commit messages (thanks SZEDER Gábor)
Removed some #ifdefs, moving all backend setup to one place (thanks to Duy
Nguyen)

Thanks to all for reviews.

David Turner (18):
  refs: add do_for_each_per_worktree_ref
  refs: add methods for reflog
  refs: add method for initial ref transaction commit
  refs: add method for delete_refs
  refs: add methods to init refs db
  refs: add method to rename refs
  refs: make lock generic
  refs: move duplicate check to common code
  refs: allow log-only updates
  refs: resolve symbolic refs first
  refs: always handle non-normal refs in files backend
  init: allow alternate ref strorage to be set for new repos
  refs: check submodules ref storage config
  clone: allow ref storage backend to be set for clone
  svn: learn ref-storage argument
  refs: add register_ref_storage_backends()
  refs: add LMDB refs storage backend
  refs: tests for lmdb backend

Ronnie Sahlberg (3):
  refs: add a backend method structure with transaction functions
  refs: add methods for misc ref operations
  refs: add methods for the ref iterators

 .gitignore |1 +
 Documentation/config.txt   |7 +
 Documentation/git-clone.txt|6 +
 Documentation/git-init-db.txt  |2 +-
 Documentation/git-init.txt |7 +-
 Documentation/technical/refs-lmdb-backend.txt  |   52 +
 Documentation/technical/repository-version.txt |5 +
 Makefile   |   12 +
 builtin/clone.c|5 +
 builtin/init-db.c  |   57 +-
 builtin/submodule--helper.c|2 +-
 cache.h|2 +
 config.c   |   25 +
 configure.ac   |   33 +
 contrib/completion/git-completion.bash |6 +-
 contrib/workdir/git-new-workdir|3 +
 git-submodule.sh   |   13 +
 git-svn.perl   |6 +-
 path.c |   29 +-
 refs.c |  486 +-
 refs.h |   21 +
 refs/files-backend.c   |  404 ++---
 refs/lmdb-backend.c| 2052 
 refs/refs-internal.h   |  128 +-
 setup.c|   23 +-
 t/t0001-init.sh|   25 +
 t/t1460-refs-lmdb-backend.sh   | 1109 +
 t/t1470-refs-lmdb-backend-reflog.sh|  359 +
 t/t1480-refs-lmdb-submodule.sh |   85 +
 t/test-lib.sh  |1 +
 test-refs-lmdb-backend.c   |   64 +
 transport.c|7 +-
 32 files changed, 4825 insertions(+), 212 deletions(-)
 create mode 100644 Documentation/technical/refs-lmdb-backend.txt
 create mode 100644 refs/lmdb-backend.c
 create mode 100755 t/t1460-refs-lmdb-backend.sh
 create mode 100755 t/t1470-refs-lmdb-backend-reflog.sh
 create mode 100755 t/t1480-refs-lmdb-submodule.sh
 create mode 100644 test-refs-lmdb-backend.c

-- 
2.4.2.749.g730654d-twtrsrc

--
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: [PATCHv8 6/9] fetching submodules: respect `submodule.fetchJobs` config option

[Junio C Hamano]

Stefan Beller  writes:

> On Thu, Feb 4, 2016 at 7:29 PM, Junio C Hamano  wrote:
>> Stefan Beller  writes:
>> ...
>>> +static unsigned long parallel_jobs = -1;
>>
>> ... but I do not think this does
>
> So if we don't get the config option from builtin/fetch, we ask for
> config_parallel_submodules(), which is what you were seeing above
> initialized as -1, too.  And if that is still -1, we default to 1 there.

But that is not really -1, but -1 casted to unsigned long that is
casted back to int.  

> So from a design perspective, you're rather saying we want to move part of
> this logic into submodule-config.c, such that
> config_parallel_submodules never returns -1,
> but either 1 as the default or the actual configuration?

That is not my design but yours ;-)

Expecting config_parallel_submodules() to return -1 when there is no
variable defined contradicts what you wrote in the documentation, I
think, where it says "this variable defaults to 1 when not set".

> Then the code in fetch_populated_submodules, would be as simple as
>
> if (max_parallel_jobs < 0)
> max_parallel_jobs = config_parallel_submodules();

Yup.

>>> @@ -233,6 +234,13 @@ static int parse_generic_submodule_config(const char 
>>> *key,
>>> const char *value,
>>> struct parse_config_parameter *me)
>>>  {
>>> + if (!strcmp(key, "fetchjobs")) {
>>> + if (!git_parse_ulong(value, _jobs)) {
>>> + warning(_("Error parsing submodule.fetchJobs; 
>>> Defaulting to 1."));
>>> + parallel_jobs = 1;
>>
>> Hmph, this is not a die() because...?  Not a rhetorical question.
>
> Because this config option doesn't alter the state of the repository.
> After the fact you should not be able to tell how much parallel operations
> were going on.
>
> (As opposed to other options which alter the state of the repository)
>
> I'll change it to die(...), though it sounds overly strict to me in this case.
> But I guess consistency beats overstrictness here.

I do not see it as being overly strict, though.

If I were a user of this feature, I'd rather see a problem pointed
out to me (e.g "you spelled 'true' but that fetchJobs expects a
positive integer") to help me fix it, rather than having to see this
warning every time I try to run submodule commands.

What benefit does a user get by being loose here?  Probably I am
not considering some valid use cases...

>>> +unsigned long config_parallel_submodules(void)
>>> +{
>>> + return parallel_jobs;
>>> +}
>>
>> It is not a crime to make this return "int", as the code that
>> eventually uses it will assign it to a variable that will be
>> passed to run_processes_parallel() that takes an "int".
>
> ok
>
>>
>> In fact, returning "int" would be preferrable.  You are causing
>> truncation somewhere in the callchain anyway.  If the truncation
>> bothers you, this function or immediately after calling
>> git_parse_ulong() would be a good place to do a range check.  The
>> variable parallel_jobs has to stay "unsigned long" in any case as
>> you are calling git_parse_ulong().
>
> ok, that should be the best place.

Alternatively (I haven't weighed pros and cons myself), you can make
parallel_jobs an "int", call git_parse_ulong() using a temporary
"unsigned long" and after doing range check assign it to
parallel_jobs.  That would make this function really trivial ;-)
--
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-curl: don't fall back to Basic auth if we haven't tried Negotiate

[brian m. carlson]

On Fri, Feb 05, 2016 at 09:54:50AM -0800, Junio C Hamano wrote:
> OK, as Brian said, that use case would need to be in the log
> message, at least.  I am curious, though, if you can give just a
> random string to username, or at least that must match what the
> underlying authentication mechanism uses.

You can give any invalid credentials you like.  When using Kerberos, the
provided username and password are ignored, because all the
authentication information is in the ticket, and it's all encrypted.

I'm happy to send a documentation patch for this, as it seems to come up
a lot.

> Brian, I can see how this would work in that use case, but I haven't
> convinced myself that the change would not affect other existing use
> cases that are supported--do you think of any that would negatively
> affect the user expeerience?

I'd have to test how it works with Basic auth as a fallback.  I don't
normally use that on my servers, so I'd have to enable it and try it
out.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: PGP signature

Re: [PATCH v7 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

On Fri, Feb 05, 2016 at 04:59:52PM -0500, Eric Sunshine wrote:
> On Fri, Feb 05, 2016 at 11:29:06PM +0200, Dan Aloni wrote:
> > It used to be that:
> > 
> >git config --global user.email "(none)"
> > 
> > was a viable way for people to force themselves to set user.email in
> > each repository.  This was helpful for people with more than one
> > email address, targeting different email addresses for different
> > clones, as it barred git from creating commit unless the user.email
> 
> Either: s/commit/a commit/ or s/commit/commits/

Thanks for all the proofing in your reply.

>[..]
> > config was set in the per-repo config to the correct email address.
> > 
> > A recent change, 19ce497c (ident: keep a flag for bogus
> > default_email, 2015-12-10), however declared that an explicitly
> 
> s/however/&,/
> 
> > configured user.email is not bogus, no matter what its value is, so
> > this hack no longer works.
> > 
> > Provide the same functionality by adding a new configuration
> > variable user.useConfigOnly; when this variable is set, the
> > user must explicitly set user.email configuration.
> > 
> > Signed-off-by: Dan Aloni 
> > Helped-by: Jeff King 
> > Signed-off-by: Junio C Hamano 
> > Cc: Eric Sunshine 
> 
> You'd generally place your sign-off last.
> [..]

Good to know :)

> This test script still has a fair amount of unnecessary cruft in it
> which obscures the important bits showing what you are really
> testing. Below is a more concise version with the unnecessary stuff
> removed:

Thanks, though I'll stick to what Jeff suggested. Also, perhaps better
to keep 'test_config' as it is instead of using '-c', to better mimick
the tested use case.

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

Re: [PATCH v8 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Junio C Hamano]

Dan Aloni  writes:

> +user.useConfigOnly::
> + Instruct Git to avoid trying to guess defaults for 'user.email'
> + and 'user.name', and instead retrieve the values only from the
> + configuration. For example, if you have multiple email addresses
> + and would like to use a different one for each repository, then
> + with this configuration option set to `true` in the global config
> + along with a name, Git will prompt you to set up an email upon
> + making new commits in a newly cloned repository.

I do not think this is wrong per-se, but s/upon/before/ may be more
accurate and would assure the users more that no commits with a
wrong identity will be created.

Other than that, these two patches look very good to me.

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

Re: [PATCH 2/8] pack-objects: produce a stable pack when --skip is given

[Duy Nguyen]

On Sat, Feb 6, 2016 at 1:43 AM, Junio C Hamano  wrote:
> Nguyễn Thái Ngọc Duy   writes:
>
>> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
>> index 417c830..c58a9cb 100644
>> --- a/builtin/pack-objects.c
>> +++ b/builtin/pack-objects.c
>> @@ -2709,6 +2709,11 @@ int cmd_pack_objects(int argc, const char **argv, 
>> const char *prefix)
>>   if (get_oid_hex(skip_hash_hex, _hash))
>>   die(_("%s is not SHA-1"), skip_hash_hex);
>>   }
>> +
>> + /*
>> +  * Parallel delta search can't produce stable packs.
>> +  */
>> + delta_search_threads = 1;
>>   }
>>
>>   argv_array_push(, "pack-objects");
>
> A multi-threaded packing is _one_ source of regenerating the same
> pack for the same set of objects, but we shouldn't be tying our
> hands by promising it will forever be the _only_ source of it by
> doing things like this.  We may want to dynamically tweak the
> packing behaviour depending on the load of the minute and such for
> example.

You noticed that tying the behavior only happens when the user asks
for it, right? I don't expect people to do resumable fetch/clone by
default. There are tradeoffs to make and they decide it, we offer
options. So, it does not really tie our hands in the normal case.

> I think a more sensible approach for "resuming" is to attack cloning
> first.  Take a reasonable baseline snapshot periodically (depending
> on the activity level of the project, the interval may span between
> 12 hours to 2 weeks and you would want to make it configurable) to
> create a bundle, teach "clone" to check the bundle first and perform
> a resumable and bulk transfer for the stable part of the history
> (e.g. up to the latest tag or a branch that does not rewind, the set
> of refs to use as the stable anchors you would want to make
> configurable), and then fill the gap between that baseline snapshot
> and up-to-date state by doing another round of "git fetch" and then
> you'd have solved the most serious problem already.

_most_. On a troubled connection, fetch can fail as well, especially
when the repo is not uptodate. What about shallow clone? I don't think
you want to prepare snapshots for all depths (or some "popular"
depths). Cloning full then cutting it shallow locally might work, but
we're wasting bandwidth and disk space.

> This is an indication that the approach this series takes is taking
> us in a wrong direction.

The way I see it, the two approaches complement each other. Snapshots,
like pack bitmaps, helps tremendously, but it has corner cases that
can be covered by this.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Eric Sunshine]

On Fri, Feb 05, 2016 at 11:29:06PM +0200, Dan Aloni wrote:
> It used to be that:
> 
>git config --global user.email "(none)"
> 
> was a viable way for people to force themselves to set user.email in
> each repository.  This was helpful for people with more than one
> email address, targeting different email addresses for different
> clones, as it barred git from creating commit unless the user.email

Either: s/commit/a commit/ or s/commit/commits/

> config was set in the per-repo config to the correct email address.
> 
> A recent change, 19ce497c (ident: keep a flag for bogus
> default_email, 2015-12-10), however declared that an explicitly

s/however/&,/

> configured user.email is not bogus, no matter what its value is, so
> this hack no longer works.
> 
> Provide the same functionality by adding a new configuration
> variable user.useConfigOnly; when this variable is set, the
> user must explicitly set user.email configuration.
> 
> Signed-off-by: Dan Aloni 
> Helped-by: Jeff King 
> Signed-off-by: Junio C Hamano 
> Cc: Eric Sunshine 

You'd generally place your sign-off last.

> ---
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> @@ -2821,6 +2821,16 @@ user.name::
> +user.useConfigOnly::
> + This instructs Git to avoid trying to guess defaults for 'user.email'

Perhaps: s/This instructs/Instruct/

> + and 'user.name' other than strictly from config. For example, if

The way this is phrased, it sounds almost as if Git also "guesses"
the value from config. Perhaps rephrase like this:

Instruct Git to avoid trying to guess defaults for 'user.email'
and 'user.name', and instead retrieve the values only from
configuration.

> + you have multiple email addresses and would like to use a different
> + one for each repository, then with this configuration option set
> + to `true` in the global config along with a name, Git would prompt

s/would/will/

> + for you for setting up an email upon making new commits in a newly

s/for you for setting/you to set/

More below...

> + cloned repository.
> + Defaults to `false`.
> diff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh
> @@ -0,0 +1,55 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2016 Dan Aloni
> +#
> +
> +test_description='per-repo forced setting of email address'
> +
> +. ./test-lib.sh
> +
> +reprepare () {
> + git reset --hard initial
> +}
> +
> +test_expect_success setup '
> + # Initial repo state
> + echo "Initial" >foo &&
> + git add foo &&
> + git commit -m foo &&
> + git tag initial &&
> +
> + # Setup a likely user.useConfigOnly use case
> + sane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&
> + sane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL &&
> + git config user.name "test" &&
> + git config --global user.useConfigOnly true &&
> +
> + reprepare
> +'
> +
> +test_expect_success 'fails committing if clone email is not set' '
> + test_when_finished reprepare &&
> +
> + test_must_fail git commit --allow-empty -m msg
> +'
> +
> +test_expect_success 'fails committing if clone email is not set, but EMAIL 
> set' '
> + test_when_finished reprepare &&
> +
> + test_must_fail env EMAIL=t...@fail.com git commit --allow-empty -m msg
> +'
> +
> +test_expect_success 'succeeds committing if clone email is set' '
> + test_when_finished reprepare &&
> +
> + test_config user.email "t...@ok.com" &&
> + git commit --allow-empty -m msg
> +'
> +
> +test_expect_success 'succeeds cloning if global email is not set' '
> + test_when_finished reprepare &&
> +
> + git clone . clone
> +'
> +
> +test_done

This test script still has a fair amount of unnecessary cruft in it
which obscures the important bits showing what you are really
testing. Below is a more concise version with the unnecessary stuff
removed:

--- 8< ---
#!/bin/sh
#
# Copyright (c) 2016 Dan Aloni
#

test_description='per-repo forced setting of email address'

. ./test-lib.sh

test_expect_success setup '
sane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&
sane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL &&
git config user.name "test" &&
git config --global user.useConfigOnly true
'

test_expect_success 'fails committing if clone email is not set' '
test_must_fail git commit --allow-empty -m msg
'

test_expect_success 'fails committing if clone email is not set, but EMAIL set' 
'
test_must_fail env EMAIL=t...@fail.com git commit --allow-empty -m msg
'

test_expect_success 'succeeds committing if clone email is set' '
git -c user.email=t...@ok.com commit --allow-empty -m msg
'

test_expect_success 'succeeds cloning if global email is not set' '
git clone . clone
'

test_done
--- 8< ---
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info 

Re: [PATCH v7 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

On Fri, Feb 05, 2016 at 04:48:33PM -0500, Jeff King wrote:
> On Fri, Feb 05, 2016 at 11:29:06PM +0200, Dan Aloni wrote:
> 
> > diff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh
> > new file mode 100755
> > index ..f2b33881e46b
> > --- /dev/null
> > +++ b/t/t9904-per-repo-email.sh
> 
> Is t9904 the right place for this? Usually t99xx is for very separate
> components.
> 
> This is sort-of about "commit", which would put it in the t75xx range.
> But in some ways, it is even more fundamental than that. We don't seem
> to have a lot of tests for ident stuff. The closest is the strict ident
> stuff in t0007.

Will move to t7517. IMHO it's better to verify the commit operation
itself before running further tests that rely on its proper function.

>[..]
> > +reprepare () {
> > +   git reset --hard initial
> > +}
> 
> Do we need this reprepare stuff at all now? The tests don't care which
> commit we're at when they start.
> 
> > +test_expect_success setup '
> > +   # Initial repo state
> > +   echo "Initial" >foo &&
> > +   git add foo &&
> > +   git commit -m foo &&
> > +   git tag initial &&
> 
> A shorter way of saying this is "test_commit foo".
> 
> I almost thought we could get rid of this part entirely; the commit
> tests don't care. But we do still need _a_ commit for the clone test,
> since we want to make sure a reflog is written. It would be nice to push
> it down there, but our test environment doesn't allow creating commits,
> because of of useConfigOnly. So it's probably fine to leave it here.
> 
> Technically, the final "commit" test does make a commit for us to push,
> but we do generally try to avoid unnecessary dependencies between the
> individual tests.
> 
> So all together, maybe:
>[..]

Yes, shorted is better.

I'm squashing in these changes and adding you as Signed-off for v8.

-- 
Dan Aloni
--
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: Clarification on the git+ssh and ssh+git schemes

[Junio C Hamano]

Linus Torvalds  writes:

> On Fri, Feb 5, 2016 at 11:30 AM, Jeff King  wrote:
>>
>> I suspect they were not really documented because nobody wanted to
>> encourage their use. I don't think it would be wrong to document that
>> they exist and are deprecated, though.
>
> They exist because some people seemed to think that people shouldn't
> use "ssh://" since they thought that only ssh should use that.
>
> Which is obviously bullshit, since by that logic all the other formats
> should have that idiotic "git+" format too ("git+https", anybody?). It
> doesn't actually help anything, and it only pushes somebodys broken
> agenda.
>
> So there was a push for that silly thing by a couple of people, but it
> was always wrong. Don't even document it.

"git+https://; is actually an interesting thing to think about.

Those who argued that "ssh://" should only be used by ssh would say
for the same reason that "http://; is OK only when Git is using the
old "commit walker" aka "dumb" HTTP transport, and the modern "Git
protocol over HTTP" aka "smart" HTTP should not use "http://;.

Which is a problematic stance to take.  It would force inconvenience
on our users, especially when the server side support for the modern
"Git over HTTP" is done in such a way that it can co-exist with the
commit walker transport.

So in that sense, "git+https://; does not help anybody.  I however
wouldn't use such strong words like you did ;-)

 - If we see "hg+http://; or "svn+http://; URL, that would help
   people to immediately know that "git clone" would not work
   against them, so for us who live in Git world, "git+http:// does
   not buy anything (assuming that we know better than assuming all
   "http://; are clonable, e.g. "git clone http://nytimes.com;), it
   would help if others marked their non-Git URL as such.

 - During technical discussion inside Git circle when we need to
   differentiate the "smart" and "dump" HTTP transports, it may help
   to be able to say "git+http://; and "http://;.  And people who
   heard such a conversation may be tempted to say "git+http://; to
   talk to a Git repository that is known to talk the "smart HTTP"
   protocol.

Devil's advocate mode off.

> Leave it in the source code as an option, and maybe add a comment
> about "This is stupid, but we support it for hysterical raisins".

Sounds good.


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

Re: [PATCH v7 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Junio C Hamano]

Jeff King  writes:

> This is sort-of about "commit", which would put it in the t75xx range.
> But in some ways, it is even more fundamental than that. We don't seem
> to have a lot of tests for ident stuff. The closest is the strict ident
> stuff in t0007.

Good point.

>> +reprepare () {
>> +git reset --hard initial
>> +}
>
> Do we need this reprepare stuff at all now? The tests don't care which
> commit we're at when they start.

Ah, I thought the function was needed to make sure we have something
to commit, but I agree that you do not need it, if you are going to
use "commit --allow-empty" in the tests.

> So all together, maybe:

Sounds sensible.

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 v4 20/21] refs: add LMDB refs storage backend

[David Turner]

Add a database backend for refs using LMDB.  This backend runs git
for-each-ref about 30% faster than the files backend with fully-packed
refs on a repo with ~120k refs.  It's also about 4x faster than using
fully-unpacked refs.  In addition, and perhaps more importantly, it
avoids case-conflict issues on OS X.

LMDB has a few features that make it suitable for usage in git:

1. It is relatively lightweight; it requires only one header file, and
the library code takes under 64k at runtime.

2. It is well-tested: it's been used in OpenLDAP for years.

3. It's very fast.  LMDB's benchmarks show that it is among
the fastest key-value stores.

4. It has a relatively simple concurrency story; readers don't
block writers and writers don't block readers.

Ronnie Sahlberg's original version of this patchset used tdb.  The
major disadvantage of tdb is that tdb is hard to build on OS X.  It's
also not in homebrew.  So lmdb seemed simpler.

To test this backend's correctness, I hacked test-lib.sh and
test-lib-functions.sh to run all tests under the refs backend. Dozens
of tests use manual ref/reflog reading/writing, or create submodules
without passing --ref-storage to git init.  If those tests are
changed to use the update-ref machinery or test-refs-lmdb-backend (or,
in the case of packed-refs, corrupt refs, and dumb fetch tests, are
skipped), the only remaining failing tests are the git-new-workdir
tests and the gitweb tests.

Signed-off-by: David Turner 
---
 .gitignore |1 +
 Documentation/config.txt   |7 +
 Documentation/git-clone.txt|3 +-
 Documentation/git-init.txt |2 +-
 Documentation/technical/refs-lmdb-backend.txt  |   52 +
 Documentation/technical/repository-version.txt |5 +
 Makefile   |   12 +
 configure.ac   |   33 +
 contrib/workdir/git-new-workdir|3 +
 refs.c |3 +
 refs.h |2 +
 refs/lmdb-backend.c| 2052 
 setup.c|   11 +-
 test-refs-lmdb-backend.c   |   64 +
 transport.c|7 +-
 15 files changed, 2250 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/technical/refs-lmdb-backend.txt
 create mode 100644 refs/lmdb-backend.c
 create mode 100644 test-refs-lmdb-backend.c

diff --git a/.gitignore b/.gitignore
index 1c2f832..87d45a2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -199,6 +199,7 @@
 /test-path-utils
 /test-prio-queue
 /test-read-cache
+/test-refs-lmdb-backend
 /test-regex
 /test-revision-walking
 /test-run-command
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 06d3659..bc222c9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1153,6 +1153,13 @@ difftool..cmd::
 difftool.prompt::
Prompt before each invocation of the diff tool.
 
+extensions.refsStorage::
+   Type of refs storage backend. Default is to use the original
+   files based ref storage.  When set to "lmdb", refs are stored in
+   the lmdb database backend.  This setting reflects the refs
+   backend that is currently in use; it is not possible to change
+   the backend by changing this setting.
+
 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-clone.txt b/Documentation/git-clone.txt
index 68f56a7..b9c52ce 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -227,7 +227,8 @@ objects from the source repository into a pack in the 
cloned repository.
 
 --ref-storage=::
Type of ref storage backend. Default is to use the original files
-   based ref storage.
+   based ref storage. Set to "lmdb" to store refs in the lmdb database
+   backend.
 
 ::
The (possibly remote) repository to clone from.  See the
diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index d2b150f..3b30bf3 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -116,7 +116,7 @@ does not exist, it will be created.
 --ref-storage=::
 Type of refs storage backend. Default is to use the original "files"
 storage, which stores ref data in files in .git/refs and
-.git/packed-refs.
+.git/packed-refs. Set to "lmdb" to activate the lmdb storage backend.
 
 TEMPLATE DIRECTORY
 --
diff --git a/Documentation/technical/refs-lmdb-backend.txt 
b/Documentation/technical/refs-lmdb-backend.txt
new file mode 100644
index 000..d2cddb6
--- /dev/null
+++ b/Documentation/technical/refs-lmdb-backend.txt
@@ -0,0 +1,52 @@
+Notes on the LMDB refs backend
+==
+

[PATCH v4 19/21] refs: add register_ref_storage_backends()

[David Turner]

This new function will register all known ref storage backends... once
there are any other than the default.  For now, it's a no-op.

Signed-off-by: David Turner 
---
 builtin/init-db.c |  3 +++
 config.c  | 25 +
 refs.c|  8 
 refs.h|  2 ++
 4 files changed, 38 insertions(+)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index d331ce8..4209b67 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -226,6 +226,7 @@ static int create_default_files(const char *template_path)
if (strcmp(ref_storage_backend, "files")) {
git_config_set("extensions.refStorage", ref_storage_backend);
git_config_set("core.repositoryformatversion", 
ref_storage_backend);
+   register_ref_storage_backends();
if (set_ref_storage_backend(ref_storage_backend))
die(_("Unknown ref storage backend %s"),
ref_storage_backend);
@@ -503,6 +504,8 @@ int cmd_init_db(int argc, const char **argv, const char 
*prefix)
 
argc = parse_options(argc, argv, prefix, init_db_options, 
init_db_usage, 0);
 
+   register_ref_storage_backends();
+
if (requested_ref_storage_backend &&
!ref_storage_backend_exists(requested_ref_storage_backend))
die(_("Unknown ref storage backend %s"),
diff --git a/config.c b/config.c
index b95ac3a..b9ef223 100644
--- a/config.c
+++ b/config.c
@@ -11,6 +11,7 @@
 #include "strbuf.h"
 #include "quote.h"
 #include "hashmap.h"
+#include "refs.h"
 #include "string-list.h"
 #include "utf8.h"
 
@@ -1207,6 +1208,30 @@ int git_config_early(config_fn_t fn, void *data, const 
char *repo_config)
}
 
if (repo_config && !access_or_die(repo_config, R_OK, 0)) {
+   char *storage = NULL;
+
+   /*
+* make sure we always read the ref storage config
+* from the extensions section on startup
+*/
+   ret += git_config_from_file(ref_storage_backend_config,
+   repo_config, );
+
+   register_ref_storage_backends();
+   if (!storage)
+   storage = xstrdup("");
+
+   if ((!*storage) ||
+   (!strcmp(storage, "files"))) {
+   /* default backend, nothing to do */
+   free(storage);
+   } else {
+   ref_storage_backend = storage;
+   if (set_ref_storage_backend(ref_storage_backend))
+   die(_("Unknown ref storage backend %s"),
+   ref_storage_backend);
+   }
+
ret += git_config_from_file(fn, repo_config, data);
found += 1;
}
diff --git a/refs.c b/refs.c
index 715a492..e50cca0 100644
--- a/refs.c
+++ b/refs.c
@@ -1554,3 +1554,11 @@ done:
string_list_clear(_refnames, 0);
return ret;
 }
+
+void register_ref_storage_backends(void) {
+   /*
+* No need to register the files backend; it's registered by
+* default. Add register_ref_storage_backend(ptr-to-backend)
+* entries below when you add a new backend.
+*/
+}
diff --git a/refs.h b/refs.h
index 680a535..81aab6b 100644
--- a/refs.h
+++ b/refs.h
@@ -525,4 +525,6 @@ int ref_storage_backend_exists(const char *name);
 
 void register_ref_storage_backend(struct ref_storage_be *be);
 
+void register_ref_storage_backends(void);
+
 #endif /* REFS_H */
-- 
2.4.2.749.g730654d-twtrsrc

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

[PATCH v4 11/21] refs: move duplicate check to common code

[David Turner]

The check for duplicate refnames in a transaction is needed for
all backends, so move it to the common code.

ref_transaction_commit_fn gains a new argument, the sorted
string_list of affected refnames.

Signed-off-by: David Turner 
---
 refs.c   | 69 ++--
 refs/files-backend.c | 57 ---
 refs/refs-internal.h |  1 +
 3 files changed, 73 insertions(+), 54 deletions(-)

diff --git a/refs.c b/refs.c
index e04fddc..283a5ec 100644
--- a/refs.c
+++ b/refs.c
@@ -1116,6 +1116,36 @@ int rename_ref_available(const char *oldname, const char 
*newname)
return ret;
 }
 
+/*
+ * Return 1 if there are any duplicate refnames in the updates in
+ * `transaction`, and fill in err with an appropriate error message.
+ * Fill in `refnames` with the refnames from the transaction.
+ */
+static int get_affected_refnames(struct ref_transaction *transaction,
+struct string_list *refnames,
+struct strbuf *err)
+{
+   int i, n = transaction->nr;
+   struct ref_update **updates;
+
+   assert(err);
+
+   updates = transaction->updates;
+   /* Fail if a refname appears more than once in the transaction: */
+   for (i = 0; i < n; i++)
+   string_list_append(refnames, updates[i]->refname);
+   string_list_sort(refnames);
+
+   for (i = 1; i < n; i++)
+   if (!strcmp(refnames->items[i - 1].string, 
refnames->items[i].string)) {
+   strbuf_addf(err,
+   "Multiple updates for ref '%s' not 
allowed.",
+   refnames->items[i].string);
+   return 1;
+   }
+   return 0;
+}
+
 /* backend functions */
 int refs_init_db(struct strbuf *err, int shared)
 {
@@ -1125,7 +1155,29 @@ int refs_init_db(struct strbuf *err, int shared)
 int ref_transaction_commit(struct ref_transaction *transaction,
   struct strbuf *err)
 {
-   return the_refs_backend->transaction_commit(transaction, err);
+   int ret = -1;
+   struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
+
+   assert(err);
+
+   if (transaction->state != REF_TRANSACTION_OPEN)
+   die("BUG: commit called for transaction that is not open");
+
+   if (!transaction->nr) {
+   transaction->state = REF_TRANSACTION_CLOSED;
+   return 0;
+   }
+
+   if (get_affected_refnames(transaction, _refnames, err)) {
+   ret = TRANSACTION_GENERIC_ERROR;
+   goto done;
+   }
+
+   ret = the_refs_backend->transaction_commit(transaction,
+  _refnames, err);
+done:
+   string_list_clear(_refnames, 0);
+   return ret;
 }
 
 int delete_refs(struct string_list *refnames)
@@ -1277,5 +1329,18 @@ int reflog_expire(const char *refname, const unsigned 
char *sha1,
 int initial_ref_transaction_commit(struct ref_transaction *transaction,
   struct strbuf *err)
 {
-   return the_refs_backend->initial_transaction_commit(transaction, err);
+   struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
+   int ret;
+
+   if (get_affected_refnames(transaction,
+ _refnames, err)) {
+   ret = TRANSACTION_GENERIC_ERROR;
+   goto done;
+   }
+   ret = the_refs_backend->initial_transaction_commit(transaction,
+  _refnames,
+  err);
+done:
+   string_list_clear(_refnames, 0);
+   return ret;
 }
diff --git a/refs/files-backend.c b/refs/files-backend.c
index aa11ba2..0ad60bc 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3154,24 +3154,8 @@ static int files_for_each_reflog(each_ref_fn fn, void 
*cb_data)
return retval;
 }
 
-static int ref_update_reject_duplicates(struct string_list *refnames,
-   struct strbuf *err)
-{
-   int i, n = refnames->nr;
-
-   assert(err);
-
-   for (i = 1; i < n; i++)
-   if (!strcmp(refnames->items[i - 1].string, 
refnames->items[i].string)) {
-   strbuf_addf(err,
-   "Multiple updates for ref '%s' not 
allowed.",
-   refnames->items[i].string);
-   return 1;
-   }
-   return 0;
-}
-
 static int files_transaction_commit(struct ref_transaction *transaction,
+   struct string_list *affected_refnames,
struct strbuf *err)
 {
int ret = 0, i;
@@ -3179,26 +3163,6 @@ static int files_transaction_commit(struct 
ref_transaction *transaction,
struct ref_update 

Re: [PATCHv8 1/9] submodule-config: keep update strategy around

[Stefan Beller]

On Fri, Feb 5, 2016 at 12:33 PM, Jonathan Nieder  wrote:
> Stefan Beller wrote:
>> On Thu, Feb 4, 2016 at 4:59 PM, Jonathan Nieder  wrote:
>>> Stefan Beller wrote:
>
 +++ b/submodule-config.h
 @@ -14,6 +14,7 @@ struct submodule {
 + const char *update;
>>>
>>> gitmodules(5) tells me the only allowed values are checkout, rebase,
>>> merge, and none.  I wouldn't know at a glance how to match against
>>> those in calling code.  Can this be an enum?
>>
>> "Note that the !command form is intentionally ignored here for
>> security reasons."
>>
>> However you can overwrite the update field in .git/config to be "! [foo]",
>> which we also need to support, so let's keep it a string for now?
>
> I had forgotten about "!command".  I think making it a string makes
> the field hard to use: how do I determine whether it was checkout,
> rebase, merge, custom, or none?  Are they case-sensitive?  Am I
> supposed to strip whitespace?
>
> One possibility would be to have an enum and a string:
>
> enum submodule_update_type update;
> const char *update_command;

ok, that makes sense.

>
> Thanks,
> Jonathan
--
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

no luck with colors for branch names in gitk yet

[Britton Kerin]

Someone suggested using color.branch.upstream, I tried like this and variants

[color "branch"]
  local = red bold
  upstream = red bold

Doesn't seem to matter what I put in for upstream, including invalid
colors, gitk just ignores it and does the dark green for local
branches
--
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: [PATCHv8 1/9] submodule-config: keep update strategy around

[Jonathan Nieder]

Stefan Beller wrote:
> On Thu, Feb 4, 2016 at 4:59 PM, Jonathan Nieder  wrote:
>> Stefan Beller wrote:

>>> +++ b/submodule-config.h
>>> @@ -14,6 +14,7 @@ struct submodule {
>>> + const char *update;
>>
>> gitmodules(5) tells me the only allowed values are checkout, rebase,
>> merge, and none.  I wouldn't know at a glance how to match against
>> those in calling code.  Can this be an enum?
>
> "Note that the !command form is intentionally ignored here for
> security reasons."
>
> However you can overwrite the update field in .git/config to be "! [foo]",
> which we also need to support, so let's keep it a string for now?

I had forgotten about "!command".  I think making it a string makes
the field hard to use: how do I determine whether it was checkout,
rebase, merge, custom, or none?  Are they case-sensitive?  Am I
supposed to strip whitespace?

One possibility would be to have an enum and a string:

enum submodule_update_type update;
const char *update_command;

Thanks,
Jonathan
--
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 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

It used to be that:

   git config --global user.email "(none)"

was a viable way for people to force themselves to set user.email in
each repository.  This was helpful for people with more than one
email address, targeting different email addresses for different
clones, as it barred git from creating commit unless the user.email
config was set in the per-repo config to the correct email address.

A recent change, 19ce497c (ident: keep a flag for bogus
default_email, 2015-12-10), however declared that an explicitly
configured user.email is not bogus, no matter what its value is, so
this hack no longer works.

Provide the same functionality by adding a new configuration
variable user.useConfigOnly; when this variable is set, the
user must explicitly set user.email configuration.

Signed-off-by: Dan Aloni 
Helped-by: Jeff King 
Signed-off-by: Junio C Hamano 
Cc: Eric Sunshine 
---
 Documentation/config.txt  | 10 +
 ident.c   | 16 ++
 t/t9904-per-repo-email.sh | 55 +++
 3 files changed, 81 insertions(+)
 create mode 100755 t/t9904-per-repo-email.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 02bcde6bb596..0d168d92fd79 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2821,6 +2821,16 @@ user.name::
Can be overridden by the 'GIT_AUTHOR_NAME' and 'GIT_COMMITTER_NAME'
environment variables.  See linkgit:git-commit-tree[1].
 
+user.useConfigOnly::
+   This instructs Git to avoid trying to guess defaults for 'user.email'
+   and 'user.name' other than strictly from config. For example, if
+   you have multiple email addresses and would like to use a different
+   one for each repository, then with this configuration option set
+   to `true` in the global config along with a name, Git would prompt
+   for you for setting up an email upon making new commits in a newly
+   cloned repository.
+   Defaults to `false`.
+
 user.signingKey::
If linkgit:git-tag[1] or linkgit:git-commit[1] is not selecting the
key you want it to automatically when creating a signed tag or
diff --git a/ident.c b/ident.c
index f3a431f738cc..6e125821f056 100644
--- a/ident.c
+++ b/ident.c
@@ -13,11 +13,14 @@ static struct strbuf git_default_date = STRBUF_INIT;
 static int default_email_is_bogus;
 static int default_name_is_bogus;
 
+static int ident_use_config_only;
+
 #define IDENT_NAME_GIVEN 01
 #define IDENT_MAIL_GIVEN 02
 #define IDENT_ALL_GIVEN (IDENT_NAME_GIVEN|IDENT_MAIL_GIVEN)
 static int committer_ident_explicitly_given;
 static int author_ident_explicitly_given;
+static int ident_config_given;
 
 #ifdef NO_GECOS_IN_PWENT
 #define get_gecos(ignored) "&"
@@ -354,6 +357,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect name (got '%s')", 
name);
}
+   if (strict && ident_use_config_only
+   && !(ident_config_given & IDENT_NAME_GIVEN))
+   die("user.useConfigOnly set but no name given");
}
if (!*name) {
struct passwd *pw;
@@ -373,6 +379,9 @@ const char *fmt_ident(const char *name, const char *email,
fputs(env_hint, stderr);
die("unable to auto-detect email address (got '%s')", 
email);
}
+   if (strict && ident_use_config_only
+   && !(ident_config_given & IDENT_MAIL_GIVEN))
+   die("user.useConfigOnly set but no mail given");
}
 
strbuf_reset();
@@ -446,6 +455,11 @@ int author_ident_sufficiently_given(void)
 
 int git_ident_config(const char *var, const char *value, void *data)
 {
+   if (!strcmp(var, "user.useconfigonly")) {
+   ident_use_config_only = git_config_bool(var, value);
+   return 0;
+   }
+
if (!strcmp(var, "user.name")) {
if (!value)
return config_error_nonbool(var);
@@ -453,6 +467,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_name, value);
committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
author_ident_explicitly_given |= IDENT_NAME_GIVEN;
+   ident_config_given |= IDENT_NAME_GIVEN;
return 0;
}
 
@@ -463,6 +478,7 @@ int git_ident_config(const char *var, const char *value, 
void *data)
strbuf_addstr(_default_email, value);
committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
+   ident_config_given |= IDENT_MAIL_GIVEN;
 

[PATCH v7] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

Changes between v6 -> v7:

* Dropped patch: ident: cleanup wrt ident's source
* Revised the documentation of the feature according to comments.
* Revised the test according to comments.
* Styling fix.

v6: http://article.gmane.org/gmane.comp.version-control.git/285550
--
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 1/2] fmt_ident: refactor strictness checks

[Dan Aloni]

From: Jeff King 

This function has evolved quite a bit over time, and as a
result, the logic for "is this an OK ident" has been
sprinkled throughout. This ends up with a lot of redundant
conditionals, like checking want_name repeatedly. Worse,
we want to know in many cases whether we are using the
"default" ident, and we do so by comparing directly to the
global strbuf, which violates the abstraction of the
ident_default_* functions.

Let's reorganize the function into a hierarchy of
conditionals to handle similar cases together. The only
case that doesn't just work naturally for this is that of an
empty name, where our advice is different based on whether
we came from ident_default_name() or not. We can use a
simple flag to cover this case.

Signed-off-by: Jeff King 
---
 ident.c | 46 --
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/ident.c b/ident.c
index 3da555634290..f3a431f738cc 100644
--- a/ident.c
+++ b/ident.c
@@ -345,32 +345,34 @@ const char *fmt_ident(const char *name, const char *email,
int want_date = !(flag & IDENT_NO_DATE);
int want_name = !(flag & IDENT_NO_NAME);
 
-   if (want_name && !name)
-   name = ident_default_name();
-   if (!email)
-   email = ident_default_email();
-
-   if (want_name && !*name) {
-   struct passwd *pw;
-
-   if (strict) {
-   if (name == git_default_name.buf)
+   if (want_name) {
+   int using_default = 0;
+   if (!name) {
+   name = ident_default_name();
+   using_default = 1;
+   if (strict && default_name_is_bogus) {
fputs(env_hint, stderr);
-   die("empty ident name (for <%s>) not allowed", email);
+   die("unable to auto-detect name (got '%s')", 
name);
+   }
+   }
+   if (!*name) {
+   struct passwd *pw;
+   if (strict) {
+   if (using_default)
+   fputs(env_hint, stderr);
+   die("empty ident name (for <%s>) not allowed", 
email);
+   }
+   pw = xgetpwuid_self(NULL);
+   name = pw->pw_name;
}
-   pw = xgetpwuid_self(NULL);
-   name = pw->pw_name;
-   }
-
-   if (want_name && strict &&
-   name == git_default_name.buf && default_name_is_bogus) {
-   fputs(env_hint, stderr);
-   die("unable to auto-detect name (got '%s')", name);
}
 
-   if (strict && email == git_default_email.buf && default_email_is_bogus) 
{
-   fputs(env_hint, stderr);
-   die("unable to auto-detect email address (got '%s')", email);
+   if (!email) {
+   email = ident_default_email();
+   if (strict && default_email_is_bogus) {
+   fputs(env_hint, stderr);
+   die("unable to auto-detect email address (got '%s')", 
email);
+   }
}
 
strbuf_reset();
-- 
2.5.0

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

Re: [PATCH v6 3/3] ident: cleanup wrt ident's source

[Dan Aloni]

On Fri, Feb 05, 2016 at 02:24:13PM -0500, Jeff King wrote:
> On Fri, Feb 05, 2016 at 11:05:19AM -0800, Junio C Hamano wrote:
> 
> > Dan Aloni  writes:
> > 
> > > This change condenses the variables that tells where we got the user's
> > > ident into single enum, instead of a collection of booleans.
> > >
> > > In addtion, also have {committer,author}_ident_sufficiently_given
> > > directly probe the environment and the afformentioned enum instead of
> > > relying on git_{committer,author}_info to do so.
> > >
> > > Signed-off-by: Dan Aloni 
> > > Helped-by: Jeff King 
> > > Helped-by: Junio C Hamano 
> > > ---
> > >  ident.c | 126 
> > > 
> > >  1 file changed, 80 insertions(+), 46 deletions(-)
> > 
> > Peff what do you think?  I am asking you because personally I do not
> > find this particularly easier to read than the original, but since
> > you stared at the code around here recently much longer than I did
> > when doing the 1/3, I thought you may be a better judge than I am.
> 
> I'm not sure it is really worth it unless we are going to expose this to
> the user, and let them say "I am OK with IDENT_SOURCE_GUESSED, but not
> IDENT_SOURCE_GUESSED_BOGUS" or similar.
> 
> Without that, I think it is probably just making things a bit more
> brittle.

Okay, will drop it now.

-- 
Dan Aloni
--
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-curl: don't fall back to Basic auth if we haven't tried Negotiate

[Junio C Hamano]

"brian m. carlson"  writes:

> On Fri, Feb 05, 2016 at 12:18:22PM +0300, Dmitry Vilkov wrote:
>> You are right, we are using a bare URL (without a username component).
>> With username encoded in URL everything works just fine. But it's
>> generally wrong to pass creds in URL (in my opinion) and security
>> policy of my employer prohibits doing such thing.
>> Anyway, as you said libcurl needs valid (not NULL) username/password
>> to do GSS-Negotiate, so there is nothing wrong if I set empty
>> username/password combination when git prompts for creds. Even more,
>> there is no other way to let libcurl to use GSS-Negotiate without
>> username in URL.
>
> You can literally do https://:@git.crustytoothpaste.net/git/repo.git as
> the URL, and that will work.  GSS-Negotiate using Kerberos passes the
> ticket, which contains the principal name in it, so an actual username
> and password is not needed at all.  libcurl just needs something to tell
> it to do authentication.

Hmph, so documenting that :@
as a supported way might be an ugly-looking solution to the original
problem.  A less ugly-looking solution might be a boolean that can
be set per URL (we already have urlmatch-config infrastructure to
help us do so) to tell us to pass the empty credential to lubCurl,
bypassing the step to ask the user for password that we do not use.

The end-result of either of these solution would strictly be better
than the patch we discussed in that the end user will not have to
interact with the prompt at all, right?


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

Re: [PATCH] remote-curl: don't fall back to Basic auth if we haven't tried Negotiate

[brian m. carlson]

On Fri, Feb 05, 2016 at 01:02:58PM -0800, Junio C Hamano wrote:
> Hmph, so documenting that :@
> as a supported way might be an ugly-looking solution to the original
> problem.  A less ugly-looking solution might be a boolean that can
> be set per URL (we already have urlmatch-config infrastructure to
> help us do so) to tell us to pass the empty credential to lubCurl,
> bypassing the step to ask the user for password that we do not use.
> 
> The end-result of either of these solution would strictly be better
> than the patch we discussed in that the end user will not have to
> interact with the prompt at all, right?

Yes, that's true.  I'll try to come up with a patch this weekend that
implements that (maybe remote.forceAuth = true or somesuch).
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: PGP signature

[PATCH v4 09/21] refs: add method to rename refs

[David Turner]

Signed-off-by: David Turner 
---
 refs.c   | 5 +
 refs/files-backend.c | 4 +++-
 refs/refs-internal.h | 9 +
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index 7758bdc..e04fddc 100644
--- a/refs.c
+++ b/refs.c
@@ -1133,6 +1133,11 @@ int delete_refs(struct string_list *refnames)
return the_refs_backend->delete_refs(refnames);
 }
 
+int rename_ref(const char *oldref, const char *newref, const char *logmsg)
+{
+   return the_refs_backend->rename_ref(oldref, newref, logmsg);
+}
+
 const char *resolve_ref_unsafe(const char *ref, int resolve_flags,
   unsigned char *sha1, int *flags)
 {
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 45da091..685fc6f 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2502,7 +2502,8 @@ static int commit_ref_update(struct ref_lock *lock,
 const unsigned char *sha1, const char *logmsg,
 int flags, struct strbuf *err);
 
-int rename_ref(const char *oldrefname, const char *newrefname, const char 
*logmsg)
+static int files_rename_ref(const char *oldrefname, const char *newrefname,
+   const char *logmsg)
 {
unsigned char sha1[20], orig_sha1[20];
int flag = 0, logmoved = 0;
@@ -3592,6 +3593,7 @@ struct ref_storage_be refs_be_files = {
files_peel_ref,
files_create_symref,
files_delete_refs,
+   files_rename_ref,
 
files_resolve_ref_unsafe,
files_verify_refname_available,
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 5768fee..15fa99c 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -67,6 +67,13 @@ int do_for_each_per_worktree_ref(const char *submodule, 
const char *base,
 each_ref_fn fn, int trim, int flags,
 void *cb_data);
 
+/*
+ * Check if the new name does not conflict with any existing refs
+ * (other than possibly the old ref).  Return 0 if the ref can be
+ * renamed to the new name.
+ */
+int rename_ref_available(const char *oldname, const char *newname);
+
 enum peel_status {
/* object was peeled successfully: */
PEEL_PEELED = 0,
@@ -246,6 +253,7 @@ typedef const char *resolve_ref_unsafe_fn(const char *ref,
 typedef int verify_refname_available_fn(const char *refname, struct 
string_list *extra, struct string_list *skip, struct strbuf *err);
 typedef int resolve_gitlink_ref_fn(const char *path, const char *refname,
   unsigned char *sha1);
+typedef int rename_ref_fn(const char *oldref, const char *newref, const char 
*logmsg);
 
 /* iteration methods */
 typedef int head_ref_fn(each_ref_fn fn, void *cb_data);
@@ -284,6 +292,7 @@ struct ref_storage_be {
peel_ref_fn *peel_ref;
create_symref_fn *create_symref;
delete_refs_fn *delete_refs;
+   rename_ref_fn *rename_ref;
 
resolve_ref_unsafe_fn *resolve_ref_unsafe;
verify_refname_available_fn *verify_refname_available;
-- 
2.4.2.749.g730654d-twtrsrc

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

[PATCH v4 02/21] refs: add methods for misc ref operations

[David Turner]

From: Ronnie Sahlberg 

Add ref backend methods for:
resolve_ref_unsafe, verify_refname_available, pack_refs, peel_ref,
create_symref, resolve_gitlink_ref.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
---
 builtin/init-db.c|  1 +
 refs.c   | 36 
 refs/files-backend.c | 33 +++--
 refs/refs-internal.h | 23 +++
 4 files changed, 83 insertions(+), 10 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 07229d6..26e1cc3 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -8,6 +8,7 @@
 #include "builtin.h"
 #include "exec_cmd.h"
 #include "parse-options.h"
+#include "refs.h"
 
 #ifndef DEFAULT_GIT_TEMPLATE_DIR
 #define DEFAULT_GIT_TEMPLATE_DIR "/usr/share/git-core/templates"
diff --git a/refs.c b/refs.c
index 1c646f5..afdde7d 100644
--- a/refs.c
+++ b/refs.c
@@ -1122,3 +1122,39 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
 {
return the_refs_backend->transaction_commit(transaction, err);
 }
+
+const char *resolve_ref_unsafe(const char *ref, int resolve_flags,
+  unsigned char *sha1, int *flags)
+{
+   return the_refs_backend->resolve_ref_unsafe(ref, resolve_flags, sha1,
+   flags);
+}
+
+int verify_refname_available(const char *refname, struct string_list *extra,
+struct string_list *skip, struct strbuf *err)
+{
+   return the_refs_backend->verify_refname_available(refname, extra, skip, 
err);
+}
+
+int pack_refs(unsigned int flags)
+{
+   return the_refs_backend->pack_refs(flags);
+}
+
+int peel_ref(const char *refname, unsigned char *sha1)
+{
+   return the_refs_backend->peel_ref(refname, sha1);
+}
+
+int create_symref(const char *ref_target, const char *refs_heads_master,
+ const char *logmsg)
+{
+   return the_refs_backend->create_symref(ref_target, refs_heads_master,
+  logmsg);
+}
+
+int resolve_gitlink_ref(const char *path, const char *refname,
+   unsigned char *sha1)
+{
+   return the_refs_backend->resolve_gitlink_ref(path, refname, sha1);
+}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 2a73564..882d830 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1343,7 +1343,8 @@ static int resolve_gitlink_ref_recursive(struct ref_cache 
*refs,
return resolve_gitlink_ref_recursive(refs, p, sha1, recursion+1);
 }
 
-int resolve_gitlink_ref(const char *path, const char *refname, unsigned char 
*sha1)
+static int files_resolve_gitlink_ref(const char *path, const char *refname,
+unsigned char *sha1)
 {
int len = strlen(path), retval;
struct strbuf submodule = STRBUF_INIT;
@@ -1583,8 +1584,10 @@ static const char *resolve_ref_1(const char *refname,
}
 }
 
-const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
-  unsigned char *sha1, int *flags)
+static const char *files_resolve_ref_unsafe(const char *refname,
+   int resolve_flags,
+   unsigned char *sha1,
+   int *flags)
 {
static struct strbuf sb_refname = STRBUF_INIT;
struct strbuf sb_contents = STRBUF_INIT;
@@ -1633,7 +1636,7 @@ static enum peel_status peel_entry(struct ref_entry 
*entry, int repeel)
return status;
 }
 
-int peel_ref(const char *refname, unsigned char *sha1)
+static int files_peel_ref(const char *refname, unsigned char *sha1)
 {
int flag;
unsigned char base[20];
@@ -2270,7 +2273,7 @@ static void prune_refs(struct ref_to_prune *r)
}
 }
 
-int pack_refs(unsigned int flags)
+static int files_pack_refs(unsigned int flags)
 {
struct pack_refs_cb_data cbdata;
 
@@ -2461,10 +2464,10 @@ out:
return ret;
 }
 
-int verify_refname_available(const char *newname,
-struct string_list *extras,
-struct string_list *skip,
-struct strbuf *err)
+static int files_verify_refname_available(const char *newname,
+ struct string_list *extras,
+ struct string_list *skip,
+ struct strbuf *err)
 {
struct ref_dir *packed_refs = get_packed_refs(_cache);
struct ref_dir *loose_refs = get_loose_refs(_cache);
@@ -2884,7 +2887,9 @@ static int create_symref_locked(struct ref_lock *lock, 
const char *refname,
return 0;
 }
 
-int create_symref(const char *refname, const char *target, const char *logmsg)
+static int files_create_symref(const char *refname,
+  const char 

[PATCH v4 13/21] refs: resolve symbolic refs first

[David Turner]

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

It is still possible to confuse git by concurrent updates, since the
splitting of symbolic refs does not happen under lock. So a symbolic ref
could be replaced by a plain ref in the middle of this operation, which
would lead to reflog discontinuities and missed old-ref checks.

Signed-off-by: David Turner 
---
 refs.c   |  69 +++
 refs/files-backend.c | 132 ++-
 refs/refs-internal.h |   8 
 3 files changed, 145 insertions(+), 64 deletions(-)

diff --git a/refs.c b/refs.c
index 283a5ec..227c018 100644
--- a/refs.c
+++ b/refs.c
@@ -1152,6 +1152,71 @@ int refs_init_db(struct strbuf *err, int shared)
return the_refs_backend->init_db(err, shared);
 }
 
+/*
+ * Special case for symbolic refs when REF_NODEREF is not turned on.
+ * Dereference them here, mark them REF_LOG_ONLY, and add an update
+ * for the underlying ref.
+ */
+static int dereference_symrefs(struct ref_transaction *transaction,
+  struct strbuf *err)
+{
+   int i;
+   int nr = transaction->nr;
+
+   for (i = 0; i < nr; i++) {
+   struct ref_update *update = transaction->updates[i];
+   const char *resolved;
+   unsigned char sha1[20];
+   int resolve_flags = 0;
+   int mustexist = update->flags & REF_HAVE_OLD &&
+   !is_null_sha1(update->old_sha1);
+   int deleting = (update->flags & REF_HAVE_NEW) &&
+   is_null_sha1(update->new_sha1);
+
+   if (mustexist)
+   resolve_flags |= RESOLVE_REF_READING;
+   if (deleting)
+   resolve_flags |= RESOLVE_REF_ALLOW_BAD_NAME |
+   RESOLVE_REF_NO_RECURSE;
+
+   if (strcmp(update->refname, "HEAD"))
+   update->flags |= REF_IS_NOT_HEAD;
+
+   resolved = resolve_ref_unsafe(update->refname, resolve_flags,
+ sha1, >type);
+   if (!resolved) {
+   /*
+* We may notice this breakage later and die
+* with a sensible error message
+*/
+   update->type |= REF_ISBROKEN;
+   continue;
+   }
+
+   hashcpy(update->read_sha1, sha1);
+
+   if (update->flags & REF_NODEREF ||
+   !(update->type & REF_ISSYMREF))
+   continue;
+
+   /* Create a new transaction for the underlying ref */
+   if (ref_transaction_update(transaction,
+  resolved,
+  update->new_sha1,
+  (update->flags & REF_HAVE_OLD) ?
+  update->old_sha1 : NULL,
+  update->flags & ~REF_IS_NOT_HEAD,
+  update->msg, err))
+   return -1;
+
+   /* Make the symbolic ref update non-recursive */
+   update->flags |= REF_LOG_ONLY | REF_NODEREF;
+   update->flags &= ~REF_HAVE_OLD;
+   }
+
+   return 0;
+}
+
 int ref_transaction_commit(struct ref_transaction *transaction,
   struct strbuf *err)
 {
@@ -1168,6 +1233,10 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
return 0;
}
 
+   ret = dereference_symrefs(transaction, err);
+   if (ret)
+   goto done;
+
if (get_affected_refnames(transaction, _refnames, err)) {
ret = TRANSACTION_GENERIC_ERROR;
goto done;
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 0fdcdc7..d4f9040 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -7,7 +7,6 @@
 
 struct ref_lock {
char *ref_name;
-   char *orig_ref_name;
struct lock_file *lk;
struct object_id old_oid;
 };
@@ -1857,7 +1856,6 @@ static void unlock_ref(struct ref_lock *lock)
if (lock->lk)
rollback_lock_file(lock->lk);
free(lock->ref_name);
-   free(lock->orig_ref_name);
free(lock);
 }
 
@@ -1913,6 +1911,7 @@ static int remove_empty_directories(struct strbuf *path)
  */
 static struct ref_lock *lock_ref_sha1_basic(const char *refname,
const unsigned char *old_sha1,
+   const unsigned char *read_sha1,
const struct string_list 

[PATCH v4 08/21] refs: add methods to init refs db

[David Turner]

Alternate refs backends might not need the refs/heads directory and so
on, so we make ref db initialization part of the backend.

Signed-off-by: David Turner 
---
 builtin/init-db.c| 20 ++--
 refs.c   |  5 +
 refs.h   |  2 ++
 refs/files-backend.c | 16 
 refs/refs-internal.h |  2 ++
 5 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 26e1cc3..801e977 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -178,13 +178,7 @@ static int create_default_files(const char *template_path)
char junk[2];
int reinit;
int filemode;
-
-   /*
-* Create .git/refs/{heads,tags}
-*/
-   safe_create_dir(git_path_buf(, "refs"), 1);
-   safe_create_dir(git_path_buf(, "refs/heads"), 1);
-   safe_create_dir(git_path_buf(, "refs/tags"), 1);
+   struct strbuf err = STRBUF_INIT;
 
/* Just look for `init.templatedir` */
git_config(git_init_db_config, NULL);
@@ -208,12 +202,18 @@ static int create_default_files(const char *template_path)
 */
if (shared_repository) {
adjust_shared_perm(get_git_dir());
-   adjust_shared_perm(git_path_buf(, "refs"));
-   adjust_shared_perm(git_path_buf(, "refs/heads"));
-   adjust_shared_perm(git_path_buf(, "refs/tags"));
}
 
/*
+* We need to create a "refs" dir in any case so that older
+* versions of git can tell that this is a repository.
+*/
+   safe_create_dir(git_path("refs"), 1);
+   
+   if (refs_init_db(, shared_repository))
+   die("failed to set up refs db: %s", err.buf);
+
+   /*
 * Create the default symlink from ".git/HEAD" to the "master"
 * branch, if it does not exist yet.
 */
diff --git a/refs.c b/refs.c
index 5fbe794..7758bdc 100644
--- a/refs.c
+++ b/refs.c
@@ -1117,6 +1117,11 @@ int rename_ref_available(const char *oldname, const char 
*newname)
 }
 
 /* backend functions */
+int refs_init_db(struct strbuf *err, int shared)
+{
+   return the_refs_backend->init_db(err, shared);
+}
+
 int ref_transaction_commit(struct ref_transaction *transaction,
   struct strbuf *err)
 {
diff --git a/refs.h b/refs.h
index 5bc3fbc..c5ecc52 100644
--- a/refs.h
+++ b/refs.h
@@ -66,6 +66,8 @@ extern int ref_exists(const char *refname);
 
 extern int is_branch(const char *refname);
 
+extern int refs_init_db(struct strbuf *err, int shared);
+
 /*
  * If refname is a non-symbolic reference that refers to a tag object,
  * and the tag can be (recursively) dereferenced to a non-tag object,
diff --git a/refs/files-backend.c b/refs/files-backend.c
index e86849c..45da091 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3558,9 +3558,25 @@ static int files_reflog_expire(const char *refname, 
const unsigned char *sha1,
return -1;
 }
 
+static int files_init_db(struct strbuf *err, int shared)
+{
+   /*
+* Create .git/refs/{heads,tags}
+*/
+   safe_create_dir(git_path("refs/heads"), 1);
+   safe_create_dir(git_path("refs/tags"), 1);
+   if (shared) {
+   adjust_shared_perm(git_path("refs"));
+   adjust_shared_perm(git_path("refs/heads"));
+   adjust_shared_perm(git_path("refs/tags"));
+   }
+   return 0;
+}
+
 struct ref_storage_be refs_be_files = {
NULL,
"files",
+   files_init_db,
files_transaction_commit,
files_initial_transaction_commit,
 
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 1d9e5e0..5768fee 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -208,6 +208,7 @@ const char *find_descendant_ref(const char *dirname,
 int rename_ref_available(const char *oldname, const char *newname);
 
 /* refs backends */
+typedef int ref_init_db_fn(struct strbuf *err, int shared);
 typedef int ref_transaction_commit_fn(struct ref_transaction *transaction,
  struct strbuf *err);
 
@@ -267,6 +268,7 @@ typedef int for_each_replace_ref_fn(each_ref_fn fn, void 
*cb_data);
 struct ref_storage_be {
struct ref_storage_be *next;
const char *name;
+   ref_init_db_fn *init_db;
ref_transaction_commit_fn *transaction_commit;
ref_transaction_commit_fn *initial_transaction_commit;
 
-- 
2.4.2.749.g730654d-twtrsrc

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

[PATCH v4 06/21] refs: add method for initial ref transaction commit

[David Turner]

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
---
 refs.c   | 6 ++
 refs/files-backend.c | 5 +++--
 refs/refs-internal.h | 1 +
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 3254378..d481a94 100644
--- a/refs.c
+++ b/refs.c
@@ -1258,3 +1258,9 @@ int reflog_expire(const char *refname, const unsigned 
char *sha1,
   prepare_fn, should_prune_fn,
   cleanup_fn, policy_cb_data);
 }
+
+int initial_ref_transaction_commit(struct ref_transaction *transaction,
+  struct strbuf *err)
+{
+   return the_refs_backend->initial_transaction_commit(transaction, err);
+}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index b3372e6..723127e 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3337,8 +3337,8 @@ static int ref_present(const char *refname,
return string_list_has_string(affected_refnames, refname);
 }
 
-int initial_ref_transaction_commit(struct ref_transaction *transaction,
-  struct strbuf *err)
+static int files_initial_transaction_commit(struct ref_transaction 
*transaction,
+   struct strbuf *err)
 {
int ret = 0, i;
int n = transaction->nr;
@@ -3562,6 +3562,7 @@ struct ref_storage_be refs_be_files = {
NULL,
"files",
files_transaction_commit,
+   files_initial_transaction_commit,
 
files_for_each_reflog_ent,
files_for_each_reflog_ent_reverse,
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index ee2bea6..142c663 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -267,6 +267,7 @@ struct ref_storage_be {
struct ref_storage_be *next;
const char *name;
ref_transaction_commit_fn *transaction_commit;
+   ref_transaction_commit_fn *initial_transaction_commit;
 
for_each_reflog_ent_fn *for_each_reflog_ent;
for_each_reflog_ent_reverse_fn *for_each_reflog_ent_reverse;
-- 
2.4.2.749.g730654d-twtrsrc

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

[PATCH v4 05/21] refs: add methods for reflog

[David Turner]

In the file-based backend, the reflog piggybacks on the ref lock.
Since other backends won't have the same sort of ref lock, ref backends
must also handle reflogs.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
---
 refs.c   | 46 ++
 refs/files-backend.c | 36 
 refs/refs-internal.h | 27 +++
 3 files changed, 97 insertions(+), 12 deletions(-)

diff --git a/refs.c b/refs.c
index e598b73..3254378 100644
--- a/refs.c
+++ b/refs.c
@@ -1212,3 +1212,49 @@ int for_each_replace_ref(each_ref_fn fn, void *cb_data)
 {
return the_refs_backend->for_each_replace_ref(fn, cb_data);
 }
+
+int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn,
+   void *cb_data)
+{
+   return the_refs_backend->for_each_reflog_ent_reverse(refname, fn,
+cb_data);
+}
+
+int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn,
+   void *cb_data)
+{
+   return the_refs_backend->for_each_reflog_ent(refname, fn, cb_data);
+}
+
+int for_each_reflog(each_ref_fn fn, void *cb_data)
+{
+   return the_refs_backend->for_each_reflog(fn, cb_data);
+}
+
+int reflog_exists(const char *refname)
+{
+   return the_refs_backend->reflog_exists(refname);
+}
+
+int safe_create_reflog(const char *refname, int force_create,
+  struct strbuf *err)
+{
+   return the_refs_backend->create_reflog(refname, force_create, err);
+}
+
+int delete_reflog(const char *refname)
+{
+   return the_refs_backend->delete_reflog(refname);
+}
+
+int reflog_expire(const char *refname, const unsigned char *sha1,
+ unsigned int flags,
+ reflog_expiry_prepare_fn prepare_fn,
+ reflog_expiry_should_prune_fn should_prune_fn,
+ reflog_expiry_cleanup_fn cleanup_fn,
+ void *policy_cb_data)
+{
+   return the_refs_backend->reflog_expire(refname, sha1, flags,
+  prepare_fn, should_prune_fn,
+  cleanup_fn, policy_cb_data);
+}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 3d678ad..b3372e6 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2667,7 +2667,8 @@ static int log_ref_setup(const char *refname, struct 
strbuf *logfile, struct str
 }
 
 
-int safe_create_reflog(const char *refname, int force_create, struct strbuf 
*err)
+static int files_create_reflog(const char *refname, int force_create,
+  struct strbuf *err)
 {
int ret;
struct strbuf sb = STRBUF_INIT;
@@ -2923,7 +2924,7 @@ static int files_create_symref(const char *refname,
return ret;
 }
 
-int reflog_exists(const char *refname)
+static int files_reflog_exists(const char *refname)
 {
struct stat st;
 
@@ -2931,7 +2932,7 @@ int reflog_exists(const char *refname)
S_ISREG(st.st_mode);
 }
 
-int delete_reflog(const char *refname)
+static int files_delete_reflog(const char *refname)
 {
return remove_path(git_path("logs/%s", refname));
 }
@@ -2975,7 +2976,9 @@ static char *find_beginning_of_line(char *bob, char *scan)
return scan;
 }
 
-int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, 
void *cb_data)
+static int files_for_each_reflog_ent_reverse(const char *refname,
+each_reflog_ent_fn fn,
+void *cb_data)
 {
struct strbuf sb = STRBUF_INIT;
FILE *logfp;
@@ -3077,7 +3080,8 @@ int for_each_reflog_ent_reverse(const char *refname, 
each_reflog_ent_fn fn, void
return ret;
 }
 
-int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void 
*cb_data)
+static int files_for_each_reflog_ent(const char *refname,
+each_reflog_ent_fn fn, void *cb_data)
 {
FILE *logfp;
struct strbuf sb = STRBUF_INIT;
@@ -3139,7 +3143,7 @@ static int do_for_each_reflog(struct strbuf *name, 
each_ref_fn fn, void *cb_data
return retval;
 }
 
-int for_each_reflog(each_ref_fn fn, void *cb_data)
+static int files_for_each_reflog(each_ref_fn fn, void *cb_data)
 {
int retval;
struct strbuf name;
@@ -3449,12 +3453,12 @@ static int expire_reflog_ent(unsigned char *osha1, 
unsigned char *nsha1,
return 0;
 }
 
-int reflog_expire(const char *refname, const unsigned char *sha1,
-unsigned int flags,
-reflog_expiry_prepare_fn prepare_fn,
-reflog_expiry_should_prune_fn should_prune_fn,
-reflog_expiry_cleanup_fn cleanup_fn,
-void *policy_cb_data)
+static int files_reflog_expire(const char *refname, const unsigned char *sha1,
+ 

[PATCH v4 01/21] refs: add a backend method structure with transaction functions

[David Turner]

From: Ronnie Sahlberg 

Add a ref structure for storage backend methods. Start by adding a
method pointer for the transaction commit function.

Add a function set_refs_backend to switch between storage
backends. The files based storage backend is the default.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
---
 refs.c   | 40 
 refs.h   |  7 +++
 refs/files-backend.c | 10 --
 refs/refs-internal.h | 12 
 4 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index e2d34b2..1c646f5 100644
--- a/refs.c
+++ b/refs.c
@@ -10,6 +10,39 @@
 #include "tag.h"
 
 /*
+ * We always have a files backend and it is the default.
+ */
+static struct ref_storage_be *the_refs_backend = _be_files;
+/*
+ * List of all available backends
+ */
+static struct ref_storage_be *refs_backends = _be_files;
+
+int set_ref_storage_backend(const char *name)
+{
+   struct ref_storage_be *be;
+
+   for (be = refs_backends; be; be = be->next)
+   if (!strcmp(be->name, name)) {
+   the_refs_backend = be;
+   return 0;
+   }
+   return 1;
+}
+
+int ref_storage_backend_exists(const char *name)
+{
+   struct ref_storage_be *be;
+
+   for (be = refs_backends; be; be = be->next)
+   if (!strcmp(be->name, name)) {
+   the_refs_backend = be;
+   return 1;
+   }
+   return 0;
+}
+
+/*
  * How to handle various characters in refnames:
  * 0: An acceptable character for refs
  * 1: End-of-component
@@ -1082,3 +1115,10 @@ int rename_ref_available(const char *oldname, const char 
*newname)
strbuf_release();
return ret;
 }
+
+/* backend functions */
+int ref_transaction_commit(struct ref_transaction *transaction,
+  struct strbuf *err)
+{
+   return the_refs_backend->transaction_commit(transaction, err);
+}
diff --git a/refs.h b/refs.h
index 3c3da29..5bc3fbc 100644
--- a/refs.h
+++ b/refs.h
@@ -508,4 +508,11 @@ extern int reflog_expire(const char *refname, const 
unsigned char *sha1,
 reflog_expiry_cleanup_fn cleanup_fn,
 void *policy_cb_data);
 
+/*
+ * Switch to an alternate ref storage backend.
+ */
+int set_ref_storage_backend(const char *name);
+
+int ref_storage_backend_exists(const char *name);
+
 #endif /* REFS_H */
diff --git a/refs/files-backend.c b/refs/files-backend.c
index b569762..2a73564 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3146,8 +3146,8 @@ static int ref_update_reject_duplicates(struct 
string_list *refnames,
return 0;
 }
 
-int ref_transaction_commit(struct ref_transaction *transaction,
-  struct strbuf *err)
+static int files_transaction_commit(struct ref_transaction *transaction,
+   struct strbuf *err)
 {
int ret = 0, i;
int n = transaction->nr;
@@ -3533,3 +3533,9 @@ int reflog_expire(const char *refname, const unsigned 
char *sha1,
unlock_ref(lock);
return -1;
 }
+
+struct ref_storage_be refs_be_files = {
+   NULL,
+   "files",
+   files_transaction_commit,
+};
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index c7dded3..fc0e852 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -197,4 +197,16 @@ const char *find_descendant_ref(const char *dirname,
 
 int rename_ref_available(const char *oldname, const char *newname);
 
+/* refs backends */
+typedef int ref_transaction_commit_fn(struct ref_transaction *transaction,
+ struct strbuf *err);
+
+struct ref_storage_be {
+   struct ref_storage_be *next;
+   const char *name;
+   ref_transaction_commit_fn *transaction_commit;
+};
+
+extern struct ref_storage_be refs_be_files;
+
 #endif /* REFS_REFS_INTERNAL_H */
-- 
2.4.2.749.g730654d-twtrsrc

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

[PATCH v4 03/21] refs: add methods for the ref iterators

[David Turner]

From: Ronnie Sahlberg 

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
---
 refs.c   | 54 
 refs/files-backend.c | 41 +++
 refs/refs-internal.h | 29 
 3 files changed, 112 insertions(+), 12 deletions(-)

diff --git a/refs.c b/refs.c
index afdde7d..e598b73 100644
--- a/refs.c
+++ b/refs.c
@@ -1158,3 +1158,57 @@ int resolve_gitlink_ref(const char *path, const char 
*refname,
 {
return the_refs_backend->resolve_gitlink_ref(path, refname, sha1);
 }
+
+int head_ref(each_ref_fn fn, void *cb_data)
+{
+   return the_refs_backend->head_ref(fn, cb_data);
+}
+
+int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+{
+   return the_refs_backend->head_ref_submodule(submodule, fn, cb_data);
+}
+
+int for_each_ref(each_ref_fn fn, void *cb_data)
+{
+   return the_refs_backend->for_each_ref(fn, cb_data);
+}
+
+int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
+{
+   return the_refs_backend->for_each_ref_submodule(submodule, fn, cb_data);
+}
+
+int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
+{
+   return the_refs_backend->for_each_ref_in(prefix, fn, cb_data);
+}
+
+int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data,
+   unsigned int broken)
+{
+   return the_refs_backend->for_each_fullref_in(prefix, fn, cb_data,
+broken);
+}
+
+int for_each_ref_in_submodule(const char *submodule, const char *prefix,
+ each_ref_fn fn, void *cb_data)
+{
+   return the_refs_backend->for_each_ref_in_submodule(submodule, prefix,
+  fn, cb_data);
+}
+
+int for_each_rawref(each_ref_fn fn, void *cb_data)
+{
+   return the_refs_backend->for_each_rawref(fn, cb_data);
+}
+
+int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
+{
+   return the_refs_backend->for_each_namespaced_ref(fn, cb_data);
+}
+
+int for_each_replace_ref(each_ref_fn fn, void *cb_data)
+{
+   return the_refs_backend->for_each_replace_ref(fn, cb_data);
+}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 882d830..0772a02 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1774,32 +1774,36 @@ static int do_head_ref(const char *submodule, 
each_ref_fn fn, void *cb_data)
return 0;
 }
 
-int head_ref(each_ref_fn fn, void *cb_data)
+static int files_head_ref(each_ref_fn fn, void *cb_data)
 {
return do_head_ref(NULL, fn, cb_data);
 }
 
-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+static int files_head_ref_submodule(const char *submodule, each_ref_fn fn,
+   void *cb_data)
 {
return do_head_ref(submodule, fn, cb_data);
 }
 
-int for_each_ref(each_ref_fn fn, void *cb_data)
+static int files_for_each_ref(each_ref_fn fn, void *cb_data)
 {
return do_for_each_ref(_cache, "", fn, 0, 0, cb_data);
 }
 
-int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
+static int files_for_each_ref_submodule(const char *submodule, each_ref_fn fn,
+   void *cb_data)
 {
return do_for_each_ref(get_ref_cache(submodule), "", fn, 0, 0, cb_data);
 }
 
-int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
+static int files_for_each_ref_in(const char *prefix, each_ref_fn fn,
+void *cb_data)
 {
return do_for_each_ref(_cache, prefix, fn, strlen(prefix), 0, 
cb_data);
 }
 
-int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, 
unsigned int broken)
+static int files_for_each_fullref_in(const char *prefix, each_ref_fn fn,
+void *cb_data, unsigned int broken)
 {
unsigned int flag = 0;
 
@@ -1808,19 +1812,21 @@ int for_each_fullref_in(const char *prefix, each_ref_fn 
fn, void *cb_data, unsig
return do_for_each_ref(_cache, prefix, fn, 0, flag, cb_data);
 }
 
-int for_each_ref_in_submodule(const char *submodule, const char *prefix,
-   each_ref_fn fn, void *cb_data)
+static int files_for_each_ref_in_submodule(const char *submodule,
+  const char *prefix,
+  each_ref_fn fn, void *cb_data)
 {
-   return do_for_each_ref(get_ref_cache(submodule), prefix, fn, 
strlen(prefix), 0, cb_data);
+   return do_for_each_ref(get_ref_cache(submodule), prefix, fn,
+  strlen(prefix), 0, cb_data);
 }
 
-int for_each_replace_ref(each_ref_fn fn, void *cb_data)
+static int files_for_each_replace_ref(each_ref_fn fn, void *cb_data)
 {
return do_for_each_ref(_cache, 

[PATCH v4 14/21] refs: always handle non-normal refs in files backend

[David Turner]

Always handle non-normal (per-worktree or pseudo) refs in the files
backend instead of alternate backends.

Sometimes a ref transaction will update both a per-worktree ref and a
normal ref.  For instance, an ordinary commit might update
refs/heads/master and HEAD (or at least HEAD's reflog).

Updates to normal refs continue to go through the chosen backend.

Updates to non-normal refs are moved to a separate files backend
transaction.

Signed-off-by: David Turner 
---
 refs.c | 81 --
 1 file changed, 79 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 227c018..18ba356 100644
--- a/refs.c
+++ b/refs.c
@@ -9,6 +9,11 @@
 #include "object.h"
 #include "tag.h"
 
+static const char split_transaction_fail_warning[] = N_(
+   "A ref transaction was split across two refs backends.  Part of the "
+   "transaction succeeded, but then the update to the per-worktree refs "
+   "failed.  Your repository may be in an inconsistent state.");
+
 /*
  * We always have a files backend and it is the default.
  */
@@ -791,6 +796,13 @@ void ref_transaction_free(struct ref_transaction 
*transaction)
free(transaction);
 }
 
+static void add_update_obj(struct ref_transaction *transaction,
+  struct ref_update *update)
+{
+   ALLOC_GROW(transaction->updates, transaction->nr + 1, 
transaction->alloc);
+   transaction->updates[transaction->nr++] = update;
+}
+
 static struct ref_update *add_update(struct ref_transaction *transaction,
 const char *refname)
 {
@@ -798,8 +810,7 @@ static struct ref_update *add_update(struct ref_transaction 
*transaction,
struct ref_update *update = xcalloc(1, sizeof(*update) + len);
 
memcpy((char *)update->refname, refname, len); /* includes NUL */
-   ALLOC_GROW(transaction->updates, transaction->nr + 1, 
transaction->alloc);
-   transaction->updates[transaction->nr++] = update;
+   add_update_obj(transaction, update);
return update;
 }
 
@@ -1217,11 +1228,38 @@ static int dereference_symrefs(struct ref_transaction 
*transaction,
return 0;
 }
 
+/*
+ * Move all non-normal ref updates into a specially-created
+ * files-backend transaction
+ */
+static int move_abnormal_ref_updates(struct ref_transaction *transaction,
+struct ref_transaction *files_transaction,
+struct strbuf *err)
+{
+   int i;
+
+   for (i = 0; i < transaction->nr; i++) {
+   int last;
+   struct ref_update *update = transaction->updates[i];
+
+   if (ref_type(update->refname) == REF_TYPE_NORMAL)
+   continue;
+
+   last = --transaction->nr;
+   transaction->updates[i] = transaction->updates[last];
+   add_update_obj(files_transaction, update);
+   }
+
+   return 0;
+}
+
 int ref_transaction_commit(struct ref_transaction *transaction,
   struct strbuf *err)
 {
int ret = -1;
struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
+   struct string_list files_affected_refnames = STRING_LIST_INIT_NODUP;
+   struct ref_transaction *files_transaction = NULL;
 
assert(err);
 
@@ -1237,6 +1275,26 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
if (ret)
goto done;
 
+   if (the_refs_backend != _be_files) {
+   files_transaction = ref_transaction_begin(err);
+   if (!files_transaction)
+   goto done;
+
+   ret = move_abnormal_ref_updates(transaction, files_transaction,
+   err);
+   if (ret)
+   goto done;
+
+   /* files backend commit */
+   if (get_affected_refnames(files_transaction,
+_affected_refnames,
+err)) {
+   ret = TRANSACTION_GENERIC_ERROR;
+   goto done;
+   }
+   }
+
+   /* main backend commit */
if (get_affected_refnames(transaction, _refnames, err)) {
ret = TRANSACTION_GENERIC_ERROR;
goto done;
@@ -1244,8 +1302,24 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
 
ret = the_refs_backend->transaction_commit(transaction,
   _refnames, err);
+   if (ret)
+   goto done;
+
+   if (files_transaction) {
+   ret = refs_be_files.transaction_commit(files_transaction,
+  _affected_refnames,
+  err);
+   if (ret) {
+   

[PATCH v4 12/21] refs: allow log-only updates

[David Turner]

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

Signed-off-by: David Turner 
---
 refs/files-backend.c | 15 ++-
 refs/refs-internal.h |  2 ++
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 0ad60bc..0fdcdc7 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2845,7 +2845,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;
@@ -3199,7 +3199,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));
 
@@ -3229,7 +3230,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)) {
/*
 * We didn't have to write anything to the lockfile.
 * Close it to free up the file descriptor:
@@ -3246,7 +3249,8 @@ static int files_transaction_commit(struct 
ref_transaction *transaction,
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];
 
-   if (update->flags & REF_NEEDS_COMMIT) {
+   if (update->flags & REF_NEEDS_COMMIT ||
+   update->flags & REF_LOG_ONLY) {
if (commit_ref_update(update->backend_data,
  update->new_sha1, update->msg,
  update->flags, err)) {
@@ -3266,7 +3270,8 @@ static int files_transaction_commit(struct 
ref_transaction *transaction,
struct ref_update *update = updates[i];
struct ref_lock *lock = update->backend_data;
 
-   if (update->flags & REF_DELETING) {
+   if (update->flags & REF_DELETING &&
+   !(update->flags & REF_LOG_ONLY)) {
if (delete_ref_loose(lock, update->type, err)) {
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index fc5d1db..b5d0ab8 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -42,6 +42,8 @@
  * value to ref_update::flags
  */
 
+#define REF_LOG_ONLY 0x80
+
 /*
  * Return true iff refname is minimally safe. "Safe" here means that
  * deleting a loose reference by this name will not do any damage, for
-- 
2.4.2.749.g730654d-twtrsrc

--
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: [PATCHv8 1/9] submodule-config: keep update strategy around

[Stefan Beller]

On Thu, Feb 4, 2016 at 4:59 PM, Jonathan Nieder  wrote:
> Hi,
>
> It's been a while since I looked at this series.  Hopefully I can
> come at it with some fresh eyes.  Thanks for your perseverance.
>
> Stefan Beller wrote:
>
>> We need the submodule update strategies in a later patch.
>
> This description doesn't explain what the patch will do or why
> parse_config didn't already retain the value.  If I look back
> at this patch later and want to understand why it was written,
> what would I want to know?
>
> It could say
>
> Currently submodule..update is only handled by git-submodule.sh.
> C code will start to need to make use of that value as more of the
> functionality of git-submodule.sh moves into library code in C.
>
> Add the update field to 'struct submodule' and populate it so it can
> be read as sm->update.


ok

>
> [...]
>> +++ b/submodule-config.c
>> @@ -311,6 +312,16 @@ static int parse_config(const char *var, const char 
>> *value, void *data)
>>   free((void *) submodule->url);
>>   submodule->url = xstrdup(value);
>>   }
>> + } else if (!strcmp(item.buf, "update")) {
>> + if (!value)
>> + ret = config_error_nonbool(var);
>> + else if (!me->overwrite && submodule->update != NULL)
>> + warn_multiple_config(me->commit_sha1, submodule->name,
>> +  "update");
>> + else {
>> + free((void *) submodule->update);
>> + submodule->update = xstrdup(value);
>> + }
>
> (not about this patch) This code is repetitive.  Would there be a way
> to share code between the parsing of different per-submodule settings?
>
> [...]
>> --- a/submodule-config.h
>> +++ b/submodule-config.h
>> @@ -14,6 +14,7 @@ struct submodule {
>>   const char *url;
>>   int fetch_recurse;
>>   const char *ignore;
>> + const char *update;
>
> gitmodules(5) tells me the only allowed values are checkout, rebase,
> merge, and none.  I wouldn't know at a glance how to match against
> those in calling code.  Can this be an enum?

"Note that the !command form is intentionally ignored here for
security reasons."

However you can overwrite the update field in .git/config to be "! [foo]",
which we also need to support, so let's keep it a string for now?

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

[PATCH v4 15/21] init: allow alternate ref strorage to be set for new repos

[David Turner]

git init learns a new argument --ref-storage.  Presently, only
"files" is supported, but later we will add other storage backends.

When this argument is used, the repository's extensions.refStorage
configuration value is set (as well as core.repositoryformatversion),
and the ref storage backend's initdb function is used to set up the
ref database.

Signed-off-by: David Turner 
Signed-off-by: SZEDER Gábor 
---
 Documentation/git-init-db.txt  |  2 +-
 Documentation/git-init.txt |  7 +-
 builtin/init-db.c  | 44 +++---
 cache.h|  2 ++
 contrib/completion/git-completion.bash |  3 ++-
 path.c | 29 --
 refs.c |  8 +++
 refs.h |  8 +++
 setup.c| 12 ++
 t/t0001-init.sh| 25 +++
 10 files changed, 127 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-init-db.txt b/Documentation/git-init-db.txt
index 648a6cd..d03fb69 100644
--- a/Documentation/git-init-db.txt
+++ b/Documentation/git-init-db.txt
@@ -9,7 +9,7 @@ git-init-db - Creates an empty Git repository
 SYNOPSIS
 
 [verse]
-'git init-db' [-q | --quiet] [--bare] [--template=] 
[--separate-git-dir ] [--shared[=]]
+'git init-db' [-q | --quiet] [--bare] [--template=] 
[--separate-git-dir ] [--shared[=]] [--ref-storage=]
 
 
 DESCRIPTION
diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index 8174d27..d2b150f 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git init' [-q | --quiet] [--bare] [--template=]
  [--separate-git-dir ]
  [--shared[=]] [directory]
-
+ [--ref-storage=]
 
 DESCRIPTION
 ---
@@ -113,6 +113,11 @@ does not exist, it will be created.
 
 --
 
+--ref-storage=::
+Type of refs storage backend. Default is to use the original "files"
+storage, which stores ref data in files in .git/refs and
+.git/packed-refs.
+
 TEMPLATE DIRECTORY
 --
 
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 801e977..d331ce8 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -24,6 +24,7 @@ static int init_is_bare_repository = 0;
 static int init_shared_repository = -1;
 static const char *init_db_template_dir;
 static const char *git_link;
+static char *requested_ref_storage_backend;
 
 static void copy_templates_1(struct strbuf *path, struct strbuf *template,
 DIR *dir)
@@ -179,6 +180,7 @@ static int create_default_files(const char *template_path)
int reinit;
int filemode;
struct strbuf err = STRBUF_INIT;
+   int repo_version = 0;
 
/* Just look for `init.templatedir` */
git_config(git_init_db_config, NULL);
@@ -205,6 +207,32 @@ static int create_default_files(const char *template_path)
}
 
/*
+* Create the default symlink from ".git/HEAD" to the "master"
+* branch, if it does not exist yet.
+*/
+   path = git_path_buf(, "HEAD");
+   reinit = (!access(path, R_OK)
+ || readlink(path, junk, sizeof(junk)-1) != -1);
+   if (reinit) {
+   if (requested_ref_storage_backend &&
+   strcmp(ref_storage_backend, requested_ref_storage_backend))
+   die(_("You can't change the refs storage type (was %s; 
you requested %s)"),
+ ref_storage_backend,
+ requested_ref_storage_backend);
+   }
+
+   if (requested_ref_storage_backend)
+   ref_storage_backend = requested_ref_storage_backend;
+   if (strcmp(ref_storage_backend, "files")) {
+   git_config_set("extensions.refStorage", ref_storage_backend);
+   git_config_set("core.repositoryformatversion", 
ref_storage_backend);
+   if (set_ref_storage_backend(ref_storage_backend))
+   die(_("Unknown ref storage backend %s"),
+   ref_storage_backend);
+   repo_version = 1;
+   }
+
+   /*
 * We need to create a "refs" dir in any case so that older
 * versions of git can tell that this is a repository.
 */
@@ -213,13 +241,6 @@ static int create_default_files(const char *template_path)
if (refs_init_db(, shared_repository))
die("failed to set up refs db: %s", err.buf);
 
-   /*
-* Create the default symlink from ".git/HEAD" to the "master"
-* branch, if it does not exist yet.
-*/
-   path = git_path_buf(, "HEAD");
-   reinit = (!access(path, R_OK)
- || readlink(path, junk, sizeof(junk)-1) != -1);
if (!reinit) {
if (create_symref("HEAD", "refs/heads/master", NULL) < 0)
   

[PATCH v4 21/21] refs: tests for lmdb backend

[David Turner]

Add tests for the database backend.

Signed-off-by: David Turner 
Helped-by: Dennis Kaarsemaker 
---
 t/t1460-refs-lmdb-backend.sh| 1109 +++
 t/t1470-refs-lmdb-backend-reflog.sh |  359 
 t/t1480-refs-lmdb-submodule.sh  |   85 +++
 t/test-lib.sh   |1 +
 4 files changed, 1554 insertions(+)
 create mode 100755 t/t1460-refs-lmdb-backend.sh
 create mode 100755 t/t1470-refs-lmdb-backend-reflog.sh
 create mode 100755 t/t1480-refs-lmdb-submodule.sh

diff --git a/t/t1460-refs-lmdb-backend.sh b/t/t1460-refs-lmdb-backend.sh
new file mode 100755
index 000..31442e7
--- /dev/null
+++ b/t/t1460-refs-lmdb-backend.sh
@@ -0,0 +1,1109 @@
+#!/bin/sh
+#
+# Copyright (c) 2015 Twitter, Inc
+# Copyright (c) 2006 Shawn Pearce
+# This test is based on t1400-update-ref.sh
+#
+
+test_description='Test lmdb refs backend'
+TEST_NO_CREATE_REPO=1
+. ./test-lib.sh
+
+if ! test_have_prereq LMDB
+then
+   skip_all="Skipping lmdb refs backend tests, lmdb backend not built"
+   test_done
+fi
+
+raw_ref() {
+   test-refs-lmdb-backend "$1"
+}
+
+delete_ref() {
+   test-refs-lmdb-backend -d "$1"
+}
+
+write_ref() {
+   test-refs-lmdb-backend "$1" "$2"
+}
+
+raw_reflog() {
+   test-refs-lmdb-backend -l "$1"
+}
+
+delete_all_reflogs() {
+   test-refs-lmdb-backend -c
+}
+
+append_reflog() {
+   test-refs-lmdb-backend -a "$1"
+}
+
+Z=$_z40
+
+test_expect_success setup '
+   git init --ref-storage=lmdb &&
+   for name in A B C D E F
+   do
+   test_tick &&
+   T=$(git write-tree) &&
+   sha1=$(echo $name | git commit-tree $T) &&
+   eval $name=$sha1
+   done
+'
+
+m=refs/heads/master
+n_dir=refs/heads/gu
+n=$n_dir/fixes
+
+test_expect_success \
+   "create $m" \
+   "git update-ref $m $A &&
+test $A"' = $(raw_ref '"$m"')'
+test_expect_success \
+   "create $m" \
+   "git update-ref $m $B $A &&
+test $B"' = $(raw_ref '"$m"')'
+test_expect_success "fail to delete $m with stale ref" '
+   test_must_fail git update-ref -d $m $A &&
+   test $B = "$(raw_ref $m)"
+'
+test_expect_success "delete $m" '
+   git update-ref -d $m $B &&
+   ! raw_ref $m
+'
+delete_ref $m
+
+test_expect_success "delete $m without oldvalue verification" "
+   git update-ref $m $A &&
+   test $A = \$(raw_ref $m) &&
+   git update-ref -d $m &&
+   ! raw_ref $m
+"
+delete_ref $m
+
+test_expect_success \
+   "fail to create $n" \
+   "git update-ref $n_dir $A &&
+test_must_fail git update-ref $n $A >out 2>err"
+
+delete_ref $n_dir
+rm -f out err
+
+test_expect_success \
+   "create $m (by HEAD)" \
+   "git update-ref HEAD $A &&
+test $A"' = $(raw_ref '"$m"')'
+test_expect_success \
+   "create $m (by HEAD)" \
+   "git update-ref HEAD $B $A &&
+test $B"' = $(raw_ref '"$m"')'
+test_expect_success "fail to delete $m (by HEAD) with stale ref" '
+   test_must_fail git update-ref -d HEAD $A &&
+   test $B = $(raw_ref '"$m"')
+'
+test_expect_success "delete $m (by HEAD)" '
+   git update-ref -d HEAD $B &&
+   ! raw_ref $m
+'
+delete_ref $m
+
+test_expect_success \
+   "create $m (by HEAD)" \
+   "git update-ref HEAD $A &&
+test $A"' = $(raw_ref '"$m"')'
+test_expect_success \
+   "pack refs" \
+   "git pack-refs --all"
+test_expect_success \
+   "move $m (by HEAD)" \
+   "git update-ref HEAD $B $A &&
+test $B"' = $(raw_ref '"$m"')'
+test_expect_success "delete $m (by HEAD) should remove both packed and loose 
$m" '
+   git update-ref -d HEAD $B &&
+   ! raw_ref $m
+'
+delete_ref $m
+
+OLD_HEAD=$(raw_ref HEAD)
+test_expect_success "delete symref without dereference" '
+   git update-ref --no-deref -d HEAD &&
+   ! raw_ref HEAD
+'
+write_ref HEAD "$OLD_HEAD"
+
+test_expect_success "delete symref without dereference when the referred ref 
is packed" '
+   echo foo >foo.c &&
+   git add foo.c &&
+   git commit -m foo &&
+   git pack-refs --all &&
+   git update-ref --no-deref -d HEAD &&
+   ! raw_ref HEAD
+'
+write_ref HEAD "$OLD_HEAD"
+delete_ref $m
+
+test_expect_success 'update-ref -d is not confused by self-reference' '
+   git symbolic-ref refs/heads/self refs/heads/self &&
+   test_when_finished "delete_ref refs/heads/self" &&
+   test_must_fail git update-ref -d refs/heads/self
+'
+
+test_expect_success 'update-ref --no-deref -d can delete self-reference' '
+   git symbolic-ref refs/heads/self refs/heads/self &&
+   test_when_finished "delete_ref refs/heads/self" &&
+   git update-ref --no-deref -d refs/heads/self
+'
+
+test_expect_success 'update-ref --no-deref -d can delete reference to bad ref' 
'
+   test-refs-lmdb-backend refs/heads/bad "" &&
+   test_when_finished "delete_ref refs/heads/bad" &&
+   git symbolic-ref 

[PATCH v4 07/21] refs: add method for delete_refs

[David Turner]

In the file-based backend, delete_refs has some special optimization
to deal with packed refs.  In other backends, we might be able to make
ref deletion faster by putting all deletions into a single
transaction.  So we need a special backend function for this.

Signed-off-by: David Turner 
---
 refs.c   | 5 +
 refs/files-backend.c | 3 ++-
 refs/refs-internal.h | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index d481a94..5fbe794 100644
--- a/refs.c
+++ b/refs.c
@@ -1123,6 +1123,11 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
return the_refs_backend->transaction_commit(transaction, err);
 }
 
+int delete_refs(struct string_list *refnames)
+{
+   return the_refs_backend->delete_refs(refnames);
+}
+
 const char *resolve_ref_unsafe(const char *ref, int resolve_flags,
   unsigned char *sha1, int *flags)
 {
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 723127e..e86849c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2380,7 +2380,7 @@ static int delete_ref_loose(struct ref_lock *lock, int 
flag, struct strbuf *err)
return 0;
 }
 
-int delete_refs(struct string_list *refnames)
+static int files_delete_refs(struct string_list *refnames)
 {
struct strbuf err = STRBUF_INIT;
int i, result = 0;
@@ -3575,6 +3575,7 @@ struct ref_storage_be refs_be_files = {
files_pack_refs,
files_peel_ref,
files_create_symref,
+   files_delete_refs,
 
files_resolve_ref_unsafe,
files_verify_refname_available,
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 142c663..1d9e5e0 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -236,6 +236,7 @@ typedef int peel_ref_fn(const char *refname, unsigned char 
*sha1);
 typedef int create_symref_fn(const char *ref_target,
 const char *refs_heads_master,
 const char *logmsg);
+typedef int delete_refs_fn(struct string_list *refnames);
 
 /* resolution methods */
 typedef const char *resolve_ref_unsafe_fn(const char *ref,
@@ -280,6 +281,7 @@ struct ref_storage_be {
pack_refs_fn *pack_refs;
peel_ref_fn *peel_ref;
create_symref_fn *create_symref;
+   delete_refs_fn *delete_refs;
 
resolve_ref_unsafe_fn *resolve_ref_unsafe;
verify_refname_available_fn *verify_refname_available;
-- 
2.4.2.749.g730654d-twtrsrc

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

[PATCH v4 04/21] refs: add do_for_each_per_worktree_ref

[David Turner]

Alternate refs backends might still use files to store per-worktree
refs.  So the files backend's ref-loading infrastructure should be
available to those backends, just for use on per-worktree refs.  Add
do_for_each_per_worktree_ref, which iterates over per-worktree refs.

Signed-off-by: David Turner 
---
 refs/files-backend.c | 15 ---
 refs/refs-internal.h | 10 ++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 0772a02..3d678ad 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -518,9 +518,6 @@ static void sort_ref_dir(struct ref_dir *dir)
dir->sorted = dir->nr = i;
 }
 
-/* Include broken references in a do_for_each_ref*() iteration: */
-#define DO_FOR_EACH_INCLUDE_BROKEN 0x01
-
 /*
  * Return true iff the reference described by entry can be resolved to
  * an object in the database.  Emit a warning if the referred-to
@@ -568,6 +565,10 @@ static int do_one_ref(struct ref_entry *entry, void 
*cb_data)
struct ref_entry *old_current_ref;
int retval;
 
+   if (data->flags & DO_FOR_EACH_PER_WORKTREE_ONLY &&
+   ref_type(entry->name) != REF_TYPE_PER_WORKTREE)
+   return 0;
+
if (!starts_with(entry->name, data->base))
return 0;
 
@@ -1756,6 +1757,14 @@ static int do_for_each_ref(struct ref_cache *refs, const 
char *base,
return do_for_each_entry(refs, base, do_one_ref, );
 }
 
+int do_for_each_per_worktree_ref(const char *submodule, const char *base,
+each_ref_fn fn, int trim, int flags,
+void *cb_data)
+{
+   return do_for_each_ref(get_ref_cache(submodule), base, fn, trim,
+  flags | DO_FOR_EACH_PER_WORKTREE_ONLY, cb_data);
+}
+
 static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
 {
struct object_id oid;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 4a1d215..cb949c5 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -57,6 +57,16 @@
  */
 int refname_is_safe(const char *refname);
 
+/* Include broken references in a do_for_each_ref*() iteration */
+#define DO_FOR_EACH_INCLUDE_BROKEN 0x01
+
+/* Only include per-worktree refs in a do_for_each_ref*() iteration */
+#define DO_FOR_EACH_PER_WORKTREE_ONLY 0x02
+
+int do_for_each_per_worktree_ref(const char *submodule, const char *base,
+each_ref_fn fn, int trim, int flags,
+void *cb_data);
+
 enum peel_status {
/* object was peeled successfully: */
PEEL_PEELED = 0,
-- 
2.4.2.749.g730654d-twtrsrc

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

[PATCH v4 18/21] svn: learn ref-storage argument

[David Turner]

git svn learns to pass the ref-storage command-line argument (to init
and clone) through to git init.

Signed-off-by: David Turner 
Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 2 +-
 git-svn.perl   | 6 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index cb9c473..ba4137d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2477,7 +2477,7 @@ _git_svn ()
--branches= --stdlayout --minimize-url
--no-metadata --use-svm-props --use-svnsync-props
--rewrite-root= --prefix= --use-log-author
-   --add-author-from $remote_opts
+   --add-author-from --ref-storage= $remote_opts
"
local cmt_opts="
--edit --rmdir --find-copies-harder --copy-similarity=
diff --git a/git-svn.perl b/git-svn.perl
index fa5f253..15d1544 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -141,7 +141,7 @@ my %fc_opts = ( 'follow-parent|follow!' => 
\$Git::SVN::_follow_parent,
'localtime' => \$Git::SVN::_localtime,
%remote_opts );
 
-my ($_trunk, @_tags, @_branches, $_stdlayout);
+my ($_trunk, @_tags, @_branches, $_stdlayout, $_ref_storage);
 my %icv;
 my %init_opts = ( 'template=s' => \$_template, 'shared:s' => \$_shared,
   'trunk|T=s' => \$_trunk, 'tags|t=s@' => \@_tags,
@@ -153,6 +153,7 @@ my %init_opts = ( 'template=s' => \$_template, 'shared:s' 
=> \$_shared,
  'use-svnsync-props' => sub { $icv{useSvnsyncProps} = 1 },
  'rewrite-root=s' => sub { $icv{rewriteRoot} = $_[1] },
  'rewrite-uuid=s' => sub { $icv{rewriteUUID} = $_[1] },
+ 'ref-storage=s' => \$_ref_storage,
   %remote_opts );
 my %cmt_opts = ( 'edit|e' => \$_edit,
'rmdir' => \$Git::SVN::Editor::_rmdir,
@@ -469,6 +470,9 @@ sub do_git_init_db {
push @init_db, "--shared";
}
}
+   if (defined $_ref_storage) {
+   push @init_db, "--ref-storage=" . $_ref_storage;
+   }
command_noisy(@init_db);
$_repository = Git->repository(Repository => ".git");
}
-- 
2.4.2.749.g730654d-twtrsrc

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

[PATCH v4 16/21] refs: check submodules ref storage config

[David Turner]

All submodules must have the same ref storage (for now).  Confirm that
this is so before attempting to do anything with submodule refs.

Signed-off-by: David Turner 
---
 refs.c   | 56 
 refs/files-backend.c |  8 ++--
 2 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/refs.c b/refs.c
index 191b0a2..715a492 100644
--- a/refs.c
+++ b/refs.c
@@ -308,6 +308,54 @@ int for_each_tag_ref(each_ref_fn fn, void *cb_data)
return for_each_ref_in("refs/tags/", fn, cb_data);
 }
 
+static int submodule_backend(const char *key, const char *value, void *data)
+{
+   const char **path = data;
+   char **old_path = data;
+   if (!strcmp(key, "extensions.refstorage") &&
+   !git_config_string(path, key, "extensions.refstorage"))
+   free(*old_path);
+
+   return 0;
+}
+
+/*
+ * Check that a submodule exists. If its ref storage backend differs
+ * from the current backend, die.  If the submodule exists, return
+ * 0. Else return -1.
+ */
+static int check_submodule_backend(const char *submodule)
+{
+   struct strbuf sb = STRBUF_INIT;
+   char *submodule_storage_backend;
+   int result = -1;
+
+   if (!submodule)
+   return 0;
+
+   submodule_storage_backend = xstrdup("files");
+
+   strbuf_addstr(, submodule);
+   if (!is_nonbare_repository_dir())
+   goto done;
+
+   strbuf_reset();
+   strbuf_git_path_submodule(, submodule, "config");
+
+   git_config_from_file(submodule_backend, sb.buf,
+_storage_backend);
+   if (strcmp(submodule_storage_backend, ref_storage_backend))
+   die(_("Ref storage '%s' for submodule '%s' does not match our 
storage, '%s'"),
+   submodule_storage_backend, submodule, ref_storage_backend);
+
+   result = 0;
+done:
+   free(submodule_storage_backend);
+   strbuf_release();
+
+   return result;
+}
+
 int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
 {
return for_each_ref_in_submodule(submodule, "refs/tags/", fn, cb_data);
@@ -320,6 +368,7 @@ int for_each_branch_ref(each_ref_fn fn, void *cb_data)
 
 int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
 {
+   check_submodule_backend(submodule);
return for_each_ref_in_submodule(submodule, "refs/heads/", fn, cb_data);
 }
 
@@ -330,6 +379,7 @@ int for_each_remote_ref(each_ref_fn fn, void *cb_data)
 
 int for_each_remote_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
 {
+   check_submodule_backend(submodule);
return for_each_ref_in_submodule(submodule, "refs/remotes/", fn, 
cb_data);
 }
 
@@ -1377,6 +1427,9 @@ int create_symref(const char *ref_target, const char 
*refs_heads_master,
 int resolve_gitlink_ref(const char *path, const char *refname,
unsigned char *sha1)
 {
+   if (check_submodule_backend(path))
+   return -1;
+
return the_refs_backend->resolve_gitlink_ref(path, refname, sha1);
 }
 
@@ -1387,6 +1440,7 @@ int head_ref(each_ref_fn fn, void *cb_data)
 
 int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 {
+   check_submodule_backend(submodule);
return the_refs_backend->head_ref_submodule(submodule, fn, cb_data);
 }
 
@@ -1397,6 +1451,7 @@ int for_each_ref(each_ref_fn fn, void *cb_data)
 
 int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
 {
+   check_submodule_backend(submodule);
return the_refs_backend->for_each_ref_submodule(submodule, fn, cb_data);
 }
 
@@ -1415,6 +1470,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn 
fn, void *cb_data,
 int for_each_ref_in_submodule(const char *submodule, const char *prefix,
  each_ref_fn fn, void *cb_data)
 {
+   check_submodule_backend(submodule);
return the_refs_backend->for_each_ref_in_submodule(submodule, prefix,
   fn, cb_data);
 }
diff --git a/refs/files-backend.c b/refs/files-backend.c
index d4f9040..00cd1be 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1357,13 +1357,9 @@ static int files_resolve_gitlink_ref(const char *path, 
const char *refname,
 
strbuf_add(, path, len);
refs = lookup_ref_cache(submodule.buf);
-   if (!refs) {
-   if (!is_nonbare_repository_dir()) {
-   strbuf_release();
-   return -1;
-   }
+   if (!refs)
refs = create_ref_cache(submodule.buf);
-   }
+
strbuf_release();
 
retval = resolve_gitlink_ref_recursive(refs, refname, sha1, 0);
-- 
2.4.2.749.g730654d-twtrsrc

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

[PATCH v4 17/21] clone: allow ref storage backend to be set for clone

[David Turner]

Add a new option, --ref-storage, to allow the ref storage backend to
be set on new clones.

Submodules must use the same ref storage as the parent repository, so
we also pass the --ref-storage option option when cloning
submodules.

Signed-off-by: David Turner 
Signed-off-by: SZEDER Gábor 
---
 Documentation/git-clone.txt|  5 +
 builtin/clone.c|  5 +
 builtin/submodule--helper.c|  2 +-
 contrib/completion/git-completion.bash |  1 +
 git-submodule.sh   | 13 +
 5 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 45d74be..68f56a7 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -14,6 +14,7 @@ SYNOPSIS
  [-o ] [-b ] [-u ] [--reference ]
  [--dissociate] [--separate-git-dir ]
  [--depth ] [--[no-]single-branch]
+ [--ref-storage=]
  [--recursive | --recurse-submodules] [--jobs ] [--] 
  []
 
@@ -224,6 +225,10 @@ objects from the source repository into a pack in the 
cloned repository.
The number of submodules fetched at the same time.
Defaults to the `submodule.fetchJobs` option.
 
+--ref-storage=::
+   Type of ref storage backend. Default is to use the original files
+   based ref storage.
+
 ::
The (possibly remote) repository to clone from.  See the
<> section below for more information on specifying
diff --git a/builtin/clone.c b/builtin/clone.c
index 191f522..5ba0514 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -95,6 +95,8 @@ static struct option builtin_clone_options[] = {
   N_("separate git dir from working tree")),
OPT_STRING_LIST('c', "config", _config, N_("key=value"),
N_("set config inside the new repository")),
+   OPT_STRING(0, "ref-storage", _storage_backend,
+  N_("name"), N_("name of backend type to use")),
OPT_END()
 };
 
@@ -733,6 +735,9 @@ static int checkout(void)
if (max_jobs != -1)
argv_array_pushf(, "--jobs=%d", max_jobs);
 
+   argv_array_pushl(, "--ref-storage",
+ref_storage_backend, NULL);
+
err = run_command_v_opt(args.argv, RUN_GIT_CMD);
argv_array_clear();
}
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 689c354..879d6fb 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -457,7 +457,7 @@ 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);
-
+   argv_array_pushl(, "--ref-storage", ref_storage_backend, NULL);
argv_array_push(, url);
argv_array_push(, path);
 
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 0138d03..cb9c473 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1092,6 +1092,7 @@ _git_clone ()
--depth
--single-branch
--branch
+   --ref-storage=
"
return
;;
diff --git a/git-submodule.sh b/git-submodule.sh
index 6fce0dc..e7ac98b 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -527,6 +527,14 @@ cmd_update()
--checkout)
update="checkout"
;;
+   --ref-storage=*)
+   ref_storage="$1"
+   ;;
+   --ref-storage)
+   case "$2" in '') usage ;; esac
+   ref_storage="$2"
+   shift
+   ;;
--depth)
case "$2" in '') usage ;; esac
depth="--depth=$2"
@@ -560,6 +568,11 @@ cmd_update()
if test -n "$init"
then
cmd_init "--" "$@" || return
+   else
+   if test -n "$ref_storage"
+   then
+   usage
+   fi
fi
 
git submodule--helper update-clone ${GIT_QUIET:+--quiet} \
-- 
2.4.2.749.g730654d-twtrsrc

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

[PATCH v4 10/21] refs: make lock generic

[David Turner]

Instead of using a files-backend-specific struct ref_lock, the generic
ref_transaction struct should provide a void pointer that backends can use
for their own lock data.

Signed-off-by: David Turner 
---
 refs/files-backend.c | 29 -
 refs/refs-internal.h |  2 +-
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 685fc6f..aa11ba2 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3208,11 +3208,12 @@ static int files_transaction_commit(struct 
ref_transaction *transaction,
 */
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];
+   struct ref_lock *lock;
 
if ((update->flags & REF_HAVE_NEW) &&
is_null_sha1(update->new_sha1))
update->flags |= REF_DELETING;
-   update->lock = lock_ref_sha1_basic(
+   lock = lock_ref_sha1_basic(
update->refname,
((update->flags & REF_HAVE_OLD) ?
 update->old_sha1 : NULL),
@@ -3220,7 +3221,8 @@ static int files_transaction_commit(struct 
ref_transaction *transaction,
update->flags,
>type,
err);
-   if (!update->lock) {
+   update->backend_data = lock;
+   if (!lock) {
char *reason;
 
ret = (errno == ENOTDIR)
@@ -3238,12 +3240,12 @@ static int files_transaction_commit(struct 
ref_transaction *transaction,
  (update->flags & 
REF_NODEREF));
 
if (!overwriting_symref &&
-   !hashcmp(update->lock->old_oid.hash, 
update->new_sha1)) {
+   !hashcmp(lock->old_oid.hash, update->new_sha1)) {
/*
 * The reference already has the desired
 * value, so we don't need to write it.
 */
-   } else if (write_ref_to_lockfile(update->lock,
+   } else if (write_ref_to_lockfile(lock,
 update->new_sha1,
 err)) {
char *write_err = strbuf_detach(err, NULL);
@@ -3252,7 +3254,7 @@ static int files_transaction_commit(struct 
ref_transaction *transaction,
 * The lock was freed upon failure of
 * write_ref_to_lockfile():
 */
-   update->lock = NULL;
+   update->backend_data = NULL;
strbuf_addf(err,
"cannot update the ref '%s': %s",
update->refname, write_err);
@@ -3268,7 +3270,7 @@ static int files_transaction_commit(struct 
ref_transaction *transaction,
 * We didn't have to write anything to the lockfile.
 * Close it to free up the file descriptor:
 */
-   if (close_ref(update->lock)) {
+   if (close_ref(lock)) {
strbuf_addf(err, "Couldn't close %s.lock",
update->refname);
goto cleanup;
@@ -3281,16 +3283,16 @@ static int files_transaction_commit(struct 
ref_transaction *transaction,
struct ref_update *update = updates[i];
 
if (update->flags & REF_NEEDS_COMMIT) {
-   if (commit_ref_update(update->lock,
+   if (commit_ref_update(update->backend_data,
  update->new_sha1, update->msg,
  update->flags, err)) {
/* freed by commit_ref_update(): */
-   update->lock = NULL;
+   update->backend_data = NULL;
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
} else {
/* freed by commit_ref_update(): */
-   update->lock = NULL;
+   update->backend_data = NULL;
}
}
}
@@ -3298,16 +3300,17 @@ static int files_transaction_commit(struct 
ref_transaction *transaction,
/* Perform deletes now that updates are safely completed */
for (i = 0; i < n; i++) {
struct ref_update *update = 

Re: [PATCH] remote-curl: don't fall back to Basic auth if we haven't tried Negotiate

[brian m. carlson]

On Fri, Feb 05, 2016 at 12:18:22PM +0300, Dmitry Vilkov wrote:
> You are right, we are using a bare URL (without a username component).
> With username encoded in URL everything works just fine. But it's
> generally wrong to pass creds in URL (in my opinion) and security
> policy of my employer prohibits doing such thing.
> Anyway, as you said libcurl needs valid (not NULL) username/password
> to do GSS-Negotiate, so there is nothing wrong if I set empty
> username/password combination when git prompts for creds. Even more,
> there is no other way to let libcurl to use GSS-Negotiate without
> username in URL.

You can literally do https://:@git.crustytoothpaste.net/git/repo.git as
the URL, and that will work.  GSS-Negotiate using Kerberos passes the
ticket, which contains the principal name in it, so an actual username
and password is not needed at all.  libcurl just needs something to tell
it to do authentication.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: PGP signature

Re: [PATCH v6 2/3] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni]

On Fri, Feb 05, 2016 at 11:31:34AM -0800, Junio C Hamano wrote:
> > +   If you have multiple email addresses that you would like to set
> > +   up per repository, you may want to set this to 'true' in the global
> > +   config, and then Git would prompt you to set user.email separately,
> > +   in each of the cloned repositories.
> 
> The first sentence mentioned both name and email, but here the
> example is only about email.  A first time reader might be led into
> thinking this is only about email and not name, but I am assuming
> that is not the intention (i.e. this is merely showing just one use
> case).
>[..]

Going to revise per yours and Jeff's suggestions.

>[..]
> I can read the split expression either with && hanging at the end of
> line or && leading the next line just fine, but you'd want to be
> consistent especially when you are writing two almost identical
> things.

Sure.

>[..]
>   test_expect_success 'suceed with config' '
>   test_when_finished reprepare &&
>   test_config user.email t...@ok.com &&
> test_must_fail git commit -m msg
>   '
> 
> Note that you do not need "test_unconfig user.email" in reprepare,
> as the variable is set in one test with test_config, which uses
> test_when_finished to arrange the variable to be removed after
> running the test.

Alright. It was worth to understand the differing behavior between
'test_config' and 'git config'.

-- 
Dan Aloni
--
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: no luck with colors for branch names in gitk yet

[Philip Oakley]

From: "Britton Kerin" 
Someone suggested using color.branch.upstream, I tried like this and 
variants


[color "branch"]
 local = red bold
 upstream = red bold

Doesn't seem to matter what I put in for upstream, including invalid
colors, gitk just ignores it and does the dark green for local
branches
--

Alternate, try
https://github.com/oumu/mintty-color-schemes/blob/master/base16-mintty/base16-default.minttyrc
(or any of the other colour schemes) and copy them into your .minttyrc file
(works for me on g4w : git version 2.7.0.windows.1 )

--

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

[PATCH 1/8] pack-objects: add --skip and --skip-hash

[Nguyễn Thái Ngọc Duy]

The idea is, a pack is requested the first time with --skip=0. If pack
transfer is interrupted, the client will ask for the same pack again,
but this time it asks the server not to send what it already has. The
client hashes what it has and sends the SHA-1 to the server. If the
server finds out the skipped part does not match, it can abort early.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/pack-objects.c | 21 
 csum-file.c| 52 ++
 csum-file.h|  3 +++
 3 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 4dae5b1..417c830 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -75,6 +75,9 @@ static unsigned long cache_max_small_delta_size = 1000;
 
 static unsigned long window_memory_limit = 0;
 
+static struct object_id skip_hash;
+static int skip_opt = -1;
+
 /*
  * stats
  */
@@ -781,6 +784,11 @@ static void write_pack_file(void)
else
f = create_tmp_packfile(_tmp_name);
 
+   if (skip_opt > 0) {
+   f->skip = skip_opt;
+   hashcpy(f->skip_hash, skip_hash.hash);
+   }
+
offset = write_pack_header(f, nr_remaining);
 
if (reuse_packfile) {
@@ -2597,6 +2605,7 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
struct argv_array rp = ARGV_ARRAY_INIT;
int rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0;
int rev_list_index = 0;
+   const char *skip_hash_hex = NULL;
struct option pack_objects_options[] = {
OPT_SET_INT('q', "quiet", ,
N_("do not show progress meter"), 0),
@@ -2669,6 +2678,10 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
 N_("use a bitmap index if available to speed up 
counting objects")),
OPT_BOOL(0, "write-bitmap-index", _bitmap_index,
 N_("write a bitmap index together with the pack 
index")),
+   OPT_STRING(0, "skip-hash", _hash_hex, "hash",
+  N_("hash of the skipped part of the pack")),
+   OPT_INTEGER(0, "skip", _opt,
+  N_("do not produce the first  bytes of the 
pack")),
OPT_END(),
};
 
@@ -2689,6 +2702,14 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
}
if (pack_to_stdout != !base_name || argc)
usage_with_options(pack_usage, pack_objects_options);
+   if (skip_opt >= 0) {
+   if (skip_opt > 0) {
+   if (!skip_hash_hex)
+   die(_("--skip-hash is required if --skip is 
non-zero"));
+   if (get_oid_hex(skip_hash_hex, _hash))
+   die(_("%s is not SHA-1"), skip_hash_hex);
+   }
+   }
 
argv_array_push(, "pack-objects");
if (thin) {
diff --git a/csum-file.c b/csum-file.c
index a172199..284847f 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -11,8 +11,27 @@
 #include "progress.h"
 #include "csum-file.h"
 
+static void check_skip_hash(struct sha1file *f, off_t old_total)
+{
+   if (old_total < f->skip && f->total > f->skip)
+   die("BUG: flush() must stop at skip boundary");
+
+   if (f->total == f->skip) {
+   git_SHA_CTX ctx;
+   unsigned char hash[20];
+
+   ctx = f->ctx;
+   git_SHA1_Final(hash, );
+   if (hashcmp(hash, f->skip_hash))
+   die("skip hash does not match, expected %s got %s",
+   sha1_to_hex(f->skip_hash), sha1_to_hex(hash));
+   }
+}
+
 static void flush(struct sha1file *f, const void *buf, unsigned int count)
 {
+   off_t old_total = f->total;
+
if (0 <= f->check_fd && count)  {
unsigned char check_buffer[8192];
ssize_t ret = read_in_full(f->check_fd, check_buffer, count);
@@ -26,7 +45,11 @@ static void flush(struct sha1file *f, const void *buf, 
unsigned int count)
}
 
for (;;) {
-   int ret = xwrite(f->fd, buf, count);
+   int ret;
+   if (f->total + count <= f->skip)
+   ret = count;
+   else
+   ret = xwrite(f->fd, buf, count);
if (ret > 0) {
f->total += ret;
display_throughput(f->tp, f->total);
@@ -34,6 +57,8 @@ static void flush(struct sha1file *f, const void *buf, 
unsigned int count)
count -= ret;
if (count)
continue;
+   if (f->skip > 0)
+   check_skip_hash(f, old_total);
 

[PATCH 2/8] pack-objects: produce a stable pack when --skip is given

[Nguyễn Thái Ngọc Duy]

Parallel delta search does not produce a stable pack so it's disabled
when --skip is used. zlib compression algorithm is stable. Ref
negotiation should be stable (at least on smart http). Ref changes
will be addressed separately.

So unless configuration files or git binary is changed, we should be
able to produce a stable pack. And if config or binaries are changed,
it's ok to abort and fetch a new (resumable) pack again.

Alternatively (and not implemented), pack-objects can be taught to
save the output pack when --skip=0 is passed in, then somehow find the
cached version and send the remaining when --skip= is
given. This saves processing power at the cost of disk and cache
management.

The key thing for caching though, is "find the cached version". We do
not provide any way for pack-objects to send a "key", like http
cookies, to the client, so that the client can pass it back on the
next fetch. Regardless, the input from client must be identical
between fetches, the server should be able to extract a signature from
that and use it to associate with a cached pack. So no need for
sending keys from the server side.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/pack-objects.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 417c830..c58a9cb 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2709,6 +2709,11 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
if (get_oid_hex(skip_hash_hex, _hash))
die(_("%s is not SHA-1"), skip_hash_hex);
}
+
+   /*
+* Parallel delta search can't produce stable packs.
+*/
+   delta_search_threads = 1;
}
 
argv_array_push(, "pack-objects");
-- 
2.7.0.377.g4cd97dd

--
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 3/8] index-pack: add --append-pack=

[Nguyễn Thái Ngọc Duy]

In this mode (--append-pack --stdin), index-pack consumes current
content in  first without producing anything else, then
continues to read from stdin and append to . This is the
consumer end of "pack-objects --skip".

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/index-pack.c | 50 --
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 6a01509..f099ac2 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -73,6 +73,7 @@ static int nr_resolved_deltas;
 static int nr_threads;
 
 static int from_stdin;
+static int skip_mode;
 static int strict;
 static int do_fsck_object;
 static struct fsck_options fsck_options = FSCK_OPTIONS_STRICT;
@@ -272,7 +273,29 @@ static void *fill(int min)
do {
ssize_t ret = xread(input_fd, input_buffer + input_len,
sizeof(input_buffer) - input_len);
-   if (ret <= 0) {
+
+   if (ret == 0 && skip_mode) {
+   output_fd = input_fd;
+   input_fd = 0;
+   skip_mode = 0;
+   /*
+* At this point we have 'input_len' bytes in
+* input_buffer, which is from the existing pack.
+* Assuming that we still need to read more, the
+* next loop will read from stdin instead.
+*
+* What's read from stdin must be written down. The
+* next flush() will write from _buffer[0],
+* which appends the 'input_len' bytes from existing
+* pack to the pack again.
+*
+* Seek back to make this an overwrite (of same
+* content) instead of an append.
+*/
+   if (input_len && lseek(output_fd, -input_len, SEEK_CUR))
+   die_errno(_("cannot seek back %d bytes"), 
input_len);
+   }
+   else if (ret <= 0) {
if (!ret)
die(_("early EOF"));
die_errno(_("read error on input"));
@@ -323,6 +346,26 @@ static const char *open_pack_file(const char *pack_name)
return pack_name;
 }
 
+static const char *open_pack_for_append(const char *path)
+{
+   if (!from_stdin)
+   die(_("--append-pack must be used with --stdin"));
+
+   input_fd = open(path, O_CREAT|O_RDWR, 0600);
+   if (input_fd < 0)
+   die_errno(_("cannot open packfile '%s'"), path);
+   output_fd = -1;
+   nothread_data.pack_fd = input_fd;
+   git_SHA1_Init(_ctx);
+
+   /*
+* fill() will eventually turn this flag off and set output_fd
+* after reading everything
+*/
+   skip_mode = 1;
+   return xstrdup(path);
+}
+
 static void parse_pack_header(void)
 {
struct pack_header *hdr = fill(sizeof(struct pack_header));
@@ -1692,6 +1735,8 @@ int cmd_index_pack(int argc, const char **argv, const 
char *prefix)
opts.off32_limit = strtoul(c+1, , 0);
if (*c || opts.off32_limit & 0x8000)
die(_("bad %s"), arg);
+   } else if (skip_prefix(arg, "--append-pack=", )) {
+   curr_pack = open_pack_for_append(arg);
} else
usage(index_pack_usage);
continue;
@@ -1742,7 +1787,8 @@ int cmd_index_pack(int argc, const char **argv, const 
char *prefix)
}
 #endif
 
-   curr_pack = open_pack_file(pack_name);
+   if (!curr_pack)
+   curr_pack = open_pack_file(pack_name);
parse_pack_header();
objects = xcalloc(nr_objects + 1, sizeof(struct object_entry));
if (show_stat)
-- 
2.7.0.377.g4cd97dd

--
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 8/8] one ugly test to verify basic functionality

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 t/t5544-fetch-resume.sh (new +x) | 42 
 1 file changed, 42 insertions(+)
 create mode 100755 t/t5544-fetch-resume.sh

diff --git a/t/t5544-fetch-resume.sh b/t/t5544-fetch-resume.sh
new file mode 100755
index 000..dfa033d
--- /dev/null
+++ b/t/t5544-fetch-resume.sh
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+test_description='what'
+
+. ./test-lib.sh
+
+test_expect_success 'what' '
+   test_commit one &&
+   git clone --no-local .git abc &&
+   (
+   cd abc &&
+   mv `ls .git/objects/pack/*.pack` pack &&
+   git unpack-objects < pack &&
+   rm pack &&
+   git fsck
+   ) &&
+   test_commit two &&
+   test_commit three &&
+   (
+   cd abc &&
+   git fetch --resume-pack=foo origin HEAD &&
+   git log --format=%s origin/master >actual &&
+   echo one >expected &&
+   test_cmp expected actual &&
+   rm .git/FETCH_HEAD &&
+   mv `ls .git/objects/pack/*.pack` pack &&
+   head -c 123 pack >tmp &&
+   git fetch --resume-pack=tmp origin &&
+   test_path_is_missing tmp &&
+   cmp pack .git/objects/pack/*.pack &&
+   git fsck &&
+   git log --format=%s origin/master >actual &&
+   cat >expected 

[PATCH 6/8] fetch: add --resume-pack=

[Nguyễn Thái Ngọc Duy]

This is a low-level option for resumable fetch. You start a resumable
fetch with

git fetch --resume-pack=blah 

where "blah" file does not exist. If the fetch is interrupted, "blah"
will contain what's been fetched so far. Run the same command again.

On the server side, pack-objects performs the exact same operation to
produce full pack again, but it will not send what is already in
"blah". pack-objects does check if the skipped part is the same between
two sides, in case of configuration change or whatever, and abort early.

On the client side, index-pack feeds itself with what's in "blah",
then the input stream from pack-objects. index-pack does strict
verification as usual. Even if pack-objects fails to produce a stable
pack, index-pack should catch it and complain loudly. If everything
goes well, "blah" is removed and a new good pack is put in $GIT_DIR.

Improvement point. We should be able to perform some heavy operations
locally before connecting to the server (e.g. produce the skip hash
from "blah", or index-pack consuming "blah").

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/fetch-pack.c | 4 
 builtin/fetch.c  | 5 +
 remote-curl.c| 8 +++-
 transport.c  | 4 
 transport.h  | 4 
 5 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 9b2a514..996ad30 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -129,6 +129,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char 
*prefix)
args.update_shallow = 1;
continue;
}
+   if (skip_prefix(arg, "--resume-path=", )) {
+   args.resume_path = arg;
+   continue;
+   }
usage(fetch_pack_usage);
}
 
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8e74213..34f32c6 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -48,6 +48,7 @@ static const char *recurse_submodules_default;
 static int shown_url = 0;
 static int refmap_alloc, refmap_nr;
 static const char **refmap_array;
+static const char *resume_path;
 
 static int option_parse_recurse_submodules(const struct option *opt,
   const char *arg, int unset)
@@ -115,6 +116,8 @@ static struct option builtin_fetch_options[] = {
OPT_BOOL(0, "progress", , N_("force progress reporting")),
OPT_STRING(0, "depth", , N_("depth"),
   N_("deepen history of shallow clone")),
+   OPT_FILENAME(0, "resume-pack", _path,
+N_("perform resumable fetch on the given pack")),
{ OPTION_SET_INT, 0, "unshallow", , NULL,
   N_("convert to a complete repository"),
   PARSE_OPT_NONEG | PARSE_OPT_NOARG, NULL, 1 },
@@ -872,6 +875,8 @@ static struct transport *prepare_transport(struct remote 
*remote)
set_option(transport, TRANS_OPT_DEPTH, depth);
if (update_shallow)
set_option(transport, TRANS_OPT_UPDATE_SHALLOW, "yes");
+   if (resume_path)
+   set_option(transport, TRANS_OPT_RESUME_PATH, resume_path);
return transport;
 }
 
diff --git a/remote-curl.c b/remote-curl.c
index c704857..36835fb 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -20,6 +20,7 @@ static struct strbuf url = STRBUF_INIT;
 struct options {
int verbosity;
unsigned long depth;
+   const char *resume_path;
unsigned progress : 1,
check_self_contained_and_connected : 1,
cloning : 1,
@@ -119,6 +120,9 @@ static int set_option(const char *name, const char *value)
else
return -1;
return 0;
+   } else if (!strcmp(name, "resume-path")) {
+   options.resume_path = xstrdup(value);
+   return 0;
} else {
return 1 /* unsupported */;
}
@@ -727,7 +731,7 @@ static int fetch_git(struct discovery *heads,
struct strbuf preamble = STRBUF_INIT;
char *depth_arg = NULL;
int argc = 0, i, err;
-   const char *argv[17];
+   const char *argv[18];
 
argv[argc++] = "fetch-pack";
argv[argc++] = "--stateless-rpc";
@@ -755,6 +759,8 @@ static int fetch_git(struct discovery *heads,
depth_arg = strbuf_detach(, NULL);
argv[argc++] = depth_arg;
}
+   if (options.resume_path)
+   argv[argc++] = xstrfmt("--resume-path=%s", options.resume_path);
argv[argc++] = url.buf;
argv[argc++] = NULL;
 
diff --git a/transport.c b/transport.c
index 67f3666..6378bed 100644
--- a/transport.c
+++ b/transport.c
@@ -467,6 +467,9 @@ static int set_git_option(struct git_transport_options 
*opts,
} else if (!strcmp(name, TRANS_OPT_UPDATE_SHALLOW)) {
opts->update_shallow = !!value;
return 0;
+   } else 

[PATCH 5/8] fetch-pack.c: send "skip" line to pack-objects

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 fetch-pack.c | 40 
 fetch-pack.h |  3 +++
 2 files changed, 43 insertions(+)

diff --git a/fetch-pack.c b/fetch-pack.c
index 01e34b6..ffb5254 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -15,6 +15,7 @@
 #include "version.h"
 #include "prio-queue.h"
 #include "sha1-array.h"
+#include "dir.h"
 
 static int transfer_unpack_limit = -1;
 static int fetch_unpack_limit = -1;
@@ -330,6 +331,10 @@ static int find_common(struct fetch_pack_args *args,
write_shallow_commits(_buf, 1, NULL);
if (args->depth > 0)
packet_buf_write(_buf, "deepen %d", args->depth);
+   if (args->resume_path)
+   packet_buf_write(_buf, "skip %s %d",
+sha1_to_hex(args->skip_hash),
+args->skip_offset);
packet_buf_flush(_buf);
state_len = req_buf.len;
 
@@ -730,6 +735,9 @@ static int get_pack(struct fetch_pack_args *args,
argv_array_push(, "-v");
if (args->use_thin_pack)
argv_array_push(, "--fix-thin");
+   if (args->resume_path && args->skip_offset)
+   argv_array_pushf(, "--append-pack=%s",
+args->resume_path);
if (args->lock_pack || unpack_limit) {
char hostname[256];
if (gethostname(hostname, sizeof(hostname)))
@@ -856,6 +864,8 @@ static struct ref *do_fetch_pack(struct fetch_pack_args 
*args,
fprintf(stderr, "Server supports ofs-delta\n");
} else
prefer_ofs_delta = 0;
+   if (args->resume_path && !server_supports("partial"))
+   die(_("Server does not support resume capability\n"));
 
if ((agent_feature = server_feature_value("agent", _len))) {
agent_supported = 1;
@@ -1030,6 +1040,35 @@ static void update_shallow(struct fetch_pack_args *args,
sha1_array_clear();
 }
 
+static void prepare_resume_fetch(struct fetch_pack_args *args)
+{
+   git_SHA_CTX c;
+   char buf[8192];
+   int fd, len;
+
+   if (!args->resume_path)
+   return;
+
+   args->skip_offset = 0;
+   args->keep_pack = 1;
+   if (!file_exists(args->resume_path))
+   return;
+
+   fd = open(args->resume_path, O_RDONLY);
+   if (fd == -1)
+   die_errno(_("failed to open '%s'"), args->resume_path);
+
+   git_SHA1_Init();
+   while ((len = xread(fd, buf, sizeof(buf))) > 0) {
+   git_SHA1_Update(, buf, len);
+   args->skip_offset += len;
+   }
+   if (len < 0)
+   die_errno(_("failed to read '%s'"), args->resume_path);
+   git_SHA1_Final(args->skip_hash, );
+   close(fd);
+}
+
 struct ref *fetch_pack(struct fetch_pack_args *args,
   int fd[], struct child_process *conn,
   const struct ref *ref,
@@ -1050,6 +1089,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
die("no matching remote head");
}
prepare_shallow_info(, shallow);
+   prepare_resume_fetch(args);
ref_cpy = do_fetch_pack(args, fd, ref, sought, nr_sought,
, pack_lockfile);
reprepare_packed_git();
diff --git a/fetch-pack.h b/fetch-pack.h
index bb7fd76..46f0997 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -10,6 +10,9 @@ struct fetch_pack_args {
const char *uploadpack;
int unpacklimit;
int depth;
+   const char *resume_path;
+   int skip_offset;
+   unsigned char skip_hash[20];
unsigned quiet:1;
unsigned keep_pack:1;
unsigned lock_pack:1;
-- 
2.7.0.377.g4cd97dd

--
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 7/8] index-pack: --append-pack implies --strict

[Nguyễn Thái Ngọc Duy]

A frankenstein pack, generated by multiple pack-objects runs,
certainly has higher risk of broken, especially when the server side
could be some other implementation than pack-objects. Be safe and
strict.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/index-pack.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index f099ac2..cc60aee 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1736,6 +1736,8 @@ int cmd_index_pack(int argc, const char **argv, const 
char *prefix)
if (*c || opts.off32_limit & 0x8000)
die(_("bad %s"), arg);
} else if (skip_prefix(arg, "--append-pack=", )) {
+   strict = 1;
+   do_fsck_object = 1;
curr_pack = open_pack_for_append(arg);
} else
usage(index_pack_usage);
-- 
2.7.0.377.g4cd97dd

--
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 4/8] upload-pack: new capability to pass --skip* to pack-objects

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/technical/pack-protocol.txt |  2 ++
 Documentation/technical/protocol-capabilities.txt |  9 +++
 upload-pack.c | 30 +--
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index c6977bb..5207d08 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -167,6 +167,7 @@ MUST peel the ref if it's an annotated tag.
 
   advertised-refs  =  (no-refs / list-of-refs)
  *shallow
+ skip
  flush-pkt
 
   no-refs  =  PKT-LINE(zero-id SP "capabilities^{}"
@@ -181,6 +182,7 @@ MUST peel the ref if it's an annotated tag.
   other-peeled =  obj-id SP refname "^{}"
 
   shallow  =  PKT-LINE("shallow" SP obj-id)
+  skip =  PKT-LINE("skip" SP obj-id SP 1*DIGIT)
 
   capability-list  =  capability *(SP capability)
   capability   =  1*(LC_ALPHA / DIGIT / "-" / "_")
diff --git a/Documentation/technical/protocol-capabilities.txt 
b/Documentation/technical/protocol-capabilities.txt
index eaab6b4..0567970 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -275,3 +275,12 @@ to accept a signed push certificate, and asks the  
to be
 included in the push certificate.  A send-pack client MUST NOT
 send a push-cert packet unless the receive-pack server advertises
 this capability.
+
+partial
+--
+
+This capability adds "skip" line to the protocol, which passes --skip
+and --skip-hash to pack-objects. When "skip" line is present, given
+the same set of input from the client (e.g. have, want and shallow
+lines, "skip" line excluded), the exact same pack must be produced.
+
diff --git a/upload-pack.c b/upload-pack.c
index b3f6653..5565afe 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -53,6 +53,9 @@ static int use_sideband;
 static int advertise_refs;
 static int stateless_rpc;
 
+static struct object_id skip_hash;
+static int skip_opt = -1;
+
 static void reset_timeout(void)
 {
alarm(timeout);
@@ -90,7 +93,7 @@ static void create_pack_file(void)
"corruption on the remote side.";
int buffered = -1;
ssize_t sz;
-   const char *argv[13];
+   const char *argv[17];
int i, arg = 0;
FILE *pipe_fd;
 
@@ -112,6 +115,14 @@ static void create_pack_file(void)
argv[arg++] = "--delta-base-offset";
if (use_include_tag)
argv[arg++] = "--include-tag";
+   if (skip_opt >= 0) {
+   argv[arg++] = "--skip";
+   argv[arg++] = xstrfmt("%d", skip_opt);
+   if (skip_opt > 0) {
+   argv[arg++] = "--skip-hash";
+   argv[arg++] = xstrdup(oid_to_hex(_hash));
+   }
+   }
argv[arg++] = NULL;
 
pack_objects.in = -1;
@@ -550,6 +561,8 @@ static void receive_needs(void)
const char *features;
unsigned char sha1_buf[20];
char *line = packet_read_line(0, NULL);
+   const char *arg;
+
reset_timeout();
if (!line)
break;
@@ -577,6 +590,19 @@ static void receive_needs(void)
die("Invalid deepen: %s", line);
continue;
}
+   if (skip_prefix(line, "skip ", )) {
+   char *end = NULL;
+
+   if (get_oid_hex(arg, _hash))
+   die("invalid skip line: %s", line);
+   arg += 40;
+   if (*arg++ != ' ')
+   die("invalid skip line: %s", line);
+   skip_opt = strtol(arg, , 0);
+   if (!end || *end || skip_opt < 0)
+   die("Invalid skip line: %s", line);
+   continue;
+   }
if (!starts_with(line, "want ") ||
get_sha1_hex(line+5, sha1_buf))
die("git upload-pack: protocol error, "
@@ -725,7 +751,7 @@ static int send_ref(const char *refname, const struct 
object_id *oid,
 {
static const char *capabilities = "multi_ack thin-pack side-band"
" side-band-64k ofs-delta shallow no-progress"
-   " include-tag multi_ack_detailed";
+   " include-tag multi_ack_detailed partial";
const char *refname_nons = strip_namespace(refname);
struct object_id peeled;
 
-- 
2.7.0.377.g4cd97dd

--
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 4/8] upload-pack: new capability to pass --skip* to pack-objects

[Johannes Schindelin]

Hi Duy,

On Fri, 5 Feb 2016, Nguyễn Thái Ngọc Duy wrote:

> [... empty commit message body...]
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>
> [...]
> diff --git a/Documentation/technical/protocol-capabilities.txt 
> b/Documentation/technical/protocol-capabilities.txt
> index eaab6b4..0567970 100644
> --- a/Documentation/technical/protocol-capabilities.txt
> +++ b/Documentation/technical/protocol-capabilities.txt
> @@ -275,3 +275,12 @@ to accept a signed push certificate, and asks the 
>  to be
>  included in the push certificate.  A send-pack client MUST NOT
>  send a push-cert packet unless the receive-pack server advertises
>  this capability.
> +
> +partial
> +--
> +
> +This capability adds "skip" line to the protocol, which passes --skip
> +and --skip-hash to pack-objects. When "skip" line is present, given
> +the same set of input from the client (e.g. have, want and shallow
> +lines, "skip" line excluded), the exact same pack must be produced.
> +

Would you care to elaborate on this idea a bit more? The commit message
would make an excellent place for describing the underlying idea, as well
as comparing it to the alternatives.

Ciao,
Dscho

Re: [PATCH] remote-curl: don't fall back to Basic auth if we haven't tried Negotiate

[Dmitry Vilkov]

2016-02-03 2:29 GMT+03:00 brian m. carlson :
> I'm unclear in what case you'd need to have a username and password
> combination with GSS-Negotiate.  Kerberos doesn't use your password,
> although you need some indication of a username (valid or not) to get
> libcurl to do authentication.
>
> Are you basically using a bare URL (without a username component) and
> waiting for git to prompt you for the username, so that it will then
> enable authentication?  If so, this patch looks fine for that, although
> I'd expand on the commit message.  If not, could you provide an example
> of what you're trying to do?

You are right, we are using a bare URL (without a username component).
With username encoded in URL everything works just fine. But it's
generally wrong to pass creds in URL (in my opinion) and security
policy of my employer prohibits doing such thing.
Anyway, as you said libcurl needs valid (not NULL) username/password
to do GSS-Negotiate, so there is nothing wrong if I set empty
username/password combination when git prompts for creds. Even more,
there is no other way to let libcurl to use GSS-Negotiate without
username in URL.
--
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/8] pack-objects: add --skip and --skip-hash

[Johannes Schindelin]

Hi Duy,

On Fri, 5 Feb 2016, Nguyễn Thái Ngọc Duy wrote:

> The idea is, a pack is requested the first time with --skip=0. If pack
> transfer is interrupted, the client will ask for the same pack again,
> but this time it asks the server not to send what it already has. The
> client hashes what it has and sends the SHA-1 to the server. If the
> server finds out the skipped part does not match, it can abort early.

Ah, here it is. This description should definitely go into Documentation/,
methinks. Maybe elaborate a little bit more on the "what it has" part?

Ciao,
Dscho

Quick Loan Approval

[Hijab Loan Firm]

We can help you with a genuine loan to meet your needs.
Do you need a personal or business loan without stress and
quick approval?
Do you need an urgent loan today? No Credit Checks

* LOAN APPROVAL IN 60MINS !!
* GUARANTEED SAME DAY TRANSFER !!
* 100% APPROVAL RATE !!
* LOW INTEREST RATE !!

Contact US for more information about loan offer and we will solve your 
financial problem. contact us via email: loa...@foxmail.com

NOTE:You might receive this message in your spam or junk folder depending on 
your web host.
--
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] config: add '--sources' option to print the source of a config value

[Sebastian Schuberth]

On 2/5/2016 9:42, larsxschnei...@gmail.com wrote:


Teach 'git config' the '--sources' option to print the source
configuration file for every printed value.


Yay, not being able to see where a config setting originates from has 
bothered me in the past, too. So thanks for working on this.


However, the naming of the '--sources' option sounds a bit misleading to 
me. It has nothing to do with source code. So maybe better name it 
'--origin', or even more verbose '--show-origin' or '--show-filename'?


Regards,
Sebastian

--
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] config: add '--sources' option to print the source of a config value

[Jeff King]

On Fri, Feb 05, 2016 at 12:13:04PM +0100, Sebastian Schuberth wrote:

> On 2/5/2016 9:42, larsxschnei...@gmail.com wrote:
> 
> >Teach 'git config' the '--sources' option to print the source
> >configuration file for every printed value.
> 
> Yay, not being able to see where a config setting originates from has
> bothered me in the past, too. So thanks for working on this.
> 
> However, the naming of the '--sources' option sounds a bit misleading to me.
> It has nothing to do with source code. So maybe better name it '--origin',
> or even more verbose '--show-origin' or '--show-filename'?

I think he inherited the "--sources" name from me. :) I agree it could
be better. I think "--show-filename" is not right as there are
non-filename cases.  Just "--origin" sounds funny to me, perhaps because
of git's normal use of the word "origin".

I like "--show-origin" the best of the ones suggested.

-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 8/8] one ugly test to verify basic functionality

[Elia Pinto]

2016-02-05 9:57 GMT+01:00 Nguyễn Thái Ngọc Duy :
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  t/t5544-fetch-resume.sh (new +x) | 42 
> 
>  1 file changed, 42 insertions(+)
>  create mode 100755 t/t5544-fetch-resume.sh
>
> diff --git a/t/t5544-fetch-resume.sh b/t/t5544-fetch-resume.sh
> new file mode 100755
> index 000..dfa033d
> --- /dev/null
> +++ b/t/t5544-fetch-resume.sh
> @@ -0,0 +1,42 @@
> +#!/bin/sh
> +
> +test_description='what'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'what' '
> +   test_commit one &&
> +   git clone --no-local .git abc &&
> +   (
> +   cd abc &&
> +   mv `ls .git/objects/pack/*.pack` pack &&

No, please. From the git coding guideline : "We prefer $( ... ) for
command substitution; unlike ``, it properly nests.
It should have been the way Bourne spelled it from day one, but
unfortunately isn't."

http://stackoverflow.com/questions/4708549/whats-the-difference-between-command-and-command-in-shell-programming

Thank you
> +   git unpack-objects < pack &&
> +   rm pack &&
> +   git fsck
> +   ) &&
> +   test_commit two &&
> +   test_commit three &&
> +   (
> +   cd abc &&
> +   git fetch --resume-pack=foo origin HEAD &&
> +   git log --format=%s origin/master >actual &&
> +   echo one >expected &&
> +   test_cmp expected actual &&
> +   rm .git/FETCH_HEAD &&
> +   mv `ls .git/objects/pack/*.pack` pack &&
> +   head -c 123 pack >tmp &&
> +   git fetch --resume-pack=tmp origin &&
> +   test_path_is_missing tmp &&
> +   cmp pack .git/objects/pack/*.pack &&
> +   git fsck &&
> +   git log --format=%s origin/master >actual &&
> +   cat >expected < +three
> +two
> +one
> +EOF
> +   test_cmp expected actual
> +   )
> +'
> +
> +test_done
> --
> 2.7.0.377.g4cd97dd
>
> --
> 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 14/15] git-am.sh: replace using expr for arithmetic operations with the equivalent shell builtin

[Elia Pinto]

2016-02-04 20:33 GMT+01:00 Junio C Hamano :
> As pointed out already, quoting of "$this" inside the arithmetic
> expansion would not work very well, so [14/15] needs fixing.
>
> I do not see 01/15 thru 13/15 here, by the way.  Is it just me?

Excuse me, everyone. Yesterday was a bad day. I did a bit of confusion :=(.
But this is not a patch series, yes. The patch is for git-am in
contrib: so it is not important.

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

Re: [PATCH v1] config: add '--sources' option to print the source of a config value

[Jeff King]

On Fri, Feb 05, 2016 at 09:42:30AM +0100, larsxschnei...@gmail.com wrote:

> @@ -538,6 +569,17 @@ int cmd_config(int argc, const char **argv, const char 
> *prefix)
>   error("--name-only is only applicable to --list or 
> --get-regexp");
>   usage_with_options(builtin_config_usage, 
> builtin_config_options);
>   }
> +
> + const int is_query_action = actions & (
> + ACTION_GET|ACTION_GET_ALL|ACTION_GET_REGEXP|ACTION_LIST|
> + ACTION_GET_COLOR|ACTION_GET_COLORBOOL|ACTION_GET_URLMATCH
> + );
> +
> + if (show_sources && !is_query_action) {
> + error("--sources is only applicable to --list or --get-* 
> actions");
> + usage_with_options(builtin_config_usage, 
> builtin_config_options);
> + }

Hmm. I had originally envisioned this only being used with "--list", but
I guess it makes sense to say "--sources --get" to show where the value
for a particular option is coming from.

The plural "sources" is a little funny there, though, as we list only
the "last one wins" final value, not all of them (for that, you can use
"--sources --get-all", which seems handy). I think it would be
sufficient for the documentation to make this clear (speaking of which,
this patch needs documentation...).

Also, I don't think the feature works with --get-color, --get-colorbool,
or --get-urlmatch (which don't do their output in quite the same way).
I think it would be fine to simply disallow --sources with those options
rather than worry about making it work.

> +/* output to either fp or buf; only one should be non-NULL */
> +static void show_config_source(struct strbuf *buf, FILE *fp)
> +{
> + const char *fn = current_config_filename();
> + if (!fn)
> + return;

I'm not sure returning here is the best idea. We won't have a config
filename if we are reading from "-c", but if we return early from this
function, it parses differently than every other line. E.g., with your
patch, if I do:

  git config -c foo.bar=true config --sources --list

I'll get:

  /home/peff/.gitconfig  user.name=Jeff King
  /home/peff/.gitconfig  user.email=p...@peff.net
  ...etc...
  foo.bar=true

If somebody is parsing this as a tab-delimited list, then instead of the
source field for that line being empty, it is missing (and it looks like
"foo.bar=true" is the source file). I think it would be more friendly to
consumers of the output to have a blank (i.e., set "fn" to the empty
string and continue in the function).

> +
> + char term = '\t';

This declaration comes after the "if" above, but git style doesn't allow
declaration-after-statement (to support ancient compilers).

> +test_expect_success '--sources' '
> + >.git/config &&
> + >"$HOME"/.gitconfig &&
> + INCLUDE_DIR="$HOME/include" &&
> + mkdir -p "$INCLUDE_DIR" &&
> + cat >"$INCLUDE_DIR"/include.conf <<-EOF &&
> + [user]
> + include = true
> + EOF
> + cat >"$HOME"/file.conf <<-EOF &&
> + [user]
> + custom = true
> + EOF
> + test_config_global user.global "true" &&
> + test_config_global user.override "global" &&
> + test_config_global include.path "$INCLUDE_DIR"/include.conf &&

Here you include the file by its absolute path. I wondered what would
happen if we used a relative path. E.g.:

  git config include.path=foo
  git config -f .git/foo included.config=true
  git config --sources --list

which shows it as ".git/foo", because we resolved it by manipulating the
relative path ".git/config". Whereas including it from ~/.gitconfig will
show the absolute path, because we use the absolute path to get to
~/.gitconfig in the first place.

I think that's all sane. I don't know if it's worth noting it in the
documentation or not.

> + cat >expect <<-EOF &&
> + $HOME/.gitconfiguser.global=true
> + $HOME/.gitconfiguser.override=global
> + $HOME/.gitconfiginclude.path=$INCLUDE_DIR/include.conf
> + $INCLUDE_DIR/include.conf   user.include=true
> + .git/config user.local=true
> + .git/config user.override=local
> + user.cmdline=true
> + EOF

If the filename has funny characters (e.g., a literal tab), it will be
quoted here (but not in the --null output below). Worth including in the
test?

> + cat >expect <<-EOF &&
> + .git/config local
> + EOF
> + git config --sources user.override >output &&
> + test_cmp expect output &&

Good thoroughness in checking the override case.

> + cat >expect <<-EOF &&
> + a9d9f9e555b5c6f07cbe09d3f06fe3df11e09c08user.custom=true
> + EOF
> + blob=$(git hash-object -w "$HOME"/file.conf) &&
> + git config --blob=$blob --sources --list >output &&
> + test_cmp expect output

This one was unexpected to me, but I think it makes sense. The option is
"--sources" and not 

[BUG] git describe number of commits different from git log count

[Jan Hudec]

Hello,

I have a repository with following situation:

$ git describe next
v4.1-2196-g5a414d7
$ git describe next --match=v4.2
v4.2-4757-g5a414d7

Since the tag with fewest commits since is selected, it appears logical. 
However, v4.2 is descendant of v4.1, so it does not make sense for it to have 
more commits since. And rev-list or log confirm that:

$ git rev-list v4.1..next | wc -l
2196
$ git rev-list v4.2..next | wc -l
1152

The number of commits since v4.1 matches what the describe counted, but the 
number of commits since v4.2 does not.

The v4.1 tag should be reachable along the first parent path, the v4.2 
definitely isn't, but I am not asking for first parent path.

I am using

$ git --version
git version 2.7.0

from Debian git 1:2.7.0-1 package locally, but our build server is still using 
rather ancient

$ git --version
git version 1.8.4.msysgit.0

with exactly the same result.

Unfortunately I can't share the repository, but I could run some tests on it, 
including rebuilding git with some diagnostics and trying that.

Note that the tag v4.2 is otherwise perfectly good candidate for describe and 
gets used just fine when describing master:

$ git describe master
v4.2-2-g34eb80b
$ git describe master --match=v4.1
v4.1-1046-g34eb80b

However when I merged master into next, it started producing those incorrect 
results.

-- 
 - Jan Hudec 
--
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] config: add '--sources' option to print the source of a config value

[Sebastian Schuberth]

On 2/5/2016 12:20, Jeff King wrote:


Hmm. I had originally envisioned this only being used with "--list", but
I guess it makes sense to say "--sources --get" to show where the value
for a particular option is coming from.


Being able to use "--sources --get" is a feature that I'd definitely 
like to see, too.



I'm not sure returning here is the best idea. We won't have a config
filename if we are reading from "-c", but if we return early from this
function, it parses differently than every other line. E.g., with your
patch, if I do:

   git config -c foo.bar=true config --sources --list

I'll get:

   /home/peff/.gitconfig  user.name=Jeff King
   /home/peff/.gitconfig  user.email=p...@peff.net
   ...etc...
   foo.bar=true

If somebody is parsing this as a tab-delimited list, then instead of the
source field for that line being empty, it is missing (and it looks like
"foo.bar=true" is the source file). I think it would be more friendly to
consumers of the output to have a blank (i.e., set "fn" to the empty
string and continue in the function).


Or to come up with a special string to denote config values specified on 
the command line. Maybe somehting like


  foo.bar=true

I acknowledge that "" would be a valid filename on some 
filesystems, but I think the risk is rather low that someone would 
actually be using that name for a Git config file.


Regards,
Sebastian

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

酷帅型系扣牛仔裤

[酷帅型系扣牛仔裤]

你的老朋友邀你来Q群:343257759

Re: [PATCH v1] config: add '--sources' option to print the source of a config value

[Jeff King]

On Fri, Feb 05, 2016 at 12:31:15PM +0100, Sebastian Schuberth wrote:

> >I'm not sure returning here is the best idea. We won't have a config
> >filename if we are reading from "-c", but if we return early from this
> >function, it parses differently than every other line. E.g., with your
> >patch, if I do:
> >
> >   git config -c foo.bar=true config --sources --list
> >
> >I'll get:
> >
> >   /home/peff/.gitconfig  user.name=Jeff King
> >   /home/peff/.gitconfig  user.email=p...@peff.net
> >   ...etc...
> >   foo.bar=true
> >
> >If somebody is parsing this as a tab-delimited list, then instead of the
> >source field for that line being empty, it is missing (and it looks like
> >"foo.bar=true" is the source file). I think it would be more friendly to
> >consumers of the output to have a blank (i.e., set "fn" to the empty
> >string and continue in the function).
> 
> Or to come up with a special string to denote config values specified on the
> command line. Maybe somehting like
> 
>   foo.bar=true
> 
> I acknowledge that "" would be a valid filename on some
> filesystems, but I think the risk is rather low that someone would actually
> be using that name for a Git config file.

Yeah, I agree it's unlikely. And the output is already ambiguous, as the
first field could be a blob (though I guess the caller knows if they
passed "--blob" or not). If we really wanted an unambiguous output, we
could have something like "file:...", "blob:...", etc. But that's a bit
less readable for humans, and I don't think solves any real-world
problems.

So I think it would be OK to use "" here, as long as the
token is documented.

Are there any other reasons why current_config_filename() would return
NULL, besides command-line config? I don't think so. It looks like we
can read config from stdin, but we use the token "" there for the
name field (another ambiguity!).

-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