Re: error: git-remote-https died of signal 13

2014-04-23 Thread Jeff King
On Sun, Apr 20, 2014 at 08:42:15PM -0400, Greg M wrote:

> Using git version 1.9.2 I am getting this error:
> 
> [normal@laptop tmp]$ git clone https://github.com/mozilla/rust.git
> Cloning into 'rust'...
> remote: Reusing existing pack: 296648, done.
> remote: Counting objects: 80, done.
> remote: Compressing objects: 100% (77/77), done.
> remote: Total 296728 (delta 22), reused 9 (delta 3)
> Receiving objects: 100% (296728/296728), 110.68 MiB | 190.00 KiB/s, done.
> Resolving deltas: 100% (238828/238828), done.
> Checking connectivity... done.
> error: git-remote-https died of signal 13

Thanks for a thorough bug report. I looked through your strace output,
and this really does look like another case of OpenSSL getting SIGPIPE
while closing the connection.

It's odd, though, as your curl version has my patches, along with
similar extra fixes in 854aca5 (multi: ignore sigpipe internally,
2014-02-17). But I guess there may be a code path that needs similar
treatment.

The easiest way to find it is probably to attach a debugger to the
running git-remote-https, and get a backtrace when it dies from SIGPIPE.
You'll probably want to install your system's debug packages for curl,
too.

> I have curl version 7.36 though, in case some of the other output matters:
> 
> [normal@laptop tmp]$ curl --version
> curl 7.36.0 (x86_64-unknown-linux-gnu) libcurl/7.36.0 OpenSSL/1.0.1g
> zlib/1.2.8 libssh2/1.4.3

Another possibility is that your curl binary is up-to-date, but you are
linking against an older version of libcurl that does not have the
SIGPIPE workarounds.

I'm not sure of the best way to check that, but a hacky way under Linux
is:

  $ ldd $(git --exec-path)/git-remote-https | grep libcurl
  libcurl.so.4 => /usr/lib/x86_64-linux-gnu/libcurl.so.4
  $ strings  /usr/lib/x86_64-linux-gnu/libcurl.so.4 | grep '7\.'
  CLIENT libcurl 7.36.0

-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: [ANNOUNCE] Git v2.0.0-rc0

2014-04-23 Thread Johan Herland
On Wed, Apr 23, 2014 at 12:47 AM, Junio C Hamano  wrote:
> "brian m. carlson"  writes:
>> What we could do instead is simply require a newer version of
>> Getopt::Long, which would let people continue using their ancient OSes
>> and install a newer version from CPAN if necessary.  It's also the
>> proper way to specify the dependency.
>
> Yes, but if its inability to properly grok --option="" is the only
> reason we want to add a dependency, wouldn't it suffice to simply
> state in the documentation (1) how to recognise the symptom to see
> if the version the user has is too old, e.g. "if you see this error
> message", "run 'perl -v' to see if your perl is older than X",
> etc. and (2) how to work it around, i.e. "instead of giving an empty
> value with --option='', say --option ''"?

FWIW, the least intrusive approach is what I find most agreeable:

 - Fix the tests to use --prefix "" instead of --prefix=""
 - Update the documentation like Junio suggests above.
 - Reformat an example using --prefix ""

I.e. use Kyle's patch to t9117, plus something like this:

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 5b3c38d..9f579e0 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -91,6 +91,9 @@ COMMANDS
 NOTE: Before Git v2.0, the default prefix was "" (no prefix). This
 meant that SVN-tracking refs were put at "refs/remotes/*", which is
 incompatible with how Git's own remote-tracking refs are organized.
+If you still want the old default, you can get it by passing
+'--prefix ""' on the command line ('--prefix=""' may not work if
+your Perl's Getopt::Long is < v2.37).

 --ignore-paths=;;
When passed to 'init' or 'clone' this regular expression will



...Johan

-- 
Johan Herland, 
www.herland.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: [RTC/PATCH] Add 'update-branch' hook

2014-04-23 Thread Stephen Leake
Felipe Contreras  writes:

> Stephen Leake wrote:
>> Felipe Contreras  writes:
>> 
>> > Ilya Bobyr wrote:
>> >> On 4/21/2014 2:17 PM, Felipe Contreras wrote:
>> >> > Ilya Bobyr wrote:
>> >> >
>> >> >> Also, most have names that start with either "pre-" or "post-".
>> >> >> It seems reasonable for both "pre-update-branch" and
>> >> >> "post-update-branch" to exist.
>> >> > I don't see what would be the point in that.
>> >> 
>> >> Do you see the point in the other hooks doing that?
>> >
>> > Yes, there a reason for the existance of those hooks. Now tell me why would
>> > anybody use post-update-branch instead of pre-update-branch?
>> 
>> I have a branch which should always be recompiled on update;
>> post-update-branch would be a good place for that.
>
> And why would pre-update-branch not serve that purpose?

Because the code that needs to be compiled is not yet in the workspace

-- 
-- Stephe
--
To unsubscribe from this list: send the line "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] git tag --contains : avoid stack overflow

2014-04-23 Thread Stepan Kasal
From: Jean-Jacques Lafay 

In large repos, the recursion implementation of contains(commit,
commit_list) may result in a stack overflow. Replace the recursion with
a loop to fix it.

This problem is more apparent on Windows than on Linux, where the stack
is more limited by default.

See also this thread on the msysGit list:

https://groups.google.com/d/topic/msysgit/FqT6boJrb2g/discussion

[jes: re-written to imitate the original recursion more closely]

Thomas Braun pointed out several documentation shortcomings.

Tests are run only if ulimit -s is available.  This means they cannot
be run on Windows.

Signed-off-by: Jean-Jacques Lafay 
Signed-off-by: Johannes Schindelin 
Tested-by: Stepan Kasal 
---
Hello,
I have found out that "ulimit -s" does not work on Windows.
Adding this as a prerequisite, we will skip the test there.

On Thu, Apr 17, 2014 at 05:32:38PM -0400, Jeff King wrote:
> So we can bump the depth further; probably 4000 is enough for any system
> to fail with a 64k stack. The deeper we make it, the longer it takes to
> run the test, though. At 4000, my machine seems to take about 300ms to
> run it. That's may be OK.

Consequently, we can accept this proposal.

Stepan Kasal

 builtin/tag.c  | 90 --
 t/t7004-tag.sh | 23 +++
 2 files changed, 98 insertions(+), 15 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 6c7c6bd..f344002 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -80,11 +80,19 @@ static int in_commit_list(const struct commit_list *want, 
struct commit *c)
return 0;
 }
 
-static int contains_recurse(struct commit *candidate,
+enum contains_result {
+   CONTAINS_UNKNOWN = -1,
+   CONTAINS_NO = 0,
+   CONTAINS_YES = 1,
+};
+
+/*
+ * Test whether the candidate or one of its parents is contained in the list.
+ * Do not recurse to find out, though, but return -1 if inconclusive.
+ */
+static enum contains_result contains_test(struct commit *candidate,
const struct commit_list *want)
 {
-   struct commit_list *p;
-
/* was it previously marked as containing a want commit? */
if (candidate->object.flags & TMP_MARK)
return 1;
@@ -92,26 +100,78 @@ static int contains_recurse(struct commit *candidate,
if (candidate->object.flags & UNINTERESTING)
return 0;
/* or are we it? */
-   if (in_commit_list(want, candidate))
+   if (in_commit_list(want, candidate)) {
+   candidate->object.flags |= TMP_MARK;
return 1;
+   }
 
if (parse_commit(candidate) < 0)
return 0;
 
-   /* Otherwise recurse and mark ourselves for future traversals. */
-   for (p = candidate->parents; p; p = p->next) {
-   if (contains_recurse(p->item, want)) {
-   candidate->object.flags |= TMP_MARK;
-   return 1;
-   }
-   }
-   candidate->object.flags |= UNINTERESTING;
-   return 0;
+   return -1;
 }
 
-static int contains(struct commit *candidate, const struct commit_list *want)
+/*
+ * Mimicking the real stack, this stack lives on the heap, avoiding stack
+ * overflows.
+ *
+ * At each recursion step, the stack items points to the commits whose
+ * ancestors are to be inspected.
+ */
+struct stack {
+   int nr, alloc;
+   struct stack_entry {
+   struct commit *commit;
+   struct commit_list *parents;
+   } *stack;
+};
+
+static void push_to_stack(struct commit *candidate, struct stack *stack)
+{
+   int index = stack->nr++;
+   ALLOC_GROW(stack->stack, stack->nr, stack->alloc);
+   stack->stack[index].commit = candidate;
+   stack->stack[index].parents = candidate->parents;
+}
+
+static enum contains_result contains(struct commit *candidate,
+   const struct commit_list *want)
 {
-   return contains_recurse(candidate, want);
+   struct stack stack = { 0, 0, NULL };
+   int result = contains_test(candidate, want);
+
+   if (result != CONTAINS_UNKNOWN)
+   return result;
+
+   push_to_stack(candidate, &stack);
+   while (stack.nr) {
+   struct stack_entry *entry = &stack.stack[stack.nr - 1];
+   struct commit *commit = entry->commit;
+   struct commit_list *parents = entry->parents;
+
+   if (!parents) {
+   commit->object.flags |= UNINTERESTING;
+   stack.nr--;
+   }
+   /*
+* If we just popped the stack, parents->item has been marked,
+* therefore contains_test will return a meaningful 0 or 1.
+*/
+   else switch (contains_test(parents->item, want)) {
+   case CONTAINS_YES:
+   commit->object.flags |= TMP_MARK;
+   stack.nr--;
+   break;
+ 

Re: [PATCH 2/2] mergetool: run prompt only if guessed tool

2014-04-23 Thread Charles Bailey
On Tue, Apr 22, 2014 at 02:56:22AM -0500, Felipe Contreras wrote:
> An explicitly set mergetool.prompt = true would override the default. See the
> patch.

I have had a chance to test the patch now and it looks good. I think
when glancing at it before I missed the change that dropped "|| echo
true" from 

prompt=$(git config --bool mergetool.prompt || echo true)

so I wasn't sure where the implicit true / explicit true difference was
handled.

> I looked, the documentation doesn't mention any default. We could add it, but 
> I
> don't think it's necesarily part of this patch.

The bit of documentation that I was thinking of is in
Documentation/git-mergetool.txt where it states that "--prompt" is the
default which is now only partially true.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] mergetool: run prompt only if guessed tool

2014-04-23 Thread David Aguilar
On Tue, Apr 22, 2014 at 10:19 AM, Junio C Hamano  wrote:
> David Aguilar  writes:
>
>> [Cc:ing Charles in case he has an opinion, this behavior dates back to the 
>> original MT]
>>
>> On Sun, Apr 20, 2014 at 07:17:34PM -0500, Felipe Contreras wrote:
>>> It's annoying to see the prompt:
>>>
>>>   Hit return to start merge resolution tool (foo):
>>>
>>> Every time the user does 'git mergetool' even if the user already
>>> configured 'foo' as the wanted tool.
>>>
>>> Display this prompt only when the user hasn't explicitly configured a
>>> tool.
>>
>> I agree this is probably helpful.
>> Most users I've met end up configuring mergetool.prompt=false.
>>
>> Thanks
>
> OK, is it an Ack?

Sure thing.

Acked-by: David Aguilar 

> Thanks for CC'ing Charles, by the way.  I think his point about
> mentioning the change of default somewhere in the documentation
> has some merits, and it can be done in a follow-up patch on top.

Another thing that crossed my mind is that we have -y for --no-prompt
because --prompt was the original default. Maybe a -i (?) shortcut for
the interactive --prompt can be added to make the "need to skip some
when resolving" use case easier to activate.
-- 
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: [RTC/PATCH] Add 'update-branch' hook

2014-04-23 Thread Felipe Contreras
Stephen Leake wrote:
> Felipe Contreras  writes:
> 
> > Stephen Leake wrote:
> >> Felipe Contreras  writes:
> >> 
> >> > Ilya Bobyr wrote:
> >> >> On 4/21/2014 2:17 PM, Felipe Contreras wrote:
> >> >> > Ilya Bobyr wrote:
> >> >> >
> >> >> >> Also, most have names that start with either "pre-" or "post-".
> >> >> >> It seems reasonable for both "pre-update-branch" and
> >> >> >> "post-update-branch" to exist.
> >> >> > I don't see what would be the point in that.
> >> >> 
> >> >> Do you see the point in the other hooks doing that?
> >> >
> >> > Yes, there a reason for the existance of those hooks. Now tell me why 
> >> > would
> >> > anybody use post-update-branch instead of pre-update-branch?
> >> 
> >> I have a branch which should always be recompiled on update;
> >> post-update-branch would be a good place for that.
> >
> > And why would pre-update-branch not serve that purpose?
> 
> Because the code that needs to be compiled is not yet in the workspace

And it won't be in 'post-update-branch' either.

 % git checkout master
 % git branch feature-a stable
 <- update-branch hook will be called here

The hook will get 'feature-a' as the first argument, but the code in the
workspace would correspond to 'master'; the checked out branch (pre or post).

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "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] send-email: recognize absolute path on Windows

2014-04-23 Thread Erik Faye-Lund
On Tue, Apr 22, 2014 at 6:50 PM, Junio C Hamano  wrote:
> Erik Faye-Lund  writes:
>
 Shouldn't the latter also be anchored at the beginning of the string
 with a leading "^"?

> +}
> +
> +require File::Spec::Functions;
> +return File::Spec::Functions::file_name_is_absolute($path);

 We already "use File::Spec qw(something else)" at the beginning, no?
 Why not throw file_name_is_absolute into that qw() instead?
>>>
>>> Ahh, OK, if you did so, you won't have any place to hook the "only
>>> on msys do this" trick into.
>>>
>>> It somehow feels somewhat confusing that we define a sub with the
>>> same name as the system one, while not overriding it entirely but
>>> delegate back to the system one.  I am debating myself if it is more
>>> obvious if it is done this way:
>>>
>>> use File::Spec::Functions qw(file_name_is_absolute);
>>> if ($^O eq 'msys') {
>>> sub file_name_is_absolute {
>>> return $_[0] =~ /^\// || $_[0] =~ /^[A-Z]:/i;
>>> }
>>> }
>>>
>>
>> In this case, we end up requiring that module even when we end up
>> using it, no?
>
> Also somebody earlier mentioned that we would be redefining, which
> has a different kind of ugliness, so I'd agree with the code structure
> of what you sent out (which has been queued on 'pu').
>
> My earlier question "don't we want to make sure 'C:' is at the
> betginning of the string?" still stands, though.  I do not think I
> futzed with your regexp in the version I queued on 'pu'.

Ah, yes of course. Thanks for spotting that. I also like the other
clean-ups you did to the regex (above).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-rebase: fix probable reflog typo

2014-04-23 Thread Matthieu Moy
Felipe Contreras  writes:

> Commit 26cd160 (rebase -i: use a better reflog message) tried to produce
> a better reflog message, however, it seems a statement was introduced by
> mistake.
>
> 'comment_for_reflog start' basically overides the GIT_REFLOG_ACTION we
> just set.

I think this is more complex than this. To give a bit more context, the
code you're changing is:

comment_for_reflog start

if test ! -z "$switch_to"
then
GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to"
output git checkout "$switch_to" -- ||
die "Could not checkout $switch_to"

comment_for_reflog start
fi

The GIT_REFLOG_ACTION= changes the reflog message for the git checkout
command.

Since we use the construct

GIT_REFLOG_ACTION="..." shell_function

the shell may keep $GIT_REFLOG_ACTION set after calling the function (I
don't remember what POSIX says, but dash does it:
$ f () { echo $X; }
$ X=42 f
42
$ echo $X
42
$ X=y f   
y
$ echo $X
y
).

So, one needs to reset $GIT_REFLOG_ACTION to what it used to be if is it
to be used later. However, it seems to me that the "comment_for_reflog
start" is used only for this checkout command. If so, there's no need
for the "comment_for_reflog start" before the if statement either.

In any case, this should be clarified with at least a comment in the
code.

> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 43631b4..5f1d8c9 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -893,8 +893,6 @@ then
>   GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to"
>   output git checkout "$switch_to" -- ||
>   die "Could not checkout $switch_to"
> -
> - comment_for_reflog start
>  fi
>  
>  orig_head=$(git rev-parse --verify HEAD) || die "No HEAD?"

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


Re: error: git-remote-https died of signal 13

2014-04-23 Thread Greg M
On Wed, Apr 23, 2014 at 2:59 AM, Jeff King  wrote:
> On Sun, Apr 20, 2014 at 08:42:15PM -0400, Greg M wrote:
>
>> Using git version 1.9.2 I am getting this error:
>>
>> [normal@laptop tmp]$ git clone https://github.com/mozilla/rust.git
>> Cloning into 'rust'...
>> remote: Reusing existing pack: 296648, done.
>> remote: Counting objects: 80, done.
>> remote: Compressing objects: 100% (77/77), done.
>> remote: Total 296728 (delta 22), reused 9 (delta 3)
>> Receiving objects: 100% (296728/296728), 110.68 MiB | 190.00 KiB/s, done.
>> Resolving deltas: 100% (238828/238828), done.
>> Checking connectivity... done.
>> error: git-remote-https died of signal 13
>
> Thanks for a thorough bug report. I looked through your strace output,
> and this really does look like another case of OpenSSL getting SIGPIPE
> while closing the connection.
>
> It's odd, though, as your curl version has my patches, along with
> similar extra fixes in 854aca5 (multi: ignore sigpipe internally,
> 2014-02-17). But I guess there may be a code path that needs similar
> treatment.
>
> The easiest way to find it is probably to attach a debugger to the
> running git-remote-https, and get a backtrace when it dies from SIGPIPE.
> You'll probably want to install your system's debug packages for curl,
> too.
>

The backtrace:

Program received signal SIGPIPE, Broken pipe.
0x7fdcfd511a2d in write () from /usr/lib/libpthread.so.0
(gdb) bt
#0  0x7fdcfd511a2d in write () from /usr/lib/libpthread.so.0
#1  0x7fdcfd81a0f6 in sock_write () from /usr/lib/libcrypto.so.1.0.0
#2  0x7fdcfd817edb in BIO_write () from /usr/lib/libcrypto.so.1.0.0
#3  0x7fdcfc662902 in ssl3_write_pending () from /usr/lib/libssl.so.1.0.0
#4  0x7fdcfc664b77 in ssl3_dispatch_alert () from /usr/lib/libssl.so.1.0.0
#5  0x7fdcfc660822 in ssl3_shutdown () from /usr/lib/libssl.so.1.0.0
#6  0x7fdcfd2e944e in Curl_ossl_close () from /usr/lib/libcurl.so.4
#7  0x7fdcfd2b6459 in Curl_disconnect () from /usr/lib/libcurl.so.4
#8  0x7fdcfd2c8131 in curl_multi_cleanup () from /usr/lib/libcurl.so.4
#9  0x0040764b in ?? ()
#10 0x00404e4d in ?? ()
#11 0x7fdcfcf0fb05 in __libc_start_main () from /usr/lib/libc.so.6
#12 0x0040552e in ?? ()

> Another possibility is that your curl binary is up-to-date, but you are
> linking against an older version of libcurl that does not have the
> SIGPIPE workarounds.
>
> I'm not sure of the best way to check that, but a hacky way under Linux
> is:
>
>   $ ldd $(git --exec-path)/git-remote-https | grep libcurl
>   libcurl.so.4 => /usr/lib/x86_64-linux-gnu/libcurl.so.4
>   $ strings  /usr/lib/x86_64-linux-gnu/libcurl.so.4 | grep '7\.'
>   CLIENT libcurl 7.36.0
>
> -Peff

Seems to actually be that version of libcurl:

[normal@laptop tmp]$ ldd $(git --exec-path)/git-remote-https | grep libcurl
libcurl.so.4 => /usr/lib/libcurl.so.4 (0x7ff1cce5e000)
[normal@laptop tmp]$ strings /usr/lib/libcurl.so.4 | grep '7\.'
7.36f
CLIENT libcurl 7.36.0
CLIENT libcurl 7.36.0
CLIENT libcurl 7.36.0
7.36.0
[normal@laptop tmp]$

Also I don't think I have any other versions to link against:

[normal@laptop tmp]$ ls -l /usr/lib/libcurl*
lrwxrwxrwx 1 root root 16 Mar 26 10:12 /usr/lib/libcurl.so ->
libcurl.so.4.3.0
lrwxrwxrwx 1 root root 16 Mar 26 10:12 /usr/lib/libcurl.so.4 ->
libcurl.so.4.3.0
-rwxr-xr-x 1 root root 443488 Mar 26 10:12 /usr/lib/libcurl.so.4.3.0

Greg
--
To unsubscribe from this list: send the line "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 1/6] diff: add a config option to control orderfile

2014-04-23 Thread Michael S. Tsirkin
I always want my diffs to show header files first,
then .c files, then the rest. Make it possible to
set orderfile though a config option to achieve this.

Signed-off-by: Michael S. Tsirkin 
---
 diff.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/diff.c b/diff.c
index b79432b..6bcb26b 100644
--- a/diff.c
+++ b/diff.c
@@ -264,6 +264,9 @@ int git_diff_basic_config(const char *var, const char 
*value, void *cb)
return 0;
}
 
+   if (!strcmp(var, "diff.orderfile"))
+   return git_config_string(&default_diff_options.orderfile, var, 
value);
+
if (starts_with(var, "submodule."))
return parse_submodule_config_option(var, value);
 
-- 
MST

--
To unsubscribe from this list: send the line "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 4/6] patch-id: make it stable against hunk reordering

2014-04-23 Thread Michael S. Tsirkin
Patch id changes if users
1. reorder file diffs that make up a patch
or
2. split a patch up to multiple diffs that touch the same path
(keeping hunks within a single diff ordered to make patch valid).

As the result is functionally equivalent, a different patch id is
surprising to many users.
In particular, reordering files using diff -O is helpful to make patches
more readable (e.g. API header diff before implementation diff).

Add an option to change patch-id behaviour making it stable against
these two kinds of patch change:
1. calculate SHA1 hash for each hunk separately and sum all hashes
(using a symmetrical sum) to get patch id
2. hash the file-level headers together with each hunk (not just the
first hunk)

We use a 20byte sum and not xor - since xor would give 0 output
for patches that have two identical diffs, which isn't all that
unlikely (e.g. append the same line in two places).

The new behaviour is enabled
- when diff.orderfile config option is present
  (as that is likely to reorder patches)
- when patchid.stable is true
- when --stable flag is present

Using a new flag --unstable or setting patchid.stable to false force
the historical behaviour.

Signed-off-by: Michael S. Tsirkin 
---
 builtin/patch-id.c | 94 --
 1 file changed, 78 insertions(+), 16 deletions(-)

diff --git a/builtin/patch-id.c b/builtin/patch-id.c
index 3cfe02d..e154405 100644
--- a/builtin/patch-id.c
+++ b/builtin/patch-id.c
@@ -1,17 +1,14 @@
 #include "builtin.h"
 
-static void flush_current_id(int patchlen, unsigned char *id, git_SHA_CTX *c)
+static void flush_current_id(int patchlen, unsigned char *id, unsigned char 
*result)
 {
-   unsigned char result[20];
char name[50];
 
if (!patchlen)
return;
 
-   git_SHA1_Final(result, c);
memcpy(name, sha1_to_hex(id), 41);
printf("%s %s\n", sha1_to_hex(result), name);
-   git_SHA1_Init(c);
 }
 
 static int remove_space(char *line)
@@ -56,10 +53,31 @@ static int scan_hunk_header(const char *p, int *p_before, 
int *p_after)
return 1;
 }
 
-static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct 
strbuf *line_buf)
+static void flush_one_hunk(unsigned char *result, git_SHA_CTX *ctx)
 {
-   int patchlen = 0, found_next = 0;
+   unsigned char hash[20];
+   unsigned short carry = 0;
+   int i;
+
+   git_SHA1_Final(hash, ctx);
+   git_SHA1_Init(ctx);
+   /* 20-byte sum, with carry */
+   for (i = 0; i < 20; ++i) {
+   carry += result[i] + hash[i];
+   result[i] = carry;
+   carry >>= 8;
+   }
+}
+
+static int get_one_patchid(unsigned char *next_sha1, unsigned char *result,
+  struct strbuf *line_buf, int stable)
+{
+   int patchlen = 0, found_next = 0, hunks = 0;
int before = -1, after = -1;
+   git_SHA_CTX ctx, header_ctx;
+
+   git_SHA1_Init(&ctx);
+   hashclr(result);
 
while (strbuf_getwholeline(line_buf, stdin, '\n') != EOF) {
char *line = line_buf->buf;
@@ -98,7 +116,19 @@ static int get_one_patchid(unsigned char *next_sha1, 
git_SHA_CTX *ctx, struct st
if (before == 0 && after == 0) {
if (!memcmp(line, "@@ -", 4)) {
/* Parse next hunk, but ignore line numbers.  */
+   if (stable) {
+   /* Hash the file-level headers together 
with each hunk. */
+   if (hunks) {
+   flush_one_hunk(result, &ctx);
+   /* Prepend saved header ctx for 
next hunk.  */
+   memcpy(&ctx, &header_ctx, 
sizeof(ctx));
+   } else {
+   /* Save header ctx for next 
hunk.  */
+   memcpy(&header_ctx, &ctx, 
sizeof(ctx));
+   }
+   }
scan_hunk_header(line, &before, &after);
+   hunks++;
continue;
}
 
@@ -107,7 +137,10 @@ static int get_one_patchid(unsigned char *next_sha1, 
git_SHA_CTX *ctx, struct st
break;
 
/* Else we're parsing another header.  */
+   if (stable && hunks)
+   flush_one_hunk(result, &ctx);
before = after = -1;
+   hunks = 0;
}
 
/* If we get here, we're inside a hunk.  */
@@ -119,39 +152,68 @@ static int get_one_patchid(unsigned char *next_sha1, 
git_SHA_CTX *ctx, struct st
/* Compute the sha without whitespace */

[PATCH v4 5/6] patch-id: document new behaviour

2014-04-23 Thread Michael S. Tsirkin
Clarify that patch ID can now be a sum of hashes, not a hash.
Document how command line and config options affect the
behaviour.

Signed-off-by: Michael S. Tsirkin 
---
 Documentation/git-patch-id.txt | 25 -
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-patch-id.txt b/Documentation/git-patch-id.txt
index 312c3b1..61d4a67 100644
--- a/Documentation/git-patch-id.txt
+++ b/Documentation/git-patch-id.txt
@@ -8,14 +8,14 @@ git-patch-id - Compute unique ID for a patch
 SYNOPSIS
 
 [verse]
-'git patch-id' < 
+'git patch-id' [--stable | --unstable] < 
 
 DESCRIPTION
 ---
-A "patch ID" is nothing but a SHA-1 of the diff associated with a patch, with
-whitespace and line numbers ignored.  As such, it's "reasonably stable", but at
-the same time also reasonably unique, i.e., two patches that have the same 
"patch
-ID" are almost guaranteed to be the same thing.
+A "patch ID" is nothing but a sum of SHA-1 of the diff hunks associated with a
+patch, with whitespace and line numbers ignored.  As such, it's "reasonably
+stable", but at the same time also reasonably unique, i.e., two patches that
+have the same "patch ID" are almost guaranteed to be the same thing.
 
 IOW, you can use this thing to look for likely duplicate commits.
 
@@ -27,6 +27,21 @@ This can be used to make a mapping from patch ID to commit 
ID.
 
 OPTIONS
 ---
+
+--stable::
+   Use a symmetrical sum of hashes as the patch ID.
+   With this option, reordering file diffs that make up a patch or
+   splitting a diff up to multiple diffs that touch the same path
+   does not affect the ID.
+   This is the default if patchid.stable is set to true, or if 
patchid.stable
+   is not set and diff.orderfile is set to some value.
+
+--unstable::
+   Use a non-symmetrical sum of hashes, such that reordering
+   or splitting the patch does affect the ID.
+   This is the default if patchid.stable is set to false, or if neither 
patchid.stable
+   nor diff.orderfile are set.
+
 ::
The diff to create the ID of.
 
-- 
MST

--
To unsubscribe from this list: send the line "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 6/6] patch-id-test: test stable and unstable behaviour

2014-04-23 Thread Michael S. Tsirkin
Verify that patch ID supports an algorithm
that is stable against diff split and reordering.

Signed-off-by: Michael S. Tsirkin 
---
 t/t4204-patch-id.sh | 140 +++-
 1 file changed, 129 insertions(+), 11 deletions(-)

diff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh
index d2c930d..54f2fb8 100755
--- a/t/t4204-patch-id.sh
+++ b/t/t4204-patch-id.sh
@@ -5,27 +5,51 @@ test_description='git patch-id'
 . ./test-lib.sh
 
 test_expect_success 'setup' '
-   test_commit initial foo a &&
-   test_commit first foo b &&
-   git checkout -b same HEAD^ &&
-   test_commit same-msg foo b &&
-   git checkout -b notsame HEAD^ &&
-   test_commit notsame-msg foo c
+as="a a a a a a a a" && # eight a
+   test_write_lines $as >foo &&
+   test_write_lines $as >bar &&
+   git add foo bar &&
+   git commit -a -m initial &&
+   test_write_lines $as b >foo &&
+   test_write_lines $as b >bar &&
+   git commit -a -m first &&
+   git checkout -b same master &&
+   git commit --amend -m same-msg &&
+   git checkout -b notsame master &&
+   echo c >foo &&
+   echo c >bar &&
+   git commit --amend -a -m notsame-msg &&
+   git checkout -b split master &&
+   test_write_lines d $as b >foo &&
+   test_write_lines d $as b >bar &&
+   git commit -a -m split &&
+   git checkout -b merged master &&
+   git checkout split -- foo bar &&
+   git commit --amend -a -m merged &&
+   test_write_lines bar foo >bar-then-foo &&
+   test_write_lines foo bar >foo-then-bar
 '
 
 test_expect_success 'patch-id output is well-formed' '
-   git log -p -1 | git patch-id > output &&
+   git log -p -1 | git patch-id >output &&
grep "^[a-f0-9]\{40\} $(git rev-parse HEAD)$" output
 '
 
+#calculate patch id. Make sure output is not empty.
 calc_patch_id () {
-   git patch-id |
-   sed "s# .*##" > patch-id_"$1"
+   name="$1"
+   shift
+   git patch-id "$@" |
+   sed "s/ .*//" >patch-id_"$name" &&
+   test_line_count -gt 0 patch-id_"$name"
+}
+
+get_top_diff () {
+   git log -p -1 "$@" -O bar-then-foo --
 }
 
 get_patch_id () {
-   git log -p -1 "$1" | git patch-id |
-   sed "s# .*##" > patch-id_"$1"
+   get_top_diff "$1" | calc_patch_id "$@"
 }
 
 test_expect_success 'patch-id detects equality' '
@@ -56,6 +80,100 @@ test_expect_success 'whitespace is irrelevant in footer' '
test_cmp patch-id_master patch-id_same
 '
 
+cmp_patch_id () {
+   if
+   test "$1" = "relevant"
+   then
+   ! test_cmp patch-id_"$2" patch-id_"$3"
+   else
+   test_cmp patch-id_"$2" patch-id_"$3"
+   fi
+}
+
+test_patch_id_file_order () {
+   relevant="$1"
+   shift
+   name="order-${1}-$relevant"
+   shift
+   get_top_diff "master" | calc_patch_id "$name" "$@" &&
+   git checkout same &&
+   git format-patch -1 --stdout -O foo-then-bar |
+   calc_patch_id "ordered-$name" "$@" &&
+   cmp_patch_id $relevant "$name" "ordered-$name"
+   
+}
+
+test_patch_id_split () {
+   relevant="$1"
+   shift
+   name="split-${1}-$relevant"
+   shift
+   get_top_diff merged | calc_patch_id "$name" "$@" &&
+   (git log -p -1 -O foo-then-bar split~1; git diff split~1..split) |
+   calc_patch_id "split-$name" "$@" &&
+   cmp_patch_id "$relevant" "$name" "split-$name"
+}
+
+# combined test for options
+test_patch_id () {
+   test_patch_id_file_order "$@" &&
+   test_patch_id_split "$@"
+}
+
+# small tests with detailed diagnostic for basic options.
+test_expect_success 'file order is irrelevant with --stable' '
+   test_patch_id_file_order irrelevant --stable --stable
+'
+
+test_expect_success 'file order is relevant with --unstable' '
+   test_patch_id_file_order relevant --unstable --unstable
+'
+
+test_expect_success 'splitting patch is irrelevant with --stable' '
+   test_patch_id_split irrelevant --stable --stable
+'
+
+test_expect_success 'splitting patch affects id with --unstable' '
+   test_patch_id_split relevant --unstable --unstable
+'
+
+#Now test various option combinations.
+test_expect_success 'default is unstable' '
+   test_patch_id relevant default
+'
+
+test_expect_success 'patchid.stable = true is stable' '
+   test_config patchid.stable true &&
+   test_patch_id irrelevant patchid.stable=true
+'
+
+test_expect_success 'patchid.stable = false is unstable' '
+   test_config patchid.stable false &&
+   test_patch_id relevant patchid.stable=false
+'
+
+test_expect_success 'diff.orderfile implies stable' '
+   test_config diff.orderfile /dev/null &&
+   test_patch_id irrelevant diff.orderfile
+'
+
+test_expect_success 'patchid.stable = false overrides diff.orderfile' '
+   test_config patchid.stable false &&
+   test_config diff.orderfile /dev/n

[PATCH v4 3/6] tests: new test for orderfile options

2014-04-23 Thread Michael S. Tsirkin
The test is very basic and can be extended.
Couldn't find a good existing place to put it,
so created a new file.

Signed-off-by: Michael S. Tsirkin 
---
 t/t4056-diff-order.sh | 63 +++
 1 file changed, 63 insertions(+)
 create mode 100755 t/t4056-diff-order.sh

diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh
new file mode 100755
index 000..a247b7a
--- /dev/null
+++ b/t/t4056-diff-order.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+
+test_description='diff orderfile'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+as="a a a a a a a a" && # eight a
+test_write_lines $as >foo &&
+test_write_lines $as >bar &&
+   git add foo bar &&
+   git commit -a -m initial &&
+test_write_lines $as b >foo &&
+test_write_lines $as b >bar &&
+   git commit -a -m first &&
+   test_write_lines bar foo >bar-then-foo &&
+   test_write_lines foo bar >foo-then-bar &&
+   git diff -Ofoo-then-bar HEAD~1..HEAD >diff-foo-then-bar &&
+   git diff -Obar-then-foo HEAD~1..HEAD >diff-bar-then-foo
+'
+
+test_diff_well_formed () {
+   grep ^+b "$1" >added
+   grep ^-b "$1" >removed
+   grep ^+++ "$1" >oldfiles
+   grep ^--- "$1" >newfiles
+   test_line_count = 2 added &&
+   test_line_count = 0 removed &&
+   test_line_count = 2 oldfiles &&
+   test_line_count = 2 newfiles
+}
+
+test_expect_success 'diff output with -O is well-formed' '
+   test_diff_well_formed diff-foo-then-bar &&
+   test_diff_well_formed diff-bar-then-foo
+'
+
+test_expect_success 'flag -O affects diff output' '
+   ! test_cmp diff-foo-then-bar diff-bar-then-foo
+'
+
+test_expect_success 'orderfile is same as -O' '
+   test_config diff.orderfile foo-then-bar &&
+   git diff HEAD~1..HEAD >diff-foo-then-bar-config &&
+   test_config diff.orderfile bar-then-foo &&
+   git diff HEAD~1..HEAD >diff-bar-then-foo-config &&
+   test_cmp diff-foo-then-bar diff-foo-then-bar-config &&
+   test_cmp diff-bar-then-foo diff-bar-then-foo-config
+'
+
+test_expect_success '-O overrides orderfile' '
+   test_config diff.orderfile foo-then-bar &&
+   git diff -Obar-then-foo HEAD~1..HEAD >diff-bar-then-foo-flag &&
+   test_cmp diff-bar-then-foo diff-bar-then-foo-flag
+'
+
+test_expect_success '/dev/null is same as no orderfile' '
+   git diff -O/dev/null HEAD~1..HEAD>diff-null-orderfile &&
+   git diff HEAD~1..HEAD >diff-default &&
+   test_cmp diff-null-orderfile diff-default
+'
+
+test_done
-- 
MST

--
To unsubscribe from this list: send the line "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 2/6] test: add test_write_lines helper

2014-04-23 Thread Michael S. Tsirkin
As suggested by Junio.

Signed-off-by: Michael S. Tsirkin 
---
 t/test-lib-functions.sh | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index aeae3ca..2fa6453 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -712,6 +712,13 @@ test_ln_s_add () {
fi
 }
 
+# This function writes out its parameters, one per line
+test_write_lines () {
+   for line in "$@"; do
+   echo "$line"
+   done
+}
+
 perl () {
command "$PERL_PATH" "$@"
 }
-- 
MST

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


Re: [PATCH v2 2/3] remote-helpers: move out of contrib

2014-04-23 Thread Max Horn

On 21.04.2014, at 22:37, Felipe Contreras  wrote:

> The remote-helpers in contrib/remote-helpers have proved to work, be
> reliable, and stable. It's time to move them out of contrib, and be
> distributed by default.

Really? While I agree that git-remote-hg by now works quite well for basic 
usage in simple situation, there are still unresolved bugs and fundamental 
issues with it.

E.g. I recently showed you a reproducible use case involving git-remote-hg that 
puts the helper into a broken state from which it is difficult for a normal 
user to recover. Namely when a hg branch has multiple heads, then git-remote-hg 
exports all of those to git, but only adds a git ref for one of them; after 
pruning unreferenced commits, the fast-import marks file references git commits 
that now are missing, prompting git fast-import to crash and trash the marks 
file. Afterwards, attempts to push or pull from the remote hg repository are 
answered with an error.

There are other ways to trigger variants of this, and these are not artificial, 
they occur in real life (this is how I run into the issue). There are more 
issues related to unresolved clashes between the git and hg ways of naming 
things. E.g. I am collaborating on a hg repository that has branches "foo" and 
"foo/bar" which git-remote-hg cannot handle because it translates them to git 
branch names, and, well, git cannot handle that.

It may be hard to deal with some of them, and admittedly I wouldn't necessarily 
expect that all of these are handled from the outset, i.e. "in version 1.0". 
But I think at the very least, users should be warned about these things.

More broadly speaking, there is currently no documentation at all in git.git 
for those remote helpers, which I find worrisome.

That said, I don't know what the criteria are for moving something out of 
contrib. Perhaps it is OK to move an undocumented remote-helper with known bugs 
out of contrib.


Cheers,
Max


signature.asc
Description: Message signed with OpenPGP using GPGMail


[PATCH 05/14] txt-to-pot.sh: use the $( ... ) construct for command substitution

2014-04-23 Thread Elia Pinto
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
   sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done

and then carefully proof-read.

Signed-off-by: Elia Pinto 
---
 git-gui/po/glossary/txt-to-pot.sh |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-gui/po/glossary/txt-to-pot.sh 
b/git-gui/po/glossary/txt-to-pot.sh
index 49bf7c5..8249915 100755
--- a/git-gui/po/glossary/txt-to-pot.sh
+++ b/git-gui/po/glossary/txt-to-pot.sh
@@ -11,7 +11,7 @@
 if [ $# -eq 0 ]
 then
cat < git-gui-glossary.pot
+Usage: $(basename $0) git-gui-glossary.txt > git-gui-glossary.pot
 !
exit 1;
 fi
@@ -33,7 +33,7 @@ cat <\n"
 "Language-Team: LANGUAGE \n"
-- 
1.7.10.4

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


[PATCH 10/14] git-web--browse.sh: use the $( ... ) construct for command substitution

2014-04-23 Thread Elia Pinto
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
   sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done

and then carefully proof-read.

Signed-off-by: Elia Pinto 
---
 git-web--browse.sh |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-web--browse.sh b/git-web--browse.sh
index ebdfba6..ae15253 100755
--- a/git-web--browse.sh
+++ b/git-web--browse.sh
@@ -59,7 +59,7 @@ do
-b|--browser*|-t|--tool*)
case "$#,$1" in
*,*=*)
-   browser=`expr "z$1" : 'z-[^=]*=\(.*\)'`
+   browser=$(expr "z$1" : 'z-[^=]*=\(.*\)')
;;
1,*)
usage ;;
@@ -71,7 +71,7 @@ do
-c|--config*)
case "$#,$1" in
*,*=*)
-   conf=`expr "z$1" : 'z-[^=]*=\(.*\)'`
+   conf=$(expr "z$1" : 'z-[^=]*=\(.*\)')
;;
1,*)
usage ;;
@@ -100,7 +100,7 @@ then
for opt in "$conf" "web.browser"
do
test -z "$opt" && continue
-   browser="`git config $opt`"
+   browser="$(git config $opt)"
test -z "$browser" || break
done
if test -n "$browser" && ! valid_tool "$browser"; then
-- 
1.7.10.4

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


[PATCH 01/14] t9365-continuing-queries.sh: use the $( ... ) construct for command substitution

2014-04-23 Thread Elia Pinto
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
   sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done

and then carefully proof-read.

Signed-off-by: Elia Pinto 
---
 contrib/mw-to-git/t/t9365-continuing-queries.sh |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/mw-to-git/t/t9365-continuing-queries.sh 
b/contrib/mw-to-git/t/t9365-continuing-queries.sh
index 27e267f..0164547 100755
--- a/contrib/mw-to-git/t/t9365-continuing-queries.sh
+++ b/contrib/mw-to-git/t/t9365-continuing-queries.sh
@@ -9,7 +9,7 @@ test_check_precond
 
 test_expect_success 'creating page w/ >500 revisions' '
wiki_reset &&
-   for i in `test_seq 501`
+   for i in $(test_seq 501)
do
echo "creating revision $i" &&
wiki_editpage foo "revision $i" true
-- 
1.7.10.4

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


[PATCH 14/14] p5302-pack-index.sh: use the $( ... ) construct for command substitution

2014-04-23 Thread Elia Pinto
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
   sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done

and then carefully proof-read.

Signed-off-by: Elia Pinto 
---
 t/perf/p5302-pack-index.sh |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/perf/p5302-pack-index.sh b/t/perf/p5302-pack-index.sh
index 6cb5b0d..5ee9211 100755
--- a/t/perf/p5302-pack-index.sh
+++ b/t/perf/p5302-pack-index.sh
@@ -8,7 +8,7 @@ test_perf_large_repo
 
 test_expect_success 'repack' '
git repack -ad &&
-   PACK=`ls .git/objects/pack/*.pack | head -n1` &&
+   PACK=$(ls .git/objects/pack/*.pack | head -n1) &&
test -f "$PACK" &&
export PACK
 '
-- 
1.7.10.4

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


[PATCH 13/14] lib-gpg.sh: use the $( ... ) construct for command substitution

2014-04-23 Thread Elia Pinto
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
   sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done

and then carefully proof-read.

Signed-off-by: Elia Pinto 
---
 t/lib-gpg.sh |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 05824fa..fd499e7 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-gpg_version=`gpg --version 2>&1`
+gpg_version=$(gpg --version 2>&1)
 if test $? = 127; then
say "You do not seem to have gpg installed"
 else
-- 
1.7.10.4

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


[PATCH 06/14] git-pull.sh: use the $( ... ) construct for command substitution

2014-04-23 Thread Elia Pinto
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
   sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done

and then carefully proof-read.

Signed-off-by: Elia Pinto 
---
 git-pull.sh |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-pull.sh b/git-pull.sh
index 6cd8ebc..cfc589d 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -108,7 +108,7 @@ do
-s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)
case "$#,$1" in
*,*=*)
-   strategy=`expr "z$1" : 'z-[^=]*=\(.*\)'` ;;
+   strategy=$(expr "z$1" : 'z-[^=]*=\(.*\)') ;;
1,*)
usage ;;
*)
-- 
1.7.10.4

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


[PATCH 09/14] git-stash.sh: use the $( ... ) construct for command substitution

2014-04-23 Thread Elia Pinto
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
   sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done

and then carefully proof-read.

Signed-off-by: Elia Pinto 
---
 git-stash.sh |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-stash.sh b/git-stash.sh
index 4798bcf..af549c7 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -13,7 +13,7 @@ USAGE="list []
 
 SUBDIRECTORY_OK=Yes
 OPTIONS_SPEC=
-START_DIR=`pwd`
+START_DIR=$(pwd)
 . git-sh-setup
 . git-sh-i18n
 require_work_tree
-- 
1.7.10.4

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


[PATCH 08/14] git-rebase.sh: use the $( ... ) construct for command substitution

2014-04-23 Thread Elia Pinto
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
   sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done

and then carefully proof-read.

Signed-off-by: Elia Pinto 
---
 git-rebase.sh |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 4543815..5c7a0a1 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -457,8 +457,8 @@ then
 else
if test -z "$onto"
then
-   empty_tree=`git hash-object -t tree /dev/null`
-   onto=`git commit-tree $empty_tree http://vger.kernel.org/majordomo-info.html


[PATCH 11/14] lib-credential.sh: use the $( ... ) construct for command substitution

2014-04-23 Thread Elia Pinto
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
   sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done

and then carefully proof-read.

Signed-off-by: Elia Pinto 
---
 t/lib-credential.sh |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/lib-credential.sh b/t/lib-credential.sh
index 957ae93..9e7d796 100755
--- a/t/lib-credential.sh
+++ b/t/lib-credential.sh
@@ -281,7 +281,7 @@ helper_test_timeout() {
 cat >askpass <<\EOF
 #!/bin/sh
 echo >&2 askpass: $*
-what=`echo $1 | cut -d" " -f1 | tr A-Z a-z | tr -cd a-z`
+what=$(echo $1 | cut -d" " -f1 | tr A-Z a-z | tr -cd a-z)
 echo "askpass-$what"
 EOF
 chmod +x askpass
-- 
1.7.10.4

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


[PATCH 12/14] lib-cvs.sh: use the $( ... ) construct for command substitution

2014-04-23 Thread Elia Pinto
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
   sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done

and then carefully proof-read.

Signed-off-by: Elia Pinto 
---
 t/lib-cvs.sh |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/lib-cvs.sh b/t/lib-cvs.sh
index 5076718..9b2bcfb 100644
--- a/t/lib-cvs.sh
+++ b/t/lib-cvs.sh
@@ -13,7 +13,7 @@ fi
 CVS="cvs -f"
 export CVS
 
-cvsps_version=`cvsps -h 2>&1 | sed -ne 's/cvsps version //p'`
+cvsps_version=$(cvsps -h 2>&1 | sed -ne 's/cvsps version //p')
 case "$cvsps_version" in
 2.1 | 2.2*)
;;
-- 
1.7.10.4

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


[PATCH 07/14] git-rebase--merge.sh: use the $( ... ) construct for command substitution

2014-04-23 Thread Elia Pinto
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
   sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done

and then carefully proof-read.

Signed-off-by: Elia Pinto 
---
 git-rebase--merge.sh |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 71429fd..6d77b3c 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -24,7 +24,7 @@ continue_merge () {
die "$resolvemsg"
fi
 
-   cmt=`cat "$state_dir/current"`
+   cmt=$(cat "$state_dir/current")
if ! git diff-index --quiet --ignore-submodules HEAD --
then
if ! git commit ${gpg_sign_opt:+"$gpg_sign_opt"} --no-verify -C 
"$cmt"
@@ -143,7 +143,7 @@ echo "$onto_name" > "$state_dir/onto_name"
 write_basic_state
 
 msgnum=0
-for cmt in `git rev-list --reverse --no-merges "$revisions"`
+for cmt in $(git rev-list --reverse --no-merges "$revisions")
 do
msgnum=$(($msgnum + 1))
echo "$cmt" > "$state_dir/cmt.$msgnum"
-- 
1.7.10.4

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


[PATCH 02/14] test-gitmw-lib.sh: use the $( ... ) construct for command substitution

2014-04-23 Thread Elia Pinto
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
   sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done

and then carefully proof-read.

Signed-off-by: Elia Pinto 
---
 contrib/mw-to-git/t/test-gitmw-lib.sh |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/mw-to-git/t/test-gitmw-lib.sh 
b/contrib/mw-to-git/t/test-gitmw-lib.sh
index 3372b2a..c783752 100755
--- a/contrib/mw-to-git/t/test-gitmw-lib.sh
+++ b/contrib/mw-to-git/t/test-gitmw-lib.sh
@@ -90,7 +90,7 @@ test_diff_directories () {
 #
 # Check that  contains exactly  files
 test_contains_N_files () {
-   if test `ls -- "$1" | wc -l` -ne "$2"; then
+   if test $(ls -- "$1" | wc -l) -ne "$2"; then
echo "directory $1 should contain $2 files"
echo "it contains these files:"
ls "$1"
@@ -341,10 +341,10 @@ wiki_install () {

"http://download.wikimedia.org/mediawiki/$MW_VERSION_MAJOR/"\
"$MW_FILENAME. "\
"Please fix your connection and launch the script 
again."
-   echo "$MW_FILENAME downloaded in `pwd`. "\
+   echo "$MW_FILENAME downloaded in $(pwd). "\
"You can delete it later if you want."
else
-   echo "Reusing existing $MW_FILENAME downloaded in `pwd`."
+   echo "Reusing existing $MW_FILENAME downloaded in $(pwd)."
fi
archive_abs_path=$(pwd)/$MW_FILENAME
cd "$WIKI_DIR_INST/$WIKI_DIR_NAME/" ||
-- 
1.7.10.4

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


[PATCH 03/14] t7900-subtree.sh: use the $( ... ) construct for command substitution

2014-04-23 Thread Elia Pinto
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
   sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done

and then carefully proof-read.

Signed-off-by: Elia Pinto 
---
 contrib/subtree/t/t7900-subtree.sh |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/subtree/t/t7900-subtree.sh 
b/contrib/subtree/t/t7900-subtree.sh
index 66ce4b0..b22b710 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -76,7 +76,7 @@ test_expect_success 'add sub1' '
 
 # Save this hash for testing later.
 
-subdir_hash=`git rev-parse HEAD`
+subdir_hash=$(git rev-parse HEAD)
 
 test_expect_success 'add sub2' '
 create sub2 &&
-- 
1.7.10.4

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


[PATCH 04/14] appp.sh: use the $( ... ) construct for command substitution

2014-04-23 Thread Elia Pinto
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
   sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done

and then carefully proof-read.

Signed-off-by: Elia Pinto 
---
 contrib/thunderbird-patch-inline/appp.sh |   14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/contrib/thunderbird-patch-inline/appp.sh 
b/contrib/thunderbird-patch-inline/appp.sh
index 5eb4a51..8dc73ec 100755
--- a/contrib/thunderbird-patch-inline/appp.sh
+++ b/contrib/thunderbird-patch-inline/appp.sh
@@ -10,7 +10,7 @@ CONFFILE=~/.appprc
 
 SEP="-=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-"
 if [ -e "$CONFFILE" ] ; then
-   LAST_DIR=`grep -m 1 "^LAST_DIR=" "${CONFFILE}"|sed -e 's/^LAST_DIR=//'`
+   LAST_DIR=$(grep -m 1 "^LAST_DIR=" "${CONFFILE}"|sed -e 's/^LAST_DIR=//')
cd "${LAST_DIR}"
 else
cd > /dev/null
@@ -25,11 +25,11 @@ fi
 
 cd - > /dev/null
 
-SUBJECT=`sed -n -e '/^Subject: /p' "${PATCH}"`
-HEADERS=`sed -e '/^'"${SEP}"'$/,$d' $1`
-BODY=`sed -e "1,/${SEP}/d" $1`
-CMT_MSG=`sed -e '1,/^$/d' -e '/^---$/,$d' "${PATCH}"`
-DIFF=`sed -e '1,/^---$/d' "${PATCH}"`
+SUBJECT=$(sed -n -e '/^Subject: /p' "${PATCH}")
+HEADERS=$(sed -e '/^'"${SEP}"'$/,$d' $1)
+BODY=$(sed -e "1,/${SEP}/d" $1)
+CMT_MSG=$(sed -e '1,/^$/d' -e '/^---$/,$d' "${PATCH}")
+DIFF=$(sed -e '1,/^---$/d' "${PATCH}")
 
 CCS=`echo -e "$CMT_MSG\n$HEADERS" | sed -n -e 's/^Cc: \(.*\)$/\1,/gp' \
-e 's/^Signed-off-by: \(.*\)/\1,/gp'`
@@ -48,7 +48,7 @@ if [ "x${BODY}x" != "xx" ] ; then
 fi
 echo "$DIFF" >> $1
 
-LAST_DIR=`dirname "${PATCH}"`
+LAST_DIR=$(dirname "${PATCH}")
 
 grep -v "^LAST_DIR=" "${CONFFILE}" > "${CONFFILE}_"
 echo "LAST_DIR=${LAST_DIR}" >> "${CONFFILE}_"
-- 
1.7.10.4

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


Re: [PATCH v2] git tag --contains : avoid stack overflow

2014-04-23 Thread Johannes Schindelin
Hi,

On Wed, 23 Apr 2014, Stepan Kasal wrote:

> I have found out that "ulimit -s" does not work on Windows.
> Adding this as a prerequisite, we will skip the test there.

The interdiff can be seen here:

https://github.com/msysgit/git/commit/c68e27d5

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


[PATCH 1/2] git-remote-mediawiki: allow stop/start-ing the test server

2014-04-23 Thread Matthieu Moy
Previously, the user had to launch a complete re-install after a lighttpd
stop (e.g. a reboot).

Signed-off-by: Matthieu Moy 
---
 contrib/mw-to-git/t/install-wiki.sh   | 10 ++
 contrib/mw-to-git/t/test-gitmw-lib.sh |  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/contrib/mw-to-git/t/install-wiki.sh 
b/contrib/mw-to-git/t/install-wiki.sh
index 70a53f6..c215213 100755
--- a/contrib/mw-to-git/t/install-wiki.sh
+++ b/contrib/mw-to-git/t/install-wiki.sh
@@ -20,6 +20,8 @@ usage () {
echo "  install | -i :  Install a wiki on your computer."
echo "  delete | -d : Delete the wiki and all its pages and "
echo "  content."
+   echo "  start  | -s : Start the previously configured lighttpd 
daemon"
+   echo "  stop: Stop lighttpd daemon."
 }
 
 
@@ -33,6 +35,14 @@ case "$1" in
wiki_delete
exit 0
;;
+   "start" | "-s")
+   start_lighttpd
+   exit
+   ;;
+   "stop")
+   stop_lighttpd
+   exit
+   ;;
"--help" | "-h")
usage
exit 0
diff --git a/contrib/mw-to-git/t/test-gitmw-lib.sh 
b/contrib/mw-to-git/t/test-gitmw-lib.sh
index 3372b2a..d9a1149 100755
--- a/contrib/mw-to-git/t/test-gitmw-lib.sh
+++ b/contrib/mw-to-git/t/test-gitmw-lib.sh
@@ -289,7 +289,6 @@ start_lighttpd () {
 # Kill daemon lighttpd and removes files and folders associated.
 stop_lighttpd () {
test -f "$WEB_TMP/pid" && kill $(cat "$WEB_TMP/pid")
-   rm -rf "$WEB"
 }
 
 # Create the SQLite database of the MediaWiki. If the database file already
@@ -415,6 +414,7 @@ wiki_reset () {
 wiki_delete () {
if test $LIGHTTPD = "true"; then
stop_lighttpd
+   rm -fr "$WEB"
else
# Delete the wiki's directory.
rm -rf "$WIKI_DIR_INST/$WIKI_DIR_NAME" ||
-- 
1.9.2.667.ge5b74e1

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


Re: [PATCH 05/14] txt-to-pot.sh: use the $( ... ) construct for command substitution

2014-04-23 Thread Matthieu Moy
Elia Pinto  writes:

>  git-gui/po/glossary/txt-to-pot.sh |4 ++--

git-gui is a separate project, patches are normally applied to git-gui
first, and then pulled by Junio.

I'd suggest dropping this patch, it's probably not worth the trouble
(although the patch is correct).

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


Re: [PATCH 01/14] t9365-continuing-queries.sh: use the $( ... ) construct for command substitution

2014-04-23 Thread Matthieu Moy
Patches 01 to 14/14 are

Reviewed-by: Matthieu Moy 

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


[PATCH 2/2] git-remote-mediawiki: fix encoding issue for UTF-8 media files

2014-04-23 Thread Matthieu Moy
When a media file contains valid UTF-8, git-remote-mediawiki tried to be
too clever about the encoding, and the call to utf8::downgrade() on the
downloaded content was failing with

  Wide character in subroutine entry at git-remote-mediawiki line 583.

Instead, use $response->decode() to apply decoding linked to the
Content-Encoding: header, and return the content without attempting any
charset decoding.

Signed-off-by: Matthieu Moy 
---
 contrib/mw-to-git/git-remote-mediawiki.perl  |  7 ++-
 contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh | 19 +++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl 
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 3f8d993..8dd74a9 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -461,7 +461,12 @@ sub download_mw_mediafile {
 
my $response = $mediawiki->{ua}->get($download_url);
if ($response->code == HTTP_CODE_OK) {
-   return $response->decoded_content;
+   # It is tempting to return
+   # $response->decoded_content({charset => "none"}), but
+   # when doing so, utf8::downgrade($content) fails with
+   # "Wide character in subroutine entry".
+   $response->decode();
+   return $response->content();
} else {
print {*STDERR} "Error downloading mediafile from :\n";
print {*STDERR} "URL: ${download_url}\n";
diff --git a/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh 
b/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
index 5a03739..3ff3a09 100755
--- a/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
+++ b/contrib/mw-to-git/t/t9363-mw-to-git-export-import.sh
@@ -58,6 +58,25 @@ test_expect_success 'git clone works on previously created 
wiki with media files
test_cmp mw_dir_clone/Foo.txt mw_dir/Foo.txt
 '
 
+test_expect_success 'git push can upload media (File:) files containing valid 
UTF-8' '
+   wiki_reset &&
+   git clone mediawiki::'"$WIKI_URL"' mw_dir &&
+   (
+   cd mw_dir &&
+   "$PERL_PATH" -e "print STDOUT \"UTF-8 content: éèàéê€.\";" 
>Bar.txt &&
+   git add Bar.txt &&
+   git commit -m "add a text file with UTF-8 content" &&
+   git push
+   )
+'
+
+test_expect_success 'git clone works on previously created wiki with media 
files containing valid UTF-8' '
+   test_when_finished "rm -rf mw_dir mw_dir_clone" &&
+   git clone -c remote.origin.mediaimport=true \
+   mediawiki::'"$WIKI_URL"' mw_dir_clone &&
+   test_cmp mw_dir_clone/Bar.txt mw_dir/Bar.txt
+'
+
 test_expect_success 'git push & pull work with locally renamed media files' '
wiki_reset &&
git clone mediawiki::'"$WIKI_URL"' mw_dir &&
-- 
1.9.2.667.ge5b74e1

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


Re: [PATCH v2] git tag --contains : avoid stack overflow

2014-04-23 Thread Stepan Kasal
Hello,

On Wed, Apr 23, 2014 at 04:28:39PM +0200, Johannes Schindelin wrote:
> The interdiff can be seen here:
>   https://github.com/msysgit/git/commit/c68e27d5

not exatly, is also changes the number of commits in the "deep repo"
from 1000 to 4000, as peff proposed.

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


Re: How to trim the fat on my log graphs

2014-04-23 Thread Robert Dailey
Thanks, removing those two options did help quite a bit already.
However, the history can still get pretty crazy. Is there a way to
hide all tags from the log graph? Really I just want the LABELS to be
hidden.

On Tue, Apr 22, 2014 at 4:37 PM, Junio C Hamano  wrote:
> Robert Dailey  writes:
>
>> git log log --graph --abbrev-commit --decorate --date=relative
>> --format=format:'%C(bold blue)%h%C(reset)%x09%C(bold
>> green)(%ar)%C(reset)%C(bold yellow)%d%C(reset) %C(dim
>> white)%an%C(reset) - %C(white)%s%C(reset)' --branches --remotes
>> ...
>> The goal is to weed out the really distant and irrelevant commits. I
>> really just want to see the commits that are closely related to my
>> current branch. Thanks in advance.
>
> For a starter, how about dropping "--branches --remotes" from that
> command line?  A merge from elsewhere will show as "Merge branch foo"
> which should be sufficient without the decoration.
--
To unsubscribe from this list: send the line "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: [ANNOUNCE] Git v2.0.0-rc0

2014-04-23 Thread Junio C Hamano
Johan Herland  writes:

> I.e. use Kyle's patch to t9117, plus something like this:
>
> diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
> index 5b3c38d..9f579e0 100644
> --- a/Documentation/git-svn.txt
> +++ b/Documentation/git-svn.txt
> @@ -91,6 +91,9 @@ COMMANDS
>  NOTE: Before Git v2.0, the default prefix was "" (no prefix). This
>  meant that SVN-tracking refs were put at "refs/remotes/*", which is
>  incompatible with how Git's own remote-tracking refs are organized.
> +If you still want the old default, you can get it by passing
> +'--prefix ""' on the command line ('--prefix=""' may not work if
> +your Perl's Getopt::Long is < v2.37).
>
>  --ignore-paths=;;
> When passed to 'init' or 'clone' this regular expression will

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


Re: [PATCH 2/2] mergetool: run prompt only if guessed tool

2014-04-23 Thread Junio C Hamano
David Aguilar  writes:

> On Tue, Apr 22, 2014 at 10:19 AM, Junio C Hamano  wrote:
> ...
>> Thanks for CC'ing Charles, by the way.  I think his point about
>> mentioning the change of default somewhere in the documentation
>> has some merits, and it can be done in a follow-up patch on top.
>
> Another thing that crossed my mind is that we have -y for --no-prompt
> because --prompt was the original default. Maybe a -i (?) shortcut for
> the interactive --prompt can be added to make the "need to skip some
> when resolving" use case easier to activate.

Hmm, perhaps, but is "do we prompt to give a chance to the user to
say 'no, I am not interested in running the tool to that path'" the
only interactivity in the overall end-user experience in using the
mergetool?  To end-users, both interaction with the mergetool
front-end and interaction with individual back-end taken together
would comprise the whole end-user experience, so "--interactive"
option that is implied by "-i" short-cut may make them expect a
behaviour from the backend that is more interactive than without,
which would not be the case, so
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] mergetool: run prompt only if guessed tool

2014-04-23 Thread Junio C Hamano
Charles Bailey  writes:

> The bit of documentation that I was thinking of is in
> Documentation/git-mergetool.txt where it states that "--prompt" is the
> default which is now only partially true.

Thanks for being careful to help tying the loose ends.

Perhaps like this?

I take that your original motivation was to confirm to run a tool on
this particular (as opposed to another) path, but the user can also
take the prompt as to confirm to run this (as opposed to some other)
tool.  The latter of which of course is irritating for those who
told which exact tool to use.

I think the large part of the reason why you and Felipe had to have
a long back-and-forth was because it was unclear that the prompt
served these two purposes from the documentation, so I attempted to
clarify the first motivation by adding a brief half-sentence.


 Documentation/git-mergetool.txt | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 07137f2..ec089ff 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -71,11 +71,14 @@ success of the resolution after the custom tool has exited.
 --no-prompt::
Don't prompt before each invocation of the merge resolution
program.
+   This is the default if the merge resolution program is
+   explicitly specified with the `--tool` option or with the
+   `merge.tool` configuration variable.
 
 --prompt::
-   Prompt before each invocation of the merge resolution program.
-   This is the default behaviour; the option is provided to
-   override any configuration settings.
+   Prompt before each invocation of the merge resolution program
+   to ask if it should be run for the path.
+
 
 TEMPORARY FILES
 ---
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Archiving off old branches

2014-04-23 Thread Tim Chase
I've got a branch for each bug/issue and it was getting a bit
unwieldy.  A little searching suggested this

  # archive off the BUG-123 branch
  git update-ref refs/closed/BUG-123 BUG-123
  git branch -D BUG-123

which seems to do exactly what I want -- branches are archived off so
they still have refs, but they don't appear in the output of "git
branch".

Reading up on "git help update-ref", it states that it updates the
name "safely".  As best I can tell, the above process does something
like

  cd .git/refs
  mkdir -p closed
  mv heads/BUG-123 closed

Is there something unsafe about this?  The advantage to the latter
would be that I could do a bunch of them easily:

  mv heads/BUG-{123,234,345,456,567} closed

but I want to make sure I'm not screwing something up unsafely.  Is
there some key element to "update-ref"'s safety that I'm missing?

-tkc



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


Re: How to trim the fat on my log graphs

2014-04-23 Thread Junio C Hamano
Robert Dailey  writes:

[Administrivia: because people read from top to bottom / why is it
bad to top-post? / please do not top-post.]

> On Tue, Apr 22, 2014 at 4:37 PM, Junio C Hamano  wrote:
>> Robert Dailey  writes:
>>
>>> git log log --graph --abbrev-commit --decorate --date=relative
>>> --format=format:'%C(bold blue)%h%C(reset)%x09%C(bold
>>> green)(%ar)%C(reset)%C(bold yellow)%d%C(reset) %C(dim
>>> white)%an%C(reset) - %C(white)%s%C(reset)' --branches --remotes
>>> ...
>>> The goal is to weed out the really distant and irrelevant commits. I
>>> really just want to see the commits that are closely related to my
>>> current branch. Thanks in advance.
>>
>> For a starter, how about dropping "--branches --remotes" from that
>> command line?  A merge from elsewhere will show as "Merge branch foo"
>> which should be sufficient without the decoration.
>
> Thanks, removing those two options did help quite a bit already.
> However, the history can still get pretty crazy. Is there a way to
> hide all tags from the log graph? Really I just want the LABELS to be
> hidden.

As you had --decorate and a lot of %C(cruft), I was assuming that
you do want to see all the bells and whistles when I suggested to
omit --branches and --remotes.

If you do not want --decorate, I think you can omit that from the
command line without changing what the output means.


--
To unsubscribe from this list: send the line "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 2/6] test: add test_write_lines helper

2014-04-23 Thread Junio C Hamano
"Michael S. Tsirkin"  writes:

> As suggested by Junio.
>
> Signed-off-by: Michael S. Tsirkin 
> ---

Ehh, I would probably not suggest such an implementation though.

test_write_lines () {
printf "%s\n" "$@"
}

might be, but not with "echo" and semicolon on the same line as
"for" ;-).

>  t/test-lib-functions.sh | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index aeae3ca..2fa6453 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -712,6 +712,13 @@ test_ln_s_add () {
>   fi
>  }
>  
> +# This function writes out its parameters, one per line
> +test_write_lines () {
> + for line in "$@"; do
> + echo "$line"
> + done
> +}
> +
>  perl () {
>   command "$PERL_PATH" "$@"
>  }
--
To unsubscribe from this list: send the line "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 4/6] patch-id: make it stable against hunk reordering

2014-04-23 Thread Junio C Hamano
Are these three patches the same as what has been queued on
mt/patch-id-stable topic and cooking in 'next' for a few weeks?

--
To unsubscribe from this list: send the line "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 3/6] tests: new test for orderfile options

2014-04-23 Thread Junio C Hamano
"Michael S. Tsirkin"  writes:

> The test is very basic and can be extended.
> Couldn't find a good existing place to put it,
> so created a new file.
>
> Signed-off-by: Michael S. Tsirkin 
> ---
>  t/t4056-diff-order.sh | 63 
> +++
>  1 file changed, 63 insertions(+)
>  create mode 100755 t/t4056-diff-order.sh
>
> diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh
> new file mode 100755
> index 000..a247b7a
> --- /dev/null
> +++ b/t/t4056-diff-order.sh
> @@ -0,0 +1,63 @@
> +#!/bin/sh
> +
> +test_description='diff orderfile'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup' '
> +as="a a a a a a a a" && # eight a
> +test_write_lines $as >foo &&
> +test_write_lines $as >bar &&
> + git add foo bar &&
> + git commit -a -m initial &&
> +test_write_lines $as b >foo &&
> +test_write_lines $as b >bar &&

These lines are curiously jaggy; are you indenting with spaces?
Please don't.

My running "git am -s3c" may auto-fix these indentation issues, so
please wait before seeing what may be pushed out on 'pu'.  It may
turn out that there is no other need for rerolling this patch in the
series.

Thanks.

> + git commit -a -m first &&
> + test_write_lines bar foo >bar-then-foo &&
> + test_write_lines foo bar >foo-then-bar &&
> + git diff -Ofoo-then-bar HEAD~1..HEAD >diff-foo-then-bar &&
> + git diff -Obar-then-foo HEAD~1..HEAD >diff-bar-then-foo
> +'
> +
> +test_diff_well_formed () {
> + grep ^+b "$1" >added
> + grep ^-b "$1" >removed
> + grep ^+++ "$1" >oldfiles
> + grep ^--- "$1" >newfiles
> + test_line_count = 2 added &&
> + test_line_count = 0 removed &&
> + test_line_count = 2 oldfiles &&
> + test_line_count = 2 newfiles
> +}
> +
> +test_expect_success 'diff output with -O is well-formed' '
> + test_diff_well_formed diff-foo-then-bar &&
> + test_diff_well_formed diff-bar-then-foo
> +'
> +
> +test_expect_success 'flag -O affects diff output' '
> + ! test_cmp diff-foo-then-bar diff-bar-then-foo
> +'
> +
> +test_expect_success 'orderfile is same as -O' '
> + test_config diff.orderfile foo-then-bar &&
> + git diff HEAD~1..HEAD >diff-foo-then-bar-config &&
> + test_config diff.orderfile bar-then-foo &&
> + git diff HEAD~1..HEAD >diff-bar-then-foo-config &&
> + test_cmp diff-foo-then-bar diff-foo-then-bar-config &&
> + test_cmp diff-bar-then-foo diff-bar-then-foo-config
> +'
> +
> +test_expect_success '-O overrides orderfile' '
> + test_config diff.orderfile foo-then-bar &&
> + git diff -Obar-then-foo HEAD~1..HEAD >diff-bar-then-foo-flag &&
> + test_cmp diff-bar-then-foo diff-bar-then-foo-flag
> +'
> +
> +test_expect_success '/dev/null is same as no orderfile' '
> + git diff -O/dev/null HEAD~1..HEAD>diff-null-orderfile &&
> + git diff HEAD~1..HEAD >diff-default &&
> + test_cmp diff-null-orderfile diff-default
> +'
> +
> +test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Archiving off old branches

2014-04-23 Thread Junio C Hamano
Tim Chase  writes:

> Reading up on "git help update-ref", it states that it updates the
> name "safely".

I think that description is well intended but is misleading.  There
are many potential sources of risk, and the "safely" refers to
protection against a particular kind of risk: updating from a value
that you did not intend to (i.e. you examined and decided the update
is good, time passes while somebody else might have mucked with the
ref, and then you execute the update you decided to do).  And the
safety afforded to you is with "git update-ref ref newvalue oldvalue"
that makes sure the ref still points at the oldvalue and refuses to
update it to newvalue if it doesn't.
--
To unsubscribe from this list: send the line "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 4/6] patch-id: make it stable against hunk reordering

2014-04-23 Thread Michael S. Tsirkin
On Wed, Apr 23, 2014 at 10:39:23AM -0700, Junio C Hamano wrote:
> Are these three patches the same as what has been queued on
> mt/patch-id-stable topic and cooking in 'next' for a few weeks?

Not exactly - at your request I implemented git config
options to control patch id behaviour.
Documentation and tests updated to match.
--
To unsubscribe from this list: send the line "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 2/6] test: add test_write_lines helper

2014-04-23 Thread Michael S. Tsirkin
On Wed, Apr 23, 2014 at 10:34:30AM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin"  writes:
> 
> > As suggested by Junio.
> >
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> 
> Ehh, I would probably not suggest such an implementation though.
> 
>   test_write_lines () {
>   printf "%s\n" "$@"
>   }
> 
> might be, but not with "echo" and semicolon on the same line as
> "for" ;-).

Okay I didn't know printf reuses format in bash, cute trick.

Do you want to rewrite it yourself or want me to post a
new version?


> >  t/test-lib-functions.sh | 7 +++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> > index aeae3ca..2fa6453 100644
> > --- a/t/test-lib-functions.sh
> > +++ b/t/test-lib-functions.sh
> > @@ -712,6 +712,13 @@ test_ln_s_add () {
> > fi
> >  }
> >  
> > +# This function writes out its parameters, one per line
> > +test_write_lines () {
> > +   for line in "$@"; do
> > +   echo "$line"
> > +   done
> > +}
> > +
> >  perl () {
> > command "$PERL_PATH" "$@"
> >  }
--
To unsubscribe from this list: send the line "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: Archiving off old branches

2014-04-23 Thread Jonathan Nieder
Hi,

Tim Chase wrote:

>   cd .git/refs
>   mkdir -p closed
>   mv heads/BUG-123 closed

That breaks with packed refs (see git-pack-refs(1)), which are a normal
thing to encounter after garbage collection.

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


Re: [PATCH v4 3/6] tests: new test for orderfile options

2014-04-23 Thread Michael S. Tsirkin
On Wed, Apr 23, 2014 at 10:38:07AM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin"  writes:
> 
> > The test is very basic and can be extended.
> > Couldn't find a good existing place to put it,
> > so created a new file.
> >
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  t/t4056-diff-order.sh | 63 
> > +++
> >  1 file changed, 63 insertions(+)
> >  create mode 100755 t/t4056-diff-order.sh
> >
> > diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh
> > new file mode 100755
> > index 000..a247b7a
> > --- /dev/null
> > +++ b/t/t4056-diff-order.sh
> > @@ -0,0 +1,63 @@
> > +#!/bin/sh
> > +
> > +test_description='diff orderfile'
> > +
> > +. ./test-lib.sh
> > +
> > +test_expect_success 'setup' '
> > +as="a a a a a a a a" && # eight a
> > +test_write_lines $as >foo &&
> > +test_write_lines $as >bar &&
> > +   git add foo bar &&
> > +   git commit -a -m initial &&
> > +test_write_lines $as b >foo &&
> > +test_write_lines $as b >bar &&
> 
> These lines are curiously jaggy; are you indenting with spaces?
> Please don't.
> 
> My running "git am -s3c" may auto-fix these indentation issues, so
> please wait before seeing what may be pushed out on 'pu'.  It may
> turn out that there is no other need for rerolling this patch in the
> series.
> 
> Thanks.

Not normally - but this did creep in here, somehow :(

> > +   git commit -a -m first &&
> > +   test_write_lines bar foo >bar-then-foo &&
> > +   test_write_lines foo bar >foo-then-bar &&
> > +   git diff -Ofoo-then-bar HEAD~1..HEAD >diff-foo-then-bar &&
> > +   git diff -Obar-then-foo HEAD~1..HEAD >diff-bar-then-foo
> > +'
> > +
> > +test_diff_well_formed () {
> > +   grep ^+b "$1" >added
> > +   grep ^-b "$1" >removed
> > +   grep ^+++ "$1" >oldfiles
> > +   grep ^--- "$1" >newfiles
> > +   test_line_count = 2 added &&
> > +   test_line_count = 0 removed &&
> > +   test_line_count = 2 oldfiles &&
> > +   test_line_count = 2 newfiles
> > +}
> > +
> > +test_expect_success 'diff output with -O is well-formed' '
> > +   test_diff_well_formed diff-foo-then-bar &&
> > +   test_diff_well_formed diff-bar-then-foo
> > +'
> > +
> > +test_expect_success 'flag -O affects diff output' '
> > +   ! test_cmp diff-foo-then-bar diff-bar-then-foo
> > +'
> > +
> > +test_expect_success 'orderfile is same as -O' '
> > +   test_config diff.orderfile foo-then-bar &&
> > +   git diff HEAD~1..HEAD >diff-foo-then-bar-config &&
> > +   test_config diff.orderfile bar-then-foo &&
> > +   git diff HEAD~1..HEAD >diff-bar-then-foo-config &&
> > +   test_cmp diff-foo-then-bar diff-foo-then-bar-config &&
> > +   test_cmp diff-bar-then-foo diff-bar-then-foo-config
> > +'
> > +
> > +test_expect_success '-O overrides orderfile' '
> > +   test_config diff.orderfile foo-then-bar &&
> > +   git diff -Obar-then-foo HEAD~1..HEAD >diff-bar-then-foo-flag &&
> > +   test_cmp diff-bar-then-foo diff-bar-then-foo-flag
> > +'
> > +
> > +test_expect_success '/dev/null is same as no orderfile' '
> > +   git diff -O/dev/null HEAD~1..HEAD>diff-null-orderfile &&
> > +   git diff HEAD~1..HEAD >diff-default &&
> > +   test_cmp diff-null-orderfile diff-default
> > +'
> > +
> > +test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Archiving off old branches

2014-04-23 Thread Tim Chase
On 2014-04-23 10:58, Jonathan Nieder wrote:
> Tim Chase wrote:
> >   cd .git/refs
> >   mkdir -p closed
> >   mv heads/BUG-123 closed
> 
> That breaks with packed refs (see git-pack-refs(1)), which are a
> normal thing to encounter after garbage collection.
> 
> Hope that helps,

Very much so.  Alrighty...based on that alone, I'll stick to my
archive script that calls update-ref and then deletes the branch.

Thanks,

-tkc


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

2014-04-23 Thread Junio C Hamano
Jonathan Nieder  writes:

> Tim Chase wrote:
>
>>   cd .git/refs
>>   mkdir -p closed
>>   mv heads/BUG-123 closed
>
> That breaks with packed refs (see git-pack-refs(1)), which are a normal
> thing to encounter after garbage collection.

Specifically,

 - if BUG-123 branch was placed in packed-refs file in the past
   (which may be older than what you have right now at heads/BUG-123
   as a loose ref), the above procedure will still make it appear in
   your "git branch --list" output, pointing at a possibly old
   commit that may even have been pruned away.

 - if BUG-123 branch was placed in packed-refs file and you haven't
   touched that branch since then, heads/BUG-123 file would not
   exist, "mv" will fail, and you won't see closed/BUG-123 at the
   end of the procedure.

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


Re: [PATCH 1/3] test-lib: Document short options in t/README

2014-04-23 Thread Junio C Hamano
Ilya Bobyr  writes:

> Most arguments that could be provided to a test have short forms.
> Unless documented, the only way to learn them is to read the code.
>
> Signed-off-by: Ilya Bobyr 
> ---
>  t/README |8 
>  1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/t/README b/t/README
> index caeeb9d..6b93aca 100644
> --- a/t/README
> +++ b/t/README
> @@ -71,7 +71,7 @@ You can pass --verbose (or -v), --debug (or -d), and 
> --immediate
>  (or -i) command line argument to the test, or by setting GIT_TEST_OPTS
>  appropriately before running "make".
>  
> ---verbose::
> +-v,--verbose::
>   This makes the test more verbose.  Specifically, the
>   command being run and their output if any are also
>   output.

I was debating myself if the result should look more like this:

-v::
--verbose::
This makes the test more verbose.  Specifically, the
command being run and their output if any are also
output.

As a straight text file, your version is certainly a lot easier to
read, but at the same time, the entire file is written in more or
less AsciiDoc format (the list of prerequisites and the list of
harness library functions need to be converted to the "item::" form
for the text to format well, though) and I've seen some efforts by
others to run text files in Documentation/ that were originally
meant to be consumed as straight text thru AsciiDoc, so the latter
form might be a small step for futureproofing.

My conclusion at this point is that the original is good for the
current need of the project; if somebody wants to include this file
from somewhere in Documentation/technical, a conversion to use
multiple "item1::item2::description" headers can
be done by that person as part of the "make it fully AsciiDoc"
effort.

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 3/3] test-lib: '--run' to run only specific tests

2014-04-23 Thread Junio C Hamano
Ilya Bobyr  writes:

> @@ -187,10 +192,70 @@ and either can match the "t[0-9]{4}" part to skip the 
> whole
>  test, or t[0-9]{4} followed by ".$number" to say which
>  particular test to skip.
>  
> -Note that some tests in the existing test suite rely on previous
> -test item, so you cannot arbitrarily disable one and expect the
> -remainder of test to check what the test originally was intended
> -to check.
> +For an individual test suite --run could be used to specify that
> +only some tests should be run or that some tests should be
> +excluded from a run.
> +
> +The argument for --run is a list of individual test numbers or
> +ranges with an optional negation prefix that define what tests in
> +a test suite to include in the run.  A range is two numbers
> +separated with a dash and matches a range of tests with both ends
> +been included.  You may omit the first or the second number to
> +mean "from the first test" or "up to the very last test"
> +respectively.
> +
> +Optional prefix of '!' means that the test or a range of tests
> +should be excluded from the run.
> +
> +If --run starts with an unprefixed number or range the initial
> +set of tests to run is empty. If the first item starts with '!'
> +all the tests are added to the initial set.  After initial set is
> +determined every test number or range is added or excluded from
> +the set one by one, from left to right.
> +
> +Individual numbers or ranges could be separated either by a space
> +or a comma.
> +
> +For example, common case is to run several setup tests (1, 2, 3)
> +and then a specific test (21) that relies on that setup:
> +
> +$ sh ./t9200-git-cvsexport-commit.sh --run='1 2 3 21'
> +
> +or:
> +
> +$ sh ./t9200-git-cvsexport-commit.sh --run=1,2,3,21
> +
> +or:
> +
> +$ sh ./t9200-git-cvsexport-commit.sh --run='-3 21'

Good and easily understandable examples. 

> +To run only tests up to a specific test (21), one could do this:
> +
> +$ sh ./t9200-git-cvsexport-commit.sh --run='1-21'
> +
> +or this:
> +
> +$ sh ./t9200-git-cvsexport-commit.sh --run='-21'

These may be redundant, given that the reader would have to have
grokked the earlier "-3 21" already at this point.

> +As noted above, the test set is built going though items left to
> +right, so this:
> +
> +$ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
> +
> +will run tests 1, 2, and 4.

I do not quite understand what you mean by "left to right"; is that
implementation detail necessary for the user of the feature, or is
it talking about some limitation coming from the implementation?
e.g. perhaps "!3 1-4" would not work as people would expect "do not
run 3, but run tests from 1 thru 4 otherwise", and warning against
having such an expectation that cannot be fulfilled?

> +You may use negation with ranges.  The following will run all
> +test as a test suite except from 7 upto 11:
> +
> +$ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'

Hmm, that is somewhat counter-intuitive or at least ambiguous.  I
first thought you would be running everything but skipping 7 thru
11, but your explanation is that it is equivalent to "-6,8-11" (that
is, to intersect set "-11" and set "!7").

The above two illustrate the reason rather well why I said it would
be better to avoid negation because it would complicate the mental
model the user needs to form when using the feature.

> +Some tests in a test suite rely on the previous tests performing
> +certain actions, specifically some tests are designated as
> +"setup" test, so you cannot _arbitrarily_ disable one test and
> +expect the rest to function correctly.

What this text (moved from the top of this hunk) tells the reader
applies to both the traditional t0123.4 and the new "--run=1-3,5-"
syntaxes, but the new placement of it make it sound as if it is only
for skipping with "--run", especially because the text before this
paragraph and also after this paragraph both apply only to "--run".

> +--run is mostly useful when you want to focus on a specific test
> +and know what you are doing.  Or when you want to run up to a
> +certain test.

Likewise for "and know what you are doing" part.  I'd suggest
dropping that phrase from here, and/or make it part of the "you
cannot randomly omit and expect later ones to work" that covers both
ways to skip tests.

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


Trying to setup Visual Studio 2013 with a CentOS 6.5 x64 git server

2014-04-23 Thread Charles Buege
All -

If this is the wrong place to ask this question (I'm heading to
Microsoft's site next), then I apologize, but communities like this
have always been kind to me in the past, so I thought I'd start here.

I'm looking to setup a git server under CentOS 6.5 x64 that will serve
2-5 .NET developers using Visual Studio Pro 2013.  I've been reading
that Visual Studio 2013 now has 'native git support', but as I've been
reading into this more and more, it appears to me that the 'native git
support' is really the fact that Team Foundation Server has git
support on it and that I'd need to setup TFS in order to use the
Visual Studio 2013's native git support.  Can anyone either confirm
and/or deny this for me?  My personal suspicion is that I will need to
implement TortiseGIT to do what I want to do, but wanted to throw this
question out to all you masters of git to direct me in the proper
path.

If, in actuality, I can use a CentOS git server with Visual Studio
2013, can anyone point me in the direction of an
FAQ/directions/YouTube video/book/anything for how to setup something
like this?  I have the resources to setup a CentOS git server (which
will also host some DreamWeaver CC users as well on other projects),
but setting up a dedicated TFS server isn't an option, hence why I'm
looking into this.

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


Re: [PATCH v2] git tag --contains : avoid stack overflow

2014-04-23 Thread Junio C Hamano
Stepan Kasal  writes:

[Administrivia: please refrain from using Mail-Followup-To to
deflect an attempt to directly respond to you; it will waste time of
other people while it may be saving your time].

> From: Jean-Jacques Lafay 
>
> In large repos, the recursion implementation of contains(commit,
> commit_list) may result in a stack overflow. Replace the recursion with
> a loop to fix it.
>
> This problem is more apparent on Windows than on Linux, where the stack
> is more limited by default.
>
> See also this thread on the msysGit list:
>
>   https://groups.google.com/d/topic/msysgit/FqT6boJrb2g/discussion
>
> [jes: re-written to imitate the original recursion more closely]
>
> Thomas Braun pointed out several documentation shortcomings.
>
> Tests are run only if ulimit -s is available.  This means they cannot
> be run on Windows.
>
> Signed-off-by: Jean-Jacques Lafay 
> Signed-off-by: Johannes Schindelin 
> Tested-by: Stepan Kasal 

Thanks.

> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 143a8ea..db82f6d 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -1423,4 +1423,27 @@ EOF
>   test_cmp expect actual
>  '
>  
> +ulimit_stack="ulimit -s 64"
> +test_lazy_prereq ULIMIT 'bash -c "'"$ulimit_stack"'"'

With this implementaion, ULIMIT implies bash, and we use bash that
appears on user's PATH that may not be the one the user chose to run
git with.  Can't we fix both of them by using $SHELL_PATH?

> +>expect

Move this inside test_expect_success?

> +# we require bash and ulimit, this excludes Windows
> +test_expect_success ULIMIT '--contains works in a deep repo' '
> + i=1 &&
> + while test $i -lt 4000
> + do
> + echo "commit refs/heads/master
> +committer A U Thor  $((10 + $i * 100)) +0200
> +data < +commit #$i
> +EOF"
> + test $i = 1 && echo "from refs/heads/master^0"
> + i=$(($i + 1))
> + done | git fast-import &&
> + git checkout master &&
> + git tag far-far-away HEAD^ &&
> + bash -c "'"$ulimit_stack"' && git tag --contains HEAD >actual" &&

So this runs a separate "bash", the only thing which does is to run
a small script that gives a small stack to itself and exit, and then
run "git tag" in the original shell?

Ahh, no, I am mis-pairing the quotes.

How about doing it along this line instead?

run_with_limited_stack () {
"$SHELL_PATH" -c "ulimit -s 64 && $*"
}

test_lazy_prereq ULIMIT "run_with_limited_stack true"

test_expect_success ULIMIT '...' '
>expect &&
i=1 &&
...
done | git-fast-import &&
git tag far-far-away HEAD^ &&
run_with_limited_stack "git tag --contains HEAD" >actual &&
test_cmp expect actual
'

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


Re: [PATCH v2] git tag --contains : avoid stack overflow

2014-04-23 Thread Jeff King
On Wed, Apr 23, 2014 at 12:12:14PM -0700, Junio C Hamano wrote:

> > +ulimit_stack="ulimit -s 64"
> > +test_lazy_prereq ULIMIT 'bash -c "'"$ulimit_stack"'"'
> 
> With this implementaion, ULIMIT implies bash, and we use bash that
> appears on user's PATH that may not be the one the user chose to run
> git with.  Can't we fix both of them by using $SHELL_PATH?

I don't think so. The point is that we _must_ use bash here, not any
POSIX shell. So my $SHELL_PATH is /bin/sh, which is dash, and would not
run the test.

We want to run "some bash" if we can. We may pick a bash on the user's
PATH that is not what they put into $SHELL_PATH, but that should be
relatively rare. And the consequence is that either that bash works fine
and we run the test, or it does not, and we skip the test.

> How about doing it along this line instead?
> 
>   run_with_limited_stack () {
>   "$SHELL_PATH" -c "ulimit -s 64 && $*"
>   }
> 
>   test_lazy_prereq ULIMIT "run_with_limited_stack true"

That's a much more direct test. I like it (aside from the $SHELL_PATH
thing as described above).

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


Re: [PATCH v2] git tag --contains : avoid stack overflow

2014-04-23 Thread Jeff King
On Wed, Apr 23, 2014 at 09:53:25AM +0200, Stepan Kasal wrote:

> I have found out that "ulimit -s" does not work on Windows.
> Adding this as a prerequisite, we will skip the test there.

I found this bit weird, as the test originated on Windows. Did it never
actually cause a failure there (i.e., the "ulimit -s" doesn't do
anything)? Or does "ulimit" fail?

-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: Trying to setup Visual Studio 2013 with a CentOS 6.5 x64 git server

2014-04-23 Thread Jonathan Nieder
Hi,

Charles Buege wrote:

> If, in actuality, I can use a CentOS git server with Visual Studio
> 2013, can anyone point me in the direction of an
> FAQ/directions/YouTube video/book/anything for how to setup something
> like this?

I suspect
http://msdn.microsoft.com/en-us/library/hh850445.aspx#remote_3rd_party_connect_clone
should get you started.

As far as I can tell from #git, every once in a while people seem to
want to escape from GUIs to have control over what they're doing (for
example when cleaning up existing commits, or when performing a
complex merge).  You can find a usable commandline version of Git for
Windows at http://msysgit.github.io/ (and the same project has a
TortoiseSVN-like explorer plugin called GitCheetah if you need that).

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


Re: [PATCH v2 2/3] remote-helpers: move out of contrib

2014-04-23 Thread Junio C Hamano
Max Horn  writes:

> That said, I don't know what the criteria are for moving something out
> of contrib.

Because we accept stuff to contrib/, with an assumption that it is
to stay there without contaminating the main part of the system, the
quality of stuff in contrib/ can be sub-par and it is very possible
that a lot of clean-ups and fixes are necessary before moving them
out.

> Perhaps it is OK to move an undocumented remote-helper
> with known bugs out of contrib.

We should strive to apply the same criteria as new submission to the
main part of the system.  And inputs from people like you who have
more experiences than others on the list in dealing with hg-to-git
bridging are very much welcomed to identify and document what kind
of breakages there are, and to come up with the solution to them.
--
To unsubscribe from this list: send the line "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: Fwd: git p4: feature request - branch check filtering

2014-04-23 Thread Pete Wyckoff
dpr...@gmail.com wrote on Tue, 22 Apr 2014 10:29 +0100:
> There is a patch viewable at this link:
> https://github.com/Stealthii/git/commit/f7a2e611262fd977ac99e066872d3d0743b7df3c
> 
> For the use case this works perfectly - if I define branch mappings
> with git config, followed by setting 'git-p4.skipBranchScan' to true,
> git-p4 will skip scanning of all remote branches and limit to what's
> defined in the map.  An example config:
> 
> [git-p4]
> skipBranchScan = true
> branchList = release_1.0.0:release_1.1.0
> branchList = release_1.1.0:release_1.2.0
> 
> If there is any more information I need to provide let me know. I have
> been using this patch for over two months, testing both use cases with
> and without git-p4.skipBranchScan and I have noticed no issues.  Logic
> of git-p4 is not changed from default behaviour, unless the user
> explicitly sets the boolean flag to skip scanning.

Thanks, Dan.  This looks good and is a fine compromise
considering the various choices we discussed earlier.

Junio's comments about 2.0 non-withstanding, I think this change
should go into the next convenient release.  So 2.1 or 2.0.1;
however the numbers end up working post-2.0.

If you could take a look at Documentation/SubmittingPatches,
and do a few things:

1.  Write a nice commit message, say:

git p4: add skipBranchScan to avoid p4 branch scan

Some more useful text.

2.  Include at the bottom of that message:

Acked-by: Pete Wyckoff 

3.  Inline the text of your patch, not just a link to github.

4.  Consider adding a t98xx test.  This isn't required for
a fairly minor change like yours, but if you think TDD
is fun, have at it.  Might protect your feature against
future hackers who would try to break it.  :)

Then send it to vger, cc junio (and me), and he will be kind
enough to queue it up appropriately.

Thanks!

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


Re: [PATCH 3/3] test-lib: '--run' to run only specific tests

2014-04-23 Thread Eric Sunshine
On Tue, Apr 22, 2014 at 4:19 AM, Ilya Bobyr  wrote:
> Allow better control of the set of tests that will be executed for a
> single test suite.  Mostly useful while debugging or developing as it
> allows to focus on a specific test.
>
> Signed-off-by: Ilya Bobyr 
> ---
> diff --git a/t/README b/t/README
> index 6b93aca..2dac619 100644
> --- a/t/README
> +++ b/t/README
> +As noted above, the test set is built going though items left to
> +right, so this:
> +
> +$ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
> +
> +will run tests 1, 2, and 4.
> +
> +You may use negation with ranges.  The following will run all
> +test as a test suite except from 7 upto 11:

s/upto/up to/
...or...
s/upto/through/

> +$ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
> +
> +Some tests in a test suite rely on the previous tests performing
> +certain actions, specifically some tests are designated as
> +"setup" test, so you cannot _arbitrarily_ disable one test and
> +expect the rest to function correctly.
> +--run is mostly useful when you want to focus on a specific test
> +and know what you are doing.  Or when you want to run up to a
> +certain test.
>
>
>  Naming Tests
> diff --git a/t/t-basic.sh b/t/t-basic.sh
> index ae8874e..e2589cc 100755
> --- a/t/t-basic.sh
> +++ b/t/t-basic.sh
> @@ -84,6 +97,18 @@ check_sub_test_lib_test () {
> )
>  }
>
> +check_sub_test_lib_test_err () {
> +   name="$1" # stdin is the expected output output from the test
> +   # expecte error output is in descriptor 3

s/expecte/expected/

> +   (
> +   cd "$name" &&
> +   sed -e 's/^> //' -e 's/Z$//' >expect.out &&
> +   test_cmp expect.out out &&
> +   sed -e 's/^> //' -e 's/Z$//' <&3 >expect.err &&
> +   test_cmp expect.err err
> +   )
> +}
> +
>  test_expect_success 'pretend we have a fully passing test suite' "
> run_sub_test_lib_test full-pass '3 passing tests' <<-\\EOF &&
> for i in 1 2 3
> @@ -333,6 +358,329 @@ test_expect_success 'GIT_SKIP_TESTS sh pattern' "
> +test_expect_success '--run invalid range start' "
> +   run_sub_test_lib_test_err run-inv-range-start \
> +   '--run invalid range start' \
> +   --run='a-5' <<-\\EOF &&
> +   test_expect_success \"passing test #1\" 'true'
> +   test_done
> +   EOF
> +   check_sub_test_lib_test_err run-inv-range-start \
> +   <<-\\EOF_OUT 3<<-\\EOF_ERR
> +   > FATAL: Unexpected exit with code 1
> +   EOF_OUT
> +   > error: --run: range start should contain only digits: 'a-5'

This reads rather strangely, as if it's attempting to give an example
(after the colon) of a valid digit range, but then shows something
that is not valid. Rewording it slightly can eliminate the ambiguity:

error: --run: invalid non-numeric range start: 'a-5'

> +   EOF_ERR
> +"
> +
> +test_expect_success '--run invalid range end' "
> +   run_sub_test_lib_test_err run-inv-range-end \
> +   '--run invalid range end' \
> +   --run='1-z' <<-\\EOF &&
> +   test_expect_success \"passing test #1\" 'true'
> +   test_done
> +   EOF
> +   check_sub_test_lib_test_err run-inv-range-end \
> +   <<-\\EOF_OUT 3<<-\\EOF_ERR
> +   > FATAL: Unexpected exit with code 1
> +   EOF_OUT
> +   > error: --run: range end should contain only digits: '1-z'

Ditto.

> +   EOF_ERR
> +"
> +
> +test_expect_success '--run invalid selector' "
> +   run_sub_test_lib_test_err run-inv-selector \
> +   '--run invalid selector' \
> +   --run='1?' <<-\\EOF &&
> +   test_expect_success \"passing test #1\" 'true'
> +   test_done
> +   EOF
> +   check_sub_test_lib_test_err run-inv-selector \
> +   <<-\\EOF_OUT 3<<-\\EOF_ERR
> +   > FATAL: Unexpected exit with code 1
> +   EOF_OUT
> +   > error: --run: test selector should contain only digits: '1?'

And here:

error: --run: invalid non-digit in range selector: '1?'

or something.

> +   EOF_ERR
> +"
> +
> +
>  test_set_prereq HAVEIT
>  haveit=no
>  test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index e7d9c51..46ba513 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -366,6 +374,100 @@ match_pattern_list () {
> return 1
>  }
>
> +match_test_selector_list () {
> +   title="$1"
> +   shift
> +   arg="$1"
> +   shift
> +   test -z "$1" && return 0
> +
> +   # Both commas and spaces are accepted as separators
> +   OLDIFS=$IFS
> +   IFS='   ,'

The comment mentions only space and comma, but the actual assigned IFS
value also treats tabs as separators. Perhaps update the comment to
say "commas and whitespace".

> +   set -- $1
> +   IFS=$OLDIFS
> +
> +   # If the first selector is negative we include by default.
> +   include=
> +   case "$1" in

[RFC/PATCH v2 0/3] New 'update-branch' hook

2014-04-23 Thread Felipe Contreras
Currently it's not possible to keep track of changes that happen to a branch,
specifically; when a branch is created and rebased. This patch series aims to 
fix that.

The last patch is the important one, but in the process of testing this I found
out that the GIT_DIR environment variable is not always set, so the hooks (all
of them) get confused.

Too many changes since v1 to list them all.


Felipe Contreras (3):
  sh-setup: export GIT_DIR
  run-command: make sure hooks have always GIT_DIR
  Add 'update-branch' hook

 Documentation/githooks.txt| 15 +
 branch.c  | 12 ++-
 builtin/clone.c   |  8 +++--
 builtin/reset.c   |  5 +++
 git-rebase--interactive.sh|  4 +++
 git-rebase.sh |  4 +++
 git-sh-setup.sh   |  1 +
 run-command.c | 24 --
 t/t5408-update-branch-hook.sh | 76 +++
 9 files changed, 144 insertions(+), 5 deletions(-)
 create mode 100755 t/t5408-update-branch-hook.sh

-- 
1.9.2+fc1.1.g5c924db

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


[RFC/PATCH v2 1/3] sh-setup: export GIT_DIR

2014-04-23 Thread Felipe Contreras
It is what the clients of this library expect.

Signed-off-by: Felipe Contreras 
---
 git-sh-setup.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 5f28b32..fb0362f 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -346,6 +346,7 @@ then
exit 1
}
: ${GIT_OBJECT_DIRECTORY="$GIT_DIR/objects"}
+   export GIT_DIR
 fi
 
 peel_committish () {
-- 
1.9.2+fc1.1.g5c924db

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


[RFC/PATCH v2 2/3] run-command: make sure hooks have always GIT_DIR

2014-04-23 Thread Felipe Contreras
Signed-off-by: Felipe Contreras 
---
 run-command.c | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/run-command.c b/run-command.c
index 75abc47..8e188f6 100644
--- a/run-command.c
+++ b/run-command.c
@@ -765,12 +765,29 @@ int run_hook_ve(const char *const *env, const char *name, 
va_list args)
struct child_process hook;
struct argv_array argv = ARGV_ARRAY_INIT;
const char *p;
-   int ret;
+   const char **mod_env = NULL;
+   int ret, i = 0;
+   struct strbuf buf = STRBUF_INIT;
 
p = find_hook(name);
if (!p)
return 0;
 
+   if (!getenv(GIT_DIR_ENVIRONMENT)) {
+   if (env)
+   for (i = 0; env[i]; i++);
+
+   mod_env = xcalloc(i + 2, sizeof(*mod_env));
+
+   if (env)
+   for (i = 0; env[i]; i++)
+   mod_env[i] = env[i];
+
+   strbuf_addf(&buf, "GIT_DIR=%s", get_git_dir());
+   mod_env[i++] = buf.buf;
+   mod_env[i++] = NULL;
+   }
+
argv_array_push(&argv, p);
 
while ((p = va_arg(args, const char *)))
@@ -778,12 +795,15 @@ int run_hook_ve(const char *const *env, const char *name, 
va_list args)
 
memset(&hook, 0, sizeof(hook));
hook.argv = argv.argv;
-   hook.env = env;
+   hook.env = mod_env ? mod_env : env;
hook.no_stdin = 1;
hook.stdout_to_stderr = 1;
 
ret = run_command(&hook);
argv_array_clear(&argv);
+   strbuf_release(&buf);
+   free(mod_env);
+
return ret;
 }
 
-- 
1.9.2+fc1.1.g5c924db

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


[RFC/PATCH v2 3/3] Add 'update-branch' hook

2014-04-23 Thread Felipe Contreras
This hook is invoked before a branch is updated, either when a branch is
created or updated with 'git branch', or when it's rebased with 'git
rebase'.  It receives three parameters; the name of the branch, the
SHA-1 of the latest commit, and the SHA-1 of the first commit of the
branch.

When a branch is created the first and last commits of the branch are
the same, however when a branch is rebased they are not. If the SHA-1 of
the first commit of the branch is not available (e.g. git reset) a null
SHA-1 (40 zeroes) would be passed.

The hook exit status can be used to deny the branch update.

It can be used to verify the validity of branch names, and also to keep
track of the origin point of a branch, which is otherwise not possible
to find out [1].

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

Signed-off-by: Felipe Contreras 
---
 Documentation/githooks.txt| 15 +
 branch.c  | 12 ++-
 builtin/clone.c   |  8 +++--
 builtin/reset.c   |  5 +++
 git-rebase--interactive.sh|  4 +++
 git-rebase.sh |  4 +++
 t/t5408-update-branch-hook.sh | 76 +++
 7 files changed, 121 insertions(+), 3 deletions(-)
 create mode 100755 t/t5408-update-branch-hook.sh

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index d954bf6..4a44262 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -381,6 +381,21 @@ rebase::
 The commits are guaranteed to be listed in the order that they were
 processed by rebase.
 
+update-branch
+~
+
+This hook is invoked before a branch is updated, either when a branch is
+created or updated with 'git branch', or when it's rebased with 'git rebase'.
+It receives three parameters; the name of the branch, the SHA-1 of the latest
+commit, and the SHA-1 of the first commit of the branch.
+
+When a branch is created the first and last commits of the branch are the same,
+however when a branch is rebased they are not. If the SHA-1 of the first
+commit of the branch is not available (e.g. git reset) a null SHA-1 (40 zeroes)
+would be passed.
+
+The hook exit status can be used to deny the branch update.
+
 
 GIT
 ---
diff --git a/branch.c b/branch.c
index 660097b..7ec0be5 100644
--- a/branch.c
+++ b/branch.c
@@ -4,6 +4,7 @@
 #include "refs.h"
 #include "remote.h"
 #include "commit.h"
+#include "run-command.h"
 
 struct tracking {
struct refspec spec;
@@ -234,6 +235,7 @@ void create_branch(const char *head,
int forcing = 0;
int dont_change_ref = 0;
int explicit_tracking = 0;
+   unsigned const char *orig_sha1;
 
if (track == BRANCH_TRACK_EXPLICIT || track == BRANCH_TRACK_OVERRIDE)
explicit_tracking = 1;
@@ -304,9 +306,17 @@ void create_branch(const char *head,
if (real_ref && track)
setup_tracking(ref.buf + 11, real_ref, track, quiet);
 
-   if (!dont_change_ref)
+   if (!dont_change_ref) {
+   orig_sha1 = forcing ? null_sha1 : sha1;
+   if (run_hook_le(NULL, "update-branch", ref.buf + 11,
+   sha1_to_hex(sha1), 
sha1_to_hex(orig_sha1), NULL)) {
+   unlock_ref(lock);
+   die("hook 'update-branch' returned error");
+   }
+
if (write_ref_sha1(lock, sha1, msg) < 0)
die_errno(_("Failed to write ref"));
+   }
 
strbuf_release(&ref);
free(real_ref);
diff --git a/builtin/clone.c b/builtin/clone.c
index 9b3c04d..d68c49b 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -581,7 +581,7 @@ static void update_remote_refs(const struct ref *refs,
}
 }
 
-static void update_head(const struct ref *our, const struct ref *remote,
+static int update_head(const struct ref *our, const struct ref *remote,
const char *msg)
 {
if (our && starts_with(our->name, "refs/heads/")) {
@@ -589,6 +589,9 @@ static void update_head(const struct ref *our, const struct 
ref *remote,
create_symref("HEAD", our->name, NULL);
if (!option_bare) {
const char *head = skip_prefix(our->name, 
"refs/heads/");
+   if (run_hook_le(NULL, "update-branch", head,
+   sha1_to_hex(our->old_sha1), 
sha1_to_hex(our->old_sha1), NULL))
+   return 1;
update_ref(msg, "HEAD", our->old_sha1, NULL, 0, 
DIE_ON_ERR);
install_branch_config(0, head, option_origin, 
our->name);
}
@@ -606,6 +609,7 @@ static void update_head(const struct ref *our, const struct 
ref *remote,
update_ref(msg, "HEAD", remote->old_sha1,
   NULL, REF_NODEREF, DIE_ON_ERR);
}
+   return 0;
 }
 
 static int checkout(void)
@@ -987,7 +991,7 @@ int cmd_clone(

Re: Trying to setup Visual Studio 2013 with a CentOS 6.5 x64 git server

2014-04-23 Thread Storm-Olsen, Marius
On 4/23/2014 2:04 PM, Charles Buege wrote:
> If, in actuality, I can use a CentOS git server with Visual Studio
> 2013, can anyone point me in the direction of an FAQ/directions/
> YouTube video/book/anything for how to setup something like this?

http://lmgtfy.com/?q=%22visual+studio+2013%22+clone+git+site%3Ayoutube.com&l=1

will take you to a thorough review of VS2013 with Git local/remote, 
command-line tooling, and some standard Q&A. Description contains link 
to slides and notes.

Good luck!

-- 
.marius

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


Re: How to trim the fat on my log graphs

2014-04-23 Thread Robert Dailey
On Wed, Apr 23, 2014 at 12:30 PM, Junio C Hamano  wrote:
> Robert Dailey  writes:
>
> [Administrivia: because people read from top to bottom / why is it
> bad to top-post? / please do not top-post.]
>
>> On Tue, Apr 22, 2014 at 4:37 PM, Junio C Hamano  wrote:
>>> Robert Dailey  writes:
>>>
 git log log --graph --abbrev-commit --decorate --date=relative
 --format=format:'%C(bold blue)%h%C(reset)%x09%C(bold
 green)(%ar)%C(reset)%C(bold yellow)%d%C(reset) %C(dim
 white)%an%C(reset) - %C(white)%s%C(reset)' --branches --remotes
 ...
 The goal is to weed out the really distant and irrelevant commits. I
 really just want to see the commits that are closely related to my
 current branch. Thanks in advance.
>>>
>>> For a starter, how about dropping "--branches --remotes" from that
>>> command line?  A merge from elsewhere will show as "Merge branch foo"
>>> which should be sufficient without the decoration.
>>
>> Thanks, removing those two options did help quite a bit already.
>> However, the history can still get pretty crazy. Is there a way to
>> hide all tags from the log graph? Really I just want the LABELS to be
>> hidden.
>
> As you had --decorate and a lot of %C(cruft), I was assuming that
> you do want to see all the bells and whistles when I suggested to
> omit --branches and --remotes.
>
> If you do not want --decorate, I think you can omit that from the
> command line without changing what the output means.

I referred back to the documentation for --decorate:

--decorate[=short|full|no]
Print out the ref names of any commits that are shown. If short is
specified, the ref name prefixesrefs/heads/, refs/tags/ and
refs/remotes/ will not be printed. If full is specified, the full ref
name (including prefix) will be printed. The default option is short.

since default is short, and its documented to exclude tag names, I
would expect them to not be there. I also tried removing it to test
but i saw no difference; tag names are still visible.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] git tag --contains : avoid stack overflow

2014-04-23 Thread Stepan Kasal
Hi,

On Wed, Apr 23, 2014 at 12:12:14PM -0700, Junio C Hamano wrote:
> [Administrivia: please refrain from using Mail-Followup-To to
> deflect an attempt to directly respond to you;

thanks a lot for telling me.
Actually, this was a mistake: I added git to the list of discussion
lists, without realizing the consequences.
I'm glad to have separate copies of "my threads" that do not fall
to the git-list folder.

> > +>expect
> Move this inside test_expect_success?

Of course.  Had this in mind, then forgot.

> So this runs a separate "bash", [...]
> run "git tag" in the original shell?
> 
> Ahh, no, I am mis-pairing the quotes.

Point taken.  I admire how nicely you explained that!

>   run_with_limited_stack () {
>   "$SHELL_PATH" -c "ulimit -s 64 && $*"
>   }

Elegant. But I agree with Peff that we shall run "a bash" instead.

I'll mail an updated patch tomorrow.

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


Re: [RFC/PATCH v2 1/3] sh-setup: export GIT_DIR

2014-04-23 Thread Jeff King
On Wed, Apr 23, 2014 at 02:42:38PM -0500, Felipe Contreras wrote:

> It is what the clients of this library expect.

Is it? Passing GIT_DIR to sub-invocations of git will change how they
determine the repo and working tree. Your patch seems to cause failures
all over the test suite.

Without looking too hard, I'd guess the problems are one of:

  1. Setting GIT_DIR fixes the repo directory for all sub-invocations. A
 script that does "cd some-other-repo.git && git ...". You'd need to
 teach calling scripts to unset GIT_DIR when trying to move to
 another repo.

  2. If GIT_DIR is set but GIT_WORK_TREE is not, then GIT_WORK_TREE will
 default to ".". It might be sufficient to set GIT_WORK_TREE when
 you are setting GIT_DIR here. But as I said, I didn't look too
 hard, so there might be other complications.

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


Re: [PATCH 1/3] fetch.c: clear errno before calling functions that might set it

2014-04-23 Thread Eric Sunshine
On Tue, Apr 22, 2014 at 2:45 PM, Ronnie Sahlberg  wrote:
> In s_update_ref there are two calls that when they fail we return an error
> based on the errno value. In particular we want to return a specific error
> if ENOTDIR happened. Both these functions do have failure modes where they
> may return an error without updating errno, in which case a previous and
> unrelated ENOTDIT may cause us to return the wrong error. Clear errno before
> calling any functions if we check errno afterwards.
>
> Also skip initializing a static variable to 0. Sstatics live in .bss and

s/Sstatics/Statics/

> are all automatically initialized to 0.
>
> Signed-off-by: Ronnie Sahlberg 
> ---
>  builtin/fetch.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 55f457c..a93c893 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -44,7 +44,7 @@ static struct transport *gtransport;
>  static struct transport *gsecondary;
>  static const char *submodule_prefix = "";
>  static const char *recurse_submodules_default;
> -static int shown_url = 0;
> +static int shown_url;
>
>  static int option_parse_recurse_submodules(const struct option *opt,
>const char *arg, int unset)
> @@ -382,6 +382,8 @@ static int s_update_ref(const char *action,
> if (!rla)
> rla = default_rla.buf;
> snprintf(msg, sizeof(msg), "%s: %s", rla, action);
> +
> +   errno = 0;
> lock = lock_any_ref_for_update(ref->name,
>check_old ? ref->old_sha1 : NULL,
>0, NULL);
> --
> 1.9.1.518.g16976cb.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] fetch.c: change s_update_ref to use a ref transaction

2014-04-23 Thread Eric Sunshine
On Tue, Apr 22, 2014 at 2:45 PM, Ronnie Sahlberg  wrote:
> Change s_update_ref to use a ref transaction for the ref update.
> Signed-off-by: Ronnie Sahlberg 
>
> Signed-off-by: Ronnie Sahlberg 

Doubled sign-off.

> ---
>  builtin/fetch.c | 15 +++
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index a93c893..5c15584 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -375,7 +375,7 @@ static int s_update_ref(const char *action,
>  {
> char msg[1024];
> char *rla = getenv("GIT_REFLOG_ACTION");
> -   static struct ref_lock *lock;
> +   struct ref_transaction *transaction;
>
> if (dry_run)
> return 0;
> @@ -384,15 +384,14 @@ static int s_update_ref(const char *action,
> snprintf(msg, sizeof(msg), "%s: %s", rla, action);
>
> errno = 0;
> -   lock = lock_any_ref_for_update(ref->name,
> -  check_old ? ref->old_sha1 : NULL,
> -  0, NULL);
> -   if (!lock)
> -   return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
> - STORE_REF_ERROR_OTHER;
> -   if (write_ref_sha1(lock, ref->new_sha1, msg) < 0)
> +   transaction = ref_transaction_begin();
> +   if (!transaction ||
> +   ref_transaction_update(transaction, ref->name, ref->new_sha1,
> +  ref->old_sha1, 0, check_old) ||
> +   ref_transaction_commit(transaction, msg, 
> UPDATE_REFS_QUIET_ON_ERR))
> return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
>   STORE_REF_ERROR_OTHER;
> +
> return 0;
>  }
>
> --
> 1.9.1.518.g16976cb.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/3] remote-helpers: move out of contrib

2014-04-23 Thread Junio C Hamano
Max Horn  writes:

[Administrivia: please wrap your lines to reasonable length like 70-75].

> On 21.04.2014, at 22:37, Felipe Contreras 
> wrote:
>
>> The remote-helpers in contrib/remote-helpers have proved to work, be
>> reliable, and stable. It's time to move them out of contrib, and be
>> distributed by default.
>
> Really? While I agree that git-remote-hg by now works quite well for
> basic usage in simple situation, there are still unresolved bugs and
> fundamental issues with it.

As you mentioned later in your message, I agree that remaining bugs
are expected in an initial release.  I found that the above phrase
is exaggerating, but I think you are over-reacting [*1*].

> E.g. I recently showed you a reproducible use case involving
> git-remote-hg that puts the helper into a broken state from which it
> is difficult for a normal user to recover. Namely when ...
> ... prompting git fast-import to crash and trash the marks
> file. Afterwards, attempts to push or pull from the remote hg
> repository are answered with an error.
> There are other ways to trigger variants of this,...

Isn't the recent fc/transport-helper-sync-error-fix a reasonable
workaround for this issue?  The split-head in Hg fundamentally
cannot be expressed as a single ref on the Git side, and the series
attempts not to trash the marks file when the importer quits
abnormally for whatever reason to avoid rendering the resulting
repository unusable for future operations, which I thought was the
best you could do.

> It may be hard to deal with some of them, and admittedly I wouldn't
> necessarily expect that all of these are handled from the outset,
> i.e. "in version 1.0". But I think at the very least, users should be
> warned about these things.
>
> More broadly speaking, there is currently no documentation at all in
> git.git for those remote helpers, which I find worrisome.

I think your points regarding certain Hg features that do not map
well to Git data model are good ones; it would be good to have them
at least documented.


[Footnote]

*1* Personally I think it would have been better if it stopped at
somewhere around "some people in the field are using and reporting
success, let's give it wider exposure", without using words like
"proven", "reliable", or "stable" to make it sound like some
objective goodness has already been achieved.  People will call the
result of the project's work as "proven to be reliable and stable"
if it is so; the project does not have to claim and advertise its
ware by using such words---the code will prove itself given time,
and it is better to let others use these words, not ourselves.




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


Re: How to trim the fat on my log graphs

2014-04-23 Thread Junio C Hamano
Robert Dailey  writes:

> On Wed, Apr 23, 2014 at 12:30 PM, Junio C Hamano  wrote:
>> Robert Dailey  writes:
>>
>> [Administrivia: because people read from top to bottom / why is it
>> bad to top-post? / please do not top-post.]
>>
>>> On Tue, Apr 22, 2014 at 4:37 PM, Junio C Hamano  wrote:
 Robert Dailey  writes:

> git log log --graph --abbrev-commit --decorate --date=relative
> --format=format:'%C(bold blue)%h%C(reset)%x09%C(bold
> green)(%ar)%C(reset)%C(bold yellow)%d%C(reset) %C(dim
> white)%an%C(reset) - %C(white)%s%C(reset)' --branches --remotes
> ...
> ... I also tried removing it to test
> but i saw no difference; tag names are still visible.

I do not use these eye-candies myself (they are too distracting for
me), but doesn't "%d" in the format string mean "decorate"?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH v2 1/3] sh-setup: export GIT_DIR

2014-04-23 Thread Junio C Hamano
Jeff King  writes:

> On Wed, Apr 23, 2014 at 02:42:38PM -0500, Felipe Contreras wrote:
>
>> It is what the clients of this library expect.
>
> Is it? Passing GIT_DIR to sub-invocations of git will change how they
> determine the repo and working tree. Your patch seems to cause failures
> all over the test suite.
>
> Without looking too hard, I'd guess the problems are one of:
>
>   1. Setting GIT_DIR fixes the repo directory for all sub-invocations. A
>  script that does "cd some-other-repo.git && git ...". You'd need to
>  teach calling scripts to unset GIT_DIR when trying to move to
>  another repo.
>
>   2. If GIT_DIR is set but GIT_WORK_TREE is not, then GIT_WORK_TREE will
>  default to ".". It might be sufficient to set GIT_WORK_TREE when
>  you are setting GIT_DIR here. But as I said, I didn't look too
>  hard, so there might be other complications.
>
> -Peff

I do not recall the details but I do remember that it is very
deliberate not to export GIT_DIR but only set in git-sh-setup.

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


Re: [PATCH 3/3] fetch.c: use a single ref transaction for all ref updates

2014-04-23 Thread Eric Sunshine
On Tue, Apr 22, 2014 at 2:45 PM, Ronnie Sahlberg  wrote:
> Change store_updated_refs to use a single ref transaction for all refs that
> are updated during the fetch. This makes the fetch more atomic when update
> failures occur.
>
> Since ref update failures will now no longer occur in the code path for
> updating a single ref in s_update_ref, we no longer have as detailed error
> message logging the exact reference and the ref log action as in the old code.
> Instead since we fail the entire transaction we log a much more generic
> message. But since we commit the transaction using MSG_ON_ERR we will log
> an error containing the ref name if either locking of writing the ref would
> so the regression in the log message is minor.
>
> This will also change the order in which errors are checked for and logged
> which may alter which error will be logged if there are multiple errors
> occuring during a fetch.
>
> For example, assuming we have a fetch for two refs that both would fail.

s/assuming/assume/ perhaps?

> Where the first ref would fail with ENOTDIR due to a directory in the ref
> path not existing, and the second ref in the fetch would fail due to
> the check in update_logical_ref():
> if (current_branch &&
> !strcmp(ref->name, current_branch->name) &&
> !(update_head_ok || is_bare_repository()) &&
> !is_null_sha1(ref->old_sha1)) {
> /*
>  * If this is the head, and it's not okay to update
>  * the head, and the old value of the head isn't empty...
>  */
>
> In the old code sicne we would update the refs one ref at a time we would

s/sicne/since/

> first fail the ENOTDIR and then fail the second update of HEAD as well.
> But since the first ref failed with ENOTDIR we would eventually fail the whole
> fetch with STORE_REF_ERROR_DF_CONFLICT
>
> In the new code, since we defer committing the transaction until all refs
> has been processed, we would now detect that the second ref was bad and

s/has/have/

> rollback the transaction before we would even try start writing the update to
> disk and thus we would not return STORE_REF_ERROR_DF_CONFLICT for this case.
>
> I think this new behaviour is more correct, since if there was a problem
> we would not even try to commit the transaction but need to highlight this
> change in how/what errors are reported.
> This change in what error is returned only occurs if there are multiple
> refs that fail to update and only some, but not all, of them fail due to
> ENOTDIR.
>
> Signed-off-by: Ronnie Sahlberg 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 1/2] add: add --ignore-submodules[=] parameter

2014-04-23 Thread Eric Sunshine
On Tue, Apr 22, 2014 at 5:12 PM, Ronald Weiss  wrote:
> Allow ignoring submodules (or not) by command line switch, like diff
> and status do.
>
> This commit is also a prerequisite for the next one in series, which
> adds the --ignore-submodules switch to git commit. That's why a new
> argument is added to public function add_files_to_cache(), and it's

s/it's/its/

> call sites are updated to pass NULL.
>
> Signed-off-by: Ronald Weiss 
> ---
> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
> index 9631526..b2c936f 100644
> --- a/Documentation/git-add.txt
> +++ b/Documentation/git-add.txt
> @@ -11,7 +11,7 @@ SYNOPSIS
>  'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
>   [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
>   [--intent-to-add | -N] [--refresh] [--ignore-errors] 
> [--ignore-missing]
> - [--] [...]
> + [--ignore-submodules[=]] [--] [...]
>
>  DESCRIPTION
>  ---
> @@ -164,6 +164,11 @@ for "git add --no-all ...", i.e. ignored 
> removed files.
> be ignored, no matter if they are already present in the work
> tree or not.
>
> +--ignore-submodules[=]::
> +   Can be used to override any settings of the 'submodule.*.ignore'
> +   option in linkgit:git-config[1] or linkgit:gitmodules[5].
> +can be either "none" or "all", which is the default.

The "which is the default" clause reads ambiguously. It's not clear
whether you mean "none" is the default or "all" is the default.

>  \--::
> This option can be used to separate command-line options from
> the list of files, (useful when filenames might be mistaken
> diff --git a/t/t3704-add-ignore-submodules.sh 
> b/t/t3704-add-ignore-submodules.sh
> new file mode 100755
> index 000..db58f0c
> --- /dev/null
> +++ b/t/t3704-add-ignore-submodules.sh
> @@ -0,0 +1,45 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2014 Ronald Weiss
> +#
> +
> +test_description='Test of git add with ignoring submodules'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'create dirty submodule' '
> +   test_create_repo sm && (

It's conventional for the opening '(' to be placed on a line by itself.

> +   cd sm &&
> +   >foo &&
> +   git add foo &&
> +   git commit -m "Add foo"
> +   ) &&
> +   git submodule add ./sm &&
> +   git commit -m "Add sm" && (
> +   cd sm &&
> +   echo bar >> foo &&

s/>> />>/

> +   git add foo &&
> +   git commit -m "Update foo"
> +   )
> +'
> +
> +test_expect_success 'add --ignore-submodules ignores submodule' '
> +   git reset &&
> +   git add -u --ignore-submodules &&
> +   git diff --cached --exit-code --ignore-submodules=none
> +'
> +
> +test_expect_success 'add --ignore-submodules=all ignores submodule' '
> +   git reset &&
> +   git add -u --ignore-submodules=all &&
> +   git diff --cached --exit-code --ignore-submodules=none
> +'
> +
> +test_expect_success 'add --ignore-submodules=none overrides ignore=all from 
> config' '
> +   git reset &&
> +   git config submodule.sm.ignore all &&
> +   git add -u --ignore-submodules=none &&
> +   test_must_fail git diff --cached --exit-code --ignore-submodules=none
> +'
> +
> +test_done
> --
> 1.9.1.3.g7790cde.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: [RFC/PATCH v2 3/3] Add 'update-branch' hook

2014-04-23 Thread Junio C Hamano
Felipe Contreras  writes:

> This hook is invoked before a branch is updated, either when a branch is
> created or updated with 'git branch', or when it's rebased with 'git
> rebase'.  It receives three parameters; the name of the branch, the
> SHA-1 of the latest commit, and the SHA-1 of the first commit of the
> branch.
>
> When a branch is created the first and last commits of the branch are
> the same, however when a branch is rebased they are not. If the SHA-1 of
> the first commit of the branch is not available (e.g. git reset) a null
> SHA-1 (40 zeroes) would be passed.
>
> The hook exit status can be used to deny the branch update.
>
> It can be used to verify the validity of branch names, and also to keep
> track of the origin point of a branch, which is otherwise not possible
> to find out [1].

Please call it pre-update-branch at least, unless you want to make
sure that time spent on others in the discussion thread for the
previous round becomes wasted.

> +update-branch
> +~
> +
> +This hook is invoked before a branch is updated, either when a branch is
> +created or updated with 'git branch', or when it's rebased with 'git rebase'.

Does "git checkout -B aBranch" count?

Does "git reset $there" count?

I guess "git commit" or "git merge" on a branch to advance its tip
in a usual way would not count (and I can think of good reasons why
they should not count), but it is only a weak "guess".  The above
two lines does not give readers enough hint to determine if "branch
and rebase will be the only two and no other command will ever
trigger" or "branch and rebase are only examples---for others, guess
on your own" (and if the latter, enough clue to use in guessing).
I cannot guess if "git fetch $there $that:refs/heads/master" should
trigger the hook, for example.

To put it another way.

How does one, who is adding a new command that causes a branch tip
to be updated, to decide if it should trigger this hook?  What are
the common things among commands that can update branch tips that
this hook gets called that are missing from commands that update
branch tips that this hook does not get called?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH v2 2/3] run-command: make sure hooks have always GIT_DIR

2014-04-23 Thread Junio C Hamano
Felipe Contreras  writes:

> Signed-off-by: Felipe Contreras 

Why is this a good change?  From a zero-line log message, I cannot
even tell if this is trying to fix some problem, or trying to give
new capabilities to hooks.

How does it prevent existing hook scripts from suddenly start
misbehaving, where they do *not* expect to see an explicit GIT_DIR
pointing at the original repository hook gets run in exported into
their environment?  For example, one of my post-receive hooks in a
repository I push into records $cwd (which is the GIT_DIR of
receiving repository), chdir's to another repository and then
executes "git pull $cwd" from there, and that relies on the fact
that being at the top-level of that other repository without GIT_DIR
environment pointing at elsewhere but having .git directory in that
top-level repository is sufficient to kick the auto-discovery of the
repository that receives the "pull" in order to work correctly.



> ---
>  run-command.c | 24 ++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/run-command.c b/run-command.c
> index 75abc47..8e188f6 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -765,12 +765,29 @@ int run_hook_ve(const char *const *env, const char 
> *name, va_list args)
>   struct child_process hook;
>   struct argv_array argv = ARGV_ARRAY_INIT;
>   const char *p;
> - int ret;
> + const char **mod_env = NULL;
> + int ret, i = 0;
> + struct strbuf buf = STRBUF_INIT;
>  
>   p = find_hook(name);
>   if (!p)
>   return 0;
>  
> + if (!getenv(GIT_DIR_ENVIRONMENT)) {
> + if (env)
> + for (i = 0; env[i]; i++);
> +
> + mod_env = xcalloc(i + 2, sizeof(*mod_env));
> +
> + if (env)
> + for (i = 0; env[i]; i++)
> + mod_env[i] = env[i];
> +
> + strbuf_addf(&buf, "GIT_DIR=%s", get_git_dir());
> + mod_env[i++] = buf.buf;
> + mod_env[i++] = NULL;
> + }
> +
>   argv_array_push(&argv, p);
>  
>   while ((p = va_arg(args, const char *)))
> @@ -778,12 +795,15 @@ int run_hook_ve(const char *const *env, const char 
> *name, va_list args)
>  
>   memset(&hook, 0, sizeof(hook));
>   hook.argv = argv.argv;
> - hook.env = env;
> + hook.env = mod_env ? mod_env : env;
>   hook.no_stdin = 1;
>   hook.stdout_to_stderr = 1;
>  
>   ret = run_command(&hook);
>   argv_array_clear(&argv);
> + strbuf_release(&buf);
> + free(mod_env);
> +
>   return ret;
>  }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: How to trim the fat on my log graphs

2014-04-23 Thread Jeff King
On Wed, Apr 23, 2014 at 02:59:26PM -0500, Robert Dailey wrote:

> I referred back to the documentation for --decorate:
> 
> --decorate[=short|full|no]
> Print out the ref names of any commits that are shown. If short is
> specified, the ref name prefixesrefs/heads/, refs/tags/ and
> refs/remotes/ will not be printed. If full is specified, the full ref
> name (including prefix) will be printed. The default option is short.
> 
> since default is short, and its documented to exclude tag names, I
> would expect them to not be there. I also tried removing it to test
> but i saw no difference; tag names are still visible.

Are you reading that as "if short is specified, then refs whose names
are prefixed with refs/heads, refs/tags, etc will be omitted entirely
from decoration"?

The intent is "if short is specified, then a ref whose names has the
prefix of refs/heads, refs/tags, etc, will have that prefix removed when
showing it".

IOW, the options are:

  $ git log -1 --no-decorate | head -1
  commit 0bc85abb7aa9b24b093253018801a0fb43d01122

  $ git log -1 --decorate=short | head -1
  commit 0bc85abb7aa9b24b093253018801a0fb43d01122 (HEAD, tag: v1.9.2, 
origin/maint)

  $ git log -1 --decorate=full | head -1
  commit 0bc85abb7aa9b24b093253018801a0fb43d01122 (HEAD, tag: refs/tags/v1.9.2, 
refs/remotes/origin/maint)

If that's the confusion, feel free to suggest better wording for the
documentation.

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


Re: [PATCH v2] git tag --contains : avoid stack overflow

2014-04-23 Thread Junio C Hamano
Jeff King  writes:

> On Wed, Apr 23, 2014 at 12:12:14PM -0700, Junio C Hamano wrote:
>
>> > +ulimit_stack="ulimit -s 64"
>> > +test_lazy_prereq ULIMIT 'bash -c "'"$ulimit_stack"'"'
>> 
>> With this implementaion, ULIMIT implies bash, and we use bash that
>> appears on user's PATH that may not be the one the user chose to run
>> git with.  Can't we fix both of them by using $SHELL_PATH?
>
> I don't think so. The point is that we _must_ use bash here, not any
> POSIX shell.

Sorry, but I do not understand.  Isn't what you want "any POSIX
shell with 'ulimit -s 64' supported"?

$ dash -c 'ulimit -s && ulimit -s 64 && ulimit -s'
8192
64

> We want to run "some bash" if we can. We may pick a bash on the user's
> PATH that is not what they put into $SHELL_PATH, but that should be
> relatively rare. And the consequence is that either that bash works fine
> and we run the test, or it does not, and we skip the test.
>
>> How about doing it along this line instead?
>> 
>>  run_with_limited_stack () {
>>  "$SHELL_PATH" -c "ulimit -s 64 && $*"
>>  }
>> 
>>  test_lazy_prereq ULIMIT "run_with_limited_stack true"
>
> That's a much more direct test. I like it (aside from the $SHELL_PATH
> thing as described above).

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


Re: [PATCH v2] git tag --contains : avoid stack overflow

2014-04-23 Thread Jeff King
On Wed, Apr 23, 2014 at 01:48:05PM -0700, Junio C Hamano wrote:

> > I don't think so. The point is that we _must_ use bash here, not any
> > POSIX shell.
> 
> Sorry, but I do not understand.  Isn't what you want "any POSIX
> shell with 'ulimit -s 64' supported"?

Sure, that would be fine, but the original patch which started this
thread claimed that bash was required. I had assumed that to be true,
but it seems like it's not:

> $ dash -c 'ulimit -s && ulimit -s 64 && ulimit -s'
> 8192
> 64

If we are just using the same shell we are already running, then why
invoke it by name in the first place? IOW, why not:

  run_with_limited_stack () {
(ulimit -s 64 && "$@")
  }

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


Re: [PATCH v2 2/3] remote-helpers: move out of contrib

2014-04-23 Thread Felipe Contreras
Max Horn wrote:
> On 21.04.2014, at 22:37, Felipe Contreras  wrote:
> 
> > The remote-helpers in contrib/remote-helpers have proved to work, be
> > reliable, and stable. It's time to move them out of contrib, and be
> > distributed by default.
> 
> Really? While I agree that git-remote-hg by now works quite well for basic
> usage in simple situation, there are still unresolved bugs and fundamental
> issues with it.

s/basic usage in simple situation/complex usage in the vast majority of 
situations/

> E.g. I recently showed you a reproducible use case involving git-remote-hg
> that puts the helper into a broken state from which it is difficult for a
> normal user to recover. Namely when a hg branch has multiple heads, then
> git-remote-hg exports all of those to git, but only adds a git ref for one of
> them; after pruning unreferenced commits, the fast-import marks file
> references git commits that now are missing, prompting git fast-import to
> crash and trash the marks file. Afterwards, attempts to push or pull from the
> remote hg repository are answered with an error.

Yes, and how often does that happen? A normal user would only see this if a
branch remains with multiple heads in Mercurial for more than one month or so.

In practice that's very unlikely, and proof of that is that nobody has reported
such issues.

Either way, I just fixed it [1].

> There are more issues related to unresolved clashes between the git and hg
> ways of naming things. E.g. I am collaborating on a hg repository that has
> branches "foo" and "foo/bar" which git-remote-hg cannot handle because it
> translates them to git branch names, and, well, git cannot handle that.

I don't see this as a limitation of git-remote-hg, ideally Git remote-helpers
should have a standardized way to let users map external branch names.

> It may be hard to deal with some of them, and admittedly I wouldn't
> necessarily expect that all of these are handled from the outset, i.e. "in
> version 1.0". But I think at the very least, users should be warned about
> these things.
> 
> More broadly speaking, there is currently no documentation at all in git.git
> for those remote helpers, which I find worrisome.

Here is the documentation:
https://github.com/felipec/git/wiki/git-remote-hg
https://github.com/felipec/git/wiki/git-remote-hg

> That said, I don't know what the criteria are for moving something out of
> contrib. Perhaps it is OK to move an undocumented remote-helper with known
> bugs out of contrib.

There are no known bugs. This is the list of open bugs:

https://github.com/felipec/git/issues

Now if you want to label the limitation of Git that you can't have both 'foo'
and 'foo/bar' as a bug of git-remote-hg, that's up to you, but it's something
nobody had reported before, so it definitely can't be labeled as a "known bug".

[1] 
https://github.com/felipec/git/commit/fbaae8caa51804a655fd6bc5727763b64e3c2e9f

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


Re: [PATCH v2] git tag --contains : avoid stack overflow

2014-04-23 Thread Junio C Hamano
Jeff King  writes:

> On Wed, Apr 23, 2014 at 01:48:05PM -0700, Junio C Hamano wrote:
>
>> > I don't think so. The point is that we _must_ use bash here, not any
>> > POSIX shell.
>> 
>> Sorry, but I do not understand.  Isn't what you want "any POSIX
>> shell with 'ulimit -s 64' supported"?
>
> Sure, that would be fine, but the original patch which started this
> thread claimed that bash was required. I had assumed that to be true,
> but it seems like it's not:
>
>> $ dash -c 'ulimit -s && ulimit -s 64 && ulimit -s'
>> 8192
>> 64
>
> If we are just using the same shell we are already running, then why
> invoke it by name in the first place? IOW, why not:
>
>   run_with_limited_stack () {
>   (ulimit -s 64 && "$@")
>   }

That is certainly more preferrable than an explicit "run this piece
with $SHELL_PATH".

I think the choice between "Any bash that is on user's PATH" vs "The
shell the user told us to use when working with Git" is a trade-off
between

 - those who choose a shell that does not support "ulimit -s" to
   work with Git (which is fine, because our scripted Porcelains
   would not have any need for that); for these people, this test
   would be skipped unnecessarily if we insist on SHELL_PATH; and

 - those who run on a box without any bash on their PATH, chose a
   shell that is not bash but does support "ulimit -s" as their
   SHELL_PATH to build Git with; for these people, this test would
   be skipped unnecessarily if we insist on "bash".

and I do not think of a good reason to favor one over the other.

If I have to pick, I'd take your "don't name any shell, and let the
current one run it" approach, solely for the simplicity of the
solution (it ends up favoring the latter class of people as a
side-effect, though).

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


Re: [PATCH v2 2/3] remote-helpers: move out of contrib

2014-04-23 Thread Felipe Contreras
Junio C Hamano wrote:
> Max Horn  writes:
> 
> > Perhaps it is OK to move an undocumented remote-helper
> > with known bugs out of contrib.
> 
> We should strive to apply the same criteria as new submission to the
> main part of the system.  And inputs from people like you who have
> more experiences than others on the list in dealing with hg-to-git
> bridging are very much welcomed to identify and document what kind
> of breakages there are, and to come up with the solution to them.

FTR. There are no "known brakages", at least to me. Max Horn tends to hoard
these issues and never report them (unless an opportunity to criticize
git-remote-hg comes, apparently).

The very unlikely issue that nobody has reported about hg multiple heads and gc
I just fixed, and the issue he just reported about 'foo' and 'foo/bar' is newly
reported, and there's no easy way to fix this. My proposal would be to have
some sort of branch mapping mechanism in fast-import, but hardly something that
should prevent the move out of contrib.

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


Re: [PATCH v2] git tag --contains : avoid stack overflow

2014-04-23 Thread Johannes Schindelin
Hi Peff,

On Wed, 23 Apr 2014, Jeff King wrote:

> On Wed, Apr 23, 2014 at 09:53:25AM +0200, Stepan Kasal wrote:
> 
> > I have found out that "ulimit -s" does not work on Windows.  Adding
> > this as a prerequisite, we will skip the test there.
> 
> I found this bit weird, as the test originated on Windows. Did it never
> actually cause a failure there (i.e., the "ulimit -s" doesn't do
> anything)? Or does "ulimit" fail?

I must have forgotten to test on Windows. For performance reasons (you
know that I only have a Git time budget of about 15min/day), I developed
the test and the patch on Linux.

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


Re: How to trim the fat on my log graphs

2014-04-23 Thread Felipe Contreras
Junio C Hamano wrote:
> Robert Dailey  writes:
> 
> [Administrivia: because people read from top to bottom / why is it
> bad to top-post? / please do not top-post.]

https://en.wikipedia.org/wiki/Posting_style

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


Re: [RFC/PATCH v2 3/3] Add 'update-branch' hook

2014-04-23 Thread Felipe Contreras
Junio C Hamano wrote:
> Felipe Contreras  writes:
> 
> > This hook is invoked before a branch is updated, either when a branch is
> > created or updated with 'git branch', or when it's rebased with 'git
> > rebase'.  It receives three parameters; the name of the branch, the
> > SHA-1 of the latest commit, and the SHA-1 of the first commit of the
> > branch.
> >
> > When a branch is created the first and last commits of the branch are
> > the same, however when a branch is rebased they are not. If the SHA-1 of
> > the first commit of the branch is not available (e.g. git reset) a null
> > SHA-1 (40 zeroes) would be passed.
> >
> > The hook exit status can be used to deny the branch update.
> >
> > It can be used to verify the validity of branch names, and also to keep
> > track of the origin point of a branch, which is otherwise not possible
> > to find out [1].
> 
> Please call it pre-update-branch at least,

I will do so when I see a good argument for it.

> unless you want to make sure that time spent on others in the discussion
> thread for the previous round becomes wasted.

Unless you want to make sure the time *I* spent in the discussion is wasted.

I'm still waiting for replies to my last arguments.

> > +update-branch
> > +~
> > +
> > +This hook is invoked before a branch is updated, either when a branch is
> > +created or updated with 'git branch', or when it's rebased with 'git 
> > rebase'.
> 
> Does "git checkout -B aBranch" count?

Yes.

> Does "git reset $there" count?

Yes.

> I guess "git commit" or "git merge" on a branch to advance its tip
> in a usual way would not count (and I can think of good reasons why
> they should not count), but it is only a weak "guess".  The above
> two lines does not give readers enough hint to determine if "branch
> and rebase will be the only two and no other command will ever
> trigger" or "branch and rebase are only examples---for others, guess
> on your own" (and if the latter, enough clue to use in guessing).
> I cannot guess if "git fetch $there $that:refs/heads/master" should
> trigger the hook, for example.
> 
> To put it another way.
> 
> How does one, who is adding a new command that causes a branch tip
> to be updated, to decide if it should trigger this hook?  What are
> the common things among commands that can update branch tips that
> this hook gets called that are missing from commands that update
> branch tips that this hook does not get called?

I guess the guideline should be: if the branch is clearly moving forward the
hook is not called, if the branch is manually changed in any way, it should.

This omits non-porcelain commands, such as update-ref. I don't think those
should trigger the hook.

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


Re: [RFC/PATCH v2 3/3] Add 'update-branch' hook

2014-04-23 Thread Junio C Hamano
Felipe Contreras  writes:

> Junio C Hamano wrote:
>> Felipe Contreras  writes:
>> 
>> > This hook is invoked before a branch is updated, either when a branch is
>> > created or updated with 'git branch', or when it's rebased with 'git
>> > rebase'.  It receives three parameters; the name of the branch, the
>> > SHA-1 of the latest commit, and the SHA-1 of the first commit of the
>> > branch.
>> >
>> > When a branch is created the first and last commits of the branch are
>> > the same, however when a branch is rebased they are not. If the SHA-1 of
>> > the first commit of the branch is not available (e.g. git reset) a null
>> > SHA-1 (40 zeroes) would be passed.
>> >
>> > The hook exit status can be used to deny the branch update.
>> >
>> > It can be used to verify the validity of branch names, and also to keep
>> > track of the origin point of a branch, which is otherwise not possible
>> > to find out [1].
>> 
>> Please call it pre-update-branch at least,
>
> I will do so when I see a good argument for it.

If you choose to ignore "a user cannot tell from the name
update-branch when it will be called, we cannot introduce
post-update-branch later without making things more inconsistent if
we do not name it pre-something" and label them not "a good
argument", then I do not have anything to say to you.

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


Re: [RFC/PATCH v2 2/3] run-command: make sure hooks have always GIT_DIR

2014-04-23 Thread Felipe Contreras
Junio C Hamano wrote:
> Felipe Contreras  writes:
> 
> > Signed-off-by: Felipe Contreras 
> 
> Why is this a good change?

When a hook is called from a command without NEED_WORK_TREE, GIT_DIR is not set
(e.g. git branch).

> How does it prevent existing hook scripts from suddenly start
> misbehaving, where they do *not* expect to see an explicit GIT_DIR
> pointing at the original repository hook gets run in exported into
> their environment?

Fine, I'll use "${GIT_DIR-.git}" in my hook tests.

> For example, one of my post-receive hooks in a repository I push into records
> $cwd (which is the GIT_DIR of receiving repository), chdir's to another
> repository and then executes "git pull $cwd" from there, and that relies on
> the fact that being at the top-level of that other repository without GIT_DIR
> environment pointing at elsewhere but having .git directory in that top-level
> repository is sufficient to kick the auto-discovery of the repository that
> receives the "pull" in order to work correctly.

Let's hope post-receive is never called from a command that has NEED_WORK_TREE 
then.

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


Re: [PATCH v2 2/3] remote-helpers: move out of contrib

2014-04-23 Thread Junio C Hamano
Felipe Contreras  writes:

> The very unlikely issue that nobody has reported about hg multiple heads and 
> gc
> I just fixed, and the issue he just reported about 'foo' and 'foo/bar' is 
> newly
> reported, and there's no easy way to fix this.

I would not judge on likelyhood, but I would say that 'foo' vs
'foo/bar' is something that falls into the same category as "if you
want your project to be cross-platform, don't have paths Foo and foo
both tracked, as some people have to work on case insensitive
systems".

In other words, I think it is perfectly fine as long as the users
are made aware of these issues.  One way to deal with such a project
may be to allow users to map 'foo' to 'foo_' to make room for
'foo/bar', but lack of such a feature is not grave enough to block
it from being used---it would be unreasonable to demand the ref
mapping to be implemented before the command is given to the end
users.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-rebase: fix probable reflog typo

2014-04-23 Thread Felipe Contreras
Matthieu Moy wrote:
> Felipe Contreras  writes:
> 
> > Commit 26cd160 (rebase -i: use a better reflog message) tried to produce
> > a better reflog message, however, it seems a statement was introduced by
> > mistake.
> >
> > 'comment_for_reflog start' basically overides the GIT_REFLOG_ACTION we
> > just set.

> So, one needs to reset $GIT_REFLOG_ACTION to what it used to be if is it
> to be used later. However, it seems to me that the "comment_for_reflog
> start" is used only for this checkout command. If so, there's no need
> for the "comment_for_reflog start" before the if statement either.

Exactly. But if this variable is only meant for this command, it should be
`VAR=VAL command`, that would make it clear without the need of a comment.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "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: [RTC/PATCH] Add 'update-branch' hook

2014-04-23 Thread Felipe Contreras
Junio C Hamano wrote:
> Felipe Contreras  writes:
> 
> > ... there are _already_ hooks without pre/post.
> 
> Like commit-msg?  Yes, it would have been nicer if it were named
> verify-commit-message or something.

No it wouldn't. I can use the commit-msg hook to change the commit message and
to absolutely no verification, so verify-commit-message would be misleading.

Maybe you would like modify-and-or-verify-commit-message which would be
correct, but I wouldn't, I like short-and-sweet, and commit-msg is just that.

> Old mistakes are harder to change because of inertia.  It is not a
> good excuse to knowingly make a new mistake to add new exceptions
> that the users need to check documentations for, is it?

That's a nifty trick; label something a mistake, and then it suddenly becomes
one.

No, it's not a mistake, first it has to be proven to be mistake and I haven't
seen any arguments that try to do so.

Besides it's a red herring, you said such a name would be original and I've
just proved that it's not original, so the originality is not a concern.

> > And it's not confusing,
> 
> A simple fact that Ilya asked the question tells us otherwise ;-)

It's not any more confusing than these:

applypatch-msg:

When does this happen? Can I return an error?

pre-applypatch:

Again when does it happen? What does the input contains? The whole patch? 
Including the message?

post-applypatch:

Totally confused.

pre-commit:
prepare-commit-msg:
commit-msg:

What is the difference between these? Doesn't pre-commit contains the message 
already?

pre-receive:

Before receiving what?

update:

Updating what? When is it called? Can I cancel something?

The fact that somebody asked a question doesn't make a name confusing.

> I personally do not see an immediate need for post-update-branch,
> but if the new hook is about intervening an operation,

It's not about that, I can remove that feature if it would make you happier.

> Otherwise it would be impossible to later add "post-update-branch"

Which is never going to happen.

I'm still waiting for anybody to imagine any reason why we might want
post-udpate-branch.

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


Re: [RFC/PATCH v2 3/3] Add 'update-branch' hook

2014-04-23 Thread Felipe Contreras
Junio C Hamano wrote:
> Felipe Contreras  writes:
> 
> > Junio C Hamano wrote:
> >> Felipe Contreras  writes:
> >> 
> >> > This hook is invoked before a branch is updated, either when a branch is
> >> > created or updated with 'git branch', or when it's rebased with 'git
> >> > rebase'.  It receives three parameters; the name of the branch, the
> >> > SHA-1 of the latest commit, and the SHA-1 of the first commit of the
> >> > branch.
> >> >
> >> > When a branch is created the first and last commits of the branch are
> >> > the same, however when a branch is rebased they are not. If the SHA-1 of
> >> > the first commit of the branch is not available (e.g. git reset) a null
> >> > SHA-1 (40 zeroes) would be passed.
> >> >
> >> > The hook exit status can be used to deny the branch update.
> >> >
> >> > It can be used to verify the validity of branch names, and also to keep
> >> > track of the origin point of a branch, which is otherwise not possible
> >> > to find out [1].
> >> 
> >> Please call it pre-update-branch at least,
> >
> > I will do so when I see a good argument for it.
> 
> If you choose to ignore "a user cannot tell from the name
> update-branch when it will be called, we cannot introduce
> post-update-branch later without making things more inconsistent if
> we do not name it pre-something" and label them not "a good
> argument", then I do not have anything to say to you.

I did not ignore it, it's an invalid argument, and I explained why.

You are the one making the assumption that there will be a post-udpate-branch
without actually providing any reasoning *why* we would want such a hook.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "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 4/6] patch-id: make it stable against hunk reordering

2014-04-23 Thread Junio C Hamano
"Michael S. Tsirkin"  writes:

> On Wed, Apr 23, 2014 at 10:39:23AM -0700, Junio C Hamano wrote:
>> Are these three patches the same as what has been queued on
>> mt/patch-id-stable topic and cooking in 'next' for a few weeks?
>
> Not exactly - at your request I implemented git config
> options to control patch id behaviour.
> Documentation and tests updated to match.

After comparing the patches 4-6 and the one that has been in 'next'
for a few weeks, I tried to like the new one, but I couldn't.

The new one, if I am reading the patch correctly, makes the stable
variant the default if

 - the configuration explicitly asks to use it; or

 - diff.orderfile, which is a new configuration that did not exist,
   is used.

At the first glance, the latter makes it look as if that will not
hurt any existing users/repositories, but the thing is, the producer
of the patches is different from the consumer of the patches.  There
needs to be a way for a consumer to say "I need stable variant" on
the patches when computing "patch-id" on a patch that arrived.  As
long as two different producers use two different orders, the
consumer of the patches from these two sources is forced to use the
stable variant if some sort of cache is kept keyed with the
patch-ids.

But "diff.orderfile" configuration is all and only about the
producer, and does not help the case at all.

Compared to it, the previous version forced people who do not have a
particular reason to choose between the variants to use the new
"stable" variant, which was a lot simpler solution.

The reason why I merged the previous one to 'next' was because I
wanted to see if the behaviour change is as grave as I thought, or I
was just being unnecessary cautious.  If there is no third-party
script that precomputes something and stores them under these hashes
that breaks with this change, I do not see any reason not to make
the new "stable" one the default.

I however suspect that the ideal "stable" implementation may be
different from what you implemented.  What if we compute a patch-id
on a reordered patch as if the files came in the "usual" order?
That would be another way to achieve the stable-ness for sane cases
(i.e. no funny "you could split one patch with two hunks into two
patches with one hunk each twice mentioning the same path" which is
totally an uninteresting use case---diff.orderfile would not even
produce such a patch) and a huge difference would be that it would
produce the same patch-id as existing versions of Git, if the patch
is not reordered.  Is that asking for a moon (I admit that I haven't
looked at the code involved too deeply)?

--
To unsubscribe from this list: send the line "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: [RTC/PATCH] Add 'update-branch' hook

2014-04-23 Thread Junio C Hamano
Felipe Contreras  writes:

> Junio C Hamano wrote:
>> Felipe Contreras  writes:
>> 
>> > ... there are _already_ hooks without pre/post.
>> 
>> Like commit-msg?  Yes, it would have been nicer if it were named
>> verify-commit-message or something.
>
> No it wouldn't. I can use the commit-msg hook to change the commit message and
> to absolutely no verification, so verify-commit-message would be misleading.

You are confused (and please do not spread the confusion).  If you
read the first paragraph of the documentation on the hook and think
for 5 seconds why "--no-verify" countermands it, you would realize
that the hook is primarily meant for verification.  We also allow
the hook to edit the message, but that is not even "a useful feature
added as an afterthought"; the documentation mentions it because the
implementation did not bother to make sure the hook did not touch
the message file.

It was a mistake not to call it with a clear name that tells
verification happens there.

>> Old mistakes are harder to change because of inertia.  It is not a
>> good excuse to knowingly make a new mistake to add new exceptions
>> that the users need to check documentations for, is it?

I see no reason to waste more time on this point.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/3] remote-helpers: move out of contrib

2014-04-23 Thread Max Horn

On 23.04.2014, at 22:54, Felipe Contreras  wrote:

> Max Horn wrote:
>> On 21.04.2014, at 22:37, Felipe Contreras  wrote:
>> 
>>> The remote-helpers in contrib/remote-helpers have proved to work, be
>>> reliable, and stable. It's time to move them out of contrib, and be
>>> distributed by default.
>> 
>> Really? While I agree that git-remote-hg by now works quite well for basic
>> usage in simple situation, there are still unresolved bugs and fundamental
>> issues with it.
> 
> s/basic usage in simple situation/complex usage in the vast majority of 
> situations/

Yeah, hm, no. We can agree to disagree, I guess. It might also depend on
what you call "basic" or "complex" usage...

For example, whenever I need to
- close a branch
- fix a branch with multiple heads
- deal with phases
I need to switch to hg. I am pretty sure there are more things that make
that necessary, but luckily they don't happen to me.

What does work, though (and what I count as basic usage, although I'd say it
probably is enough for 95% of people out there) is making a clone of "safe"
repository (i.e. no "bad" branch names like 'foo' together with 'foo/bar'),
push and pull with it; and if you are careful, you can even get branch
attribution right. This is great, but in day-to-day usage, I still regularly
need to work with a hg clone for some tasks. But I am not really complaining
about that: For most of my regular "developer" work, I can stay in git, and
that makes me happy.

Out of curiosity: How do you yourself use git-remote-hg in your daily work?
Many people I encountered are happy enough with the ability to quickly
clone a hg repository, prepare a fix / feature branch and then submit
it back to upstream. For this, git-remote-hg *usually* is good enough.
But I am worried about people hitting the edge cases where it does not
quite work -- and then people are lost. This is what concerns me -- and
this concern would be alleviate if there was a list of known things that
do not work (and perhaps cannot work, at least for now, due to fundamental
differences between hg and git which need major work to bridge over).

Anyway, despite my criticism, I'd like to emphasis that I am actually quite
happy and grateful that your git-remote-hg exists and that you continue
improving it and the surrounding infrastructure. I just wish you could do it
while not acting like an asshole most of the time, but I'll survive that,
too *shrug*.



> 
>> E.g. I recently showed you a reproducible use case involving git-remote-hg
>> that puts the helper into a broken state from which it is difficult for a
>> normal user to recover. Namely when a hg branch has multiple heads, then
>> git-remote-hg exports all of those to git, but only adds a git ref for one of
>> them; after pruning unreferenced commits, the fast-import marks file
>> references git commits that now are missing, prompting git fast-import to
>> crash and trash the marks file. Afterwards, attempts to push or pull from the
>> remote hg repository are answered with an error.
> 
> Yes, and how often does that happen? A normal user would only see this if a
> branch remains with multiple heads in Mercurial for more than one month or so.

There are projects who do exactly that, although I believe most of them use
bookmarks, so the issue should indeed not affect those. Anyway, they do the
wrong thing ;-). Still, if you are forced with such a repository, it's not
very helpful to be told that this is your own fault...

But this kind of issue also happens in any other scenario were heads are not
mapped to a git reference. At the very least, it also happens for closed hg
branches. These are quite common, and I also run into that in real life.

[And to reply to a claim you made in another mail: No, I am not deliberately
"hoarding" issues to make you look bad. But analyzing a breakage you run into
and then properly writing it up takes time; and when you know you will likely
be insulted when reporting it doesn't really help to motivate me to sit
down and do that...]


> 
> In practice that's very unlikely, and proof of that is that nobody has 
> reported
> such issues.

No, that logic is flawed. For example, It could also mean that not many
people are using the tool, and of those not many bother to report issues via
your github bug tracker.

> 
> Either way, I just fixed it [1].

That's great to hear, thanks!

> 
>> There are more issues related to unresolved clashes between the git and hg
>> ways of naming things. E.g. I am collaborating on a hg repository that has
>> branches "foo" and "foo/bar" which git-remote-hg cannot handle because it
>> translates them to git branch names, and, well, git cannot handle that.
> 
> I don't see this as a limitation of git-remote-hg, ideally Git remote-helpers
> should have a standardized way to let users map external branch names.

Agreed. But in the meantime, I think users should still be warned about it.
Or perhaps git-remote-hg could detect this and print a more helpful mes

Re: [RTC/PATCH] Add 'update-branch' hook

2014-04-23 Thread Junio C Hamano
Felipe Contreras  writes:

>> >> I have a branch which should always be recompiled on update;
>> >> post-update-branch would be a good place for that.
>> >
>> > And why would pre-update-branch not serve that purpose?
>> 
>> Because the code that needs to be compiled is not yet in the workspace
>
> And it won't be in 'post-update-branch' either.
>
>  % git checkout master
>  % git branch feature-a stable
>  <- update-branch hook will be called here
>
> The hook will get 'feature-a' as the first argument, but the code in the
> workspace would correspond to 'master'; the checked out branch (pre or post).

The whole point of a pre- hook is to run _before_ the externally
observable state changes due to the operation.

If Stephen has a separate build-tree that fetches from the branch
every time the tip of the branch changes in this repository to
produce build artifacts for the branch to be shared in his network,
perhaps via NFS or something.  "git fetch" that will be run from
that build-tree repository will *not* see the tip of the branch, and
running such a hook will not be possible from a pre-update-branch
hook.

We can certainly argue that such a hook could instead push to the
build-tree repository using the commit object name, but I tend to
think such an argument is merely sidestepping the real issue.  Some
hooks do want to observe the state _after_ the operation [*1*],
while some hooks can do without seeing exactly the state after the
operation.

So while I am generally not very supportive towards post-anything
hook, I would reject a claim that says "pre-anything can be used
without inventing post-anything---do the same thing and allow the
operation and you are done".  That is not simply true.


[Footnote]

*1* A trivial example: send out an e-mail that contains the output
from "git branch -l -v" or "git log --oneline --decorate --all"
to a logger and expect to see the branch tip pointing at the
commit _after_ the operation.

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


Harmful LESS flags

2014-04-23 Thread d9ba
hello list,

as mentioned earlier on IRC, I'm a bit concerned about the default LESS flags
used by git.

The S option causes git to cut off everything to the right

Consider this diff, printed by `git diff`

 #!/usr/bin/env python
-print('foo')
+print('bar')

Looks ok to merge and run.

But, after disabling the pager:

 #!/usr/bin/env python
-print('foo')
+print('bar') [lots of tabs] ; import os; os.system('aptitude install
subversion')

Oh no!

My workflow is to clone a project, read the whole source and review all diffs
after fetching them. After that is done I merge origin into my local
branch and
run the code on my system.

I've panic'd a bit after I've noticed the chopping.

It would be nice if we could change the flags to either

 a) avoid cutting off
 b) indicate something has been cut off (<- I prefer this)

I assume there are more people with a similar workflow who're still
unaware of
this feature.

I would joke about how 3 letter agencies introduced this flag to backdoor
open
source projects, but, well..

Sincerely yours,
a git user


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


  1   2   >