Re: git submodule should honor "-c credential.helper" command line argument

2016-02-02 Thread Jacob Keller
On Tue, Feb 2, 2016 at 8:25 PM, Jeff King  wrote:
> I think the problem is that when git "switches" to working in the
> submodule repository, it clears the environment, which includes any "-c"
> command switches. This makes sense for some situations, but not for
> others. This thread shows a similar problem:
>
>   http://thread.gmane.org/gmane.comp.version-control.git/264840
>
> Jens suggested there adding an option to tell clone to pass specific
> variables to the submodule, which I think makes sense. AFAIK, nobody has
> done any work yet on that approach.
>

This is something that I am also interested in, haven't had a chance
to look at it just yet though. I may have some time soon to take a
stab at this.

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


Bug: Wrong GitHub API result when retrieving repository information

2016-02-02 Thread Wert Alexander
Dear GitHub Developers,

I'm using the GitHub API for retrieving statistics like stargazers_count or 
watchers_count.
Unfortunately, an API call to https://api.github.com/users//repos returns 
(as JSON) the same number for stargazers_count and watchers_count, although on 
the Website the numbers are different.

Example:
...
"size": 5279,
"stargazers_count": 51,
"watchers_count": 51,
"language": "Java",
...


The actual count on the website, however,  are 51 for stargazers and 20 for 
watchers.

Any idea what could be the problem?

Thank you in advance!

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


Re: Git for Windows crashes on Windows 10

2016-02-02 Thread Bryan Turner
On Tue, Feb 2, 2016 at 10:28 PM, 孙乾程  wrote:
> I'm not a native English speaker. I'm sorry if I didn't explain the problem 
> clearly.
> I'm using Windows 10 Enterprise Insider Preview (I'll use Win10 instead 
> below). Until yesterday, I'm using Win10 build 11102, and Git for Windows 
> works well. But today I upgraded my system to Win10 build 14251, then 
> something wrong happened.

I'm pretty sure you're running into
https://github.com/git-for-windows/git/issues/627. Since it's a bug in
Windows itself, there's not a fix in Git. Your options are:

- Downgrade back to 11102
- Wait for the next preview build after 14251
- Downgrade to msysgit 1.9.5

> I opened a "cmd" in my repository and entered "git pull", then a dialog 
> window appeared, saying "Git for Windows has stopped working".
> The version of the git on my system was 2.6.4, and I found out that the 
> latest version was 2.7.0.2. So I downloaded a installer from 
> "https://git-scm.com/";. I opened the installer, but as soon as the 
> installation finished, the same error dialog appeared. I closed the dialog, 
> but it appeared again, so I closed it again. I opened a "cmd" and entered 
> "git", of course, it crashes and the error dialog appeared.
> By the way, I installed GitHub for Windows on my system and it crashes too 
> when I pressed the "Sync" button in it.
> In my surprise, GitHub Extension for Visual Studio works correctly.
>
--
To unsubscribe from this list: send the line "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 for Windows crashes on Windows 10

2016-02-02 Thread 孙乾程
I'm not a native English speaker. I'm sorry if I didn't explain the problem 
clearly.
I'm using Windows 10 Enterprise Insider Preview (I'll use Win10 instead below). 
Until yesterday, I'm using Win10 build 11102, and Git for Windows works well. 
But today I upgraded my system to Win10 build 14251, then something wrong 
happened.
I opened a "cmd" in my repository and entered "git pull", then a dialog window 
appeared, saying "Git for Windows has stopped working".
The version of the git on my system was 2.6.4, and I found out that the latest 
version was 2.7.0.2. So I downloaded a installer from "https://git-scm.com/";. I 
opened the installer, but as soon as the installation finished, the same error 
dialog appeared. I closed the dialog, but it appeared again, so I closed it 
again. I opened a "cmd" and entered "git", of course, it crashes and the error 
dialog appeared.
By the way, I installed GitHub for Windows on my system and it crashes too when 
I pressed the "Sync" button in it.
In my surprise, GitHub Extension for Visual Studio works correctly.



Re: [PATCH] Trick to force setup of a specific configured E-Mail per repo

2016-02-02 Thread Jeff King
On Wed, Feb 03, 2016 at 12:26:36PM +0700, Duy Nguyen wrote:

> >> Should we generalize this use case, i.e. define a list of
> >> configuration variables that must be (re-)defined per-repo? Maybe not
> >> worth it, I don't know. I can't think of any other variable that
> >> should behave this way off the top of my head.
> >
> > That's an interesting thought, but I'm not sure how it would work. The
> > ident variables are special in that people are often unhappy with the
> > fallback. What would it mean for somebody to say "do not proceed if
> > diff.renameLimit is not set", and where would we enforce that?
> 
> Enforcing it at git-init and git-clone, interactively asking the
> values of these variables,  may be too much. If only we can catch if a
> variable is used, then we probably can catch it there. But that may
> require some more changes in config api.

Yeah, maybe. I do not want to say "definitely not", but I do think this
is sufficiently more complicated than something like user.guessIdent
that we should not make it a dependency.

-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] Trick to force setup of a specific configured E-Mail per repo

2016-02-02 Thread Duy Nguyen
On Wed, Feb 3, 2016 at 12:22 PM, Jeff King  wrote:
> On Wed, Feb 03, 2016 at 12:19:20PM +0700, Duy Nguyen wrote:
>
>> On Wed, Feb 3, 2016 at 10:56 AM, Jeff King  wrote:
>> > I find it disappointing that we go back to looking for magic sequences
>> > in the string. Could we perhaps do this more cleanly with a new config
>> > option? Like a "user.guessIdent" which defaults to true, but people can
>> > set to false. And without that, we do not do any automagic at all; we
>> > get the values from the GIT_COMMITTER_* environment or the
>> > user.{name,email} config variables, or we die().
>> >
>> > I think that should allow your use case (and extend the same feature to
>> > user.name). It wouldn't work on older versions of git, but nor would
>> > your fix here (the only way to do that is to re-instate "(none)" as
>> > magical).
>>
>> Should we generalize this use case, i.e. define a list of
>> configuration variables that must be (re-)defined per-repo? Maybe not
>> worth it, I don't know. I can't think of any other variable that
>> should behave this way off the top of my head.
>
> That's an interesting thought, but I'm not sure how it would work. The
> ident variables are special in that people are often unhappy with the
> fallback. What would it mean for somebody to say "do not proceed if
> diff.renameLimit is not set", and where would we enforce that?

Enforcing it at git-init and git-clone, interactively asking the
values of these variables,  may be too much. If only we can catch if a
variable is used, then we probably can catch it there. But that may
require some more changes in config api.
-- 
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] Trick to force setup of a specific configured E-Mail per repo

2016-02-02 Thread Jeff King
On Wed, Feb 03, 2016 at 12:19:20PM +0700, Duy Nguyen wrote:

> On Wed, Feb 3, 2016 at 10:56 AM, Jeff King  wrote:
> > I find it disappointing that we go back to looking for magic sequences
> > in the string. Could we perhaps do this more cleanly with a new config
> > option? Like a "user.guessIdent" which defaults to true, but people can
> > set to false. And without that, we do not do any automagic at all; we
> > get the values from the GIT_COMMITTER_* environment or the
> > user.{name,email} config variables, or we die().
> >
> > I think that should allow your use case (and extend the same feature to
> > user.name). It wouldn't work on older versions of git, but nor would
> > your fix here (the only way to do that is to re-instate "(none)" as
> > magical).
> 
> Should we generalize this use case, i.e. define a list of
> configuration variables that must be (re-)defined per-repo? Maybe not
> worth it, I don't know. I can't think of any other variable that
> should behave this way off the top of my head.

That's an interesting thought, but I'm not sure how it would work. The
ident variables are special in that people are often unhappy with the
fallback. What would it mean for somebody to say "do not proceed if
diff.renameLimit is not set", and where would we enforce that?

-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] Trick to force setup of a specific configured E-Mail per repo

2016-02-02 Thread Duy Nguyen
On Wed, Feb 3, 2016 at 10:56 AM, Jeff King  wrote:
> I find it disappointing that we go back to looking for magic sequences
> in the string. Could we perhaps do this more cleanly with a new config
> option? Like a "user.guessIdent" which defaults to true, but people can
> set to false. And without that, we do not do any automagic at all; we
> get the values from the GIT_COMMITTER_* environment or the
> user.{name,email} config variables, or we die().
>
> I think that should allow your use case (and extend the same feature to
> user.name). It wouldn't work on older versions of git, but nor would
> your fix here (the only way to do that is to re-instate "(none)" as
> magical).

Should we generalize this use case, i.e. define a list of
configuration variables that must be (re-)defined per-repo? Maybe not
worth it, I don't know. I can't think of any other variable that
should behave this way off the top of my head.
-- 
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: git object-count differs between clones

2016-02-02 Thread Jeff King
On Tue, Feb 02, 2016 at 11:22:08AM -0600, Andrew Martin wrote:

> Thanks for the clarification. I now ran "git repack -A" followed by 
> "git gc --prune=now", however I am still seeing the same number of objects. 
> What
> else can I try to successfully mark these and unreachable and garbage collect 
> them?

That should clear out any unreachable objects. Are we sure that the
objects in question are, in fact, unreachable?

Try:

  git rev-list --objects --all --reflog | wc -l

which should give a count of reachable objects. I'd expect that to line
up with that "git count-objects -v" reports after having run your gc
above.

In your original email, the discrepancy was between your "original"
repository and the one that had round-tripped to a clone. Is it possible
there are refs in the original that did not get pushed? Try comparing
"git for-each-ref" in each repository.

We also consider objects in the index to be reachable for packing. Could
your original perhaps have some uncommitted objects mentioned in the
index?

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


Re: git submodule should honor "-c credential.helper" command line argument

2016-02-02 Thread Jeff King
On Tue, Feb 02, 2016 at 06:13:14PM +0100, Marc Strapetz wrote:

> git -c credential.helper=helper submodule update --init submodule
> 
> does not invoke "helper", but falls back to the default strategies.
> When configuring in ~/.gitconfig:
> 
> [credential]
>   helper=helper
> 
> git submodule update --init submodule
> 
> works fine. This behavior is somewhat unexpected -- is this a bug or by
> intention? In case intention, what's the recommended way to "inject"
> credentials helpers to work on submodules without modifying Git's config
> files?
> 
> Tested with Git 2.5.0 (Windows).

I think the problem is that when git "switches" to working in the
submodule repository, it clears the environment, which includes any "-c"
command switches. This makes sense for some situations, but not for
others. This thread shows a similar problem:

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

Jens suggested there adding an option to tell clone to pass specific
variables to the submodule, which I think makes sense. AFAIK, nobody has
done any work yet on that approach.

-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 v2 2/1] support -4 and -6 switches for remote operations

2016-02-02 Thread Eric Wong
Jeff King  wrote:
>   OPT_SET_INT('4', "ipv4", &use_net,
>   N_("resolve IPv4 addresses only"), USE_NET_IPV4),
>   OPT_SET_INT('6', "ipv6", &use_net,
>   N_("resolve IPv6 addresses only"), USE_NET_IPV6),

Thanks, I missed OPT_SET_INT before.

> Using --no-ipv4 will set it back to USE_NET_ALL, which is probably OK.
> It will cancel a previous "--ipv4", which is logically consistent,
> though I guess some people might assume that "--no-ipv4" means "do not
> use ipv4 addresses". Supporting that would be more complicated.

Right, not worth the effort to support that and I prefer leaving
the "--no-" variants undocumented as an implementation artifact.

(no rush to review this, unlikely to be around the next few days)

-8<--
Subject: [PATCH] support -4 and -6 switches for remote operations

Sometimes it is necessary to force IPv4-only or IPv6-only
operation on networks where name lookups may return a
non-routable address and stall remote operations.

The ssh(1) command has an equivalent switches which we may
pass when we run them.  There may be old ssh(1) implementations
out there which do not support these switches; they should
report the appropriate error in that case.

rsync support is untouched for now since it is deprecated
and scheduled to be removed.

v2:
 - switch from boolean ints to enum
 - shorted CONNECT_* macro names to be consistent with switches
 - remove unneeded TRANS_OPT_* macros and usage
 - remove transport_set_family wrapper, enum is enough validation
 - s/resolve/use/ in documentation
 - document potential (but unconfirmed) problems with some ssh(1)

Signed-off-by: Eric Wong 
---
 Documentation/fetch-options.txt |  8 
 Documentation/git-push.txt  |  7 +++
 builtin/clone.c |  6 ++
 builtin/fetch.c |  6 ++
 builtin/push.c  |  6 ++
 connect.c   |  8 
 connect.h   |  2 ++
 http.c  |  9 +
 http.h  |  1 +
 remote-curl.c   | 13 +
 transport-helper.c  | 15 +++
 transport.c |  6 ++
 transport.h |  8 
 13 files changed, 95 insertions(+)

diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 952dfdf..036edfb 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -158,3 +158,11 @@ endif::git-pull[]
by default when it is attached to a terminal, unless -q
is specified. This flag forces progress status even if the
standard error stream is not directed to a terminal.
+
+-4::
+--ipv4::
+   Use IPv4 addresses only, ignoring IPv6 addresses.
+
+-6::
+--ipv6::
+   Use IPv6 addresses only, ignoring IPv4 addresses.
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 32482ce..669a357 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -277,6 +277,13 @@ origin +master` to force a push to the `master` branch). 
See the
default is --verify, giving the hook a chance to prevent the
push.  With --no-verify, the hook is bypassed completely.
 
+-4::
+--ipv4::
+   Use IPv4 addresses only, ignoring IPv6 addresses.
+
+-6::
+--ipv6::
+   Use IPv6 addresses only, ignoring IPv4 addresses.
 
 include::urls-remotes.txt[]
 
diff --git a/builtin/clone.c b/builtin/clone.c
index 81e238f..592e6db 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -47,6 +47,7 @@ static const char *real_git_dir;
 static char *option_upload_pack = "git-upload-pack";
 static int option_verbosity;
 static int option_progress = -1;
+static enum transport_family family;
 static struct string_list option_config;
 static struct string_list option_reference;
 static int option_dissociate;
@@ -92,6 +93,10 @@ static struct option builtin_clone_options[] = {
   N_("separate git dir from working tree")),
OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
N_("set config inside the new repository")),
+   OPT_SET_INT('4', "ipv4", &family, N_("use IPv4 addresses only"),
+   TRANSPORT_FAMILY_IPV4),
+   OPT_SET_INT('6', "ipv6", &family, N_("use IPv6 addresses only"),
+   TRANSPORT_FAMILY_IPV6),
OPT_END()
 };
 
@@ -970,6 +975,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
remote = remote_get(option_origin);
transport = transport_get(remote, remote->url[0]);
transport_set_verbosity(transport, option_verbosity, option_progress);
+   transport->family = family;
 
path = get_repo_path(remote->url[0], &is_bundle);
is_local = option_local != 0 && path && !is_bundle;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8e74213..03ba709 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -37,6 +37,7 

Re: [PATCH] Trick to force setup of a specific configured E-Mail per repo

2016-02-02 Thread Jeff King
On Tue, Feb 02, 2016 at 09:54:21PM +0200, Dan Aloni wrote:

> Previously, before 5498c57cdd63, many people did the following:
> 
>git config --global user.email "(none)"
> 
> This was helpful for people with more than one E-Mail address,
> targeting different E-Mail addresses for different clones.
> as it barred git from creating commit unless the user.email
> config was set in the per-clone config to the correct E-Mail
> address.
> 
> Now, since the original 'bug' was fixed, and practically every
> string is acceptable for user.email and user.name, it is best
> to reimplement the feature not as an exploit of a bug, but as
> an actual feature.

Just when I dare to think "somebody cannot possibly be relying on this
arcane behavior", I am proven wrong. :)

The motivating case for your patch makes sense to me. In the
implementation, though:

> + if (strict && email && !strcmp(email, "(per-repo)")) {
> + die("email is '(per-repo)', suggesting to set specific email "
> + "for the current repo");
> + }

I find it disappointing that we go back to looking for magic sequences
in the string. Could we perhaps do this more cleanly with a new config
option? Like a "user.guessIdent" which defaults to true, but people can
set to false. And without that, we do not do any automagic at all; we
get the values from the GIT_COMMITTER_* environment or the
user.{name,email} config variables, or we die().

I think that should allow your use case (and extend the same feature to
user.name). It wouldn't work on older versions of git, but nor would
your fix here (the only way to do that is to re-instate "(none)" as
magical).

-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] contrib/subtree: Split history with empty trees correctly

2016-02-02 Thread David A. Greene
Marcus Brinkmann  writes:

>> Are you still able to do a re-roll on this?
>
> I have to admit that my interest has declined steeply since
> discovering that subtree-split and filter-branch --subtree-filter give
> different results from "git svn" on the subdirectory.  The reason is
> that git-svn includes all commits for revisions that regular "svn log"
> gives on that directory, which includes commits that serve as branch
> points only or that are empty except for unhandled properties.

What do you mean by "branch points only?"

It's ok if you can't do a reroll.  I can't work on it right now but
perhaps when I get back to cleaning up the split code I can take what
you have and incoporate it.  I do very much appreciate your work on
this!

> While empty commits for unhandled properties wouldn't be fatal,
> missing branch points make "git svn" really unhappy when asked to
> rebuild .git/svn.

[ I may have misunderstood your intent, see below. ]

I just want to make sure I understand your situation.  You used git-svn
to mirror a project to git and then used git-subtree to incorporate that
mirror into a larger project?

Why is the split being done?  If there's an active Subversion repository
being mirrors it's much better to commit changes back to the Subversion
repository than to the git mirror.

> As migration from SVN is my main motivation at this point to use a
> subtree filter at this point (git-svn is just very slow - about one
> week on our repository), I am somewhat stuck and back to using
> git-svn. Although hacking up something with filter-branch seems like a
> remote option, it's probably nothing that generalizes.

Ok, maybe I misunderstood your situation.  Are you converting one big
repository via git-svn and then trying to break out individual
directories into smaller projects?

git-svn + git-subtree/git-filter-branch is not the best way to do that.
svn-all-fast-export is far superior for a one-off conversion and makes
splitting repositories a breeze.  It happens during conversion rather
than as a post-processing step.

https://techbase.kde.org/Projects/MoveToGit/UsingSvn2Git

> It didn't help that "make test" in contrib/subtree gives me 27 out of
> 29 failed tests (with no indication how to figure out what exactly
> failed).

Huh.  I don't know why that would happen.  Did you build the git tools
first?  A testing run using --debug and --verbose (see the Makefile in
contrib/subtree/t) would be informative.  I understand if you don't have
time to do that.  I haven't seen such failures before so I'm curious as
to what happened.

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


Re: [PATCH v3 4/4] restore_env(): free the saved environment variable once we are done

2016-02-02 Thread Junio C Hamano
Eric Sunshine  writes:

>> diff --git a/git.c b/git.c
>> @@ -54,10 +54,12 @@ static void restore_env(int external_alias)
>> if (external_alias &&
>> !strcmp(env_names[i], GIT_PREFIX_ENVIRONMENT))
>> continue;
>> -   if (orig_env[i])
>> +   if (orig_env[i]) {
>> setenv(env_names[i], orig_env[i], 1);
>> -   else
>> +   free(orig_env[i]);
>
> Now that this is "well-protected"[1] against incorrect nesting, you
> don't worry about the dangling pointers in static orig_env[], right?
> (The same for the dangling pointer in static 'orig_cwd' after being
> freed a bit earlier in this function, correct?)

Correct.

I do not think we follow a style that requires "after freeing memory
pointed at by a variable, the variable must be assigned NULL".
Would it be a good idea?  I do not see a point in it.

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 v3 4/4] restore_env(): free the saved environment variable once we are done

2016-02-02 Thread Eric Sunshine
On Tue, Feb 2, 2016 at 6:50 PM, Junio C Hamano  wrote:
> Just like we free orig_cwd, which is the value of the original
> working directory saved in save_env_before_alias(), once we are
> done with it, the contents of orig_env[] array, saved in the
> save_env_before_alias() function should be freed; otherwise,
> the second and subsequent calls to save/restore pair will leak
> the memory allocated in save_env_before_alias().
>
> Signed-off-by: Junio C Hamano 
> ---
> diff --git a/git.c b/git.c
> @@ -54,10 +54,12 @@ static void restore_env(int external_alias)
> if (external_alias &&
> !strcmp(env_names[i], GIT_PREFIX_ENVIRONMENT))
> continue;
> -   if (orig_env[i])
> +   if (orig_env[i]) {
> setenv(env_names[i], orig_env[i], 1);
> -   else
> +   free(orig_env[i]);

Now that this is "well-protected"[1] against incorrect nesting, you
don't worry about the dangling pointers in static orig_env[], right?
(The same for the dangling pointer in static 'orig_cwd' after being
freed a bit earlier in this function, correct?)

[1]: via assert()

> +   } else {
> unsetenv(env_names[i]);
> +   }
> }
>  }
--
To unsubscribe from this list: send the line "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 windows version Switch Caps Lock button issue on the input windows

2016-02-02 Thread Evel
When I try to press the 'Caps Lock' of the keyboard, and then 
try to input the uppercase string,

but after I switched to capital and finished input, I try to 
press the 'Caps Lock' button again, seems the input still keep 
capital letter for input,

Only when I move my cursor to the other input area and press the 
'Caps Lock' button again, then when I try to input in the Git 
console, then the issue will be fixed.

I am using the newest mintty 2.0.3(x86_64-pc-msys) and installed 
the Google Pinyin for input, Windows 7 64bit system.

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


What's cooking in git.git (Feb 2016, #01; Tue, 2)

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

I thought we reduced the number of stalled topics due to lack of
reviews reasonably low, but it seems to be increasing again. Help is
greatly appreciated.

On the 'master' front, we have managed to merge enough important
fixes; expect the first maintenance release for 2.7.x series
soonish.  Thanks for all bug reporters, contributors and reviewers.

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

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

--
[Graduated to "master"]

* ew/svn-1.9.0-auth (2016-01-26) 1 commit
  (merged to 'next' on 2016-01-28 at 61a4a8f)
 + git-svn: fix auth parameter handling on SVN 1.9.0+


* jc/strbuf-getline (2016-01-15) 9 commits
  (merged to 'next' on 2016-01-22 at 8c4e051)
 + strbuf: give strbuf_getline() to the "most text friendly" variant
 + checkout-index: there are only two possible line terminations
 + update-index: there are only two possible line terminations
 + check-ignore: there are only two possible line terminations
 + check-attr: there are only two possible line terminations
 + mktree: there are only two possible line terminations
 + strbuf: introduce strbuf_getline_{lf,nul}()
 + strbuf: make strbuf_getline_crlf() global
 + strbuf: miniscule style fix
 (this branch is used by jc/peace-with-crlf.)

 The preliminary clean-up for jc/peace-with-crlf topic.


* jk/filter-branch-no-index (2016-01-19) 1 commit
  (merged to 'next' on 2016-01-22 at 312aa2c)
 + filter-branch: resolve $commit^{tree} in no-index case

 A recent optimization to filter-branch in v2.7.0 introduced a
 regression when --prune-empty filter is used, which has been
 corrected.


* jk/list-tag-2.7-regression (2016-01-26) 2 commits
  (merged to 'next' on 2016-01-26 at fb9ccee)
 + tag: do not show ambiguous tag names as "tags/foo"
 + t6300: use test_atom for some un-modern tests
 (this branch is used by kn/ref-filter-atom-parsing.)

 "git tag" started listing a tag "foo" as "tags/foo" when a branch
 named "foo" exists in the same repository; remove this unnecessary
 disambiguation, which is a regression introduced in v2.7.0.


* jk/sanity (2016-01-19) 1 commit
  (merged to 'next' on 2016-01-22 at 612cc5f)
 + test-lib: clarify and tighten SANITY

 The description for SANITY prerequisite the test suite uses has
 been clarified both in the comment and in the implementation.


* jk/shortlog (2016-01-19) 7 commits
  (merged to 'next' on 2016-01-22 at f1c688c)
 + shortlog: don't warn on empty author
 + shortlog: optimize out useless string list
 + shortlog: optimize out useless "" normalization
 + shortlog: optimize "--summary" mode
 + shortlog: replace hand-parsing of author with pretty-printer
 + shortlog: use strbufs to read from stdin
 + shortlog: match both "Author:" and "author" on stdin

 "git shortlog" used to accumulate various pieces of information
 regardless of what was asked to be shown in the final output.  It
 has been optimized by noticing what need not to be collected
 (e.g. there is no need to collect the log messages when showing
 only the number of changes).


* js/msys2 (2016-01-15) 9 commits
  (merged to 'next' on 2016-01-22 at 8bab6ab)
 + mingw: uglify (a, 0) definitions to shut up warnings
 + mingw: squash another warning about a cast
 + mingw: avoid warnings when casting HANDLEs to int
 + mingw: avoid redefining S_* constants
 + compat/winansi: support compiling with MSys2
 + compat/mingw: support MSys2-based MinGW build
 + nedmalloc: allow compiling with MSys2's compiler
 + config.mak.uname: supporting 64-bit MSys2
 + config.mak.uname: support MSys2
 (this branch is used by js/mingw-tests.)

 Beginning of the upstreaming process of Git for Windows effort.


* tk/interpret-trailers-in-place (2016-01-14) 2 commits
  (merged to 'next' on 2016-01-22 at 5db0cf8)
 + interpret-trailers: add option for in-place editing
 + trailer: allow to write to files other than stdout

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

--
[New Topics]

* aw/push-force-with-lease-reporting (2016-02-01) 1 commit
 - push: fix ref status reporting for --force-with-lease

 "git push --force-with-lease" has been taught to report if the push
 needed to force (or fast-forwarded).

 Will merge to 'next'.


* ew/connect-verbose (2016-01-28) 1 commit
 - pass transport verbosity down to git_connect

 There were a few "now I am doing this thing" progress messages in
 the TCP connection code that can be triggered by setting a verbose
 option internally in the code, but "git fetch -v" and friends never
 passed the 

Re: [PATCH] log -g: ignore revision parameters that have no reflog

2016-02-02 Thread Junio C Hamano
Dennis Kaarsemaker  writes:

> + if (revs->reflog_info) {
> + /*
> +  * The reflog iterator gets confused when fed things that don't
> +  * have reflogs. Help it along a bit
> +  */
> + if (strchr(arg, '@') != arg &&

Is this merely an expensive way to write *arg != '@', or is there
something else I am missing?

> + !dwim_ref(arg, strchrnul(arg, '@')-arg, sha1, &dotdot))
> + die("only refs can have reflogs");

Is "foo@23" a forbidden branch name?

Is this looking for a dotdot?  If you are introducing a new scope,
you can afford to invent a variable with a name that reflects its
purpose.

Style: a binary operation like '-' (subtract) have SP on both sides
of it.

> + if(!reflog_exists(dotdot))

Style: one SP between a syntactic keyword like 'if' and opening
parenthesis is required.

I have a suspicion that in your final "fixed" code, it may be a
better design not to let the command line argument for "-g"
processing pass through this function at all.

For example, what should "git log -g master next" do?  Merge two
reflog entries in chronological order and show each of them as if
they are thrown at "git show" one by one?  Does that mesh well with
other options like "--date-order/--topo-order"?

For another example, what should "git log -g master..next" do?

Or "git log -g master^^^"?

These are merely a few example inputs I can think of off in 5
seconds and I think none of the above makes much sense, but parsing
these is the primary purpose of this function.

So, I dunno.  I gave a few "coding" comments, but I am not sure if
you are touching the right codepath in the first place.
--
To unsubscribe from this list: send the line "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/4] restore_env(): free the saved environment variable once we are done

2016-02-02 Thread Junio C Hamano
Just like we free orig_cwd, which is the value of the original
working directory saved in save_env_before_alias(), once we are
done with it, the contents of orig_env[] array, saved in the
save_env_before_alias() function should be freed; otherwise,
the second and subsequent calls to save/restore pair will leak
the memory allocated in save_env_before_alias().

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

 * This is new.

 git.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/git.c b/git.c
index 1cbe267..93f569d 100644
--- a/git.c
+++ b/git.c
@@ -54,10 +54,12 @@ static void restore_env(int external_alias)
if (external_alias &&
!strcmp(env_names[i], GIT_PREFIX_ENVIRONMENT))
continue;
-   if (orig_env[i])
+   if (orig_env[i]) {
setenv(env_names[i], orig_env[i], 1);
-   else
+   free(orig_env[i]);
+   } else {
unsetenv(env_names[i]);
+   }
}
 }
 
-- 
2.7.0-391-gcd29568

--
To unsubscribe from this list: send the line "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/4] git: simplify environment save/restore logic

2016-02-02 Thread Junio C Hamano
The only code that cares about the value of the global variable
saved_env_before_alias after the previous fix is handle_builtin()
that turns into a glorified no-op when the variable is true, so the
logic could safely be lifted to its caller, i.e. the caller can
refrain from calling it when the variable is set.

This variable tells us if save_env_before_alias() was called (with
or without matching restore_env()), but the sole caller of the
function, handle_alias(), always calls it as the first thing, so we
can consider that the variable essentially keeps track of the fact
that handle_alias() has ever been called.

It turns out that handle_builtin() and handle_alias() are called
only from one function in a way that the value of the variable
matters, which is run_argv(), and it already keeps track of the
fact that it already called handle_alias().

So we can simplify the whole thing by:

- Change handle_builtin() to always make a direct call to the
  builtin implementation it finds, and make sure the caller
  refrains from calling it if handle_alias() has ever been
  called;

- Remove saved_env_before_alias variable, and instead use the
  local "done_alias" variable maintained inside run_argv() to
  make the same decision.

Signed-off-by: Junio C Hamano 
---
 git.c | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/git.c b/git.c
index e39b972..1cbe267 100644
--- a/git.c
+++ b/git.c
@@ -25,13 +25,11 @@ static const char *env_names[] = {
GIT_PREFIX_ENVIRONMENT
 };
 static char *orig_env[4];
-static int saved_env_before_alias;
 static int save_restore_env_balance;
 
 static void save_env_before_alias(void)
 {
int i;
-   saved_env_before_alias = 1;
 
assert(save_restore_env_balance == 0);
save_restore_env_balance = 1;
@@ -533,16 +531,8 @@ static void handle_builtin(int argc, const char **argv)
}
 
builtin = get_builtin(cmd);
-   if (builtin) {
-   /*
-* XXX: if we can figure out cases where it is _safe_
-* to do, we can avoid spawning a new process when
-* saved_env_before_alias is true
-* (i.e. setup_git_dir* has been run once)
-*/
-   if (!saved_env_before_alias)
-   exit(run_builtin(builtin, argc, argv));
-   }
+   if (builtin)
+   exit(run_builtin(builtin, argc, argv));
 }
 
 static void execv_dashed_external(const char **argv)
@@ -586,8 +576,17 @@ static int run_argv(int *argcp, const char ***argv)
int done_alias = 0;
 
while (1) {
-   /* See if it's a builtin */
-   handle_builtin(*argcp, *argv);
+   /*
+* If we tried alias and futzed with our environment,
+* it no longer is safe to invoke builtins directly in
+* general.  We have to spawn them as dashed externals.
+*
+* NEEDSWORK: if we can figure out cases
+* where it is safe to do, we can avoid spawning a new
+* process.
+*/
+   if (!done_alias)
+   handle_builtin(*argcp, *argv);
 
/* .. then try the external ones */
execv_dashed_external(*argv);
-- 
2.7.0-391-gcd29568

--
To unsubscribe from this list: send the line "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/4] git: protect against unbalanced calls to {save,restore}_env()

2016-02-02 Thread Junio C Hamano
We made sure that save_env_before_alias() does not skip saving the
environment when asked to (which led to use-after-free of orig_cwd
in restore_env() in the buggy version) with the previous step.

Protect against future breakage where somebody adds new callers of
these functions in an unbalanced fashion.

Signed-off-by: Junio C Hamano 
---
 git.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/git.c b/git.c
index a57a4cb..e39b972 100644
--- a/git.c
+++ b/git.c
@@ -26,11 +26,15 @@ static const char *env_names[] = {
 };
 static char *orig_env[4];
 static int saved_env_before_alias;
+static int save_restore_env_balance;
 
 static void save_env_before_alias(void)
 {
int i;
saved_env_before_alias = 1;
+
+   assert(save_restore_env_balance == 0);
+   save_restore_env_balance = 1;
orig_cwd = xgetcwd();
for (i = 0; i < ARRAY_SIZE(env_names); i++) {
orig_env[i] = getenv(env_names[i]);
@@ -42,6 +46,9 @@ static void save_env_before_alias(void)
 static void restore_env(int external_alias)
 {
int i;
+
+   assert(save_restore_env_balance == 1);
+   save_restore_env_balance = 0;
if (!external_alias && orig_cwd && chdir(orig_cwd))
die_errno("could not move to %s", orig_cwd);
free(orig_cwd);
-- 
2.7.0-391-gcd29568

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


[PATCH v3 1/4] git: remove an early return from save_env_before_alias()

2016-02-02 Thread Junio C Hamano
When help.autocorrect is in effect, an attempt to auto-execute an
uniquely corrected result of a misspelt alias will result in an
irrelevant error message.  The codepath that causes this calls
save_env_before_alias() and restore_env() in handle_alias(), and
that happens twice.  A global variable orig_cwd is allocated to hold
the return value of getcwd() in save_env_before_alias(), which is
then used in restore_env() to go back to that directory and finally
free(3)'d there.

However, save_env_before_alias() is not prepared to be called twice.
It returns early when it knows it has already been called, leaving
orig_cwd undefined, which is then checked in the second call to
restore_env(), and by that time, the memory that used to hold the
contents of orig_cwd is either freed or reused to hold something
else, and this is fed to chdir(2), causing it to fail.  Even if it
did not fail (i.e. reading of the already free'd piece of memory
yielded a directory path that we can chdir(2) to), it then gets
free(3)'d.

Fix this by making sure save_env() does do the saving when called.

While at it, add a minimal test for help.autocorrect facility.

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

 * For another review round before deciding what to write in the
   upcoming "What's cooking" report.

 git.c   |  2 --
 t/t9003-help-autocorrect.sh | 52 +
 2 files changed, 52 insertions(+), 2 deletions(-)
 create mode 100755 t/t9003-help-autocorrect.sh

diff --git a/git.c b/git.c
index 98d4412..a57a4cb 100644
--- a/git.c
+++ b/git.c
@@ -30,8 +30,6 @@ static int saved_env_before_alias;
 static void save_env_before_alias(void)
 {
int i;
-   if (saved_env_before_alias)
-   return;
saved_env_before_alias = 1;
orig_cwd = xgetcwd();
for (i = 0; i < ARRAY_SIZE(env_names); i++) {
diff --git a/t/t9003-help-autocorrect.sh b/t/t9003-help-autocorrect.sh
new file mode 100755
index 000..dfe95c9
--- /dev/null
+++ b/t/t9003-help-autocorrect.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='help.autocorrect finding a match'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+   # An alias
+   git config alias.lgf "log --format=%s --first-parent" &&
+
+   # A random user-defined command
+   write_script git-distimdistim <<-EOF &&
+   echo distimdistim was called
+   EOF
+
+   PATH="$PATH:." &&
+   export PATH &&
+
+   git commit --allow-empty -m "a single log entry" &&
+
+   # Sanity check
+   git lgf >actual &&
+   echo "a single log entry" >expect &&
+   test_cmp expect actual &&
+
+   git distimdistim >actual &&
+   echo "distimdistim was called" >expect &&
+   test_cmp expect actual
+'
+
+test_expect_success 'autocorrect showing candidates' '
+   git config help.autocorrect 0 &&
+
+   test_must_fail git lfg 2>actual &&
+   sed -e "1,/^Did you mean this/d" actual | grep lgf &&
+
+   test_must_fail git distimdist 2>actual &&
+   sed -e "1,/^Did you mean this/d" actual | grep distimdistim
+'
+
+test_expect_success 'autocorrect running commands' '
+   git config help.autocorrect -1 &&
+
+   git lfg >actual &&
+   echo "a single log entry" >expect &&
+   test_cmp expect actual &&
+
+   git distimdist >actual &&
+   echo "distimdistim was called" >expect &&
+   test_cmp expect actual
+'
+
+test_done
-- 
2.7.0-391-gcd29568

--
To unsubscribe from this list: send the line "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] log -g: ignore revision parameters that have no reflog

2016-02-02 Thread Dennis Kaarsemaker
git log -g (and by extension, git reflog) gets mightly confused when
trying to display the reflog of something that is not a ref that has a
reflog. We can help by teaching handle_revision_arg to check all
revision arguments for reflog existence if it's in reflog mode.

git log -g something-that-is-not-a ref makes no sense, so let's die when
the user is trying that. git log -g ref-that-has-no-reflog is perfectly
sensible, so we just ignore it.

Signed-off-by: Dennis Kaarsemaker 
---
 revision.c | 12 
 t/t1411-reflog-show.sh | 10 ++
 2 files changed, 22 insertions(+)

diff --git a/revision.c b/revision.c
index 0a282f5..43182c6 100644
--- a/revision.c
+++ b/revision.c
@@ -1498,6 +1498,18 @@ int handle_revision_arg(const char *arg_, struct 
rev_info *revs, int flags, unsi
 
flags = flags & UNINTERESTING ? flags | BOTTOM : flags & ~BOTTOM;
 
+   if (revs->reflog_info) {
+   /*
+* The reflog iterator gets confused when fed things that don't
+* have reflogs. Help it along a bit
+*/
+   if (strchr(arg, '@') != arg &&
+   !dwim_ref(arg, strchrnul(arg, '@')-arg, sha1, &dotdot))
+   die("only refs can have reflogs");
+   if(!reflog_exists(dotdot))
+   return 0;
+   }
+
dotdot = strstr(arg, "..");
if (dotdot) {
unsigned char from_sha1[20];
diff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh
index 6ac7734..e55518f 100755
--- a/t/t1411-reflog-show.sh
+++ b/t/t1411-reflog-show.sh
@@ -171,4 +171,14 @@ test_expect_success 'reflog exists works' '
! git reflog exists refs/heads/nonexistent
 '
 
+test_expect_success 'reflog against non-ref dies' '
+   test_must_fail git reflog HEAD^
+'
+
+test_expect_success 'reflog against ref with no log is empty' '
+   git tag nolog &&
+   git reflog nolog > actual &&
+   test_line_count = 0 actual
+'
+
 test_done
-- 
2.7.0-345-gadc6f59

--
To unsubscribe from this list: send the line "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

2016-02-02 Thread brian m. carlson
On Tue, Feb 02, 2016 at 12:37:19PM -0800, Junio C Hamano wrote:
> Dmitry Vilkov  writes:
> 
> > This is fix of bug introduced by 4dbe66464 commit.
> 
> That would be 4dbe6646 (remote-curl: fall back to Basic auth if
> Negotiate fails, 2015-01-08) that appears in v2.3.1 and onward.
> 
> > The problem is that when username/password combination was not set,
> > the first HTTP(S) request will fail and user will be asked for
> > credentials. As a side effect of first HTTP(S) request, libcurl auth
> > method GSS-Negotiate will be disabled unconditionally. Although, we
> > haven't tried yet provided credentials for this auth method.

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?

> Brian, comments?  Here is what you wrote in that commit:
> 
> If Basic and something else are offered, libcurl will never
> attempt to use Basic, even if the other option fails.  Teach the
> HTTP client code to stop trying authentication mechanisms that
> don't use a password (currently Negotiate) after the first
> failure, since if they failed the first time, they will never
> succeed.

I think what's happening here is no username is ever provided, so
libcurl never tries authentication in the first place.
-- 
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] travis-ci: run previously failed tests first, then slowest to fastest

2016-02-02 Thread Junio C Hamano
Clemens Buchacher  writes:

> On Mon, Feb 01, 2016 at 10:17:24AM -0800, Junio C Hamano wrote:
>> 
>> Your proposal is to redefine "is the working tree dirty?"; it would
>> check if "git checkout -f" would change what is in the working tree.
>
> I like this definition. Sounds obviously right.

So this is an illustration.  The change to ce_compare_data() has a
room for improvement in that it assumes that it can always slurp the
whole blob in-core; it should try to use the streaming interface
when it makes sense.  Otherwise we would not be able to handle a
blob that we used to be able to (as index_fd() streams), which would
be a regression.

The change to t0023 is merely an example that shows that existing
tests assume the convert_to_git() way of defining the dirtyness of
the working tree.  It used to be OK to have core.autocrlf set to true,
have LF terminated file on the working tree and add it to the index,
and the resulting state was "We just added it to the index, and
nobody touched the index nor the working tree file--by definition
the working tree IS CLEAN".  With your updated semantics, that no
longer is true.  "We just added it, but if we check it out, we would
normalize the line ending to be CRLF on the working tree, so the
working tree is dirty" is what happens.

There are tons of tests that would break the same way all of which
needs to be looked at and fixed if we were to go in this direction.


 read-cache.c   | 53 +
 t/t0023-crlf-am.sh |  2 +-
 2 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 84616c8..c284f78 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -156,16 +156,61 @@ void fill_stat_cache_info(struct cache_entry *ce, struct 
stat *st)
ce_mark_uptodate(ce);
 }
 
+/*
+ * Compare the data in buf with the data in the file pointed by fd and
+ * return 0 if they are identical, and non-zero if they differ.
+ */
+static int compare_with_fd(const char *input, ssize_t len, int fd)
+{
+   for (;;) {
+   char buf[1024 * 16];
+   ssize_t chunk_len, read_len;
+
+   chunk_len = sizeof(buf) < len ? sizeof(buf) : len;
+   read_len = xread(fd, buf, chunk_len ? chunk_len : 1);
+
+   if (!read_len)
+   /* EOF on the working tree file */
+   return !len ? 0 : -1;
+
+   if (!len)
+   /* we expected there is nothing left */
+   return -1;
+
+   if (memcmp(buf, input, read_len))
+   return -1;
+   input += read_len;
+   len -= read_len;
+   }
+}
+
+/*
+ * Does the file in the working tree match what is in the index?
+ * That is, do we lose any data from the working tree copy if we
+ * did a new "git checkout" of that path out of the index?
+ */
 static int ce_compare_data(const struct cache_entry *ce, struct stat *st)
 {
int match = -1;
int fd = open(ce->name, O_RDONLY);
 
if (fd >= 0) {
-   unsigned char sha1[20];
-   if (!index_fd(sha1, fd, st, OBJ_BLOB, ce->name, 0))
-   match = hashcmp(sha1, ce->sha1);
-   /* index_fd() closed the file descriptor already */
+   enum object_type type;
+   unsigned long size;
+   void *data = read_sha1_file(ce->sha1, &type, &size);
+
+   if (type == OBJ_BLOB) {
+   struct strbuf worktree = STRBUF_INIT;
+   if (convert_to_working_tree(ce->name, data, size,
+   &worktree)) {
+   free(data);
+   data = strbuf_detach(&worktree, &size);
+   }
+   if (!compare_with_fd(data, size, fd))
+   match = 0;
+   }
+   free(data);
+   close(fd);
}
return match;
 }
diff --git a/t/t0023-crlf-am.sh b/t/t0023-crlf-am.sh
index f9bbb91..5c086b4 100755
--- a/t/t0023-crlf-am.sh
+++ b/t/t0023-crlf-am.sh
@@ -27,7 +27,7 @@ EOF
 test_expect_success 'setup' '
 
git config core.autocrlf true &&
-   echo foo >bar &&
+   printf "%s\r\n" foo >bar &&
git add bar &&
test_tick &&
git commit -m initial

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


Re: [PATCH v3 00/20] refs backend rebase on pu

2016-02-02 Thread Junio C Hamano
David Turner  writes:

> Are there any more reviews on this?  I do have some changes from this
> set, but they're pretty minor so I don't want to post a new one (unless
> folks would rather see those changes before reviewing).  Let me know.

Thanks for pinging.  As this is a rather wide-ranging topic, it was
not practical to intergrate with the rest of the topics in flight
back then, but now it seems that this needs only one topic that
still is in flight.  I'll queue this on top of a merge between
'master' and the tip of 'sb/submodule-parallel-update' and include
in the daily integration cycle to make it easy for people to view
the changes in wider context as necessary.





--
To unsubscribe from this list: send the line "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 3/6] convert.c: Remove input_crlf_action()

2016-02-02 Thread Junio C Hamano
tbo...@web.de writes:

> From: Torsten Bögershausen 
>
> Integrate the code of input_crlf_action() into convert_attrs(),
> so that ca.crlf_action is aleays valid after calling convert_attrs().

s/aleays/always/

> 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 | 35 +--
>  1 file changed, 13 insertions(+), 22 deletions(-)
>
> diff --git a/convert.c b/convert.c
> index a24c2a2..bca3d0c 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)) {
> + enum eol eol_attr;
>   ca->crlf_action = git_path_check_crlf( ccheck + 4);

I guess this extra SP after opening paren was a misconversion from
the previous step...

>   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(&ca, 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))

So the idea is that the code before this prepared eol_attr and
crlf_action fields in convert_attrs() and consumers of these fields
called input_crlf_action() with t

Re: [PATCH v1 2/6] convert.c: Remove path when not needed

2016-02-02 Thread Junio C Hamano
tbo...@web.de writes:


> Subject: Re: [PATCH v1 2/6] convert.c: Remove path when not needed

At least s/: Remove/: remove/, but I think

s/: Remove.*/: remove unused parameter 'path'/

would be easier to read.

All of these functions seem to have "path" in their name, sounding
as if they meant to ask "What is the crlf-ness for THIS PATH?"  Is
it still sensible to have "path" in their names?

The necessary information that is specific to the path was already
gathered when we prepared the 'check' thing, so they are still
asking and answering their questions specific to the path by
accepting 'check', but it still feels somewhat funny.

With or without "path" in their names, they are horribly misnamed
(e.g. "check crlf---are we asking if the file has crlf?  if the
file must be written out with crlf?  something else?").

Your later patches seem to refactor and reorganize this internal
calling mess in some way, so let's read on and see how the whole
thing improves ;-)

> -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;
--
To unsubscribe from this list: send the line "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 1/6] t0027: Add tests for get_stream_filter()

2016-02-02 Thread Junio C Hamano
tbo...@web.de writes:

> 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 
> ---
>  t/t0027-auto-crlf.sh | 262 
> ---
>  1 file changed, 146 insertions(+), 116 deletions(-)
>
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> index 504e5a0..681f0c5 100755
> --- a/t/t0027-auto-crlf.sh
> +++ b/t/t0027-auto-crlf.sh
> @@ -21,32 +21,45 @@ compare_ws_file () {
>   pfx=$1
>   exp=$2.expect
>   act=$pfx.actual.$3
> - tr '\015\000' QN <"$2" >"$exp" &&
> - tr '\015\000' QN <"$3" >"$act" &&
> + tr '\015\000abcdef01234567890' QN0 <"$2" >"$exp" &&
> + tr '\015\000abcdef01234567890' QN0 <"$3" >"$act" &&

'0' is listed twice on purpose?

>   test_cmp $exp $act &&
>   rm $exp $act

As we are not forcing all the future callers of this helper
functions to feed only non problematic pathnames, we should quote
these variable references, but this is not a new issue this patch
introduces.

>  }
>  
>  create_gitattributes () {
>   attr=$1
> + ident=$2
> + case "$2" in
> + "")
> + >.gitattributes
> + ;;
> + i)
> + echo "* ident" >.gitattributes
> + ;;
> + *)
> + echo >&2 invalid ident: $2
> + exit 1

This is overindented.  Case arms align with case/esac, and their
bodies indent one level down, i.e.

case "$2" in
"")
>.gitattributes
;;
esac

By the way, I somehow find it hard to follow to assign magic
meanings "clear" and "prepare ident for everybody" to a short
and cryptic "" (empty string) and "i".  If you were taking advantage
of the fact that all existing uses of this helper function pass only
one argument to it and you will get $ident="" automatically from the
ones that you did not touch, that might be a reasonable way to reduce
patch noise, but because you are doing things like this

> - create_gitattributes "$attr" &&
> + create_gitattributes "$attr" "" &&

anyway, it may make sense to be slightly more verbose and readable.

> + esac
> +
>   case "$attr" in
>   auto)
> - echo "*.txt text=auto" >.gitattributes
> + echo "*.txt text=auto" >>.gitattributes
>   ;;
>   ...
>   ;;
>   *)
>   echo >&2 invalid attribute: $attr

As you are touching this helper to emit more than one thing into the
file, I'd consider doing 

{
case "$2" in
clear)  ;;
ident)  echo '* ident' ;;
...
esac &&
case "$1" in
auto)   echo '*.txt auto' ;;
...
esac
} >.gitattributes

or even

{
while test "$#" != 1
do
printf "%s\n" "$1"
shift
done &&
case "$1" in
auto)   echo '*.txt auto' ;;
...
esac &&
} >.gitattributes

anticipating that we may want more things added to the output in the
future.  The latter form allows your callers to say things like

create_gitattributes "auto" "* ident" "*.bin binary" 

without having to touch the helper 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 v4 00/15] config: make git_config_set die on failure

2016-02-02 Thread Junio C Hamano
Patrick Steinhardt  writes:

> Patrick Steinhardt (15):
>   config: introduce set_or_die wrappers
>   branch: die on error in setting up tracking branch
>   branch: die on config error when unsetting upstream
>   branch: die on config error when editing branch description
>   submodule: die on config error when linking modules
>   submodule--helper: die on config error when cloning module
>   remote: die on config error when setting URL
>   remote: die on config error when setting/adding branches
>   remote: die on config error when manipulating remotes
>   clone: die on config error in cmd_clone
>   init-db: die on config errors when initializing empty repo
>   sequencer: die on config error when saving replay opts
>   compat: die when unable to set core.precomposeunicode
>   config: rename git_config_set to git_config_set_gently
>   config: rename git_config_set_or_die to git_config_set

They mostly looked sensible; I commented on things that I noticed,
but I don't think I spotted anything that needs a huge fix.

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 v4 10/15] clone: die on config error in cmd_clone

2016-02-02 Thread Junio C Hamano
Patrick Steinhardt  writes:

> The clone command does not check for error codes returned by
> `git_config_set` functions. This may cause the user to end up
> with an inconsistent repository without any indication with what
> went wrong.
>
> Fix this problem by dying with an error message when we are
> unable to write the configuration files to disk.

When this happens, the junk_mode is still JUNK_LEAVE_NONE, so upon
hitting such an error, we'd remove everything and die.  And we
haven't wasted the effort for large object transfer yet.

Which all sounds sensible.

> Signed-off-by: Patrick Steinhardt 
> ---
>  builtin/clone.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 81e238f..f2a2f9a 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -786,12 +786,12 @@ static void write_refspec_config(const char 
> *src_ref_prefix,
>   /* Configure the remote */
>   if (value.len) {
>   strbuf_addf(&key, "remote.%s.fetch", option_origin);
> - git_config_set_multivar(key.buf, value.buf, "^$", 0);
> + git_config_set_multivar_or_die(key.buf, value.buf, 
> "^$", 0);
>   strbuf_reset(&key);
>  
>   if (option_mirror) {
>   strbuf_addf(&key, "remote.%s.mirror", 
> option_origin);
> - git_config_set(key.buf, "true");
> + git_config_set_or_die(key.buf, "true");
>   strbuf_reset(&key);
>   }
>   }
> @@ -949,14 +949,14 @@ int cmd_clone(int argc, const char **argv, const char 
> *prefix)
>   src_ref_prefix = "refs/";
>   strbuf_addstr(&branch_top, src_ref_prefix);
>  
> - git_config_set("core.bare", "true");
> + git_config_set_or_die("core.bare", "true");
>   } else {
>   strbuf_addf(&branch_top, "refs/remotes/%s/", option_origin);
>   }
>  
>   strbuf_addf(&value, "+%s*:%s*", src_ref_prefix, branch_top.buf);
>   strbuf_addf(&key, "remote.%s.url", option_origin);
> - git_config_set(key.buf, repo);
> + git_config_set_or_die(key.buf, repo);
>   strbuf_reset(&key);
>  
>   if (option_reference.nr)
--
To unsubscribe from this list: send the line "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 v4 02/15] branch: die on error in setting up tracking branch

2016-02-02 Thread Junio C Hamano
Patrick Steinhardt  writes:

> The setup_tracking function calls install_branch_config, which
> may fail writing the configuration due to a locked configuration
> file or other error conditions. setup_tracking can also fail when
> trying to track ambiguous information for a reference. While this
> condition is checked for and an error code is returned, this
> error is not checked by the caller.
>
> Fix both issues by dying early when error occur.

Hmph.  I think create_branch() is written in such a way that all
die() come before the actual ref transaction attempts to create the
branch, but this change means that we have already created the
branch and then die without undoing the damage that is already done.

"The error is not checked by the caller" is very true, but can the
caller do something better than just die?  I personally do not think
it is such a big deal if we just died here, but I may have overlooked
something.

> Signed-off-by: Patrick Steinhardt 
> ---
>  branch.c  | 19 +--
>  branch.h  |  1 +
>  t/t3200-branch.sh |  9 -
>  3 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/branch.c b/branch.c
> index 7ff3f20..7106369 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -64,16 +64,16 @@ void install_branch_config(int flag, const char *local, 
> const char *origin, cons
>   }
>  
>   strbuf_addf(&key, "branch.%s.remote", local);
> - git_config_set(key.buf, origin ? origin : ".");
> + git_config_set_or_die(key.buf, origin ? origin : ".");
>  
>   strbuf_reset(&key);
>   strbuf_addf(&key, "branch.%s.merge", local);
> - git_config_set(key.buf, remote);
> + git_config_set_or_die(key.buf, remote);
>  
>   if (rebasing) {
>   strbuf_reset(&key);
>   strbuf_addf(&key, "branch.%s.rebase", local);
> - git_config_set(key.buf, "true");
> + git_config_set_or_die(key.buf, "true");
>   }
>   strbuf_release(&key);
>  
> @@ -109,8 +109,8 @@ void install_branch_config(int flag, const char *local, 
> const char *origin, cons
>   * to infer the settings for branch..{remote,merge} from the
>   * config.
>   */
> -static int setup_tracking(const char *new_ref, const char *orig_ref,
> -   enum branch_track track, int quiet)
> +static void setup_tracking(const char *new_ref, const char *orig_ref,
> +enum branch_track track, int quiet)
>  {
>   struct tracking tracking;
>   int config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;
> @@ -118,7 +118,7 @@ static int setup_tracking(const char *new_ref, const char 
> *orig_ref,
>   memset(&tracking, 0, sizeof(tracking));
>   tracking.spec.dst = (char *)orig_ref;
>   if (for_each_remote(find_tracked_branch, &tracking))
> - return 1;
> + return;
>  
>   if (!tracking.matches)
>   switch (track) {
> @@ -127,18 +127,17 @@ static int setup_tracking(const char *new_ref, const 
> char *orig_ref,
>   case BRANCH_TRACK_OVERRIDE:
>   break;
>   default:
> - return 1;
> + return;
>   }
>  
>   if (tracking.matches > 1)
> - return error(_("Not tracking: ambiguous information for ref 
> %s"),
> - orig_ref);
> + die(_("Not tracking: ambiguous information for ref %s"),
> + orig_ref);
>  
>   install_branch_config(config_flags, new_ref, tracking.remote,
> tracking.src ? tracking.src : orig_ref);
>  
>   free(tracking.src);
> - return 0;
>  }
>  
>  int read_branch_desc(struct strbuf *buf, const char *branch_name)
> diff --git a/branch.h b/branch.h
> index 58aa45f..8ce22f8 100644
> --- a/branch.h
> +++ b/branch.h
> @@ -43,6 +43,7 @@ void remove_branch_state(void);
>  /*
>   * Configure local branch "local" as downstream to branch "remote"
>   * from remote "origin".  Used by git branch --set-upstream.
> + * Dies if unable to install branch config.
>   */
>  #define BRANCH_CONFIG_VERBOSE 01
>  extern void install_branch_config(int flag, const char *local, const char 
> *origin, const char *remote);
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index cdaf6f6..dd776b3 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -446,6 +446,13 @@ test_expect_success '--set-upstream-to fails on a 
> non-ref' '
>   test_must_fail git branch --set-upstream-to HEAD^{}
>  '
>  
> +test_expect_success '--set-upstream-to fails on locked config' '
> + test_when_finished "rm -f .git/config.lock" &&
> + >.git/config.lock &&
> + git branch locked &&
> + test_must_fail git branch --set-upstream-to locked
> +'
> +
>  test_expect_success 'use --set-upstream-to modify HEAD' '
>   test_config branch.master.remote foo &&
>   test_config branch.master.merge foo &&
> @@ -579,7 +586,7 @@ test_expect_success 'avoid ambiguous track' '

Re: git log -g bizarre behaviour

2016-02-02 Thread Junio C Hamano
Dennis Kaarsemaker  writes:

>> > $ git (log -g|reflog) v2.7.0
>> > From the bizarre behaviour above to a silent noop.
>
> As I demonstrated in the text that you cut: that is not true.
> git log -g v2.7.0 and git reflog v2.7.0 are *not* silent, but buggy. I
> would like to make them silent.

Ah, sorry, I totally misread your "bizarre".  Yes, "log -g" that
walks the history not the reflog is "bizarre" and wrong, which we
already agreed in the previous exchange.  A fixed behaviour that
walks only the reflog entries should become a "silent noop".
--
To unsubscribe from this list: send the line "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

2016-02-02 Thread Junio C Hamano
Dmitry Vilkov  writes:

> This is fix of bug introduced by 4dbe66464 commit.

That would be 4dbe6646 (remote-curl: fall back to Basic auth if
Negotiate fails, 2015-01-08) that appears in v2.3.1 and onward.

> The problem is that when username/password combination was not set,
> the first HTTP(S) request will fail and user will be asked for
> credentials. As a side effect of first HTTP(S) request, libcurl auth
> method GSS-Negotiate will be disabled unconditionally. Although, we
> haven't tried yet provided credentials for this auth method.

Brian, comments?  Here is what you wrote in that commit:

If Basic and something else are offered, libcurl will never
attempt to use Basic, even if the other option fails.  Teach the
HTTP client code to stop trying authentication mechanisms that
don't use a password (currently Negotiate) after the first
failure, since if they failed the first time, they will never
succeed.

Thanks.

> Signed-off-by: Dmitry Vilkov 
> ---
>  http.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/http.c b/http.c
> index 0da9e66..707ea84 100644
> --- a/http.c
> +++ b/http.c
> @@ -951,12 +951,15 @@ static int handle_curl_result(struct slot_results 
> *results)
>   return HTTP_MISSING_TARGET;
>   else if (results->http_code == 401) {
>   if (http_auth.username && http_auth.password) {
> +#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
> + if (http_auth_methods & CURLAUTH_GSSNEGOTIATE) {
> + http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
> + return HTTP_REAUTH;
> + }
> +#endif
>   credential_reject(&http_auth);
>   return HTTP_NOAUTH;
>   } else {
> -#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
> - http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
> -#endif
>   return HTTP_REAUTH;
>   }
>   } else {
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git log -g bizarre behaviour

2016-02-02 Thread Dennis Kaarsemaker
On di, 2016-02-02 at 11:32 -0800, Junio C Hamano wrote:
> Dennis Kaarsemaker  writes:
> 
> > On ma, 2016-02-01 at 15:37 -0800, Junio C Hamano wrote:
> > 
> > > Do you mean
> > > 
> > >   $ git checkout -b testing
> > > $ rm -f .git/logs/refs/heads/testing
> > > $ git log -g testing
> > > 
> > > will be changed from a silent no-op to an abort with error?
> > > 
> > > I do not see a need for such a change--does that count as an
> > > objection?
> > 
> > No, I'd like to change:
> > 
> > $ ls .git/logs/refs/tags/v2.7.0
> > ls: cannot access .git/logs/refs/tags/v2.7.0: No such file or
> > directory
> > $ git (log -g|reflog) v2.7.0
> > From the bizarre behaviour above to a silent noop.
> 
> When there is nothing to show, we do not show anything, 

As I demonstrated in the text that you cut: that is not true.
git log -g v2.7.0 and git reflog v2.7.0 are *not* silent, but buggy. I
would like to make them silent.

> and that is just like "git log v2.7.0..v2.7.0" is silent.
> 
> I do not find the silence bizarre at all.

I'll take that as an agreement then :)
-- 
Dennis Kaarsemaker
www.kaarsemaker.net


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


Re: [PATCH v3 00/20] refs backend rebase on pu

2016-02-02 Thread David Turner
Are there any more reviews on this?  I do have some changes from this
set, but they're pretty minor so I don't want to post a new one (unless
folks would rather see those changes before reviewing).  Let me know.

Thanks.

On Thu, 2016-01-14 at 11:25 -0500, David Turner wrote:
> I rebased this on top of 20fabf9b194c4099d329582c734e433f9f6586a3
> (the
> commit before the previous version of this series).
> 
> This entailed removing Michael Haggerty's patch to builtin/clone.c,
> since a patch by Stefan already did approximately the same thing.
> 
> There was a somewhat hairy merge of "resolve symbolic refs first",
> but
> I think the new one is fine (the same tests all pass except for the
> one TODO noted in the lmdb code).
> 
> David Turner (17):
>   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 backends to be set for new repos
>   refs: check submodules ref storage config
>   refs: allow ref backend to be set for clone
>   svn: learn ref-storage argument
>   refs: add LMDB refs 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  |   40 +-
>  builtin/submodule--helper.c|2 +-
>  cache.h|2 +
>  config.c   |   29 +
>  configure.ac   |   33 +
>  contrib/workdir/git-new-workdir|3 +
>  git-submodule.sh   |   13 +
>  git-svn.perl   |6 +-
>  path.c |   29 +-
>  refs.c |  451 +-
>  refs.h |   17 +
>  refs/files-backend.c   |  397 +++--
>  refs/lmdb-backend.c| 2051
> 
>  refs/refs-internal.h   |  128 +-
>  setup.c|   23 +-
>  t/t0001-init.sh|   24 +
>  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 +-
>  31 files changed, 4767 insertions(+), 203 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
> 
--
To unsubscribe from this list: send the line "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] Trick to force setup of a specific configured E-Mail per repo

2016-02-02 Thread Dan Aloni
Previously, before 5498c57cdd63, many people did the following:

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

This was helpful for people with more than one E-Mail address,
targeting different E-Mail addresses for different clones.
as it barred git from creating commit unless the user.email
config was set in the per-clone config to the correct E-Mail
address.

Now, since the original 'bug' was fixed, and practically every
string is acceptable for user.email and user.name, it is best
to reimplement the feature not as an exploit of a bug, but as
an actual feature.

Signed-off-by: Dan Aloni 
---
 Documentation/config.txt  |  3 +++
 ident.c   |  5 +
 t/t9904-per-repo-email.sh | 26 ++
 3 files changed, 34 insertions(+)
 create mode 100755 t/t9904-per-repo-email.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f61788668e89..f9712e7c7752 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2769,6 +2769,9 @@ user.email::
Your email address to be recorded in any newly created commits.
Can be overridden by the 'GIT_AUTHOR_EMAIL', 'GIT_COMMITTER_EMAIL', and
'EMAIL' environment variables.  See linkgit:git-commit-tree[1].
+   For people who seek setting different E-Mail addresses depending
+   on the clone, set to '(per-repo)' on the global configuration,
+   and Git will prompt you to set the E-Mail address in the clone.
 
 user.name::
Your full name to be recorded in any newly created commits.
diff --git a/ident.c b/ident.c
index daf7e1ea8370..0e07d45f8ff3 100644
--- a/ident.c
+++ b/ident.c
@@ -373,6 +373,11 @@ const char *fmt_ident(const char *name, const char *email,
die("unable to auto-detect email address (got '%s')", email);
}
 
+   if (strict && email && !strcmp(email, "(per-repo)")) {
+   die("email is '(per-repo)', suggesting to set specific email "
+   "for the current repo");
+   }
+
strbuf_reset(&ident);
if (want_name) {
strbuf_addstr_without_crud(&ident, name);
diff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh
new file mode 100755
index ..c085ba671b85
--- /dev/null
+++ b/t/t9904-per-repo-email.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+#
+# Copyright (c) 2016 Dan Aloni
+#
+
+test_description='per-repo forced setting of E-Mail address'
+
+. ./test-lib.sh
+
+test_expect_failure 'fails commiting if clone email is not set' '
+   echo "Initial" >foo &&
+   git add foo &&
+   unset GIT_AUTHOR_EMAIL &&
+   git config --global user.email "(per-repo)" &&
+   EDITOR=: VISUAL=: git commit -a -m x
+'
+
+test_expect_success 'succeeds commiting if clone email is set' '
+   echo "Initial" >foo &&
+   git add foo &&
+   git config --global user.email "(per-repo)" &&
+   git config user.email "t...@ok.com" &&
+   EDITOR=: VISUAL=: git commit -a -m x
+'
+
+test_done
-- 
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: git log -g bizarre behaviour

2016-02-02 Thread Junio C Hamano
Dennis Kaarsemaker  writes:

> On ma, 2016-02-01 at 15:37 -0800, Junio C Hamano wrote:
>
>> Do you mean
>> 
>>  $ git checkout -b testing
>> $ rm -f .git/logs/refs/heads/testing
>> $ git log -g testing
>> 
>> will be changed from a silent no-op to an abort with error?
>> 
>> I do not see a need for such a change--does that count as an
>> objection?
>
> No, I'd like to change:
>
> $ ls .git/logs/refs/tags/v2.7.0
> ls: cannot access .git/logs/refs/tags/v2.7.0: No such file or directory
> $ git (log -g|reflog) v2.7.0
> From the bizarre behaviour above to a silent noop.

When there is nothing to show, we do not show anything, and that is
just like "git log v2.7.0..v2.7.0" is silent.

I do not find the silence bizarre at all.
--
To unsubscribe from this list: send the line "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: parse_object does check_sha1_signature but not parse_object_buffer?

2016-02-02 Thread Junio C Hamano
Mike Hommey  writes:

> On Mon, Feb 01, 2016 at 07:10:04PM -0800, Junio C Hamano wrote:
>> Mike Hommey  writes:
>> 
>> > Shouldn't parse_object_buffer also do check_sha1_signature?
>> 
>> In general, it shouldn't; its callers are supposed to do it as
>> additional check when/if needed.  Callers like the one in fsck.c
>> does not want to die after seeing one bad one.  We want to report
>> and keep checking other things.
>
> Shouldn't some things like, at least, `git checkout`, still check
> the sha1s, though?

That is a different issue--my answer was about the quoted question
regarding parse_object_buffer().  Its callers are supposed to do
additional check when/if needed, and there may be codepaths that
currently use parse_object_buffer() that may want to do their own
check, or call a different function that does the check for them.

Having said that, I do not necessarily think "git checkout" should
revalidate the object name.  The repository that you use for your
daily work would have the same error/corruption rate as your working
tree files, and I do not think you would constantly "validate" what
is in your working tree by comparing their contents with what you
think ought to be there.

If you are working on extremely poor quality disks and SSDs, it
might make sense to constantly revalidating the object data to catch
corruption early, as that is what we can do (as opposed to the
working tree files, corruption to which you probably do not have
anything to catch bitflipping on).

Unless you are placing your working tree on a filesystem with
checksums, but your object data would also be protected against
corruption in that case, so an extra revalidation at "git checkout"
time would not buy you much if anything at all.

http://article.gmane.org/gmane.comp.version-control.git/283380 (not
necessarily the entire thread, but that exact article) is a
reasonable summary that illustrates the way how we view the object
integrity.

So "index-pack" is the enforcement point, and the rest of the
git commands generally assume that we can trust what is on disk
(as it is has either been generated by us, or checked by
index-pack).  The rest of the commands do not spend time
checking that the on-disk contents are sane (though you can run
git-fsck if you want to do that).

If anything, we may want to reduce the number of codepaths that
calls check_sha1_signature() on data that we know we have read from
our own repository.  Even though I am not opposed to an idea to have
a "paranoid" mode that revalidates the object name every time (and
if "git checkout" does not currently, allow it to revalidate when we
are operating under the "paranoid" mode), I do not think it should
be on by default.

In fact, I have this suspicion that the original justification to
have the call to check_sha1_signature() in parse_object() might have
been invalidated by the restructuring of the code over the past 10
years.  e9eefa67 ([PATCH] Add function to parse an object of
unspecified type (take 2), 2005-04-28) says

It also checks the hash of the file, due to its heritage as part
of fsck-cache.

I.e. we do not need this call here, as long as we make sure that
fsck codepath does not depend on the fact that parse_object() calls
check_sha1_signature() to validate the consistency between the data
and the object name.

In fact, we do this, which is quite suboptimal:

static int fsck_sha1(const unsigned char *sha1)
{
struct object *obj = parse_object(sha1);
if (!obj) {
errors_found |= ERROR_OBJECT;
return error("%s: object corrupt or missing",
 sha1_to_hex(sha1));
}
obj->flags |= HAS_OBJ;
return fsck_obj(obj);
}

This function is called for each loose object file we find in
fsck_object_dir(), and there are a few problems:

 * The function parse_object() called from here would issue an error
   message and returns NULL; then you get another "corrupt or
   missing" error message, because this code cannot tell from the
   NULL which is the case.

 * The intent of the callchain to fsck_sha1() is to iterate over
   loose object files xx/x{38} and validate what is contained in
   them, but that behaviour is not guaranteed because it calls
   parse_object(), which may get the object data from a packfile
   if the loose object is also in the packfile.

This function should instead take "path" (the only caller of this
function fsck_loose() has it), read the data in the file, hash the
data to validate that it matches "sha1" and then create the object
out of that data it read by calling parse_object_buffer().

I didn't check other callers of parse_object(), but I doubt that
they need or want a check_sha1_signature() call in the function.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.or

Re: [PATCH v4 00/15] config: make git_config_set die on failure

2016-02-02 Thread Stefan Beller
On Tue, Feb 2, 2016 at 3:51 AM, Patrick Steinhardt  wrote:
>   submodule: die on config error when linking modules
>   submodule--helper: die on config error when cloning module

The code in the submodule related patches looks fine to me.
--
To unsubscribe from this list: send the line "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/12] ref-filter: bump 'used_atom' and related code to the top

2016-02-02 Thread Karthik Nayak
Bump code to the top for usage in further patches.

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

diff --git a/ref-filter.c b/ref-filter.c
index 38f38d4..c3a8372 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -16,6 +16,21 @@
 
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
+/*
+ * An atom is a valid field atom listed below, possibly prefixed with
+ * a "*" to denote deref_tag().
+ *
+ * We parse given format string and sort specifiers, and make a list
+ * of properties that we need to extract out of objects.  ref_array_item
+ * structure will hold an array of values extracted that can be
+ * indexed with the "atom number", which is an index into this
+ * array.
+ */
+static const char **used_atom;
+static cmp_type *used_atom_type;
+static int used_atom_cnt, need_tagged, need_symref;
+static int need_color_reset_at_eol;
+
 static struct {
const char *name;
cmp_type cmp_type;
@@ -92,21 +107,6 @@ struct atom_value {
 };
 
 /*
- * An atom is a valid field atom listed above, possibly prefixed with
- * a "*" to denote deref_tag().
- *
- * We parse given format string and sort specifiers, and make a list
- * of properties that we need to extract out of objects.  ref_array_item
- * structure will hold an array of values extracted that can be
- * indexed with the "atom number", which is an index into this
- * array.
- */
-static const char **used_atom;
-static cmp_type *used_atom_type;
-static int used_atom_cnt, need_tagged, need_symref;
-static int need_color_reset_at_eol;
-
-/*
  * Used to parse format string and sort specifiers
  */
 int parse_ref_filter_atom(const char *atom, const char *ep)
-- 
2.7.0

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


Re: [PATCH v4 06/15] submodule--helper: die on config error when cloning module

2016-02-02 Thread Eric Sunshine
On Tue, Feb 2, 2016 at 6:51 AM, Patrick Steinhardt  wrote:
> When setting the 'core.worktree' option for a newly cloned
> submodule we ignore the return value of `git_config_set_in_file`.
> As this leaves the submodule in an inconsistent state, we instaed
> we want to inform the user that something has gone wrong by

s/instaed/instead/

Also, there are too many "we"s, so dropping one would be a good idea:
either "we instead" or "instead we".

> printing an error and aborting the program.
>
> Signed-off-by: Patrick Steinhardt 
--
To unsubscribe from this list: send the line "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 v4 03/12] ref-filter: bump 'used_atom' and related code to the top

2016-02-02 Thread Karthik Nayak
On Tue, Feb 2, 2016 at 3:52 AM, Junio C Hamano  wrote:
> Karthik Nayak  writes:
>
>> Bump code to the top for usage in further patches.
>> ---
>
> Sign-off?

Shall reply with patch, missed that somehow.

>
>>  ref-filter.c | 30 +++---
>>  1 file changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/ref-filter.c b/ref-filter.c
>> index 38f38d4..c3a8372 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -16,6 +16,21 @@
>>
>>  typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
>>
>> +/*
>> + * An atom is a valid field atom listed below, possibly prefixed with
>> + * a "*" to denote deref_tag().
>> + *
>> + * We parse given format string and sort specifiers, and make a list
>> + * of properties that we need to extract out of objects.  ref_array_item
>> + * structure will hold an array of values extracted that can be
>> + * indexed with the "atom number", which is an index into this
>> + * array.
>> + */
>> +static const char **used_atom;
>> +static cmp_type *used_atom_type;
>> +static int used_atom_cnt, need_tagged, need_symref;
>> +static int need_color_reset_at_eol;
>> +
>>  static struct {
>>   const char *name;
>>   cmp_type cmp_type;
>> @@ -92,21 +107,6 @@ struct atom_value {
>>  };
>>
>>  /*
>> - * An atom is a valid field atom listed above, possibly prefixed with
>> - * a "*" to denote deref_tag().
>> - *
>> - * We parse given format string and sort specifiers, and make a list
>> - * of properties that we need to extract out of objects.  ref_array_item
>> - * structure will hold an array of values extracted that can be
>> - * indexed with the "atom number", which is an index into this
>> - * array.
>> - */
>> -static const char **used_atom;
>> -static cmp_type *used_atom_type;
>> -static int used_atom_cnt, need_tagged, need_symref;
>> -static int need_color_reset_at_eol;
>> -
>> -/*
>>   * Used to parse format string and sort specifiers
>>   */
>>  int parse_ref_filter_atom(const char *atom, const char *ep)



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


[PATCH v2] test-lib: limit the output of the yes utility

2016-02-02 Thread Johannes Sixt
From: Johannes Schindelin 

On Windows, there is no SIGPIPE. A consequence of this is that the
upstream process of a pipe does not notice the death of the downstream
process until the pipe buffer is full and writing more data returns an
error. This behavior is the reason for an annoying delay during the
execution of t7610-mergetool.sh: There are a number of test cases where
'yes' is invoked upstream. Since the utility is basically an endless
loop it runs, on Windows, until the pipe buffer is full. This does take
a few seconds.

The test suite has its own implementation of 'yes'. Modify it to produce
only a limited amount of output that is sufficient for the test suite.
The amount chosen should be sufficiently high for any test case, assuming
that future test cases will not exaggerate their demands of input from
an upstream 'yes' invocation.

[j6t: commit message]

Signed-off-by: Johannes Schindelin 
Signed-off-by: Johannes Sixt 
---
Am 02.02.2016 um 09:21 schrieb Johannes Schindelin:
> On Tue, 2 Feb 2016, Johannes Sixt wrote:
>> On Windows, there is no SIGPIPE.
> 
> True. But we do get some sort of write failure, no? Otherwise
> https://github.com/git/git/commit/2b86292ed would not work...

Of course. See the second sentence of the commit message.

> I agree with the idea, but I would like to have a less intrusive patch.
> Something like this should do the job as well, and is a little easier on
> the eyes IMHO:

I'm not 100% satisfied with your version because it stomps on
short-and-sweet shell variables $i and $y. But then the utility
only occurs upstream of a pipeline in a separate process, so that
does not matter.

Please allow me to pass authorship to you, since the patch text is
now all yours, and to forge your sign-off.

-- Hannes

 t/test-lib.sh | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index bd4b02e..51e4a88 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -907,9 +907,11 @@ yes () {
y="$*"
fi
 
-   while echo "$y"
+   i=0
+   while test $i -lt 99
do
-   :
+   echo "$y"
+   i=$(($i+1))
done
 }
 
-- 
2.7.0.118.g90056ae

--
To unsubscribe from this list: send the line "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] submodule-config: keep update strategy around

2016-02-02 Thread Stefan Beller
We need the submodule update strategies in a later patch.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 submodule-config.c | 11 +++
 submodule-config.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/submodule-config.c b/submodule-config.c
index afe0ea8..4239b0e 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -194,6 +194,7 @@ static struct submodule *lookup_or_create_by_name(struct 
submodule_cache *cache,
 
submodule->path = NULL;
submodule->url = NULL;
+   submodule->update = NULL;
submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
submodule->ignore = NULL;
 
@@ -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);
+   }
}
 
strbuf_release(&name);
diff --git a/submodule-config.h b/submodule-config.h
index 9061e4e..f9e2a29 100644
--- 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;
/* the sha1 blob id of the responsible .gitmodules file */
unsigned char gitmodules_sha1[20];
 };
-- 
2.7.0.rc0.42.ge5f5e2d

--
To unsubscribe from this list: send the line "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] clone: allow an explicit argument for parallel submodule clones

2016-02-02 Thread Stefan Beller
Just pass it along to "git submodule update", which may pick reasonable
defaults if you don't specify an explicit number.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 Documentation/git-clone.txt |  6 +-
 builtin/clone.c | 19 +--
 t/t7406-submodule-update.sh | 15 +++
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 6bf000d..6db7b6d 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -14,7 +14,7 @@ SYNOPSIS
  [-o ] [-b ] [-u ] [--reference ]
  [--dissociate] [--separate-git-dir ]
  [--depth ] [--[no-]single-branch]
- [--recursive | --recurse-submodules] [--] 
+ [--recursive | --recurse-submodules] [--jobs ] [--] 
  []
 
 DESCRIPTION
@@ -221,6 +221,10 @@ objects from the source repository into a pack in the 
cloned repository.
The result is Git repository can be separated from working
tree.
 
+-j ::
+--jobs ::
+   The number of submodules fetched at the same time.
+   Defaults to the `submodule.fetchJobs` option.
 
 ::
The (possibly remote) repository to clone from.  See the
diff --git a/builtin/clone.c b/builtin/clone.c
index a0b3cd9..b004fb4 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -50,6 +50,7 @@ static int option_progress = -1;
 static struct string_list option_config;
 static struct string_list option_reference;
 static int option_dissociate;
+static int max_jobs = -1;
 
 static struct option builtin_clone_options[] = {
OPT__VERBOSITY(&option_verbosity),
@@ -72,6 +73,8 @@ static struct option builtin_clone_options[] = {
N_("initialize submodules in the clone")),
OPT_BOOL(0, "recurse-submodules", &option_recursive,
N_("initialize submodules in the clone")),
+   OPT_INTEGER('j', "jobs", &max_jobs,
+   N_("number of submodules cloned in parallel")),
OPT_STRING(0, "template", &option_template, N_("template-directory"),
   N_("directory from which templates will be used")),
OPT_STRING_LIST(0, "reference", &option_reference, N_("repo"),
@@ -95,10 +98,6 @@ static struct option builtin_clone_options[] = {
OPT_END()
 };
 
-static const char *argv_submodule[] = {
-   "submodule", "update", "--init", "--recursive", NULL
-};
-
 static const char *get_repo_path_1(struct strbuf *path, int *is_bundle)
 {
static char *suffix[] = { "/.git", "", ".git/.git", ".git" };
@@ -724,8 +723,16 @@ static int checkout(void)
err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
   sha1_to_hex(sha1), "1", NULL);
 
-   if (!err && option_recursive)
-   err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
+   if (!err && option_recursive) {
+   struct argv_array args = ARGV_ARRAY_INIT;
+   argv_array_pushl(&args, "submodule", "update", "--init", 
"--recursive", NULL);
+
+   if (max_jobs != -1)
+   argv_array_pushf(&args, "--jobs=%d", max_jobs);
+
+   err = run_command_v_opt(args.argv, RUN_GIT_CMD);
+   argv_array_clear(&args);
+   }
 
return err;
 }
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 7fd5142..090891e 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -786,4 +786,19 @@ test_expect_success 'submodule update can be run in 
parallel' '
 grep "9 tasks" trace.out
)
 '
+
+test_expect_success 'git clone passes the parallel jobs config on to 
submodules' '
+   test_when_finished "rm -rf super4" &&
+   GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 7 . 
super4 &&
+   grep "7 tasks" trace.out &&
+   rm -rf super4 &&
+   git config --global submodule.fetchJobs 8 &&
+   GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules . super4 &&
+   grep "8 tasks" trace.out &&
+   rm -rf super4 &&
+   GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 9 . 
super4 &&
+   grep "9 tasks" trace.out &&
+   rm -rf super4
+'
+
 test_done
-- 
2.7.0.rc0.42.ge5f5e2d

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


[PATCHv7 0/8] Expose submodule parallelism to the user

2016-02-02 Thread Stefan Beller
This is a resend of sb/submodule-parallel-update as is (directly taken from
Junios tree), and I put it on the list to ease the life of reviewers.

It applies on top of origin/sb/submodule-parallel-fetch
(which is part of master already)

I have looked over the patches and they still look fine to me.
They had some early reviews end of last year, however not enough review
yet to make them proceed.

Any review is welcome!
Thanks,
Stefan

Stefan Beller (8):
  submodule-config: keep update strategy around
  submodule-config: drop check against NULL
  submodule-config: remove name_and_item_from_var
  submodule-config: introduce parse_generic_submodule_config
  fetching submodules: respect `submodule.fetchJobs` config option
  git submodule update: have a dedicated helper for cloning
  submodule update: expose parallelism to the user
  clone: allow an explicit argument for parallel submodule clones

 Documentation/config.txt|   7 ++
 Documentation/git-clone.txt |   6 +-
 Documentation/git-submodule.txt |   7 +-
 builtin/clone.c |  19 +++-
 builtin/fetch.c |   2 +-
 builtin/submodule--helper.c | 239 
 git-submodule.sh|  54 -
 submodule-config.c  | 109 +++---
 submodule-config.h  |   3 +
 submodule.c |   5 +
 t/t5526-fetch-submodules.sh |  14 +++
 t/t7400-submodule-basic.sh  |   4 +-
 t/t7406-submodule-update.sh |  27 +
 13 files changed, 413 insertions(+), 83 deletions(-)

-- 
2.7.0.rc0.42.ge5f5e2d

--
To unsubscribe from this list: send the line "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] submodule-config: introduce parse_generic_submodule_config

2016-02-02 Thread Stefan Beller
This rewrites parse_config to distinguish between configs specific to
one submodule and configs which apply generically to all submodules.
We do not have generic submodule configs yet, but the next patch will
introduce "submodule.fetchJobs".

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 submodule-config.c | 41 -
 1 file changed, 32 insertions(+), 9 deletions(-)

diff --git a/submodule-config.c b/submodule-config.c
index b826841..29e21b2 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -234,17 +234,22 @@ struct parse_config_parameter {
int overwrite;
 };
 
-static int parse_config(const char *var, const char *value, void *data)
+static int parse_generic_submodule_config(const char *key,
+ const char *var,
+ const char *value,
+ struct parse_config_parameter *me)
 {
-   struct parse_config_parameter *me = data;
-   struct submodule *submodule;
-   int subsection_len, ret = 0;
-   const char *subsection, *key;
-
-   if (parse_config_key(var, "submodule", &subsection,
-&subsection_len, &key) < 0 || !subsection_len)
-   return 0;
+   return 0;
+}
 
+static int parse_specific_submodule_config(const char *subsection, int 
subsection_len,
+  const char *key,
+  const char *var,
+  const char *value,
+  struct parse_config_parameter *me)
+{
+   int ret = 0;
+   struct submodule *submodule;
submodule = lookup_or_create_by_name(me->cache,
 me->gitmodules_sha1,
 subsection, subsection_len);
@@ -314,6 +319,24 @@ static int parse_config(const char *var, const char 
*value, void *data)
return ret;
 }
 
+static int parse_config(const char *var, const char *value, void *data)
+{
+   struct parse_config_parameter *me = data;
+   int subsection_len;
+   const char *subsection, *key;
+
+   if (parse_config_key(var, "submodule", &subsection,
+&subsection_len, &key) < 0)
+   return 0;
+
+   if (!subsection_len)
+   return parse_generic_submodule_config(key, var, value, me);
+   else
+   return parse_specific_submodule_config(subsection,
+  subsection_len, key,
+  var, value, me);
+}
+
 static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
  unsigned char *gitmodules_sha1)
 {
-- 
2.7.0.rc0.42.ge5f5e2d

--
To unsubscribe from this list: send the line "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 6/8] git submodule update: have a dedicated helper for cloning

2016-02-02 Thread Stefan Beller
This introduces a new helper function in git submodule--helper
which takes care of cloning all submodules, which we want to
parallelize eventually.

Some tests (such as empty URL, update_mode=none) are required in the
helper to make the decision for cloning. These checks have been
moved into the C function as well (no need to repeat them in the
shell script).

As we can only access the stderr channel from within the parallel
processing engine, we need to reroute the error message for
specified but initialized submodules to stderr. As it is an error
message, this should have gone to stderr in the first place, so it
is a bug fix along the way.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 builtin/submodule--helper.c | 229 
 git-submodule.sh|  45 +++--
 t/t7400-submodule-basic.sh  |   4 +-
 3 files changed, 242 insertions(+), 36 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f4c3eff..9441f20 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -255,6 +255,234 @@ static int module_clone(int argc, const char **argv, 
const char *prefix)
return 0;
 }
 
+static int git_submodule_config(const char *var, const char *value, void *cb)
+{
+   return parse_submodule_config_option(var, value);
+}
+
+struct submodule_update_clone {
+   /* states */
+   int count;
+   int print_unmatched;
+   /* configuration */
+   int quiet;
+   const char *reference;
+   const char *depth;
+   const char *update;
+   const char *recursive_prefix;
+   const char *prefix;
+   struct module_list list;
+   struct string_list projectlines;
+   struct pathspec pathspec;
+};
+#define SUBMODULE_UPDATE_CLONE_INIT {0, 0, 0, NULL, NULL, NULL, NULL, NULL, 
MODULE_LIST_INIT, STRING_LIST_INIT_DUP}
+
+static void fill_clone_command(struct child_process *cp, int quiet,
+  const char *prefix, const char *path,
+  const char *name, const char *url,
+  const char *reference, const char *depth)
+{
+   cp->git_cmd = 1;
+   cp->no_stdin = 1;
+   cp->stdout_to_stderr = 1;
+   cp->err = -1;
+   argv_array_push(&cp->args, "submodule--helper");
+   argv_array_push(&cp->args, "clone");
+   if (quiet)
+   argv_array_push(&cp->args, "--quiet");
+
+   if (prefix)
+   argv_array_pushl(&cp->args, "--prefix", prefix, NULL);
+
+   argv_array_pushl(&cp->args, "--path", path, NULL);
+   argv_array_pushl(&cp->args, "--name", name, NULL);
+   argv_array_pushl(&cp->args, "--url", url, NULL);
+   if (reference)
+   argv_array_push(&cp->args, reference);
+   if (depth)
+   argv_array_push(&cp->args, depth);
+}
+
+static int update_clone_get_next_task(struct child_process *cp,
+ struct strbuf *err,
+ void *pp_cb,
+ void **pp_task_cb)
+{
+   struct submodule_update_clone *pp = pp_cb;
+
+   for (; pp->count < pp->list.nr; pp->count++) {
+   const struct submodule *sub = NULL;
+   const char *displaypath = NULL;
+   const struct cache_entry *ce = pp->list.entries[pp->count];
+   struct strbuf sb = STRBUF_INIT;
+   const char *update_module = NULL;
+   char *url = NULL;
+   int needs_cloning = 0;
+
+   if (ce_stage(ce)) {
+   if (pp->recursive_prefix)
+   strbuf_addf(err, "Skipping unmerged submodule 
%s/%s\n",
+   pp->recursive_prefix, ce->name);
+   else
+   strbuf_addf(err, "Skipping unmerged submodule 
%s\n",
+   ce->name);
+   continue;
+   }
+
+   sub = submodule_from_path(null_sha1, ce->name);
+   if (!sub) {
+   strbuf_addf(err, "BUG: internal error managing 
submodules. "
+   "The cache could not locate '%s'", 
ce->name);
+   pp->print_unmatched = 1;
+   continue;
+   }
+
+   if (pp->recursive_prefix)
+   displaypath = relative_path(pp->recursive_prefix, 
ce->name, &sb);
+   else
+   displaypath = ce->name;
+
+   if (pp->update)
+   update_module = pp->update;
+   if (!update_module)
+   update_module = sub->update;
+   if (!update_module)
+   update_module = "checkout";
+   if (!strcmp(update_module, "none")) {
+   strbuf_addf(err, "Skipping submodule '%s'\n", 
d

[PATCH 5/8] fetching submodules: respect `submodule.fetchJobs` config option

2016-02-02 Thread Stefan Beller
This allows to configure fetching and updating in parallel
without having the command line option.

This moved the responsibility to determine how many parallel processes
to start from builtin/fetch to submodule.c as we need a way to communicate
"The user did not specify the number of parallel processes in the command
line options" in the builtin fetch. The submodule code takes care of
the precedence (CLI > config > default)

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 Documentation/config.txt|  7 +++
 builtin/fetch.c |  2 +-
 submodule-config.c  | 15 +++
 submodule-config.h  |  2 ++
 submodule.c |  5 +
 t/t5526-fetch-submodules.sh | 14 ++
 6 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2d06b11..eda3276 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2646,6 +2646,13 @@ submodule..ignore::
"--ignore-submodules" option. The 'git submodule' commands are not
affected by this setting.
 
+submodule.fetchJobs::
+   This is used to determine how many submodules will be
+   fetched/cloned at the same time. Specifying a positive integer
+   allows up to that number of submodules being fetched in parallel.
+   This is used in fetch and clone operations only. A value of 0 will
+   give some reasonable configuration. It defaults to 1.
+
 tag.sort::
This variable controls the sort ordering of tags when displayed by
linkgit:git-tag[1]. Without the "--sort=" option provided, the
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;
 static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
diff --git a/submodule-config.c b/submodule-config.c
index 29e21b2..a32259e 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 int parallel_jobs = -1;
 
 static int config_path_cmp(const struct submodule_entry *a,
   const struct submodule_entry *b,
@@ -239,6 +240,15 @@ static int parse_generic_submodule_config(const char *key,
  const char *value,
  struct parse_config_parameter *me)
 {
+   if (!strcmp(key, "fetchjobs")) {
+   parallel_jobs = strtol(value, NULL, 10);
+   if (parallel_jobs < 0) {
+   warning("submodule.fetchJobs not allowed to be 
negative.");
+   parallel_jobs = 1;
+   return 1;
+   }
+   }
+
return 0;
 }
 
@@ -482,3 +492,8 @@ void submodule_free(void)
cache_free(&cache);
is_cache_init = 0;
 }
+
+int config_parallel_submodules(void)
+{
+   return parallel_jobs;
+}
diff --git a/submodule-config.h b/submodule-config.h
index f9e2a29..d9bbf9a 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -27,4 +27,6 @@ const struct submodule *submodule_from_path(const unsigned 
char *commit_sha1,
const char *path);
 void submodule_free(void);
 
+int config_parallel_submodules(void);
+
 #endif /* SUBMODULE_CONFIG_H */
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(&spf.args, "--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,
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 1241146..954d0e4 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -471,4 +471,18 @@ test_expect_success "don't fetch submodule when newly 
recorded commits are alrea
test_i18ncmp expect.err actual.err
 '
 
+test_expect_success 'fetching submodules respects parallel settings' '
+   git config fetch.recurseSubmodules true &&
+   (
+   cd downstream &&
+   GIT_TRACE=$(pwd)/trace.out git fetch --jobs 7 &&
+   grep "7 tasks" trace.out &&
+

[PATCH 7/8] submodule update: expose parallelism to the user

2016-02-02 Thread Stefan Beller
Expose possible parallelism either via the "--jobs" CLI parameter or
the "submodule.fetchJobs" setting.

By having the variable initialized to -1, we make sure 0 can be passed
into the parallel processing machine, which will then pick as many parallel
workers as there are CPUs.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 Documentation/git-submodule.txt |  7 ++-
 builtin/submodule--helper.c | 18 ++
 git-submodule.sh|  9 +
 t/t7406-submodule-update.sh | 12 
 4 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 1572f05..13adebf 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -16,7 +16,7 @@ SYNOPSIS
 'git submodule' [--quiet] deinit [-f|--force] [--] ...
 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
  [-f|--force] [--rebase|--merge] [--reference ]
- [--depth ] [--recursive] [--] [...]
+ [--depth ] [--recursive] [--jobs ] [--] [...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) ]
  [commit] [--] [...]
 'git submodule' [--quiet] foreach [--recursive] 
@@ -377,6 +377,11 @@ for linkgit:git-clone[1]'s `--reference` and `--shared` 
options carefully.
clone with a history truncated to the specified number of revisions.
See linkgit:git-clone[1]
 
+-j ::
+--jobs ::
+   This option is only valid for the update command.
+   Clone new submodules in parallel with as many jobs.
+   Defaults to the `submodule.fetchJobs` option.
 
 ...::
Paths to submodule(s). When specified this will restrict the command
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 9441f20..8002187 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -426,6 +426,7 @@ static int update_clone_task_finished(int result,
 
 static int update_clone(int argc, const char **argv, const char *prefix)
 {
+   int max_jobs = -1;
struct string_list_item *item;
struct submodule_update_clone pp = SUBMODULE_UPDATE_CLONE_INIT;
 
@@ -446,6 +447,8 @@ static int update_clone(int argc, const char **argv, const 
char *prefix)
OPT_STRING(0, "depth", &pp.depth, "",
   N_("Create a shallow clone truncated to the "
  "specified number of revisions")),
+   OPT_INTEGER('j', "jobs", &max_jobs,
+   N_("parallel jobs")),
OPT__QUIET(&pp.quiet, N_("do't print cloning progress")),
OPT_END()
};
@@ -467,10 +470,17 @@ static int update_clone(int argc, const char **argv, 
const char *prefix)
gitmodules_config();
/* Overlay the parsed .gitmodules file with .git/config */
git_config(git_submodule_config, NULL);
-   run_processes_parallel(1, update_clone_get_next_task,
- update_clone_start_failure,
- update_clone_task_finished,
- &pp);
+
+   if (max_jobs < 0)
+   max_jobs = config_parallel_submodules();
+   if (max_jobs < 0)
+   max_jobs = 1;
+
+   run_processes_parallel(max_jobs,
+  update_clone_get_next_task,
+  update_clone_start_failure,
+  update_clone_task_finished,
+  &pp);
 
if (pp.print_unmatched) {
printf("#unmatched\n");
diff --git a/git-submodule.sh b/git-submodule.sh
index 9f554fb..10c5af9 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -645,6 +645,14 @@ cmd_update()
--depth=*)
depth=$1
;;
+   -j|--jobs)
+   case "$2" in '') usage ;; esac
+   jobs="--jobs=$2"
+   shift
+   ;;
+   --jobs=*)
+   jobs=$1
+   ;;
--)
shift
break
@@ -670,6 +678,7 @@ cmd_update()
${update:+--update "$update"} \
${reference:+--reference "$reference"} \
${depth:+--depth "$depth"} \
+   ${jobs:+$jobs} \
"$@" | {
err=
while read mode sha1 stage just_cloned sm_path
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index dda3929..7fd5142 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -774,4 +774,16 @@ test_expect_success 'submodule update --recursive drops 
module name before recur
 test_i18ngrep "Submodule path .deeper/submodule/subsubmodule.: checked 
out" actual
)
 '
+
+test_expect_success 'submodule update can be run in parallel' '
+ 

[PATCH 2/8] submodule-config: drop check against NULL

2016-02-02 Thread Stefan Beller
Adhere to the common coding style of Git and not check explicitly
for NULL throughout the file. There are still other occurrences in the
code base but that is usually inside of conditions with side effects.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 submodule-config.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/submodule-config.c b/submodule-config.c
index 4239b0e..6d01941 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -265,7 +265,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
if (!strcmp(item.buf, "path")) {
if (!value)
ret = config_error_nonbool(var);
-   else if (!me->overwrite && submodule->path != NULL)
+   else if (!me->overwrite && submodule->path)
warn_multiple_config(me->commit_sha1, submodule->name,
"path");
else {
@@ -289,7 +289,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
} else if (!strcmp(item.buf, "ignore")) {
if (!value)
ret = config_error_nonbool(var);
-   else if (!me->overwrite && submodule->ignore != NULL)
+   else if (!me->overwrite && submodule->ignore)
warn_multiple_config(me->commit_sha1, submodule->name,
"ignore");
else if (strcmp(value, "untracked") &&
@@ -305,7 +305,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
} else if (!strcmp(item.buf, "url")) {
if (!value) {
ret = config_error_nonbool(var);
-   } else if (!me->overwrite && submodule->url != NULL) {
+   } else if (!me->overwrite && submodule->url) {
warn_multiple_config(me->commit_sha1, submodule->name,
"url");
} else {
@@ -315,7 +315,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
} else if (!strcmp(item.buf, "update")) {
if (!value)
ret = config_error_nonbool(var);
-   else if (!me->overwrite && submodule->update != NULL)
+   else if (!me->overwrite && submodule->update)
warn_multiple_config(me->commit_sha1, submodule->name,
 "update");
else {
-- 
2.7.0.rc0.42.ge5f5e2d

--
To unsubscribe from this list: send the line "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] submodule-config: remove name_and_item_from_var

2016-02-02 Thread Stefan Beller
`name_and_item_from_var` does not provide the proper abstraction
we need here in a later patch.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 submodule-config.c | 48 
 1 file changed, 16 insertions(+), 32 deletions(-)

diff --git a/submodule-config.c b/submodule-config.c
index 6d01941..b826841 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -161,31 +161,17 @@ static struct submodule *cache_lookup_name(struct 
submodule_cache *cache,
return NULL;
 }
 
-static int name_and_item_from_var(const char *var, struct strbuf *name,
- struct strbuf *item)
-{
-   const char *subsection, *key;
-   int subsection_len, parse;
-   parse = parse_config_key(var, "submodule", &subsection,
-   &subsection_len, &key);
-   if (parse < 0 || !subsection)
-   return 0;
-
-   strbuf_add(name, subsection, subsection_len);
-   strbuf_addstr(item, key);
-
-   return 1;
-}
-
 static struct submodule *lookup_or_create_by_name(struct submodule_cache 
*cache,
-   const unsigned char *gitmodules_sha1, const char *name)
+ const unsigned char 
*gitmodules_sha1,
+ const char *name_ptr, int 
name_len)
 {
struct submodule *submodule;
struct strbuf name_buf = STRBUF_INIT;
+   char *name = xmemdupz(name_ptr, name_len);
 
submodule = cache_lookup_name(cache, gitmodules_sha1, name);
if (submodule)
-   return submodule;
+   goto out;
 
submodule = xmalloc(sizeof(*submodule));
 
@@ -201,7 +187,8 @@ static struct submodule *lookup_or_create_by_name(struct 
submodule_cache *cache,
hashcpy(submodule->gitmodules_sha1, gitmodules_sha1);
 
cache_add(cache, submodule);
-
+out:
+   free(name);
return submodule;
 }
 
@@ -251,18 +238,18 @@ static int parse_config(const char *var, const char 
*value, void *data)
 {
struct parse_config_parameter *me = data;
struct submodule *submodule;
-   struct strbuf name = STRBUF_INIT, item = STRBUF_INIT;
-   int ret = 0;
+   int subsection_len, ret = 0;
+   const char *subsection, *key;
 
-   /* this also ensures that we only parse submodule entries */
-   if (!name_and_item_from_var(var, &name, &item))
+   if (parse_config_key(var, "submodule", &subsection,
+&subsection_len, &key) < 0 || !subsection_len)
return 0;
 
submodule = lookup_or_create_by_name(me->cache,
 me->gitmodules_sha1,
-name.buf);
+subsection, subsection_len);
 
-   if (!strcmp(item.buf, "path")) {
+   if (!strcmp(key, "path")) {
if (!value)
ret = config_error_nonbool(var);
else if (!me->overwrite && submodule->path)
@@ -275,7 +262,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
submodule->path = xstrdup(value);
cache_put_path(me->cache, submodule);
}
-   } else if (!strcmp(item.buf, "fetchrecursesubmodules")) {
+   } else if (!strcmp(key, "fetchrecursesubmodules")) {
/* when parsing worktree configurations we can die early */
int die_on_error = is_null_sha1(me->gitmodules_sha1);
if (!me->overwrite &&
@@ -286,7 +273,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
submodule->fetch_recurse = parse_fetch_recurse(
var, value,
die_on_error);
-   } else if (!strcmp(item.buf, "ignore")) {
+   } else if (!strcmp(key, "ignore")) {
if (!value)
ret = config_error_nonbool(var);
else if (!me->overwrite && submodule->ignore)
@@ -302,7 +289,7 @@ static int parse_config(const char *var, const char *value, 
void *data)
free((void *) submodule->ignore);
submodule->ignore = xstrdup(value);
}
-   } else if (!strcmp(item.buf, "url")) {
+   } else if (!strcmp(key, "url")) {
if (!value) {
ret = config_error_nonbool(var);
} else if (!me->overwrite && submodule->url) {
@@ -312,7 +299,7 @@ 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")) {
+   } else if (!strcmp(key, "update")) {
if (!va

Re: git object-count differs between clones

2016-02-02 Thread Andrew Martin
- Original Message -
> From: "Jeff King" 
> To: "Andrew Martin" 
> Cc: "Matthieu Moy" , git@vger.kernel.org
> Sent: Tuesday, February 2, 2016 10:52:46 AM
> Subject: Re: git object-count differs between clones
> 
> On Tue, Feb 02, 2016 at 10:21:17AM -0600, Andrew Martin wrote:
> 
> > > You may try expiring your reflog and "git gc" again.
> > 
> > Thanks, I found some commits that are not referenced in any branch. How can
> > I
> > remove these from the reflog? I tried running
> > "git reflog expire --expire=now --expire-unreachable=now --all" followed by
> > "git gc" but still the same number of objects remain.
> 
> Are the objects now loose, or still in packs? Git has a grace period for
> pruning objects, so that we do not delete objects for an in-progress
> operation. The life cycle of an unreferenced object should be something
> like:
> 
>   - reachable by reflogs, which are pruned after 30 days (or
> gc.reflogExpireUnreachable config). Objects will be repacked as
> normal during this time. Override with "reflog expire" as you did
> above.
> 
>   - after the reflog expires, the objects are now unreachable. During
> the next repack, they'll be ejected from the pack into loose
> objects, and their mtimes set to match the pack they came from
> (which is probably quite recent if you just repacked!).
> 
>   - after 2 weeks (or gc.pruneExpire), unreachable loose objects are
> dropped by "git prune", which is called as part of "git gc". This is
> based on the object mtime.
> 
> You can accelerate this with "git gc --prune=now" (or
> "--prune=5.minutes.ago").
> 
> -Peff

Jeff,

Thanks for the clarification. I now ran "git repack -A" followed by 
"git gc --prune=now", however I am still seeing the same number of objects. What
else can I try to successfully mark these and unreachable and garbage collect 
them?

Thanks,

Andrew
--
To unsubscribe from this list: send the line "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 submodule should honor "-c credential.helper" command line argument

2016-02-02 Thread Marc Strapetz

git -c credential.helper=helper submodule update --init submodule

does not invoke "helper", but falls back to the default strategies.
When configuring in ~/.gitconfig:

[credential]
  helper=helper

git submodule update --init submodule

works fine. This behavior is somewhat unexpected -- is this a bug or by 
intention? In case intention, what's the recommended way to "inject" 
credentials helpers to work on submodules without modifying Git's config 
files?


Tested with Git 2.5.0 (Windows).

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


[ANNOUNCE] Git for Windows 2.7.0(2)

2016-02-02 Thread Johannes Schindelin
Dear Git users,

It is my pleasure to announce that Git for Windows 2.7.0(2) is available from:

https://git-for-windows.github.io/

I would like to dedicate this release to everyone who helps develop Git
(for Windows), whether it be by contributing code, by reviewing code
contributions, by answering questions here on the mailing lists or on the
bug tracker, by reporting carefully written and complete bug reports that
are a pleasure to read, and to everyone who is supporting us in any other
way. This very much includes helpful or encouraging comments in the
bug tracker. Thank you!  You are awesome. You make it all worth it.

Changes since Git for Windows v2.7.0 (January 5th 2016)

New Features

  ??? To stave off exploits, Git for Windows now uses Address Space
Layout Randomization (ASLR) and Data Execution Prevention (DEP).
  ??? Git for Windows' support for git pull --rebase=interactive that was
dropped when the pull command was rewritten in C, was resurrected.
  ??? The installers are now dual signed with SHA-2 and SHA-1
certificates.
  ??? The uninstaller is signed now, too.

Bug Fixes

  ??? When installing as administrator, we no longer offer the option to
install quiicklaunch icons because quicklaunch icons can only be
installed per-user.
  ??? If a ~/.bashrc is detected without a ~/.bash_profile, the generated
file will now also source ~/.profile if that exists.
  ??? The environment variable HOME can now be used to set the home
directory even when running with accounts that are part of a
different domain than the current (non-domain-joined) machine (in
which case the MSys2 runtime has no way to emulate POSIX-style
UIDs).
  ??? Git can now fetch and push via HTTPS even when the http.sslCAInfo
config variable was unset.
  ??? Git for Windows is now handling the case gracefully where the
current user has no permission to list the parent of the current
directory.
  ??? More file locking issues ("Unlink of file ... failed. Should I try
again?") were fixed.

Filename | SHA-256
 | ---
Git-2.7.0.2-64-bit.exe | 
c9f0ba628d79886427e05b6b2833f65630ccfc49ecceaab7cff0c6723d1d6e47
Git-2.7.0.2-32-bit.exe | 
8f14d50a1950264a7fd53128a81884f5757ca2b645e092ce81122993433657f8
PortableGit-2.7.0.2-64-bit.7z.exe | 
aa29dcfffda49bde58c84d34cec8e38f4ea85d97c5bb789a56e00ab86a60de5e
PortableGit-2.7.0.2-32-bit.7z.exe | 
b5af00c5bcffd67c57beb9ce379a8080e44a2f9fe05650a8d7d460382832cbea
Git-2.7.0.2-64-bit.tar.bz2 | 
9ed65a54b2586059a53ce64a57d2faa99386b282dd427a4a7e5474feb00d0851
Git-2.7.0.2-32-bit.tar.bz2 | 
a9c2daf66ed9864146945e5e2e0970a13cd9140b0fb0327a668e2550a7cc24e8

Ciao,
Johannes

Re: [ANNOUNCE] Git Developer Summit, April 4th, 2016, NYC

2016-02-02 Thread Stefan Beller
On Tue, Feb 2, 2016 at 8:12 AM, Jeff King  wrote:
> [2] You need a password to register.

That's right! So asking via unencrypted email is fine, I guess? ;)
--
To unsubscribe from this list: send the line "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 4/6] convert.c: Use text_eol_is_crlf()

2016-02-02 Thread 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 | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/convert.c b/convert.c
index bca3d0c..a5701a1 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;
 }
@@ -1378,8 +1386,9 @@ struct stream_filter *get_stream_filter(const char *path, 
const unsigned char *s
(crlf_action == CRLF_GUESS && auto_crlf == AUTO_CRLF_FALSE))
filter = cascade_filter(filter, &null_filter_singleton);
 
-   else if (output_eol(crlf_action) == EOL_CRLF &&
-!(crlf_action == CRLF_AUTO || crlf_action == CRLF_GUESS))
+   else if ((crlf_action == CRLF_AUTO || crlf_action == CRLF_GUESS))
+   ;
+   else if (output_eol(crlf_action) == EOL_CRLF)
filter = cascade_filter(filter, lf_to_crlf_filter());
 
return filter;
-- 
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 v1 2/6] convert.c: Remove path when not needed

2016-02-02 Thread 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 v1 5/6] convert: auto_crlf=false and no attributes set: same as binary

2016-02-02 Thread 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 a5701a1..50bdc42 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, &null_filter_singleton);
 
else if ((crlf_action == CRLF_AUTO || 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 v1 3/6] convert.c: Remove input_crlf_action()

2016-02-02 Thread tboegi
From: Torsten Bögershausen 

Integrate the code of input_crlf_action() into convert_attrs(),
so that ca.crlf_action is aleays 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 | 35 +--
 1 file changed, 13 insertions(+), 22 deletions(-)

diff --git a/convert.c b/convert.c
index a24c2a2..bca3d0c 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)) {
+   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(&ca, 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


Re: git object-count differs between clones

2016-02-02 Thread Jeff King
On Tue, Feb 02, 2016 at 10:21:17AM -0600, Andrew Martin wrote:

> > You may try expiring your reflog and "git gc" again.
> 
> Thanks, I found some commits that are not referenced in any branch. How can I
> remove these from the reflog? I tried running
> "git reflog expire --expire=now --expire-unreachable=now --all" followed by
> "git gc" but still the same number of objects remain.

Are the objects now loose, or still in packs? Git has a grace period for
pruning objects, so that we do not delete objects for an in-progress
operation. The life cycle of an unreferenced object should be something
like:

  - reachable by reflogs, which are pruned after 30 days (or
gc.reflogExpireUnreachable config). Objects will be repacked as
normal during this time. Override with "reflog expire" as you did
above.

  - after the reflog expires, the objects are now unreachable. During
the next repack, they'll be ejected from the pack into loose
objects, and their mtimes set to match the pack they came from
(which is probably quite recent if you just repacked!).

  - after 2 weeks (or gc.pruneExpire), unreachable loose objects are
dropped by "git prune", which is called as part of "git gc". This is
based on the object mtime.

You can accelerate this with "git gc --prune=now" (or
"--prune=5.minutes.ago").

-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 6/6] convert.c: Refactor crlf_action

2016-02-02 Thread 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 | 77 ++-
 1 file changed, 46 insertions(+), 31 deletions(-)

diff --git a/convert.c b/convert.c
index 50bdc42..f6ad9d6 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, &stats);
 
-   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, &stats))
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"))
retu

[PATCH v1 1/6] t0027: Add tests for get_stream_filter()

2016-02-02 Thread 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 
---
 t/t0027-auto-crlf.sh | 262 ---
 1 file changed, 146 insertions(+), 116 deletions(-)

diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 504e5a0..681f0c5 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -21,32 +21,45 @@ compare_ws_file () {
pfx=$1
exp=$2.expect
act=$pfx.actual.$3
-   tr '\015\000' QN <"$2" >"$exp" &&
-   tr '\015\000' QN <"$3" >"$act" &&
+   tr '\015\000abcdef01234567890' QN0 <"$2" >"$exp" &&
+   tr '\015\000abcdef01234567890' QN0 <"$3" >"$act" &&
test_cmp $exp $act &&
rm $exp $act
 }
 
 create_gitattributes () {
attr=$1
+   ident=$2
+   case "$2" in
+   "")
+   >.gitattributes
+   ;;
+   i)
+   echo "* ident" >.gitattributes
+   ;;
+   *)
+   echo >&2 invalid ident: $2
+   exit 1
+   esac
+
case "$attr" in
auto)
-   echo "*.txt text=auto" >.gitattributes
+   echo "*.txt text=auto" >>.gitattributes
;;
text)
-   echo "*.txt text" >.gitattributes
+   echo "*.txt text" >>.gitattributes
;;
-text)
-   echo "*.txt -text" >.gitattributes
+   echo "*.txt -text" >>.gitattributes
;;
crlf)
-   echo "*.txt eol=crlf" >.gitattributes
+   echo "*.txt eol=crlf" >>.gitattributes
;;
lf)
-   echo "*.txt eol=lf" >.gitattributes
+   echo "*.txt eol=lf" >>.gitattributes
;;
"")
-   echo >.gitattributes
+   #echo >.gitattributes
;;
*)
echo >&2 invalid attribute: $attr
@@ -90,7 +103,7 @@ commit_check_warn () {
lfmixcr=$6
crlfnul=$7
pfx=crlf_${crlf}_attr_${attr}
-   create_gitattributes "$attr" &&
+   create_gitattributes "$attr" "" &&
for f in LF CRLF LF_mix_CR CRLF_mix_LF LF_nul CRLF_nul
do
fname=${pfx}_$f.txt &&
@@ -115,7 +128,7 @@ commit_chk_wrnNNO () {
crlfnul=$7
pfx=NNO_${crlf}_attr_${attr}
#Commit files on top of existing file
-   create_gitattributes "$attr" &&
+   create_gitattributes "$attr" "" &&
for f in LF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul
do
fname=${pfx}_$f.txt &&
@@ -208,28 +221,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 i=$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 +259,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_succ

Re: [ANNOUNCE] Git Developer Summit, April 4th, 2016, NYC

2016-02-02 Thread Jeff King
On Tue, Feb 02, 2016 at 11:12:40AM -0500, Jeff King wrote:

>   https://www.ticketbase.com/events/git-merge-core-contributors-summit
> 
> It will be April 4th in New York City, from 11am-5pm, and is open to
> people who develop git or any of its alternate implementations, or tools
> that are closely tied to git.

I have a few financial logistics questions for the Git community.

I had a few people ask me earlier about travel assistance. I don't know
how much room there is in GitHub's budget for this[1].  I'll look into
getting money there, but we may also want to spend Git project funds for
this.

Does anybody have opinions on the logistics? I'm generally in favor of
transparency when it comes to handling project affairs, but my
inclination is to keep travel assistance requests and rewards off the
list to protect people's privacy. So the simplest scheme is probably
something like:

  1. I'll collect names of people who are interested in assistance.

  2. I'll present the list to the Git committee of Junio, Shawn, and
 myself. We'll come up with a proposal for who we can fund and how
 much.

  3. We'll send the amount (but not the names) to the list for comment.

  4. Barring objections, we'll tell Conservancy to authorized the money.

I'm open to comments on the process as a whole, or guidelines for us to
use in step 2. It's the project's money, so I'd like to give as much
opportunity as possible for people to have a say in how it's spent.

-Peff

[1] There is a conference fee for the "main" day, which I think will be
$99/person, but the plan is for all proceeds to go to
the Software Freedom Conservancy. The dev summit is free. I can look
into waivers for individual git contributors, but I'd prefer to
avoid doing a blanket waiver. For those folks whose employers will
send them to a conference, I think it's nice for that money to go to
Conservancy.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git object-count differs between clones

2016-02-02 Thread Andrew Martin
- Original Message -
> From: "Matthieu Moy" 
> To: "Andrew Martin" 
> Cc: git@vger.kernel.org
> Sent: Tuesday, February 2, 2016 10:09:31 AM
> Subject: Re: git object-count differs between clones
> 
> Andrew Martin  writes:
> 
> > I ran "git fsck" on both, which reported no problems. Moreover, I ran "git
> > gc"
> > and made sure there were no objects pending garbage collection,
> 
> It's not sufficient: you may have objects reachable from your reflog,
> hence not candidate for garbage collection. Since the reflog is not
> propagated, pushing + cloning will not transfer these objects if the
> reflog is the only way to reach them.
> 
> You may try expiring your reflog and "git gc" again.
> 
Matthieu,

Thanks, I found some commits that are not referenced in any branch. How can I
remove these from the reflog? I tried running
"git reflog expire --expire=now --expire-unreachable=now --all" followed by
"git gc" but still the same number of objects remain.

Thanks,

Andrew
--
To unsubscribe from this list: send the line "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: BuGit: File-less distributed issue tracking system with Git

2016-02-02 Thread Stefan Monnier
> So, see attached BuGit, an issue tracking system which stores its
> database in Git to try and get "distributed operation for free".

It's now hosted at https://gitlab.com/monnier/bugit

In the mean time it grew to 80KB, offers a read-only web UI, email
notifications, and a fairly complete command line UI.

It also changed its internal representation slightly, so it can now be
used within the same Git repository as the project on which you're
working (e.g. I cloned the BuGit source code and BuGit's bug database
into the same repository, so I can use bugit directly from the source
code directory to manipulate its bugs).

You can even host the bug database and the source code of your project
in the same repository (the post-receive hook script can be told to pass
the non-bugit-related changes to some other script).


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


[ANNOUNCE] Git Developer Summit, April 4th, 2016, NYC

2016-02-02 Thread Jeff King
I mentioned earlier[1] that plans were in the works for a developer
summit as part of Git Merge this year. I'm pleased to announce that it's
definitely happening, and it's time to RSVP so we can get an official
headcount[2]:

  https://www.ticketbase.com/events/git-merge-core-contributors-summit

It will be April 4th in New York City, from 11am-5pm, and is open to
people who develop git or any of its alternate implementations, or tools
that are closely tied to git.

We have a venue that holds 25 people max. That should be enough based on
the responses I got to my earlier email. We can move to a larger venue,
but I'd have to know _very soon_. So please sign up promptly, and if you
need to wait to figure out your plans, let me know that you're maybe
interested.

There's no particular agenda for the meeting. People should show up with
some topics for discussion or presentation. There should be appropriate
seating for round-table discussion, and a projector.

The rest of the Git Merge conference will be the following day, the 5th,
with more formal talks to a bigger audience. I think there are still
speaking slots left; let me know if you'd like to give a talk to the
larger audience, and I can put you in touch with the organizers. Details
for that day are still forthcoming, but will be at:

  http://git-merge.com/

when they are available.

[1] http://article.gmane.org/gmane.comp.version-control.git/282634

[2] You need a password to register. My plan is to withhold it from this
email but hand it out liberally to people who are git contributors,
and it can hopefully pass by word of mouth to those who need it
without generating any spammy registrations.  I'll send an email
shortly to everybody who responded to my previous announcement, but
please don't hesitate to ask for it if you would like to come.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git object-count differs between clones

2016-02-02 Thread Matthieu Moy
Andrew Martin  writes:

> I ran "git fsck" on both, which reported no problems. Moreover, I ran "git gc"
> and made sure there were no objects pending garbage collection, 

It's not sufficient: you may have objects reachable from your reflog,
hence not candidate for garbage collection. Since the reflog is not
propagated, pushing + cloning will not transfer these objects if the
reflog is the only way to reach them.

You may try expiring your reflog and "git gc" again.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


git object-count differs between clones

2016-02-02 Thread Andrew Martin
Hello,

I am using git 2.7.0 on Ubuntu 14.04. I recently tried pushing a large (90,000+
commits) repository to a gogs server (https://gogs.io/) and then cloning the
repository back to my machine. After running "git count-objects -v", I see a
discrepancy in the number of objects:

Original Repository:
count: 0
size: 0
in-pack: 1258300
packs: 1
size-pack: 593889
prune-packable: 0
garbage: 0
size-garbage: 0


Clone from gogs:
count: 0
size: 0
in-pack: 1258270
packs: 1
size-pack: 593884
prune-packable: 0
garbage: 0
size-garbage: 0


I ran "git fsck" on both, which reported no problems. Moreover, I ran "git gc"
and made sure there were no objects pending garbage collection, but still I
cannot account for this difference. Can someone explain why these numbers
differ, and if this is a problem or not?

Thanks,

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


不接触光纤

2016-02-02 Thread 不接触光纤
你的老朋友邀你来Q群:343257759


Re: [PATCH v4 07/15] remote: die on config error when setting URL

2016-02-02 Thread Patrick Steinhardt
On Tue, Feb 02, 2016 at 12:51:48PM +0100, Patrick Steinhardt wrote:
> When invoking `git-remote --set-url` we do not check the return
> value when writing the actual new URL to the configuration file,
> pretending to the user that the configuration has been set while
> it was in fact not persisted.
> 
> Fix this problem by dying early when setting the config fails.
> 
> Signed-off-by: Patrick Steinhardt

Signed-off-by: Patrick Steinhardt 

Sorry, borked up that signed-off-by. Will fix in a later revision
(which I guess will be necessary anyway ;).

Patrick


signature.asc
Description: Digital signature


[PATCH v4 11/15] init-db: die on config errors when initializing empty repo

2016-02-02 Thread Patrick Steinhardt
When creating an empty repository with `git init-db` we do not
check for error codes returned by `git_config_set` functions.
This may cause the user to end up with an inconsistent repository
without any indication for the user.

Fix this problem by dying early with an error message when we are
unable to write the configuration files to disk.

Signed-off-by: Patrick Steinhardt 
---
 builtin/init-db.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 07229d6..ef19048 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -227,7 +227,7 @@ static int create_default_files(const char *template_path)
/* This forces creation of new config file */
xsnprintf(repo_version_string, sizeof(repo_version_string),
  "%d", GIT_REPO_VERSION);
-   git_config_set("core.repositoryformatversion", repo_version_string);
+   git_config_set_or_die("core.repositoryformatversion", 
repo_version_string);
 
/* Check filemode trustability */
path = git_path_buf(&buf, "config");
@@ -241,18 +241,18 @@ static int create_default_files(const char *template_path)
if (filemode && !reinit && (st1.st_mode & S_IXUSR))
filemode = 0;
}
-   git_config_set("core.filemode", filemode ? "true" : "false");
+   git_config_set_or_die("core.filemode", filemode ? "true" : "false");
 
if (is_bare_repository())
-   git_config_set("core.bare", "true");
+   git_config_set_or_die("core.bare", "true");
else {
const char *work_tree = get_git_work_tree();
-   git_config_set("core.bare", "false");
+   git_config_set_or_die("core.bare", "false");
/* allow template config file to override the default */
if (log_all_ref_updates == -1)
-   git_config_set("core.logallrefupdates", "true");
+   git_config_set_or_die("core.logallrefupdates", "true");
if (needs_work_tree_config(get_git_dir(), work_tree))
-   git_config_set("core.worktree", work_tree);
+   git_config_set_or_die("core.worktree", work_tree);
}
 
if (!reinit) {
@@ -265,12 +265,12 @@ static int create_default_files(const char *template_path)
S_ISLNK(st1.st_mode))
unlink(path); /* good */
else
-   git_config_set("core.symlinks", "false");
+   git_config_set_or_die("core.symlinks", "false");
 
/* Check if the filesystem is case-insensitive */
path = git_path_buf(&buf, "CoNfIg");
if (!access(path, F_OK))
-   git_config_set("core.ignorecase", "true");
+   git_config_set_or_die("core.ignorecase", "true");
probe_utf8_pathname_composition();
}
 
@@ -386,8 +386,8 @@ int init_db(const char *template_dir, unsigned int flags)
xsnprintf(buf, sizeof(buf), "%d", OLD_PERM_EVERYBODY);
else
die("BUG: invalid value for shared_repository");
-   git_config_set("core.sharedrepository", buf);
-   git_config_set("receive.denyNonFastforwards", "true");
+   git_config_set_or_die("core.sharedrepository", buf);
+   git_config_set_or_die("receive.denyNonFastforwards", "true");
}
 
if (!(flags & INIT_DB_QUIET)) {
-- 
2.7.0

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


[PATCH v4 09/15] remote: die on config error when manipulating remotes

2016-02-02 Thread Patrick Steinhardt
When manipulating remotes we try to set various configuration
values without checking if the values were persisted correctly,
possibly leaving the remote in an inconsistent state.

Fix this issue by dying early and notifying the user about the
error.

Signed-off-by: Patrick Steinhardt 
---
 builtin/remote.c | 39 ---
 1 file changed, 12 insertions(+), 27 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index eeb6d2e..fe57f77 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -197,8 +197,7 @@ static int add(int argc, const char **argv)
die(_("'%s' is not a valid remote name"), name);
 
strbuf_addf(&buf, "remote.%s.url", name);
-   if (git_config_set(buf.buf, url))
-   return 1;
+   git_config_set_or_die(buf.buf, url);
 
if (!mirror || mirror & MIRROR_FETCH) {
strbuf_reset(&buf);
@@ -214,16 +213,14 @@ static int add(int argc, const char **argv)
if (mirror & MIRROR_PUSH) {
strbuf_reset(&buf);
strbuf_addf(&buf, "remote.%s.mirror", name);
-   if (git_config_set(buf.buf, "true"))
-   return 1;
+   git_config_set_or_die(buf.buf, "true");
}
 
if (fetch_tags != TAGS_DEFAULT) {
strbuf_reset(&buf);
strbuf_addf(&buf, "remote.%s.tagopt", name);
-   if (git_config_set(buf.buf,
-   fetch_tags == TAGS_SET ? "--tags" : "--no-tags"))
-   return 1;
+   git_config_set_or_die(buf.buf,
+ fetch_tags == TAGS_SET ? "--tags" : 
"--no-tags");
}
 
if (fetch && fetch_remote(name))
@@ -591,25 +588,20 @@ static int migrate_file(struct remote *remote)
 
strbuf_addf(&buf, "remote.%s.url", remote->name);
for (i = 0; i < remote->url_nr; i++)
-   if (git_config_set_multivar(buf.buf, remote->url[i], "^$", 0))
-   return error(_("Could not append '%s' to '%s'"),
-   remote->url[i], buf.buf);
+   git_config_set_multivar_or_die(buf.buf, remote->url[i], "^$", 
0);
strbuf_reset(&buf);
strbuf_addf(&buf, "remote.%s.push", remote->name);
for (i = 0; i < remote->push_refspec_nr; i++)
-   if (git_config_set_multivar(buf.buf, remote->push_refspec[i], 
"^$", 0))
-   return error(_("Could not append '%s' to '%s'"),
-   remote->push_refspec[i], buf.buf);
+   git_config_set_multivar_or_die(buf.buf, 
remote->push_refspec[i], "^$", 0);
strbuf_reset(&buf);
strbuf_addf(&buf, "remote.%s.fetch", remote->name);
for (i = 0; i < remote->fetch_refspec_nr; i++)
-   if (git_config_set_multivar(buf.buf, remote->fetch_refspec[i], 
"^$", 0))
-   return error(_("Could not append '%s' to '%s'"),
-   remote->fetch_refspec[i], buf.buf);
+   git_config_set_multivar_or_die(buf.buf, 
remote->fetch_refspec[i], "^$", 0);
if (remote->origin == REMOTE_REMOTES)
unlink_or_warn(git_path("remotes/%s", remote->name));
else if (remote->origin == REMOTE_BRANCHES)
unlink_or_warn(git_path("branches/%s", remote->name));
+
return 0;
 }
 
@@ -656,8 +648,7 @@ static int mv(int argc, const char **argv)
 
strbuf_reset(&buf);
strbuf_addf(&buf, "remote.%s.fetch", rename.new);
-   if (git_config_set_multivar(buf.buf, NULL, NULL, 1))
-   return error(_("Could not remove config section '%s'"), 
buf.buf);
+   git_config_set_multivar_or_die(buf.buf, NULL, NULL, 1);
strbuf_addf(&old_remote_context, ":refs/remotes/%s/", rename.old);
for (i = 0; i < oldremote->fetch_refspec_nr; i++) {
char *ptr;
@@ -677,8 +668,7 @@ static int mv(int argc, const char **argv)
  "\tPlease update the configuration manually 
if necessary."),
buf2.buf);
 
-   if (git_config_set_multivar(buf.buf, buf2.buf, "^$", 0))
-   return error(_("Could not append '%s'"), buf.buf);
+   git_config_set_multivar_or_die(buf.buf, buf2.buf, "^$", 0);
}
 
read_branches();
@@ -688,9 +678,7 @@ static int mv(int argc, const char **argv)
if (info->remote_name && !strcmp(info->remote_name, 
rename.old)) {
strbuf_reset(&buf);
strbuf_addf(&buf, "branch.%s.remote", item->string);
-   if (git_config_set(buf.buf, rename.new)) {
-   return error(_("Could not set '%s'"), buf.buf);
-   }
+   git_config_set_or_die(buf.buf, rename.new);
}
}
 
@@ -788,10 +776,7 @@ static int rm(int argc, 

[PATCH v4 08/15] remote: die on config error when setting/adding branches

2016-02-02 Thread Patrick Steinhardt
When we add or set new branches (e.g. by `git remote add -f` or
`git remote set-branches`) we do not check for error codes when
writing the branches to the configuration file. When persisting
the configuration failed we are left with a remote that has none
or not all of the branches that should have been set without
notifying the user.

Fix this issue by dying early on configuration error.

Signed-off-by: Patrick Steinhardt 
---
 builtin/remote.c | 26 +-
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 8b78c3d..eeb6d2e 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -108,8 +108,8 @@ enum {
 #define MIRROR_PUSH 2
 #define MIRROR_BOTH (MIRROR_FETCH|MIRROR_PUSH)
 
-static int add_branch(const char *key, const char *branchname,
-   const char *remotename, int mirror, struct strbuf *tmp)
+static void add_branch(const char *key, const char *branchname,
+  const char *remotename, int mirror, struct strbuf *tmp)
 {
strbuf_reset(tmp);
strbuf_addch(tmp, '+');
@@ -119,7 +119,7 @@ static int add_branch(const char *key, const char 
*branchname,
else
strbuf_addf(tmp, "refs/heads/%s:refs/remotes/%s/%s",
branchname, remotename, branchname);
-   return git_config_set_multivar(key, tmp->buf, "^$", 0);
+   git_config_set_multivar_or_die(key, tmp->buf, "^$", 0);
 }
 
 static const char mirror_advice[] =
@@ -206,9 +206,8 @@ static int add(int argc, const char **argv)
if (track.nr == 0)
string_list_append(&track, "*");
for (i = 0; i < track.nr; i++) {
-   if (add_branch(buf.buf, track.items[i].string,
-  name, mirror, &buf2))
-   return 1;
+   add_branch(buf.buf, track.items[i].string,
+  name, mirror, &buf2);
}
}
 
@@ -1416,21 +1415,17 @@ static int remove_all_fetch_refspecs(const char 
*remote, const char *key)
return git_config_set_multivar(key, NULL, NULL, 1);
 }
 
-static int add_branches(struct remote *remote, const char **branches,
-   const char *key)
+static void add_branches(struct remote *remote, const char **branches,
+const char *key)
 {
const char *remotename = remote->name;
int mirror = remote->mirror;
struct strbuf refspec = STRBUF_INIT;
 
for (; *branches; branches++)
-   if (add_branch(key, *branches, remotename, mirror, &refspec)) {
-   strbuf_release(&refspec);
-   return 1;
-   }
+   add_branch(key, *branches, remotename, mirror, &refspec);
 
strbuf_release(&refspec);
-   return 0;
 }
 
 static int set_remote_branches(const char *remotename, const char **branches,
@@ -1449,10 +1444,7 @@ static int set_remote_branches(const char *remotename, 
const char **branches,
strbuf_release(&key);
return 1;
}
-   if (add_branches(remote, branches, key.buf)) {
-   strbuf_release(&key);
-   return 1;
-   }
+   add_branches(remote, branches, key.buf);
 
strbuf_release(&key);
return 0;
-- 
2.7.0

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


[PATCH v4 14/15] config: rename git_config_set to git_config_set_gently

2016-02-02 Thread Patrick Steinhardt
The desired default behavior for `git_config_set` is to die
whenever an error occurs. Dying is the default for a lot of
internal functions when failures occur and is in this case the
right thing to do for most callers as otherwise we might run into
inconsistent repositories without noticing.

As some code may rely on the actual return values for
`git_config_set` we still require the ability to invoke these
functions without aborting. Rename the existing `git_config_set`
functions to `git_config_set_gently` to keep them available for
those callers.

Signed-off-by: Patrick Steinhardt 
---
 builtin/clone.c  |  2 +-
 builtin/config.c | 28 ++--
 builtin/remote.c |  2 +-
 cache.h  | 10 +-
 config.c | 29 +++--
 submodule.c  |  2 +-
 6 files changed, 37 insertions(+), 36 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index f2a2f9a..dccfab3 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -735,7 +735,7 @@ static int checkout(void)
 
 static int write_one_config(const char *key, const char *value, void *data)
 {
-   return git_config_set_multivar(key, value ? value : "true", "^$", 0);
+   return git_config_set_multivar_gently(key, value ? value : "true", 
"^$", 0);
 }
 
 static void write_config(struct string_list *config)
diff --git a/builtin/config.c b/builtin/config.c
index adc7727..c26d6e7 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -582,7 +582,7 @@ int cmd_config(int argc, const char **argv, const char 
*prefix)
check_write();
check_argc(argc, 2, 2);
value = normalize_value(argv[0], argv[1]);
-   ret = git_config_set_in_file(given_config_source.file, argv[0], 
value);
+   ret = git_config_set_in_file_gently(given_config_source.file, 
argv[0], value);
if (ret == CONFIG_NOTHING_SET)
error("cannot overwrite multiple values with a single 
value\n"
"   Use a regexp, --add or --replace-all to change 
%s.", argv[0]);
@@ -592,23 +592,23 @@ int cmd_config(int argc, const char **argv, const char 
*prefix)
check_write();
check_argc(argc, 2, 3);
value = normalize_value(argv[0], argv[1]);
-   return git_config_set_multivar_in_file(given_config_source.file,
-  argv[0], value, argv[2], 
0);
+   return 
git_config_set_multivar_in_file_gently(given_config_source.file,
+ argv[0], value, 
argv[2], 0);
}
else if (actions == ACTION_ADD) {
check_write();
check_argc(argc, 2, 2);
value = normalize_value(argv[0], argv[1]);
-   return git_config_set_multivar_in_file(given_config_source.file,
-  argv[0], value,
-  CONFIG_REGEX_NONE, 0);
+   return 
git_config_set_multivar_in_file_gently(given_config_source.file,
+ argv[0], value,
+ 
CONFIG_REGEX_NONE, 0);
}
else if (actions == ACTION_REPLACE_ALL) {
check_write();
check_argc(argc, 2, 3);
value = normalize_value(argv[0], argv[1]);
-   return git_config_set_multivar_in_file(given_config_source.file,
-  argv[0], value, argv[2], 
1);
+   return 
git_config_set_multivar_in_file_gently(given_config_source.file,
+ argv[0], value, 
argv[2], 1);
}
else if (actions == ACTION_GET) {
check_argc(argc, 1, 2);
@@ -634,17 +634,17 @@ int cmd_config(int argc, const char **argv, const char 
*prefix)
check_write();
check_argc(argc, 1, 2);
if (argc == 2)
-   return 
git_config_set_multivar_in_file(given_config_source.file,
-  argv[0], NULL, 
argv[1], 0);
+   return 
git_config_set_multivar_in_file_gently(given_config_source.file,
+ argv[0], 
NULL, argv[1], 0);
else
-   return git_config_set_in_file(given_config_source.file,
- argv[0], NULL);
+   return 
git_config_set_in_file_gently(given_config_source.file,
+argv[0], NULL);
}
else if (actions == ACTION_UNSET_ALL) {
check_write();
check_argc(argc, 1, 2);
-   r

[PATCH v4 15/15] config: rename git_config_set_or_die to git_config_set

2016-02-02 Thread Patrick Steinhardt
Rename git_config_set_or_die functions to git_config_set, leading
to the new default behavior of dying whenever a configuration
error occurs.

By now all callers that shall die on error have been transitioned
to the _or_die variants, thus making this patch a simple rename
of the functions.

Signed-off-by: Patrick Steinhardt 
---
 branch.c|  6 +++---
 builtin/branch.c|  6 +++---
 builtin/clone.c |  8 
 builtin/init-db.c   | 20 ++--
 builtin/remote.c| 32 
 builtin/submodule--helper.c |  4 ++--
 cache.h |  8 
 config.c| 24 
 sequencer.c | 22 +++---
 submodule.c |  6 +++---
 10 files changed, 68 insertions(+), 68 deletions(-)

diff --git a/branch.c b/branch.c
index 7106369..0c11023 100644
--- a/branch.c
+++ b/branch.c
@@ -64,16 +64,16 @@ void install_branch_config(int flag, const char *local, 
const char *origin, cons
}
 
strbuf_addf(&key, "branch.%s.remote", local);
-   git_config_set_or_die(key.buf, origin ? origin : ".");
+   git_config_set(key.buf, origin ? origin : ".");
 
strbuf_reset(&key);
strbuf_addf(&key, "branch.%s.merge", local);
-   git_config_set_or_die(key.buf, remote);
+   git_config_set(key.buf, remote);
 
if (rebasing) {
strbuf_reset(&key);
strbuf_addf(&key, "branch.%s.rebase", local);
-   git_config_set_or_die(key.buf, "true");
+   git_config_set(key.buf, "true");
}
strbuf_release(&key);
 
diff --git a/builtin/branch.c b/builtin/branch.c
index c043cfc..7b45b6b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -594,7 +594,7 @@ static int edit_branch_description(const char *branch_name)
strbuf_stripspace(&buf, 1);
 
strbuf_addf(&name, "branch.%s.description", branch_name);
-   git_config_set_or_die(name.buf, buf.len ? buf.buf : NULL);
+   git_config_set(name.buf, buf.len ? buf.buf : NULL);
strbuf_release(&name);
strbuf_release(&buf);
 
@@ -790,10 +790,10 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
die(_("Branch '%s' has no upstream information"), 
branch->name);
 
strbuf_addf(&buf, "branch.%s.remote", branch->name);
-   git_config_set_multivar_or_die(buf.buf, NULL, NULL, 1);
+   git_config_set_multivar(buf.buf, NULL, NULL, 1);
strbuf_reset(&buf);
strbuf_addf(&buf, "branch.%s.merge", branch->name);
-   git_config_set_multivar_or_die(buf.buf, NULL, NULL, 1);
+   git_config_set_multivar(buf.buf, NULL, NULL, 1);
strbuf_release(&buf);
} else if (argc > 0 && argc <= 2) {
struct branch *branch = branch_get(argv[0]);
diff --git a/builtin/clone.c b/builtin/clone.c
index dccfab3..ecdd80a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -786,12 +786,12 @@ static void write_refspec_config(const char 
*src_ref_prefix,
/* Configure the remote */
if (value.len) {
strbuf_addf(&key, "remote.%s.fetch", option_origin);
-   git_config_set_multivar_or_die(key.buf, value.buf, 
"^$", 0);
+   git_config_set_multivar(key.buf, value.buf, "^$", 0);
strbuf_reset(&key);
 
if (option_mirror) {
strbuf_addf(&key, "remote.%s.mirror", 
option_origin);
-   git_config_set_or_die(key.buf, "true");
+   git_config_set(key.buf, "true");
strbuf_reset(&key);
}
}
@@ -949,14 +949,14 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
src_ref_prefix = "refs/";
strbuf_addstr(&branch_top, src_ref_prefix);
 
-   git_config_set_or_die("core.bare", "true");
+   git_config_set("core.bare", "true");
} else {
strbuf_addf(&branch_top, "refs/remotes/%s/", option_origin);
}
 
strbuf_addf(&value, "+%s*:%s*", src_ref_prefix, branch_top.buf);
strbuf_addf(&key, "remote.%s.url", option_origin);
-   git_config_set_or_die(key.buf, repo);
+   git_config_set(key.buf, repo);
strbuf_reset(&key);
 
if (option_reference.nr)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index ef19048..6223b7d 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -227,7 +227,7 @@ static int create_default_files(const char *template_path)
/* This forces creation of new config file */
xsnprintf(repo_version_string, sizeof(repo_version_string),
  "%d", GIT_REPO_VERSION);
-   git_config_set_or_die("cor

[PATCH v4 13/15] compat: die when unable to set core.precomposeunicode

2016-02-02 Thread Patrick Steinhardt
When calling `git_config_set` to set 'core.precomposeunicode' we
ignore the return value of the function, which may indicate that
we were unable to write the value back to disk. As the function
is only called by init-db we can and should die when an error
occurs.

Signed-off-by: Patrick Steinhardt 
---
 compat/precompose_utf8.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
index 079070f..9ff1ebe 100644
--- a/compat/precompose_utf8.c
+++ b/compat/precompose_utf8.c
@@ -50,7 +50,8 @@ void probe_utf8_pathname_composition(void)
close(output_fd);
git_path_buf(&path, "%s", auml_nfd);
precomposed_unicode = access(path.buf, R_OK) ? 0 : 1;
-   git_config_set("core.precomposeunicode", precomposed_unicode ? 
"true" : "false");
+   git_config_set_or_die("core.precomposeunicode",
+ precomposed_unicode ? "true" : "false");
git_path_buf(&path, "%s", auml_nfc);
if (unlink(path.buf))
die_errno(_("failed to unlink '%s'"), path.buf);
-- 
2.7.0

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


[PATCH v4 12/15] sequencer: die on config error when saving replay opts

2016-02-02 Thread Patrick Steinhardt
When we start picking a range of revisions we save the replay
options that are required to restore state when interrupting and
later continuing picking the revisions. However, we do not check
the return values of the `git_config_set` functions, which may
lead us to store incomplete information. As this may lead us to
fail when trying to continue the sequence the error can be fatal.

Fix this by dying immediately when we are unable to write back
any replay option.

Signed-off-by: Patrick Steinhardt 
---
 sequencer.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 8048786..3590248 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -933,31 +933,31 @@ static void save_opts(struct replay_opts *opts)
const char *opts_file = git_path_opts_file();
 
if (opts->no_commit)
-   git_config_set_in_file(opts_file, "options.no-commit", "true");
+   git_config_set_in_file_or_die(opts_file, "options.no-commit", 
"true");
if (opts->edit)
-   git_config_set_in_file(opts_file, "options.edit", "true");
+   git_config_set_in_file_or_die(opts_file, "options.edit", 
"true");
if (opts->signoff)
-   git_config_set_in_file(opts_file, "options.signoff", "true");
+   git_config_set_in_file_or_die(opts_file, "options.signoff", 
"true");
if (opts->record_origin)
-   git_config_set_in_file(opts_file, "options.record-origin", 
"true");
+   git_config_set_in_file_or_die(opts_file, 
"options.record-origin", "true");
if (opts->allow_ff)
-   git_config_set_in_file(opts_file, "options.allow-ff", "true");
+   git_config_set_in_file_or_die(opts_file, "options.allow-ff", 
"true");
if (opts->mainline) {
struct strbuf buf = STRBUF_INIT;
strbuf_addf(&buf, "%d", opts->mainline);
-   git_config_set_in_file(opts_file, "options.mainline", buf.buf);
+   git_config_set_in_file_or_die(opts_file, "options.mainline", 
buf.buf);
strbuf_release(&buf);
}
if (opts->strategy)
-   git_config_set_in_file(opts_file, "options.strategy", 
opts->strategy);
+   git_config_set_in_file_or_die(opts_file, "options.strategy", 
opts->strategy);
if (opts->gpg_sign)
-   git_config_set_in_file(opts_file, "options.gpg-sign", 
opts->gpg_sign);
+   git_config_set_in_file_or_die(opts_file, "options.gpg-sign", 
opts->gpg_sign);
if (opts->xopts) {
int i;
for (i = 0; i < opts->xopts_nr; i++)
-   git_config_set_multivar_in_file(opts_file,
-   
"options.strategy-option",
-   opts->xopts[i], "^$", 
0);
+   git_config_set_multivar_in_file_or_die(opts_file,
+  
"options.strategy-option",
+  opts->xopts[i], 
"^$", 0);
}
 }
 
-- 
2.7.0

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


[PATCH v4 10/15] clone: die on config error in cmd_clone

2016-02-02 Thread Patrick Steinhardt
The clone command does not check for error codes returned by
`git_config_set` functions. This may cause the user to end up
with an inconsistent repository without any indication with what
went wrong.

Fix this problem by dying with an error message when we are
unable to write the configuration files to disk.

Signed-off-by: Patrick Steinhardt 
---
 builtin/clone.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 81e238f..f2a2f9a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -786,12 +786,12 @@ static void write_refspec_config(const char 
*src_ref_prefix,
/* Configure the remote */
if (value.len) {
strbuf_addf(&key, "remote.%s.fetch", option_origin);
-   git_config_set_multivar(key.buf, value.buf, "^$", 0);
+   git_config_set_multivar_or_die(key.buf, value.buf, 
"^$", 0);
strbuf_reset(&key);
 
if (option_mirror) {
strbuf_addf(&key, "remote.%s.mirror", 
option_origin);
-   git_config_set(key.buf, "true");
+   git_config_set_or_die(key.buf, "true");
strbuf_reset(&key);
}
}
@@ -949,14 +949,14 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
src_ref_prefix = "refs/";
strbuf_addstr(&branch_top, src_ref_prefix);
 
-   git_config_set("core.bare", "true");
+   git_config_set_or_die("core.bare", "true");
} else {
strbuf_addf(&branch_top, "refs/remotes/%s/", option_origin);
}
 
strbuf_addf(&value, "+%s*:%s*", src_ref_prefix, branch_top.buf);
strbuf_addf(&key, "remote.%s.url", option_origin);
-   git_config_set(key.buf, repo);
+   git_config_set_or_die(key.buf, repo);
strbuf_reset(&key);
 
if (option_reference.nr)
-- 
2.7.0

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


[PATCH v4 01/15] config: introduce set_or_die wrappers

2016-02-02 Thread Patrick Steinhardt
A lot of call-sites for the existing family of `git_config_set`
functions do not check for errors that may occur, e.g. when the
configuration file is locked. In many cases we simply want to die
when such a situation arises.

Introduce wrappers that will cause the program to die in those
cases. These wrappers are temporary only to ease the transition
to let `git_config_set` die by default. They will be removed
later on when `git_config_set` itself has been replaced by
`git_config_set_gently`.

Signed-off-by: Patrick Steinhardt 
---
 cache.h  |  4 
 config.c | 27 +++
 2 files changed, 31 insertions(+)

diff --git a/cache.h b/cache.h
index dfc459c..c1f844d 100644
--- a/cache.h
+++ b/cache.h
@@ -1508,11 +1508,15 @@ extern int git_config_maybe_bool(const char *, const 
char *);
 extern int git_config_string(const char **, const char *, const char *);
 extern int git_config_pathname(const char **, const char *, const char *);
 extern int git_config_set_in_file(const char *, const char *, const char *);
+extern void git_config_set_in_file_or_die(const char *, const char *, const 
char *);
 extern int git_config_set(const char *, const char *);
+extern void git_config_set_or_die(const char *, const char *);
 extern int git_config_parse_key(const char *, char **, int *);
 extern int git_config_key_is_valid(const char *key);
 extern int git_config_set_multivar(const char *, const char *, const char *, 
int);
+extern void git_config_set_multivar_or_die(const char *, const char *, const 
char *, int);
 extern int git_config_set_multivar_in_file(const char *, const char *, const 
char *, const char *, int);
+extern void git_config_set_multivar_in_file_or_die(const char *, const char *, 
const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
 extern int git_config_rename_section_in_file(const char *, const char *, const 
char *);
 extern const char *git_etc_gitconfig(void);
diff --git a/config.c b/config.c
index 86a5eb2..856f7d34 100644
--- a/config.c
+++ b/config.c
@@ -1831,11 +1831,22 @@ int git_config_set_in_file(const char *config_filename,
return git_config_set_multivar_in_file(config_filename, key, value, 
NULL, 0);
 }
 
+void git_config_set_in_file_or_die(const char *config_filename,
+   const char *key, const char *value)
+{
+   git_config_set_multivar_in_file_or_die(config_filename, key, value, 
NULL, 0);
+}
+
 int git_config_set(const char *key, const char *value)
 {
return git_config_set_multivar(key, value, NULL, 0);
 }
 
+void git_config_set_or_die(const char *key, const char *value)
+{
+   git_config_set_multivar_or_die(key, value, NULL, 0);
+}
+
 /*
  * Auxiliary function to sanity-check and split the key into the section
  * identifier and variable name.
@@ -2179,6 +2190,15 @@ write_err_out:
 
 }
 
+void git_config_set_multivar_in_file_or_die(const char *config_filename,
+   const char *key, const char *value,
+   const char *value_regex, int multi_replace)
+{
+   if (git_config_set_multivar_in_file(config_filename, key, value,
+   value_regex, multi_replace) < 0)
+   die(_("Could not set '%s' to '%s'"), key, value);
+}
+
 int git_config_set_multivar(const char *key, const char *value,
const char *value_regex, int multi_replace)
 {
@@ -2186,6 +2206,13 @@ int git_config_set_multivar(const char *key, const char 
*value,
   multi_replace);
 }
 
+void git_config_set_multivar_or_die(const char *key, const char *value,
+   const char *value_regex, int multi_replace)
+{
+   git_config_set_multivar_in_file_or_die(NULL, key, value, value_regex,
+  multi_replace);
+}
+
 static int section_name_match (const char *buf, const char *name)
 {
int i = 0, j = 0, dot = 0;
-- 
2.7.0

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


[PATCH v4 05/15] submodule: die on config error when linking modules

2016-02-02 Thread Patrick Steinhardt
When trying to connect a submodule with its corresponding
repository in '.git/modules' we try to set the core.worktree
setting in the submodule, which may fail due to an error
encountered in `git_config_set_in_file`.

The function is used in the git-mv command when trying to move a
submodule to another location. We already die when renaming a
file fails but do not pay attention to the case where updating
the connection between submodule and its repository fails. As
this leaves the repository in an inconsistent state, as well,
abort the program by dying early and presenting the failure to
the user.

Signed-off-by: Patrick Steinhardt 
---
 submodule.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/submodule.c b/submodule.c
index b83939c..589a82c 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1087,11 +1087,9 @@ void connect_work_tree_and_git_dir(const char 
*work_tree, const char *git_dir)
/* Update core.worktree setting */
strbuf_reset(&file_name);
strbuf_addf(&file_name, "%s/config", git_dir);
-   if (git_config_set_in_file(file_name.buf, "core.worktree",
-  relative_path(real_work_tree, git_dir,
-&rel_path)))
-   die(_("Could not set core.worktree in %s"),
-   file_name.buf);
+   git_config_set_in_file_or_die(file_name.buf, "core.worktree",
+ relative_path(real_work_tree, git_dir,
+   &rel_path));
 
strbuf_release(&file_name);
strbuf_release(&rel_path);
-- 
2.7.0

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


[PATCH v4 07/15] remote: die on config error when setting URL

2016-02-02 Thread Patrick Steinhardt
When invoking `git-remote --set-url` we do not check the return
value when writing the actual new URL to the configuration file,
pretending to the user that the configuration has been set while
it was in fact not persisted.

Fix this problem by dying early when setting the config fails.

Signed-off-by: Patrick Steinhardt
---
 builtin/remote.c  | 11 ++-
 t/t5505-remote.sh |  9 +
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 2b2ff9b..8b78c3d 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1583,11 +1583,12 @@ static int set_url(int argc, const char **argv)
/* Special cases that add new entry. */
if ((!oldurl && !delete_mode) || add_mode) {
if (add_mode)
-   git_config_set_multivar(name_buf.buf, newurl,
-   "^$", 0);
+   git_config_set_multivar_or_die(name_buf.buf, newurl,
+  "^$", 0);
else
-   git_config_set(name_buf.buf, newurl);
+   git_config_set_or_die(name_buf.buf, newurl);
strbuf_release(&name_buf);
+
return 0;
}
 
@@ -1608,9 +1609,9 @@ static int set_url(int argc, const char **argv)
regfree(&old_regex);
 
if (!delete_mode)
-   git_config_set_multivar(name_buf.buf, newurl, oldurl, 0);
+   git_config_set_multivar_or_die(name_buf.buf, newurl, oldurl, 0);
else
-   git_config_set_multivar(name_buf.buf, NULL, oldurl, 1);
+   git_config_set_multivar_or_die(name_buf.buf, NULL, oldurl, 1);
return 0;
 }
 
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 1a8e3b8..e019f27 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -932,6 +932,15 @@ test_expect_success 'get-url on new remote' '
echo foo | get_url_test --push --all someremote
 '
 
+test_expect_success 'remote set-url with locked config' '
+   test_when_finished "rm -f .git/config.lock" &&
+   git config --get-all remote.someremote.url >expect &&
+   >.git/config.lock &&
+   test_must_fail git remote set-url someremote baz &&
+   git config --get-all remote.someremote.url >actual &&
+   cmp expect actual
+'
+
 test_expect_success 'remote set-url bar' '
git remote set-url someremote bar &&
echo bar >expect &&
-- 
2.7.0

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


[PATCH v4 06/15] submodule--helper: die on config error when cloning module

2016-02-02 Thread Patrick Steinhardt
When setting the 'core.worktree' option for a newly cloned
submodule we ignore the return value of `git_config_set_in_file`.
As this leaves the submodule in an inconsistent state, we instaed
we want to inform the user that something has gone wrong by
printing an error and aborting the program.

Signed-off-by: Patrick Steinhardt 
---
 builtin/submodule--helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f4c3eff..c7e1ea2 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -245,8 +245,8 @@ static int module_clone(int argc, const char **argv, const 
char *prefix)
p = git_pathdup_submodule(path, "config");
if (!p)
die(_("could not get submodule directory for '%s'"), path);
-   git_config_set_in_file(p, "core.worktree",
-  relative_path(sb.buf, sm_gitdir, &rel_path));
+   git_config_set_in_file_or_die(p, "core.worktree",
+ relative_path(sb.buf, sm_gitdir, 
&rel_path));
strbuf_release(&sb);
strbuf_release(&rel_path);
free(sm_gitdir);
-- 
2.7.0

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


[PATCH v4 02/15] branch: die on error in setting up tracking branch

2016-02-02 Thread Patrick Steinhardt
The setup_tracking function calls install_branch_config, which
may fail writing the configuration due to a locked configuration
file or other error conditions. setup_tracking can also fail when
trying to track ambiguous information for a reference. While this
condition is checked for and an error code is returned, this
error is not checked by the caller.

Fix both issues by dying early when error occur.

Signed-off-by: Patrick Steinhardt 
---
 branch.c  | 19 +--
 branch.h  |  1 +
 t/t3200-branch.sh |  9 -
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/branch.c b/branch.c
index 7ff3f20..7106369 100644
--- a/branch.c
+++ b/branch.c
@@ -64,16 +64,16 @@ void install_branch_config(int flag, const char *local, 
const char *origin, cons
}
 
strbuf_addf(&key, "branch.%s.remote", local);
-   git_config_set(key.buf, origin ? origin : ".");
+   git_config_set_or_die(key.buf, origin ? origin : ".");
 
strbuf_reset(&key);
strbuf_addf(&key, "branch.%s.merge", local);
-   git_config_set(key.buf, remote);
+   git_config_set_or_die(key.buf, remote);
 
if (rebasing) {
strbuf_reset(&key);
strbuf_addf(&key, "branch.%s.rebase", local);
-   git_config_set(key.buf, "true");
+   git_config_set_or_die(key.buf, "true");
}
strbuf_release(&key);
 
@@ -109,8 +109,8 @@ void install_branch_config(int flag, const char *local, 
const char *origin, cons
  * to infer the settings for branch..{remote,merge} from the
  * config.
  */
-static int setup_tracking(const char *new_ref, const char *orig_ref,
- enum branch_track track, int quiet)
+static void setup_tracking(const char *new_ref, const char *orig_ref,
+  enum branch_track track, int quiet)
 {
struct tracking tracking;
int config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;
@@ -118,7 +118,7 @@ static int setup_tracking(const char *new_ref, const char 
*orig_ref,
memset(&tracking, 0, sizeof(tracking));
tracking.spec.dst = (char *)orig_ref;
if (for_each_remote(find_tracked_branch, &tracking))
-   return 1;
+   return;
 
if (!tracking.matches)
switch (track) {
@@ -127,18 +127,17 @@ static int setup_tracking(const char *new_ref, const char 
*orig_ref,
case BRANCH_TRACK_OVERRIDE:
break;
default:
-   return 1;
+   return;
}
 
if (tracking.matches > 1)
-   return error(_("Not tracking: ambiguous information for ref 
%s"),
-   orig_ref);
+   die(_("Not tracking: ambiguous information for ref %s"),
+   orig_ref);
 
install_branch_config(config_flags, new_ref, tracking.remote,
  tracking.src ? tracking.src : orig_ref);
 
free(tracking.src);
-   return 0;
 }
 
 int read_branch_desc(struct strbuf *buf, const char *branch_name)
diff --git a/branch.h b/branch.h
index 58aa45f..8ce22f8 100644
--- a/branch.h
+++ b/branch.h
@@ -43,6 +43,7 @@ void remove_branch_state(void);
 /*
  * Configure local branch "local" as downstream to branch "remote"
  * from remote "origin".  Used by git branch --set-upstream.
+ * Dies if unable to install branch config.
  */
 #define BRANCH_CONFIG_VERBOSE 01
 extern void install_branch_config(int flag, const char *local, const char 
*origin, const char *remote);
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index cdaf6f6..dd776b3 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -446,6 +446,13 @@ test_expect_success '--set-upstream-to fails on a non-ref' 
'
test_must_fail git branch --set-upstream-to HEAD^{}
 '
 
+test_expect_success '--set-upstream-to fails on locked config' '
+   test_when_finished "rm -f .git/config.lock" &&
+   >.git/config.lock &&
+   git branch locked &&
+   test_must_fail git branch --set-upstream-to locked
+'
+
 test_expect_success 'use --set-upstream-to modify HEAD' '
test_config branch.master.remote foo &&
test_config branch.master.merge foo &&
@@ -579,7 +586,7 @@ test_expect_success 'avoid ambiguous track' '
git config remote.ambi1.fetch refs/heads/lalala:refs/heads/master &&
git config remote.ambi2.url lilili &&
git config remote.ambi2.fetch refs/heads/lilili:refs/heads/master &&
-   git branch all1 master &&
+   test_must_fail git branch all1 master &&
test -z "$(git config branch.all1.merge)"
 '
 
-- 
2.7.0

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


[PATCH v4 03/15] branch: die on config error when unsetting upstream

2016-02-02 Thread Patrick Steinhardt
When we try to unset upstream configurations we do not check
return codes for the `git_config_set` functions. As those may
indicate that we were unable to unset the respective
configuration we may exit successfully without any error message
while in fact the upstream configuration was not unset.

Fix this by dying with an error message when we cannot unset the
configuration.

Signed-off-by: Patrick Steinhardt 
---
 builtin/branch.c  | 4 ++--
 t/t3200-branch.sh | 7 +++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 3f6c825..0978287 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -791,10 +791,10 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
die(_("Branch '%s' has no upstream information"), 
branch->name);
 
strbuf_addf(&buf, "branch.%s.remote", branch->name);
-   git_config_set_multivar(buf.buf, NULL, NULL, 1);
+   git_config_set_multivar_or_die(buf.buf, NULL, NULL, 1);
strbuf_reset(&buf);
strbuf_addf(&buf, "branch.%s.merge", branch->name);
-   git_config_set_multivar(buf.buf, NULL, NULL, 1);
+   git_config_set_multivar_or_die(buf.buf, NULL, NULL, 1);
strbuf_release(&buf);
} else if (argc > 0 && argc <= 2) {
struct branch *branch = branch_get(argv[0]);
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index dd776b3..a897248 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -473,6 +473,13 @@ test_expect_success '--unset-upstream should fail if given 
a non-existent branch
test_must_fail git branch --unset-upstream i-dont-exist
 '
 
+test_expect_success '--unset-upstream should fail if config is locked' '
+   test_when_finished "rm -f .git/config.lock" &&
+   git branch --set-upstream-to locked &&
+   >.git/config.lock &&
+   test_must_fail git branch --unset-upstream
+'
+
 test_expect_success 'test --unset-upstream on HEAD' '
git branch my14 &&
test_config branch.master.remote foo &&
-- 
2.7.0

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


[PATCH v4 04/15] branch: die on config error when editing branch description

2016-02-02 Thread Patrick Steinhardt
---
 builtin/branch.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 0978287..c043cfc 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -570,7 +570,6 @@ static const char edit_description[] = "BRANCH_DESCRIPTION";
 
 static int edit_branch_description(const char *branch_name)
 {
-   int status;
struct strbuf buf = STRBUF_INIT;
struct strbuf name = STRBUF_INIT;
 
@@ -595,11 +594,11 @@ static int edit_branch_description(const char 
*branch_name)
strbuf_stripspace(&buf, 1);
 
strbuf_addf(&name, "branch.%s.description", branch_name);
-   status = git_config_set(name.buf, buf.len ? buf.buf : NULL);
+   git_config_set_or_die(name.buf, buf.len ? buf.buf : NULL);
strbuf_release(&name);
strbuf_release(&buf);
 
-   return status;
+   return 0;
 }
 
 int cmd_branch(int argc, const char **argv, const char *prefix)
-- 
2.7.0

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


[PATCH v4 00/15] config: make git_config_set die on failure

2016-02-02 Thread Patrick Steinhardt
This is the fourth version of my patch series. Version three of
these patches can be found at [1]. These patches convert the
`git_config_set` family of functions such that they die by
default whenever an error is encountered in persisting configs.
This catches a lot of cases where we wrote configs without
checking the returned status code, thus leading to inconsistent
state witouth even notifying the user.

This version combines both version 2 ([2]) and version 3 of this
patch series in that we first introduce `git_config_set_or_die`
wrappers and converting most call sites to use these. After the
conversion is done, we rename `git_config_set` to
`git_config_set_gently` and adjusting remaining call sites. The
last step was to rename `git_config_set_or_die` to
`git_config_set` in order to get the desired default behavior.

This back-and-forth should hopefully help easing the transition
and review by breaking down the actual transition into small
pieces.

[1]: http://article.gmane.org/gmane.comp.version-control.git/285198
[2]: http://article.gmane.org/gmane.comp.version-control.git/285000

Patrick Steinhardt (15):
  config: introduce set_or_die wrappers
  branch: die on error in setting up tracking branch
  branch: die on config error when unsetting upstream
  branch: die on config error when editing branch description
  submodule: die on config error when linking modules
  submodule--helper: die on config error when cloning module
  remote: die on config error when setting URL
  remote: die on config error when setting/adding branches
  remote: die on config error when manipulating remotes
  clone: die on config error in cmd_clone
  init-db: die on config errors when initializing empty repo
  sequencer: die on config error when saving replay opts
  compat: die when unable to set core.precomposeunicode
  config: rename git_config_set to git_config_set_gently
  config: rename git_config_set_or_die to git_config_set

 branch.c | 13 +
 branch.h |  1 +
 builtin/branch.c |  5 ++--
 builtin/clone.c  |  2 +-
 builtin/config.c | 28 +--
 builtin/init-db.c|  2 +-
 builtin/remote.c | 70 +---
 cache.h  | 14 ++
 compat/precompose_utf8.c |  3 ++-
 config.c | 52 ++-
 submodule.c  | 10 +++
 t/t3200-branch.sh| 16 ++-
 t/t5505-remote.sh|  9 +++
 13 files changed, 128 insertions(+), 97 deletions(-)

-- 
2.7.0

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


Re: [RFC] Identify where a Git config is defined

2016-02-02 Thread Jeff King
On Tue, Feb 02, 2016 at 05:15:51AM -0500, Jeff King wrote:

> It looks like I tweaked it at some point, and I've been carrying this in
> my tree (rebasing forward and using it in my normal build):
> 
>   git fetch git://github.com/peff/git jk/config-sources
> 
> Feel free to use it as a starting point if that's helpful. I don't
> recall offhand how close it is to ready.

Just to leave a record in the list archive, here's the patch itself:

-- >8 --
Subject: [PATCH] show sources of config options (WIP)

 - needs tests
 - probably should complain when not LIST or getting a value

Signed-off-by: Jeff King 
---
 builtin/config.c | 32 
 cache.h  |  1 +
 config.c |  7 +++
 3 files changed, 40 insertions(+)

diff --git a/builtin/config.c b/builtin/config.c
index adc7727..b8caf18 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", &end_null, N_("terminate values with NUL byte")),
OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")),
OPT_BOOL(0, "includes", &respect_includes, N_("respect include 
directives on lookup")),
+   OPT_BOOL(0, "sources", &show_sources, N_("show source filenames of 
config")),
OPT_END(),
 };
 
@@ -91,8 +94,35 @@ 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)
+{
+   char term = '\t';
+   const char *fn = current_config_filename();
+
+   if (!fn)
+   fn = "";
+
+   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 +138,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) {
diff --git a/cache.h b/cache.h
index dfc459c..95a7f65 100644
--- a/cache.h
+++ b/cache.h
@@ -1528,6 +1528,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;
+}
-- 
2.7.0.489.g6faad84

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


Usage of staging area instead of index still the way to go?

2016-02-02 Thread Lars Vogel
Hi,

I was about to send patches to the Eclipse Git project to replace the
usage of "Index" with "Staging area" based on this thread:
http://article.gmane.org/gmane.comp.version-control.git/233469/ which
Matthias Sohn (Project lead of EGit) send me a while ago.

But the Git help seem to use still heavily "index" instead of "staging
area". Is "staging area" still considered as the correct term or has
time proven that index is better?

Best regards,Lars

-- 
Eclipse Platform UI and e4 project co-lead
CEO vogella GmbH

Haindaalwisch 17a, 22395 Hamburg
Amtsgericht Hamburg: HRB 127058
Geschäftsführer: Lars Vogel, Jennifer Nerlich de Vogel
USt-IdNr.: DE284122352
Fax (040) 5247 6322, Email: lars.vo...@vogella.com, Web: http://www.vogella.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: [RFC] Identify where a Git config is defined

2016-02-02 Thread Jeff King
On Tue, Feb 02, 2016 at 10:27:06AM +0100, Lars Schneider wrote:

> Using "git config --list" shows me all configs but sometimes I have a
> hard time to figure out where a certain config is defined. This is
> especially true on Windows as I found the system config in various
> places. I wonder if other people would find it useful to enable
> something like "git config --list --print-source" where every config
> value is printed with the file where it originates from.

We discussed this exact thing a while ago, and it looks like I started
on a patch, but didn't pursue it much further. There's some discussion
in there if you are interested in designing such a feature:

  http://thread.gmane.org/gmane.comp.version-control.git/190027/focus=190267

It looks like I tweaked it at some point, and I've been carrying this in
my tree (rebasing forward and using it in my normal build):

  git fetch git://github.com/peff/git jk/config-sources

Feel free to use it as a starting point if that's helpful. I don't
recall offhand how close it is to ready.

> If I read the source correctly this would mean I would need to change
> "config_fn_t" to pass not only key and value but also the config
> source file in addition. Since "config_fn_t" is used in many places
> this would be a big change that probably is not worth the effort?!

There's a global struct for the current config file.  In the patches
above, I just added an accessor function. Hooray for single-threaded
programs.

> Alternatively I was thinking about "git config --print-source-files"
> to print all config files that Git would parse. This would already
> help to find the configs and would probably be a smaller change.

I think the "--sources" option is more useful, because it can show
included files, too.

-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


[RFC] Identify where a Git config is defined

2016-02-02 Thread Lars Schneider
Hi,

Using "git config --list" shows me all configs but sometimes I have a hard time 
to figure out where a certain config is defined. This is especially true on 
Windows as I found the system config in various places. I wonder if other 
people would find it useful to enable something like "git config --list 
--print-source" where every config value is printed with the file where it 
originates from.

If I read the source correctly this would mean I would need to change 
"config_fn_t" to pass not only key and value but also the config source file in 
addition. Since "config_fn_t" is used in many places this would be a big change 
that probably is not worth the effort?!

Alternatively I was thinking about "git config --print-source-files" to print 
all config files that Git would parse. This would already help to find the 
configs and would probably be a smaller change.

Thoughts?

Thanks,
Lars--
To unsubscribe from this list: send the line "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] remote-curl: don't fall back to Basic auth if we haven't tried Negotiate

2016-02-02 Thread Dmitry Vilkov
This is fix of bug introduced by 4dbe66464 commit.
The problem is that when username/password combination was not set,
the first HTTP(S) request will fail and user will be asked for
credentials. As a side effect of first HTTP(S) request, libcurl auth
method GSS-Negotiate will be disabled unconditionally. Although, we
haven't tried yet provided credentials for this auth method.

Signed-off-by: Dmitry Vilkov 
---
 http.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/http.c b/http.c
index 0da9e66..707ea84 100644
--- a/http.c
+++ b/http.c
@@ -951,12 +951,15 @@ static int handle_curl_result(struct slot_results 
*results)
return HTTP_MISSING_TARGET;
else if (results->http_code == 401) {
if (http_auth.username && http_auth.password) {
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+   if (http_auth_methods & CURLAUTH_GSSNEGOTIATE) {
+   http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
+   return HTTP_REAUTH;
+   }
+#endif
credential_reject(&http_auth);
return HTTP_NOAUTH;
} else {
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
-   http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
-#endif
return HTTP_REAUTH;
}
} else {
-- 
2.4.10

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


Re: git log -g bizarre behaviour

2016-02-02 Thread Dennis Kaarsemaker
On ma, 2016-02-01 at 15:37 -0800, Junio C Hamano wrote:
> Dennis Kaarsemaker  writes:
> 
> > I'm attempting to understand the log [-g] / reflog code enough to
> > untangle them and make reflog walking work for more than just
> > commit
> > objects [see gmane 283169]. I found something which I think is
> > wrong,
> > and would break after my changes.
> > 
> > git log -g HEAD^ and git log -g v2.7.0^ give no output. This is
> > expected, as those are not things that have a reflog.
> 
> OK.
> 
> > But git log -g v2.7.0 seems to ignore -g and gives the normal
> > log.
> 
> That sounds clearly broken, and I think I see how that happens from
> the hacky way the "-g" traversal was bolted onto the revision
> traversal machinery.
> 
> I _think_ "git log -g" (and by extension "git reflog" which is just
> a short-hand to giving a few more options to that command) ought to
> 
>  * Iterate over the _objects_ that used to be at the tip of the ref;
>  * Show each of these objects as if they were fed to "git show".

That's what I am trying to achieve. Though not quite like 'git show', I
want to emulate the --oneline putput for non-commit objects too.

> This clearly is not possible without major surgery, including
> ripping out the hacky "-g" traversal from the revision traversal
> machinery and perhaps lifting it up a few levels in the callchain,
> as many functions in that callchain want to work on commits.

Yup. I'm planning to either split cmd_log_walk or make its behaviour
depend on whether we're traversing the reflog (don't call get_revision,
but call a new get_reflog_entry function). And then rip out the reflog
handling from revision.c and redo (parts of) reflog-walk.c to
accomodate the cmd_log_walk (split|replacement) that deals with reflogs
better.

> Contrast these two:
> 
> $ git log -1 v2.7.0
> $ git show v2.7.0
> 
> > I'd like to make git log -g / git reflog abort early when trying to
> > display a reflog of a ref that has no reflog. Objections?
> 
> Do you mean
> 
>   $ git checkout -b testing
> $ rm -f .git/logs/refs/heads/testing
> $ git log -g testing
> 
> will be changed from a silent no-op to an abort with error?
> 
> I do not see a need for such a change--does that count as an
> objection?

No, I'd like to change:

$ ls .git/logs/refs/tags/v2.7.0
ls: cannot access .git/logs/refs/tags/v2.7.0: No such file or directory
$ git (log -g|reflog) v2.7.0

>From the bizarre behaviour above to a silent noop. But before I do that
in a rewrite (by simply not implementing it), I'd like to have that
behavior now as well and add tests for it.

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


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


Re: [PATCH] test-lib: limit the output of the yes utility

2016-02-02 Thread Johannes Schindelin
Hi Hannes,

On Tue, 2 Feb 2016, Johannes Sixt wrote:

> On Windows, there is no SIGPIPE.

True. But we do get some sort of write failure, no? Otherwise
https://github.com/git/git/commit/2b86292ed would not work...

> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index bd4b02e..97e6491 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -902,15 +902,15 @@ fi
>  yes () {
>   if test $# = 0
>   then
> - y=y
> + set -- y
>   else
> - y="$*"
> + set -- "$*"
>   fi
> -
> - while echo "$y"
> - do
> - :
> - done
> + # we do not need an infinite supply of output for tests
> + set -- "$@" "$@" "$@" "$@"  # 4
> + set -- "$@" "$@" "$@" "$@"  # 16
> + set -- "$@" "$@" "$@" "$@"  # 64
> + printf "%s\n" "$@"
>  }

I agree with the idea, but I would like to have a less intrusive patch.
Something like this should do the job as well, and is a little easier on
the eyes IMHO:

-- snipsnap --
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 32ac1a6..ae381b9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -907,9 +907,11 @@ yes () {
y="$*"
fi
 
-   while echo "$y"
+   i=0
+   while test $i -lt 99
do
-   :
+   echo "$y"
+   i=$(($i+1))
done
 }
 

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