Re: [PATCH] refs.c: get_ref_cache: use a bucket hash

[Jeff King]

On Sat, Nov 14, 2015 at 02:35:01PM +0100, Andreas Krey wrote:

> On Fri, 13 Nov 2015 19:01:18 +, Jeff King wrote:
> ...
> >   2. But for a little more work, pushing the is_git_directory() check
> >  out to the call-sites gives us probably saner semantics overall.
> 
> Oops, now I get it[1]: You mean replacing resolve_gitlink_ref usages
> with is_git_directory, like:

Yes. I mistakenly said is_git_directory, when I really meant
is_git_repository, the new function added in 0179ca7a62. You seem to
have figured out what I meant, but the critical thing is that we check
"$dir/.git", not just "$dir" (and check it both as a git dir and as a
gitfile, as is_git_repository() does).

I'm not sure if we can simply make that function public or not. It's
mostly straightforward, but it does err on the side of "yes, this is a
git repo" if we see a ".git" file we can't read. I think that's probably
reasonable in most sites, but I didn't look closely.

> diff --git a/dir.c b/dir.c
> index d2a8f06..7765dc6 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1375,8 +1375,7 @@ static enum path_treatment treat_directory(struct 
> dir_struct *dir,
>   if (dir->flags & DIR_SHOW_OTHER_DIRECTORIES)
>   break;
>   if (!(dir->flags & DIR_NO_GITLINKS)) {
> - unsigned char sha1[20];
> - if (resolve_gitlink_ref(dirname, "HEAD", sha1) == 0)
> + if (is_git_directory(dirname))
>   return path_untracked;
>   }
>   return path_recurse;
> 
> That, I like. If it is correct.

Yes, that's what I had in mind, modulo the directory/repository thing
above (the is_git_repository function also takes a strbuf, so we'd need
to handle that extra allocation somewhere).

-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: --color-diff='.' considered useful

[Matthieu Moy]

Michael J Gruber  writes:

> git tip of the day:
>
> git diff --color-words='.'
> git show --color-words='.'

Probably my main usage of --color-words indeed (except I omit the single
quotes ;-) ).

I think this deserves an explicit mention in the doc.

-- 
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] rebase-i-exec: Allow space in SHELL_PATH

[Jeff King]

On Mon, Nov 16, 2015 at 05:01:03PM +0100, Johannes Schindelin wrote:

> To clarify the Git for Windows scenario: SHELL_PATH is indeed set to
> `/bin/sh`, but reportedly it is converted into a full Windows path when we
> leave the POSIX emulation layer, i.e. when `git.exe` is called (which has
> *no* idea about POSIX paths, or at least next to none).
> 
> The reason is, of course, that regular Windows programs would not know
> what to do with the path /bin/sh.
> 
> The problem arises when we re-enter the POSIX realm, i.e. when we run a
> script (such as `git-rebase`): the path is not converted back!

Ah, thanks for this explanation. The whole thing seems much more clear
to me now.

Even if later versions give us back the POSIX name, I think Fredrik's
patch is a sane solution (and arguably how it should have been written
in the first place, even leaving aside this issue).

-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] rebase-i-exec: Allow space in SHELL_PATH

[Johannes Schindelin]

Hi Peff,

On Fri, 13 Nov 2015, Jeff King wrote:

> It's possible somebody could be doing something clever with $SHELL, but
> I kind of doubt it. If they want to do something clever, it is much
> easier to put it directly on the exec line, and have the normal $SHELL
> run their clever thing.

To clarify the Git for Windows scenario: SHELL_PATH is indeed set to
`/bin/sh`, but reportedly it is converted into a full Windows path when we
leave the POSIX emulation layer, i.e. when `git.exe` is called (which has
*no* idea about POSIX paths, or at least next to none).

The reason is, of course, that regular Windows programs would not know
what to do with the path /bin/sh.

The problem arises when we re-enter the POSIX realm, i.e. when we run a
script (such as `git-rebase`): the path is not converted back!

There are already tickets on Git for Windows' bug tracker where the case
is made that vim has serious problems with that space in the environment
variable, so the long-term fix is really to teach the POSIX emulation
layer (read: MSys2) to convert SHELL_PATH back into a POSIX path when
appropriate.

This is on my plate, but there are quite a couple more urgent tasks I need
to take care of, so please do not hold your breath (unless you are into
that kind of thing).

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

Re: [PATCH v6] Add git-grep threads param

[Jeff King]

On Mon, Nov 16, 2015 at 05:11:16AM -0800, Victor Leschuk wrote:

> The earlier version of this patch is already included in /pu branch,
> however as we all agreed ($gmane/280299) we have changed the default
> behavior and the meaning of "0". The question is: what is the right
> way to include changes from patch v6 (this one) into already merged
> patch to pu?

Merging to "pu" does not really mean anything; it is simply that the
maintainer has picked it up as a possible topic of interest. Patches can
(and often are) still re-written in that state.

Junio is on vacation for a few weeks, and I'm acting as maintainer in
the interim. I've added your v6 to my pile of patches to look at, but I
haven't gone over it carefully yet.

-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: [bug] git 2.6.2 (64-bit) blocks keyboard input (running in RDP, Win7 x64)

[Ryabov, Vasily V]

 Hi Johannes,

Thanks for your response.
I've already workarounded the issue by using OpenSSH.

It seems there is no simple way to configure Jenkins Git plugin to call Putty 
first under LOCAL_SYSTEM account, so using OpenSSH is much simpler in this case.

Best regards,
-Vasily.

-Original Message-
From: Johannes Schindelin [mailto:johannes.schinde...@gmx.de] 
Sent: Monday, November 16, 2015 3:10 PM
To: Ryabov, Vasily V
Cc: git@vger.kernel.org
Subject: Re: [bug] git 2.6.2 (64-bit) blocks keyboard input (running in RDP, 
Win7 x64)

Hi Vasily,

On Tue, 10 Nov 2015, Ryabov, Vasily V wrote:

> I'm trying to run something like this (on the remote machine (Win7 x64 with 
> git 2.6.2 64-bit) through RDP):
> ```
> git pull --tags --progress 
> ssh://@:/ 
> +refs/heads/*:refs/remotes/origin/*
> ```
> It worked on local machine with git 1.9.5.
> 
> Git asks to confirm an action:
> ```
> The server's host key is not cached in the registry. You have no 
> guarantee that the server is the computer you think it is.
> The server's dss key fingerprint is:
> ssh-dss 1024 <...>
> If you trust this host, enter "y" to add the key to PuTTY's cache and 
> carry on connecting.
> If you want to carry on connecting just once, without adding the key 
> to the cache, enter "n".
> If you do not trust this host, press Return to abandon the connection.
> Store key in cache? (y/n)
> ```
> I'm typing `y`, but there is no reaction at all. No symbols appears in the 
> console. But...
> When I'm pressing `Ctrl+C`, it's killed. And I can see `y` keys in the 
> bash console for the next command.

This is a known issue, mentioned in the release notes
(https://github.com/git-for-windows/build-extra/blob/master/installer/ReleaseNotes.md#known-issues):

> * If configured to use Plink, you will have to connect with putty first
>   and accept the host key.

Ciao,
Johannes


Joint Stock Company Intel A/O
Registered legal address: Krylatsky Hills Business Park,
17 Krylatskaya Str., Bldg 4, Moscow 121614,
Russian Federation

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

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

HVR Magnetics Co.,Ltd representative/collecting Agent Needed:Exe

[JINJING HONGHUI]

Good day to you,

It is my pleasure to forward the following offer of Appointment to you . We are 
a Chinese/Japanese based company who specializes in the production of Heavy 
duty lifting equipment, We are in search of a Reputable Company/Individual that 
can act as our Company sales representative/collecting agent in Canada and USA 
(help as a link between us and our clients in your geographical region). We 
would be obliged if you could render your services to us and get paid monthly 
with additional commission without leaving or affecting your present job.If 
interested, kindly indicate by sending an email to: upon your response to this 
letter you would be briefed on more details.

Yours Sincerely
Jinjing Honghui,
Recruitment Manager.

--
To unsubscribe from this list: send the line "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: [bug] git 2.6.2 (64-bit) blocks keyboard input (running in RDP, Win7 x64)

[Johannes Schindelin]

Hi Vasily,

On Tue, 10 Nov 2015, Ryabov, Vasily V wrote:

> I'm trying to run something like this (on the remote machine (Win7 x64 with 
> git 2.6.2 64-bit) through RDP):
> ```
> git pull --tags --progress ssh://@:/ 
> +refs/heads/*:refs/remotes/origin/*
> ```
> It worked on local machine with git 1.9.5.
> 
> Git asks to confirm an action:
> ```
> The server's host key is not cached in the registry. You
> have no guarantee that the server is the computer you
> think it is.
> The server's dss key fingerprint is:
> ssh-dss 1024 <...>
> If you trust this host, enter "y" to add the key to
> PuTTY's cache and carry on connecting.
> If you want to carry on connecting just once, without
> adding the key to the cache, enter "n".
> If you do not trust this host, press Return to abandon the
> connection.
> Store key in cache? (y/n)
> ```
> I'm typing `y`, but there is no reaction at all. No symbols appears in the 
> console. But...
> When I'm pressing `Ctrl+C`, it's killed. And I can see `y` keys in the 
> bash console for the next command.

This is a known issue, mentioned in the release notes
(https://github.com/git-for-windows/build-extra/blob/master/installer/ReleaseNotes.md#known-issues):

> * If configured to use Plink, you will have to connect with putty first
>   and accept the host key.

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] push: add recurseSubmodules config option

[Mike Crowe]

The --recurse-submodules command line parameter has existed for some
time but it has no config file equivalent.

Following the style of the corresponding parameter for git fetch, let's
invent push.recurseSubmodules to provide a default for this
parameter. This also requires the addition of --recurse-submodules=no to
allow the configuration to be overridden on the command line when
required.

The most straightforward way to implement this appears to be to make
push use code in submodule-config in a similar way to fetch.

Signed-off-by: Mike Crowe 
---
 Documentation/config.txt   |  13 +
 Documentation/git-push.txt |   4 +-
 builtin/push.c |  37 -
 submodule-config.c |  20 +++
 submodule-config.h |   1 +
 submodule.h|   1 +
 t/t5531-deep-submodule-push.sh | 123 -
 7 files changed, 182 insertions(+), 17 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 391a0c3..0546da5 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2226,6 +2226,19 @@ push.gpgSign::
override a value from a lower-priority config file. An explicit
command-line flag always overrides this config option.
 
+push.recurseSubmodules::
+   Make sure all submodule commits used by the revisions to be pushed
+   are available on a remote-tracking branch. If the value is 'check'
+   then Git will verify that all submodule commits that changed in the
+   revisions to be pushed are available on at least one remote of the
+   submodule. If any commits are missing the push will be aborted and
+   exit with non-zero status. If the value is 'on-demand' then all
+   submodules that changed in the revisions to be pushed will be
+   pushed. If on-demand was not able to push all necessary revisions
+   it will also be aborted and exit with non-zero status. You may
+   override this configuration at time of push by specifying
+   '--recurse-submodules=check|on-demand|no'.
+
 rebase.stat::
Whether to show a diffstat of what changed upstream since the last
rebase. False by default.
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 85a4d7d..fb0e9b7 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -257,7 +257,7 @@ origin +master` to force a push to the `master` branch). 
See the
is specified. This flag forces progress status even if the
standard error stream is not directed to a terminal.
 
---recurse-submodules=check|on-demand::
+--recurse-submodules=check|on-demand|no::
Make sure all submodule commits used by the revisions to be
pushed are available on a remote-tracking branch. If 'check' is
used Git will verify that all submodule commits that changed in
@@ -267,6 +267,8 @@ origin +master` to force a push to the `master` branch). 
See the
all submodules that changed in the revisions to be pushed will
be pushed. If on-demand was not able to push all necessary
revisions it will also be aborted and exit with non-zero status.
+   A value of 'no' is used to override the push.recurseSubmodules
+   variable when no submodule recursion is required.
 
 --[no-]verify::
Toggle the pre-push hook (see linkgit:githooks[5]).  The
diff --git a/builtin/push.c b/builtin/push.c
index 3bda430..dfced74 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -9,6 +9,7 @@
 #include "transport.h"
 #include "parse-options.h"
 #include "submodule.h"
+#include "submodule-config.h"
 #include "send-pack.h"
 
 static const char * const push_usage[] = {
@@ -20,7 +21,7 @@ static int thin = 1;
 static int deleterefs;
 static const char *receivepack;
 static int verbosity;
-static int progress = -1;
+static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 
 static struct push_cas_option cas;
 
@@ -452,22 +453,15 @@ static int do_push(const char *repo, int flags)
 static int option_parse_recurse_submodules(const struct option *opt,
   const char *arg, int unset)
 {
-   int *flags = opt->value;
+   int *recurse_submodules = opt->value;
 
-   if (*flags & (TRANSPORT_RECURSE_SUBMODULES_CHECK |
- TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND))
+   if (*recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
die("%s can only be used once.", opt->long_name);
 
-   if (arg) {
-   if (!strcmp(arg, "check"))
-   *flags |= TRANSPORT_RECURSE_SUBMODULES_CHECK;
-   else if (!strcmp(arg, "on-demand"))
-   *flags |= TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND;
-   else
-   die("bad %s argument: %s", opt->long_name, arg);
-   } else
-   die("option %s needs an argument (check|on-demand)",
- 

RE: [PATCH v6] Add git-grep threads param

[Victor Leschuk]

Hello all,

The earlier version of this patch is already included in /pu branch, however as 
we all agreed ($gmane/280299) we have changed the default behavior and the 
meaning of "0". The question is: what is the right way to include changes from 
patch v6 (this one) into already merged patch to pu?

Thanks.
--
Best Regards,
Victor

From: Victor Leschuk [vlesc...@gmail.com]
Sent: Wednesday, November 11, 2015 03:52
To: git@vger.kernel.org
Cc: Victor Leschuk
Subject: [PATCH v6] Add git-grep threads param

"git grep" can now be configured (or told from the command line)
 how many threads to use when searching in the working tree files.

 Changes to default behavior: number of threads now doesn't depend
 on online_cpus(), e.g. if specific number is not configured
 GREP_NUM_THREADS_DEFAULT (8) threads will be used even on 1-core CPU.

Signed-off-by: Victor Leschuk 
---
History of changes from the first version ($gmane/280053/:
* Param renamed from threads-num to threads
* Short version of '--threads' cmd key was removed
* Made num_threads 'decision-tree' more obvious
  and easy to edit for future use ($gmane/280089)
* Moved option description to more suitable place in documentation 
($gmane/280188)
* Hid threads param from 'external' grep.c, made it private for 
builtin/grep.c ($gmane/280188)
* Improved num_threads 'decision-tree', got rid of dependency on 
online_cpus ($gmane/280299)
* Improved param documentation ($gmane/280299)


 Documentation/config.txt   |  7 +
 Documentation/git-grep.txt | 15 ++
 builtin/grep.c | 50 +++---
 contrib/completion/git-completion.bash |  1 +
 4 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 391a0c3..5084e36 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1447,6 +1447,13 @@ grep.extendedRegexp::
option is ignored when the 'grep.patternType' option is set to a value
other than 'default'.

+grep.threads::
+   Number of grep worker threads, use it to tune up performance on
+   your machines. Leave it unset (or set to 0) for default behavior,
+   which for now is using 8 threads for all systems.
+   Default behavior can be changed in future versions
+   to better suit hardware and circumstances.
+
 gpg.program::
Use this custom program instead of "gpg" found on $PATH when
making or verifying a PGP signature. The program must support the
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 4a44d6d..8222a83 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -23,6 +23,7 @@ SYNOPSIS
   [--break] [--heading] [-p | --show-function]
   [-A ] [-B ] [-C ]
   [-W | --function-context]
+  [--threads ]
   [-f ] [-e] 
   [--and|--or|--not|(|)|-e ...]
   [ [--[no-]exclude-standard] [--cached | --no-index | --untracked] | 
...]
@@ -53,6 +54,13 @@ grep.extendedRegexp::
option is ignored when the 'grep.patternType' option is set to a value
other than 'default'.

+grep.threads::
+   Number of grep worker threads, use it to tune up performance on
+   your machines. Leave it unset (or set to 0) for default behavior,
+   which for now is using 8 threads for all systems.
+   Default behavior can be changed in future versions
+   to better suit hardware and circumstances.
+
 grep.fullName::
If set to true, enable '--full-name' option by default.

@@ -227,6 +235,13 @@ OPTIONS
effectively showing the whole function in which the match was
found.

+--threads ::
+   Number of grep worker threads, use it to tune up performance on
+   your machines. Leave it unset (or set to 0) for default behavior,
+   which for now is using 8 threads for all systems.
+   Default behavior can be changed in future versions
+   to better suit hardware and circumstances.
+
 -f ::
Read patterns from , one per line.

diff --git a/builtin/grep.c b/builtin/grep.c
index d04f440..f0e3dfb 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -24,11 +24,11 @@ static char const * const grep_usage[] = {
NULL
 };

-static int use_threads = 1;
+#define GREP_NUM_THREADS_DEFAULT 8
+static int num_threads = 0;

 #ifndef NO_PTHREADS
-#define THREADS 8
-static pthread_t threads[THREADS];
+static pthread_t *threads;

 /* We use one producer thread and THREADS consumer
  * threads. The producer adds struct work_items to 'todo' and the
@@ -63,13 +63,13 @@ static pthread_mutex_t grep_mutex;

 static inline void grep_lock(void)
 {
-   if (use_threads)
+   if (num_threads)
pthread_mutex_lock(_mutex);
 }

 static inline void grep_unlock(void)
 {
-   if (use_threads)
+ 

Re: [PATCH] check-ignore: correct documentation about output

[Dennis Kaarsemaker]

Ping.

On zo, 2015-11-08 at 21:10 +0100, Dennis Kaarsemaker wrote:
> By default git check-ignore shows only the filenames that will be
> ignored, not the pattern that causes their exclusion.
> 
> Signed-off-by: Dennis Kaarsemaker 
> ---
>  Documentation/git-check-ignore.txt | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/git-check-ignore.txt b/Documentation/git
> -check-ignore.txt
> index 59531ab..0a628ac 100644
> --- a/Documentation/git-check-ignore.txt
> +++ b/Documentation/git-check-ignore.txt
> @@ -16,10 +16,9 @@ DESCRIPTION
>  ---
>  
>  For each pathname given via the command-line or from a file via
> -`--stdin`, show the pattern from .gitignore (or other input files to
> -the exclude mechanism) that decides if the pathname is excluded or
> -included.  Later patterns within a file take precedence over earlier
> -ones.
> +`--stdin`, check whether the file is excluded by .gitignore (or
> other
> +input files to the exclude mechanism) and output the path if it is
> +excluded.
>  
>  By default, tracked files are not shown at all since they are not
>  subject to exclude rules; but see `--no-index'.
> -- 
> 2.6.3-495-gf0a7f49
> 
> 
-- 
Dennis Kaarsemaker
http://www.kaarsemaker.net


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

Re: GitGui: get rid of information popup window

[Johannes Schindelin]

Hi Francis,

On Thu, 12 Nov 2015, Francis ANDRE wrote:

> This popup  saying that "No difference has been detected" is little bit
> boring and useless since it states that an automatic resync will be
> started and there is no way to stop the resync.
> 
> Can GitGui remove this popup?

No, Git GUI cannot remove this popup itself. It requires your help.

Remember: Git is Open Source, i.e. it empowers you to change the software
in exactly the way you want. And it also puts the responsibility to make
it so squarely into your court.

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

--color-diff='.' considered useful

[Michael J Gruber]

git tip of the day:

git diff --color-words='.'
git show --color-words='.'

That will help you find that 1 character change that a failed default
word split hides from your eyes.

I guess everyone here will know already, but I found that super useful
and much easier than trying to get a meaningful word split for that 100+
char TeX source line without a single space...

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

Re: [PATCH v8 00/10] refs backend pre-vtable

[David Turner]

On Tue, 2015-11-10 at 12:42 +0100, Michael Haggerty wrote:
> Another re-roll of this patch series, to address the comments of
> Ramsay Jones (thanks!) about v7 [1].
> 
> This version has the following changes compared to v7:
> 
> * Drop "refs: make is_branch public" patch. This was already done
>   quite a while ago:
> 
>   e7e0f26 refs.c: add a public is_branch function (2014-07-15)
> 
> * Instead of having refs-internal.h include refs.h, have the "*.c"
>   files include both header files.
> 
> * Remove some unneeded includes from refs/files-backend.c.
> 
> Since patch 01/11 of v7 was omitted, the patches in this version are
> numbered differently. In particular, it is now patches 01 through 07
> that form the core of this patch series. The last three patches can
> easily be postponed if that will speed the progress of the first
> seven.
> 
> These patches are also available from my GitHub fork [2] as branch
> "refs-backend-pre-vtable".

This version looks good to me.

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

Re: [PATCH v2] add test to demonstrate that shallow recursive clones fail

[Jens Lehmann]

Am 14.11.2015 um 01:10 schrieb Stefan Beller:

On Fri, Nov 13, 2015 at 3:41 PM, Jeff King  wrote:

On Fri, Nov 13, 2015 at 06:38:07PM -0500, Jeff King wrote:


On Fri, Nov 13, 2015 at 03:16:01PM -0800, Stefan Beller wrote:


Junio wrote on Oct 09, 2014:

This is so non-standard a thing to do that I doubt it is worth
supporting with "git clone".  "git clone --branch", which is about

"> I want to follow that particular branch", would not mesh well with

"I want to see the history that leads to this exact commit", either.
You would not know which branch(es) is that exact commit is on in
the first place.


I disagree with this. This is the *exact* thing you actually want to do when
dealing with submodules. When fetching/cloning for a submodule, you want
to obtain the exact sha1, instead of a branch (which happens to be supported
too, but is not the original use case with submodules.)


Yes, being able to fetch certain sha1s makes lots of sense for submodules
(this has been discussed some time ago at a GitTogether). But - apart from
the extra network load - it's rather helpful to get all the submodule
branches too (though that could be limited to the branches the sha1 is on).


I think this is already implemented in 68ee628 (upload-pack: optionally
allow fetching reachable sha1, 2015-05-21), isn't it?


Note that this just implements the server side. I think to use this with
submodules right now, you'd have to manually "git init && git fetch" in
the submodule. It might make sense to teach clone to handle this, to
avoid the submodule code duplicating what the clone code does.


Yes I want to add it to clone, as that is a prerequisite for making
git clone --recursive --depth 1 to work as you'd expect. (such that
the submodule can be cloned instead of rewriting that to be
init


Cool, that should help recursive fetch too.


Thanks for pointing out that we already have some kind of server support.

I wonder if we should add an additional way to make fetching only some
sha1s possible. ("I don't want users to fetch any sha1, but only those
where superprojects point{ed} to", even if you force push a superproject,
you want to want to only allow fetching all sha1s which exist in the current
superprojects branch.)


Me thinks the restrictions for sha1-fetching could come from the branches
these sha1s are found in the upstream submodule: if the client is allowed
to fetch a branch, it should be able to fetch any sha1 on that branch.


Maybe our emails crossed, but in the other mail I pointed out we could use
some sort of hidden ref (refs/superprojects/*) for that, which are
allowed to mark
any sort of sha1, which are allowed in the superproject/submodule context
to be fetched.

So whenever you push to a superproject (a project that has a gitlink),
we would need to check serverside if that submodule is at us and mark the
correct sha1s in the submodule. Then you can disallow fetching most of the sha1s
but still could have a correctly working submodule update mechanism.


And what happens if the submodule isn't at us? Involving the serverside of
a superproject in submodule fetching sounds wrong to me. Me thinks that
the upstream of the submodule should always control if a sha1 is allowed
to be fetched. Or did I understand you wrong?
--
To unsubscribe from this list: send the line "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] push: add recurseSubmodules config option

[Jens Lehmann]

Am 16.11.2015 um 19:31 schrieb Mike Crowe:

On Monday 16 November 2015 at 10:15:24 -0800, Stefan Beller wrote:

The code itself looks good to me, one nit in the tests though.


@@ -79,6 +87,119 @@ test_expect_success 'push succeeds after commit was pushed 
to remote' '
 )
  '

+test_expect_success 'push succeeds if submodule commit not on remote but using 
on-demand on command line' '
+   (
+   cd work/gar/bage &&
+   >recurse-on-demand-on-command-line &&
+   git add recurse-on-demand-on-command-line &&
+   git commit -m "Recurse on-demand on command line junk"
+   ) &&
+   (
+   cd work &&
+   git add gar/bage &&
+   git commit -m "Recurse on-demand on command line for gar/bage" 
&&
+   git push --recurse-submodules=on-demand ../pub.git master &&
+   # Check that the supermodule commit got there
+   git fetch ../pub.git &&
+   git diff --quiet FETCH_HEAD master


Missing && chain here.


Oh, well spotted! I'll provide an updated version.


Looking good for me too!

Cool, another issue from my Wiki that's being worked on!
--
To unsubscribe from this list: send the line "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] push: add recurseSubmodules config option

[Stefan Beller]

On Mon, Nov 16, 2015 at 5:24 AM, Mike Crowe  wrote:
> The --recurse-submodules command line parameter has existed for some
> time but it has no config file equivalent.
>
> Following the style of the corresponding parameter for git fetch, let's
> invent push.recurseSubmodules to provide a default for this
> parameter. This also requires the addition of --recurse-submodules=no to
> allow the configuration to be overridden on the command line when
> required.
>
> The most straightforward way to implement this appears to be to make
> push use code in submodule-config in a similar way to fetch.
>
> Signed-off-by: Mike Crowe 
> ---

The code itself looks good to me, one nit in the tests though.

> @@ -79,6 +87,119 @@ test_expect_success 'push succeeds after commit was 
> pushed to remote' '
> )
>  '
>
> +test_expect_success 'push succeeds if submodule commit not on remote but 
> using on-demand on command line' '
> +   (
> +   cd work/gar/bage &&
> +   >recurse-on-demand-on-command-line &&
> +   git add recurse-on-demand-on-command-line &&
> +   git commit -m "Recurse on-demand on command line junk"
> +   ) &&
> +   (
> +   cd work &&
> +   git add gar/bage &&
> +   git commit -m "Recurse on-demand on command line for 
> gar/bage" &&
> +   git push --recurse-submodules=on-demand ../pub.git master &&
> +   # Check that the supermodule commit got there
> +   git fetch ../pub.git &&
> +   git diff --quiet FETCH_HEAD master

Missing && chain here.

> +   # Check that the submodule commit got there too
> +   cd gar/bage &&
> +   git diff --quiet origin/master master
> +   )
> +'
> +
--
To unsubscribe from this list: send the line "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] push: add recurseSubmodules config option

[Mike Crowe]

On Monday 16 November 2015 at 10:15:24 -0800, Stefan Beller wrote:
> The code itself looks good to me, one nit in the tests though.
> 
> > @@ -79,6 +87,119 @@ test_expect_success 'push succeeds after commit was 
> > pushed to remote' '
> > )
> >  '
> >
> > +test_expect_success 'push succeeds if submodule commit not on remote but 
> > using on-demand on command line' '
> > +   (
> > +   cd work/gar/bage &&
> > +   >recurse-on-demand-on-command-line &&
> > +   git add recurse-on-demand-on-command-line &&
> > +   git commit -m "Recurse on-demand on command line junk"
> > +   ) &&
> > +   (
> > +   cd work &&
> > +   git add gar/bage &&
> > +   git commit -m "Recurse on-demand on command line for 
> > gar/bage" &&
> > +   git push --recurse-submodules=on-demand ../pub.git master &&
> > +   # Check that the supermodule commit got there
> > +   git fetch ../pub.git &&
> > +   git diff --quiet FETCH_HEAD master
> 
> Missing && chain here.

Oh, well spotted! I'll provide an updated version.

Thanks.

Mike.
--
To unsubscribe from this list: send the line "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] add test to demonstrate that shallow recursive clones fail

[Jens Lehmann]

Am 16.11.2015 um 20:25 schrieb Stefan Beller:

On Mon, Nov 16, 2015 at 10:59 AM, Jens Lehmann  wrote:

Am 14.11.2015 um 01:10 schrieb Stefan Beller:

Thanks for pointing out that we already have some kind of server support.

I wonder if we should add an additional way to make fetching only some
sha1s possible. ("I don't want users to fetch any sha1, but only those
where superprojects point{ed} to", even if you force push a superproject,
you want to want to only allow fetching all sha1s which exist in the
current
superprojects branch.)



Me thinks the restrictions for sha1-fetching could come from the branches
these sha1s are found in the upstream submodule: if the client is allowed
to fetch a branch, it should be able to fetch any sha1 on that branch.


I'd agree on that. The server side even with uploadpack.allowTipSHA1InWant
set, is not sufficient though.

To fetch an arbitrary sha1, you would need to check if that sha1 is part
of the history of any advertised branch and then allow fetching serverside,
which sounds like some work for the server, which we may want to avoid
by having smarter data structures there.

Instead of having to search all branches for the requested sha1, we could have
some sort of data structure to make it not an O(n) operation (n being
all objects
in the repo).

Maybe I overestimate the work which needs to be done, because the server has
bitmaps nowadays.

Maybe a lazy reverse-pointer graph can be established on the serverside.
So I guess when we add the feature to fetch arbitrary sha1s, reachable from
any branch, people using submodules will make use of the feature. (such as with
git fetch --recurse --depth 1 or via a new `git fetch --recursive
--up-to-submodule-tip-only`)

So once the server is asked for a certain sha1, it will do the
reachability check,
which takes some effort, but then stores the result in the form:
"If ${current tip sha} of ${branch} is reachable, so is requested $sha1."

So when the next fetch request for $sha1 arrives, the server only needs to
check for ${current tip sha} to be part of $branch, which is expected to be
a shorter revwalk from the tip. (Because it is nearer to the tip, a bitmap could
just tell you or at least shorten the walk even more)
If the ${branch} has changed, the next evaluation for $sha1 can update
the cache,
such that the reverse lookup is not expensive on expectation.


Makes sense, although I do not know enough about the server side to tell if
it would need such an optimization or will cope with the load just fine.

But even if we'd enable such a feature without having to set an extra config
option, a submodule fetch asking for certain sha1s would have to fall back
to a simple "fetch all" like we do now when the server doesn't support that
for backwards compatibility. But maybe that's just obvious.


I assume this will mostly be used with submodules, so only a few sha1s need
this caching.


I won't bet on that, some of the submodules at $DAYJOB are rather busy and
see almost the same traffic as their superprojects ;-)


Maybe our emails crossed, but in the other mail I pointed out we could use
some sort of hidden ref (refs/superprojects/*) for that, which are
allowed to mark
any sort of sha1, which are allowed in the superproject/submodule context
to be fetched.

So whenever you push to a superproject (a project that has a gitlink),
we would need to check serverside if that submodule is at us and mark the
correct sha1s in the submodule. Then you can disallow fetching most of the
sha1s
but still could have a correctly working submodule update mechanism.



And what happens if the submodule isn't at us? Involving the serverside of
a superproject in submodule fetching sounds wrong to me. Me thinks that
the upstream of the submodule should always control if a sha1 is allowed
to be fetched. Or did I understand you wrong?


Yes and no.
The serverside submodule repository should be responsible for the ultimate
decision if you are allowed to fetch that sha1. But maybe on pushing the
superproject, we can store a hint in the submodule, that this sha1 is legit.
Although I may be missguided in my thinking here as the superproject
should have no influence on the submodule.


Submodules should never be aware of their superproject. But a superproject
does know its submodules, so I don't think the influence you describe here
is a problem per se. It's just looking like a corner case to me, as in a
lot of scenarios submodules do not live on the same server. And even if
they do, a superproject has no canonical way of finding their submodule's
repos (except for submodules that use relative URLs). So I'd rather like
to see a generic solution first, before we think about adding an optimized
version for certain setups later ;-)

The only real itch I have with the "superproject declaring submodule sha1s
fetchable on the server" approach is that it smells like a security problem.
The access rights of superprojects are often different 

Re: [PATCH 2/6] remote-http(s): Support SOCKS proxies

[Johannes Schindelin]

Hi Pat,

On Mon, 9 Nov 2015, Pat Thoyts wrote:

> Johannes Schindelin  writes:
> 
> >On Tue, 27 Oct 2015, Johannes Schindelin wrote:
> >
> >> On Mon, 26 Oct 2015, Junio C Hamano wrote:
> >> 
> >> > James McCoy  writes:
> >> > 
> >> > >> The code looks OK but the last paragraph makes _us_ worried.  What
> >> > >> is the licensing status of the original at SO?
> >> > >
> >> > > According to Stackoverflow[0],
> >> > >
> >> > >   As noted in the Stack Exchange Terms of Service[1] and in the footer 
> >> > > of
> >> > >   every page, all user contributions are licensed under Creative 
> >> > > Commons
> >> > >   Attribution-Share Alike[2]. Proper attribution[3] is required if you
> >> > >   republish any Stack Exchange content.
> >> > >
> >> > > [0]: https://stackoverflow.com/help/licensing
> >> > 
> >> > Yes, and (please correct me if I am wrong--this is one of the times
> >> > I hope I am wrong!) I thought BY-SA does not mesh well with GPLv2,
> >> > in which case we cannot use this patch (instead somebody has to
> >> > reimplement the same without copying).
> >> 
> >> Pat, could you please allow us to insert your SOB?
> >
> >On second thought... Junio, could you please sanity-check my claim that
> >this patch:
> >
> >-- snip --
> >@@ -465,6 +465,17 @@ static CURL *get_curl_handle(void)
> > 
> >if (curl_http_proxy) {
> >curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
> >+#if LIBCURL_VERSION_NUM >= 0x071800
> >+   if (starts_with(curl_http_proxy, "socks5"))
> >+   curl_easy_setopt(result,
> >+   CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5);
> >+   else if (starts_with(curl_http_proxy, "socks4a"))
> >+   curl_easy_setopt(result,
> >+   CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4A);
> >+   else if (starts_with(curl_http_proxy, "socks"))
> >+   curl_easy_setopt(result,
> >+   CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4);
> >+#endif
> >}
> > #if LIBCURL_VERSION_NUM >= 0x070a07
> >curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
> >-- snap --
> >
> >cannot be copyrighted because it is pretty much the only way to implement
> >said functionality?
> >
> >Still, Pat, if you find the time, could you please simply relicense your
> >patch (I know that you are fine with it, but we need an explicit
> >statement)?
> >
> >Ciao,
> >Johannes
> 
> A bit late to the party but 'yes'. Frankly by posting something to SO I
> rather consider it public domain

Yeah, unfortunately it needs to be stated explicitly, though... ;-)

> but I hereby license this patch as required for use by the Git project.
> 
> Signed-off-by: Pat Thoyts 

Thanks!

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

[PATCH] Optimize usage of grep by passing -q

[Stefan Beller]

Instead of redirecting all grep output to /dev/null, we can just
pass in -q instead. This preserves the exit code behavior, but is faster.
As grep returns true if it finds at least one match, grep can exit promptly
after finding the first line and doesn't need to find more occurrences
which would be redirected to /dev/null anyways.

This is true for the gnu version of grep. I am not sure if all
versions of grep support this optimization. In case it is not,
we'd revert this patch.

Signed-off-by: Stefan Beller 
---
 git-bisect.sh  | 5 ++---
 git-rebase--interactive.sh | 2 +-
 git-rebase.sh  | 2 +-
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index 5d1cb00..b909605 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -519,8 +519,7 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
 
cat "$GIT_DIR/BISECT_RUN"
 
-   if sane_grep "first $TERM_BAD commit could be any of" 
"$GIT_DIR/BISECT_RUN" \
-   >/dev/null
+   if sane_grep -q "first $TERM_BAD commit could be any of" 
"$GIT_DIR/BISECT_RUN"
then
gettextln "bisect run cannot continue any more" >&2
exit $res
@@ -533,7 +532,7 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
exit $res
fi
 
-   if sane_grep "is the first $TERM_BAD commit" 
"$GIT_DIR/BISECT_RUN" >/dev/null
+   if sane_grep -q "is the first $TERM_BAD commit" 
"$GIT_DIR/BISECT_RUN"
then
gettextln "bisect run success"
exit 0;
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d65c06e..f360ac0 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1225,7 +1225,7 @@ then
git rev-list $revisions |
while read rev
do
-   if test -f "$rewritten"/$rev && test "$(sane_grep "$rev" 
"$state_dir"/not-cherry-picks)" = ""
+   if test -f "$rewritten"/$rev && test "$(sane_grep -q "$rev" 
"$state_dir"/not-cherry-picks)"
then
# Use -f2 because if rev-list is telling us this commit 
is
# not worthwhile, we don't want to track its multiple 
heads,
diff --git a/git-rebase.sh b/git-rebase.sh
index af7ba5f..b6a5f73 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -578,7 +578,7 @@ mb=$(git merge-base "$onto" "$orig_head")
 if test "$type" != interactive && test "$upstream" = "$onto" &&
test "$mb" = "$onto" && test -z "$restrict_revision" &&
# linear history?
-   ! (git rev-list --parents "$onto".."$orig_head" | sane_grep " .* ") > 
/dev/null
+   ! (git rev-list --parents "$onto".."$orig_head" | sane_grep -q " .* ")
 then
if test -z "$force_rebase"
then
-- 
2.6.3.368.gf34be46

--
To unsubscribe from this list: send the line "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 0/9] Reduce index load time

[Johannes Schindelin]

Hi Duy,

On Sun, 1 Nov 2015, Nguyễn Thái Ngọc Duy wrote:

> This is the rebased version since last time [1] with
> s/free_index_shm/release_index_shm/ as suggested by David Turner. It
> introduces a daemon that can cache index data in memory so that
> subsequent git processes can avoid reading (and more importantly,
> verifying) the index from disk. Together with split-index it should
> keep index I/O cost down to minimum. The series can also be found at
> [2].
> 
> One of the factors that affected my design was Windows support. We
> now have Dscho back, he can evaluate my approach for Windows.

You flatter me! ;-)

Seriously again, this patch series comes at a very good time: I will have
a closer look soon (sorry about being so vague, but I am once again a
little bit short on time/brain cycles).

Thanks!
Dscho

Re: [PATCH v2] add test to demonstrate that shallow recursive clones fail

[Stefan Beller]

On Mon, Nov 16, 2015 at 10:59 AM, Jens Lehmann  wrote:
> Am 14.11.2015 um 01:10 schrieb Stefan Beller:
>>
>> On Fri, Nov 13, 2015 at 3:41 PM, Jeff King  wrote:
>>>
>>> On Fri, Nov 13, 2015 at 06:38:07PM -0500, Jeff King wrote:
>>>
 On Fri, Nov 13, 2015 at 03:16:01PM -0800, Stefan Beller wrote:

> Junio wrote on Oct 09, 2014:
>>
>> This is so non-standard a thing to do that I doubt it is worth
>> supporting with "git clone".  "git clone --branch", which is about
>
> "> I want to follow that particular branch", would not mesh well with
>>
>> "I want to see the history that leads to this exact commit", either.
>> You would not know which branch(es) is that exact commit is on in
>> the first place.
>
>
> I disagree with this. This is the *exact* thing you actually want to do
> when
> dealing with submodules. When fetching/cloning for a submodule, you
> want
> to obtain the exact sha1, instead of a branch (which happens to be
> supported
> too, but is not the original use case with submodules.)
>
>
> Yes, being able to fetch certain sha1s makes lots of sense for submodules
> (this has been discussed some time ago at a GitTogether). But - apart from
> the extra network load - it's rather helpful to get all the submodule
> branches too (though that could be limited to the branches the sha1 is on).

Ok, I did not attend that GitTogether ;)

>
 I think this is already implemented in 68ee628 (upload-pack: optionally
 allow fetching reachable sha1, 2015-05-21), isn't it?
>>>
>>>
>>> Note that this just implements the server side. I think to use this with
>>> submodules right now, you'd have to manually "git init && git fetch" in
>>> the submodule. It might make sense to teach clone to handle this, to
>>> avoid the submodule code duplicating what the clone code does.
>>
>>
>> Yes I want to add it to clone, as that is a prerequisite for making
>> git clone --recursive --depth 1 to work as you'd expect. (such that
>> the submodule can be cloned instead of rewriting that to be
>> init
>
>
> Cool, that should help recursive fetch too.
>
>> Thanks for pointing out that we already have some kind of server support.
>>
>> I wonder if we should add an additional way to make fetching only some
>> sha1s possible. ("I don't want users to fetch any sha1, but only those
>> where superprojects point{ed} to", even if you force push a superproject,
>> you want to want to only allow fetching all sha1s which exist in the
>> current
>> superprojects branch.)
>
>
> Me thinks the restrictions for sha1-fetching could come from the branches
> these sha1s are found in the upstream submodule: if the client is allowed
> to fetch a branch, it should be able to fetch any sha1 on that branch.

I'd agree on that. The server side even with uploadpack.allowTipSHA1InWant
set, is not sufficient though.

To fetch an arbitrary sha1, you would need to check if that sha1 is part
of the history of any advertised branch and then allow fetching serverside,
which sounds like some work for the server, which we may want to avoid
by having smarter data structures there.

Instead of having to search all branches for the requested sha1, we could have
some sort of data structure to make it not an O(n) operation (n being
all objects
in the repo).

Maybe I overestimate the work which needs to be done, because the server has
bitmaps nowadays.

Maybe a lazy reverse-pointer graph can be established on the serverside.
So I guess when we add the feature to fetch arbitrary sha1s, reachable from
any branch, people using submodules will make use of the feature. (such as with
git fetch --recurse --depth 1 or via a new `git fetch --recursive
--up-to-submodule-tip-only`)

So once the server is asked for a certain sha1, it will do the
reachability check,
which takes some effort, but then stores the result in the form:
"If ${current tip sha} of ${branch} is reachable, so is requested $sha1."

So when the next fetch request for $sha1 arrives, the server only needs to
check for ${current tip sha} to be part of $branch, which is expected to be
a shorter revwalk from the tip. (Because it is nearer to the tip, a bitmap could
just tell you or at least shorten the walk even more)
If the ${branch} has changed, the next evaluation for $sha1 can update
the cache,
such that the reverse lookup is not expensive on expectation.

I assume this will mostly be used with submodules, so only a few sha1s need
this caching.

>
>> Maybe our emails crossed, but in the other mail I pointed out we could use
>> some sort of hidden ref (refs/superprojects/*) for that, which are
>> allowed to mark
>> any sort of sha1, which are allowed in the superproject/submodule context
>> to be fetched.
>>
>> So whenever you push to a superproject (a project that has a gitlink),
>> we would need to check serverside if that submodule is at us and mark the
>> correct sha1s in the submodule. Then 

Re: [PATCH v5 3/6] git-p4: retry kill/cleanup operations in tests with timeout

[Eric Sunshine]

On Sun, Nov 15, 2015 at 8:08 AM,   wrote:
> From: Lars Schneider 
>
> In rare cases kill/cleanup operations in tests fail. Retry these
> operations with a timeout to make the test less flaky.
>
> Signed-off-by: Lars Schneider 
> ---
> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
> @@ -121,22 +125,33 @@ p4_add_user() {
> EOF
>  }
>
> +retry_until_success() {
> +timeout=$(($(date +%s) + $RETRY_TIMEOUT))

There was some discussion previously[1] about detecting dynamically
whether 'date +%s' was supported. Was this something that you intended
to do, or did you decide against it since p4 is not supported on such
platforms?

Same question also applies to patch 4/6.

[1]: 
http://article.gmane.org/gmane.comp.version-control.git/280978/match=lazy+prerequisite

> +until "$@" 2>/dev/null || test $(date +%s) -gt $timeout
> +do :
> +done
> +}
> +
> +retry_until_fail() {
> +timeout=$(($(date +%s) + $RETRY_TIMEOUT))
> +until ! "$@" 2>/dev/null || test $(date +%s) -gt $timeout
> +do :
> +done
> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] notes: allow merging from arbitrary references

[Jacob Keller]

On Sun, Nov 15, 2015 at 11:55 PM, Johan Herland  wrote:
> Additionally, if we suspect that passing non-notes trees to read-only
> operations will be a common error, we could add a simple heuristic to
> the notes code, to warn (or even abort) if we strongly suspect that we
> are reading in a non-notes tree. For example, if the ratio of
> non-notes to notes entries goes above, say, 1:1 (or even 10:1), then
> what we're reading is probably not a proper notes tree...
>

I agree here for this part, a possible heuristic check would maybe be
valuable.. but not sure it's super worth the effort. I doubt it would
be a common error, and I don't think the issues above would actually
cause too many problems.

The main other issue is how to get notes DWIM things to work for all
cases where we want to use notes refs, since right now the DWIM is
basically done at the top level and only handles notes like things.
The problem with it is that if you specify a full ref that *isn't*
refs/notes, you will always prefix it with refs/notes, like so:

refs/remote-notes/origin => refs/notes/refs/remote-notes/origin,

This makes it really difficult to expand a ref. However, Julio seemed
to think this was a possibly valuable expansion under normal
circumstances. The current solution is to try to do a normal lookup
first and only use the notes DWIM after we fail a lookup, which I
think is what the above patch attempts to do. This seems ok enough to
me.

Regards,
Jake

>
> ...Johan
>
--
To unsubscribe from this list: send the line "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] push: add recurseSubmodules config option

[Eric Sunshine]

On Mon, Nov 16, 2015 at 8:24 AM, Mike Crowe  wrote:
> The --recurse-submodules command line parameter has existed for some
> time but it has no config file equivalent.
>
> Following the style of the corresponding parameter for git fetch, let's
> invent push.recurseSubmodules to provide a default for this
> parameter. This also requires the addition of --recurse-submodules=no to
> allow the configuration to be overridden on the command line when
> required.
>
> The most straightforward way to implement this appears to be to make
> push use code in submodule-config in a similar way to fetch.
>
> Signed-off-by: Mike Crowe 
> ---
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> @@ -2226,6 +2226,19 @@ push.gpgSign::
> +push.recurseSubmodules::
> +   Make sure all submodule commits used by the revisions to be pushed
> +   are available on a remote-tracking branch. If the value is 'check'
> +   then Git will verify that all submodule commits that changed in the
> +   revisions to be pushed are available on at least one remote of the
> +   submodule. If any commits are missing the push will be aborted and
> +   exit with non-zero status. If the value is 'on-demand' then all
> +   submodules that changed in the revisions to be pushed will be
> +   pushed. If on-demand was not able to push all necessary revisions
> +   it will also be aborted and exit with non-zero status. You may
> +   override this configuration at time of push by specifying
> +   '--recurse-submodules=check|on-demand|no'.

Does this configuration variable also support 'no' as a value? If so,
then it probably ought to be documented. If not, shouldn't it do so to
allow a configuration file to override a 'check' or 'on-demand' value
specified in a more global git configuration file?

>  rebase.stat::
> Whether to show a diffstat of what changed upstream since the last
> rebase. False by default.
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> @@ -257,7 +257,7 @@ origin +master` to force a push to the `master` branch). 
> See the
> ---recurse-submodules=check|on-demand::
> +--recurse-submodules=check|on-demand|no::
> Make sure all submodule commits used by the revisions to be
> pushed are available on a remote-tracking branch. If 'check' is
> used Git will verify that all submodule commits that changed in
> @@ -267,6 +267,8 @@ origin +master` to force a push to the `master` branch). 
> See the
> all submodules that changed in the revisions to be pushed will
> be pushed. If on-demand was not able to push all necessary
> revisions it will also be aborted and exit with non-zero status.
> +   A value of 'no' is used to override the push.recurseSubmodules
> +   variable when no submodule recursion is required.

Does this deserve a --no-recurse-submodules alias for consistency with
how other options are turned off?

>  --[no-]verify::
> Toggle the pre-push hook (see linkgit:githooks[5]).  The
> diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
> @@ -64,7 +64,15 @@ test_expect_success 'push fails if submodule commit not on 
> remote' '
> cd work &&
> git add gar/bage &&
> git commit -m "Third commit for gar/bage" &&
> -   test_must_fail git push --recurse-submodules=check ../pub.git 
> master
> +   # the push should fail with --recurse-submodules=check
> +   # on the command line...
> +   test_must_fail git push --recurse-submodules=check ../pub.git 
> master &&
> +
> +   # ...or if specified in the configuration..
> +   git config push.recurseSubmodules check &&
> +   test_must_fail git push ../pub.git master &&
> +
> +   git config --unset push.recurseSubmodules

If something above this line fails, then 'git config --unset' will not
be invoked, so the expected cleanup won't happen. Typically, to ensure
cleanup, you'd use test_config(), however that function doesn't work
in subshells. Probably the easiest fix, in this case, is to set the
config variable as a one-shot and drop 'git config' and 'git config
--unset' altogether:

test_must_fail git -c push.recurseSubmodules check \
push ../pub.git master

> )
>  '
>
> @@ -79,6 +87,119 @@ test_expect_success 'push succeeds after commit was 
> pushed to remote' '
> +test_expect_success 'push succeeds if submodule commit not on remote but 
> using on-demand from config' '
> +   (
> +   cd work/gar/bage &&
> +   >recurse-on-demand-from-config &&
> +   git add recurse-on-demand-from-config &&
> +   git commit -m "Recurse on-demand from config junk"
> +   ) &&
> +   (
> +   cd work &&
> +   git add gar/bage &&
> +   git commit -m 

Re: [PATCH v2 2/2] completion: add support for completing email aliases

[Eric Sunshine]

On Sun, Nov 15, 2015 at 3:22 PM, Jacob Keller  wrote:
> Using the new --list-aliases option from git-send-email, add completion
> for --to, --cc, and --bcc with the available configured aliases.
>
> Signed-off-by: Jacob Keller 
> ---
> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> @@ -1711,6 +1712,15 @@ __git_send_email_suppresscc_options="author self cc 
> bodycc sob cccmd body all"
>
>  _git_send_email ()
>  {
> +   case "$prev" in
> +   --to|--cc|--bcc)

What about --from, which also undergoes alias expansion?

> +   __gitcomp "
> +   $(git --git-dir="$(__gitdir)" send-email --list-aliases 
> 2>/dev/null)
> +   " "" ""
> +   return
> +   ;;
> +   esac
> +
> case "$cur" in
> --confirm=*)
> __gitcomp "
> @@ -1735,6 +1745,12 @@ _git_send_email ()
> " "" "${cur##--thread=}"
> return
> ;;
> +   --to=*|--cc=*|--bcc=*)
> +   __gitcomp "
> +   $(git --git-dir="$(__gitdir)" send-email --list-aliases 
> 2>/dev/null)
> +   " "" "${cur#--*=}"
> +   return
> +   ;;
> --*)
> __gitcomp "--annotate --bcc --cc --cc-cmd --chain-reply-to
> --compose --confirm= --dry-run --envelope-sender
> --
> 2.6.3.491.g3e3f6ce
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

[Keller, Jacob E]

On Mon, 2015-11-16 at 18:56 -0500, Eric Sunshine wrote:
> On Mon, Nov 16, 2015 at 6:50 PM, Eric Sunshine  om> wrote:
> > Also, shouldn't --list-aliases (or --dump-aliases) be mutually
> > exclusive with many of the other options? New tests would check
> > such
> > exclusivity as well.
> 
> In fact, while I agree with Szeder that it makes sense to re-use
> send-email's aliases parsing functionality (and was going to suggest
> the same, but he beat me to it), this new option is awfully
> orthogonal
> to the overall purpose of send-email, thus, doesn't really fit in
> well
> and almost cries out for a command of its own which would be used by
> send-email and bash completion (though I'm not convinced that it's
> worth going that route for this one minor use-case).

I don't think it's worth it at this point, because we'd have to extract
out the alias parsing logic from send-email, which is not easy.

The option is pretty orthogonal to git-send-email, but until/unless
git-send-email is re-implemented in C, i don't really see value in
trying to separate the logic out... That is a lot more effort for very
little gain.

Regards,
JakeN�r��yb�X��ǧv�^�)޺{.n�+ا���ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf

Re: [PATCH v2] add test to demonstrate that shallow recursive clones fail

[Stefan Beller]

On Mon, Nov 16, 2015 at 1:42 PM, Jens Lehmann  wrote:
> Am 16.11.2015 um 20:25 schrieb Stefan Beller:
>>
>> On Mon, Nov 16, 2015 at 10:59 AM, Jens Lehmann 
>> wrote:
>>>
>>> Am 14.11.2015 um 01:10 schrieb Stefan Beller:

 Thanks for pointing out that we already have some kind of server
 support.

 I wonder if we should add an additional way to make fetching only some
 sha1s possible. ("I don't want users to fetch any sha1, but only those
 where superprojects point{ed} to", even if you force push a
 superproject,
 you want to want to only allow fetching all sha1s which exist in the
 current
 superprojects branch.)
>>>
>>>
>>>
>>> Me thinks the restrictions for sha1-fetching could come from the branches
>>> these sha1s are found in the upstream submodule: if the client is allowed
>>> to fetch a branch, it should be able to fetch any sha1 on that branch.
>>
>>
>> I'd agree on that. The server side even with uploadpack.allowTipSHA1InWant
>> set, is not sufficient though.
>>
>> To fetch an arbitrary sha1, you would need to check if that sha1 is part
>> of the history of any advertised branch and then allow fetching
>> serverside,
>> which sounds like some work for the server, which we may want to avoid
>> by having smarter data structures there.
>>
>> Instead of having to search all branches for the requested sha1, we could
>> have
>> some sort of data structure to make it not an O(n) operation (n being
>> all objects
>> in the repo).
>>
>> Maybe I overestimate the work which needs to be done, because the server
>> has
>> bitmaps nowadays.
>>
>> Maybe a lazy reverse-pointer graph can be established on the serverside.
>> So I guess when we add the feature to fetch arbitrary sha1s, reachable
>> from
>> any branch, people using submodules will make use of the feature. (such as
>> with
>> git fetch --recurse --depth 1 or via a new `git fetch --recursive
>> --up-to-submodule-tip-only`)
>>
>> So once the server is asked for a certain sha1, it will do the
>> reachability check,
>> which takes some effort, but then stores the result in the form:
>> "If ${current tip sha} of ${branch} is reachable, so is requested $sha1."
>>
>> So when the next fetch request for $sha1 arrives, the server only needs to
>> check for ${current tip sha} to be part of $branch, which is expected to
>> be
>> a shorter revwalk from the tip. (Because it is nearer to the tip, a bitmap
>> could
>> just tell you or at least shorten the walk even more)
>> If the ${branch} has changed, the next evaluation for $sha1 can update
>> the cache,
>> such that the reverse lookup is not expensive on expectation.
>
>
> Makes sense, although I do not know enough about the server side to tell if
> it would need such an optimization or will cope with the load just fine.
>
> But even if we'd enable such a feature without having to set an extra config
> option, a submodule fetch asking for certain sha1s would have to fall back
> to a simple "fetch all" like we do now when the server doesn't support that
> for backwards compatibility. But maybe that's just obvious.

It's not obvious to me.  Say you run the command:

git clone --recursive --depth=1 ...

Currently the depth argument for the submodules is ignored, because it
doesn't work out conceptually. This is because recursive fetches fetch the
branch tips and not the submodule-specified sha1.

If we want to make it work, we would need to think about, what we want
to achieve here.
depth is usually used to reduce the transmit time/bandwidth required.

So if the server tells us it's not allowing fetching of arbitrary
sha1s by its cryptic message:

$ git fetch origin 6f963a895a97d720c909fcf4eb0544a272ef7c49:refs/heads/copy
error: no such remote ref 6f963a895a97d720c909fcf4eb0544a272ef7c49

we have two choices, either error out with

die(_("Server doesn't support cloning of arbitrary sha1s"))

or we could pretend as if we know how to fix it by cloning regularly
with the whole history
attached and then present a tightened history by shallowing after
cloning. But that would
defeat the whole point of the depth argument in the first place, the
time and bandwidth would
have been wasted. So instead I'd rather have the user make the choice.



>
>> I assume this will mostly be used with submodules, so only a few sha1s
>> need
>> this caching.
>
>
> I won't bet on that, some of the submodules at $DAYJOB are rather busy and
> see almost the same traffic as their superprojects ;-)

But do you update the superproject with each submodules commit?
(We plan to update the superproject in Gerrit with each submodule eventually,
so yeah that point is nuts.)

>
> Submodules should never be aware of their superproject. But a superproject
> does know its submodules, so I don't think the influence you describe here
> is a problem per se. It's just looking like a corner case to me, as in a
> lot of scenarios submodules do not live on the 

Re: [PATCH v2 2/2] completion: add support for completing email aliases

[Keller, Jacob E]

On Mon, 2015-11-16 at 18:33 -0500, Eric Sunshine wrote:
> On Sun, Nov 15, 2015 at 3:22 PM, Jacob Keller  om> wrote:
> > Using the new --list-aliases option from git-send-email, add
> > completion
> > for --to, --cc, and --bcc with the available configured aliases.
> > 
> > Signed-off-by: Jacob Keller 
> > ---
> > diff --git a/contrib/completion/git-completion.bash
> > b/contrib/completion/git-completion.bash
> > @@ -1711,6 +1712,15 @@ __git_send_email_suppresscc_options="author
> > self cc bodycc sob cccmd body all"
> > 
> >  _git_send_email ()
> >  {
> > +   case "$prev" in
> > +   --to|--cc|--bcc)
> 
> What about --from, which also undergoes alias expansion?
> 

Makes sense, yep.

N�r��yb�X��ǧv�^�)޺{.n�+ا���ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf

Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

[Eric Sunshine]

On Mon, Nov 16, 2015 at 6:40 PM, Keller, Jacob E
 wrote:
> On Mon, 2015-11-16 at 18:30 -0500, Eric Sunshine wrote:
>> Also, is it possible that some consumer down the road might want
>> richer output which includes the expansion of each alias? For
>> instance, it could emit the alias name as the first token on each
>> line
>> and the expansion as the remainder. Consumers interested in only the
>> alias name would grab the first token on the line and ignore
>> everything else.
>
> Maybe? The problem with printing the full address is that it may not be
> quoted or similar, and it makes the bash completion require an extra
> parameter.. I am not sure how valuable the alias expansion would be for
> use? The main concern I have is we'd need to use another process on top
> to extract only alias names.

It should be possible to extract the alias within the shell itself
without a separate process. For instance:

read alias rest

will leave the first token in $alias and the remainder of the line in
$rest, and it's all done within the shell process.

>> New test(s) seem to be missing.
>
> I had removed the tests from the old version because they weren't
> necessary anymore. New ones wouldn't hurt here either, though.. I'll
> work on that.

I'm not sure which tests you mean, but I was referring to tests to
make sure that git-send-email recognizes --list-aliases (or
--dump-aliases if you switch to that) and that it produces the
expected output in the expected format.

Also, shouldn't --list-aliases (or --dump-aliases) be mutually
exclusive with many of the other options? New tests would check such
exclusivity as well.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

[Keller, Jacob E]

On Mon, 2015-11-16 at 18:50 -0500, Eric Sunshine wrote:
> It should be possible to extract the alias within the shell itself
> without a separate process. For instance:
> 
> read alias rest
> 
> will leave the first token in $alias and the remainder of the line in
> $rest, and it's all done within the shell process.
> 

I'll look into this :)

> > > New test(s) seem to be missing.
> > 
> > I had removed the tests from the old version because they weren't
> > necessary anymore. New ones wouldn't hurt here either, though..
> > I'll
> > work on that.
> 
> I'm not sure which tests you mean, but I was referring to tests to
> make sure that git-send-email recognizes --list-aliases (or
> --dump-aliases if you switch to that) and that it produces the
> expected output in the expected format.
> 

Yep, I added some in my respin.

> Also, shouldn't --list-aliases (or --dump-aliases) be mutually
> exclusive with many of the other options? New tests would check such
> exclusivity as well.

I am at a loss for how to do that correctly in the perl. Help would be
appreciated here.

Regards,
JakN�r��yb�X��ǧv�^�)޺{.n�+ا���ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf

[PATCH v3 1/2] sendemail: teach git-send-email to dump alias names

[Jacob Keller]

From: Jacob Keller 

Add an option "dump-aliases" which changes the default behavior of
git-send-email. This mode will simply read the alias files configured by
sendemail.aliasesfile and sendemail.aliasfiletype and dump a list of all
configured aliases, one per line. The intended use case for this option
is the bash-completion script which will use it to autocomplete aliases
on the options which take addresses.

Add some tests for the new option.

Signed-off-by: Jacob Keller 
---

Notes:
- v2
* Add command --list-aliases to git-send-email

- v3
* Add test
* change option to --dump-aliases
* dump both the alias and its expansion

 Documentation/git-send-email.txt | 10 ++
 git-send-email.perl  | 11 +++
 t/t9001-send-email.sh| 33 +
 3 files changed, 54 insertions(+)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index b9134d234f53..02f48e2258a6 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -10,6 +10,7 @@ SYNOPSIS
 
 [verse]
 'git send-email' [options] ...
+'git send-email' --list-aliases
 
 
 DESCRIPTION
@@ -387,6 +388,15 @@ default to '--validate'.
Send emails even if safety checks would prevent it.
 
 
+Information
+~~~
+
+--dump-aliases::
+   Instead of the standard operation, dump all aliases found in the
+   configured alias file(s), followed by its expansion. See
+   'sendemail.aliasesfile' for more information about aliases.
+
+
 CONFIGURATION
 -
 
diff --git a/git-send-email.perl b/git-send-email.perl
index e907e0eacf31..95ac9cf11155 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -46,6 +46,7 @@ package main;
 sub usage {
print < \$help,
"force" => \$force,
"xmailer!" => \$use_xmailer,
"no-xmailer" => sub {$use_xmailer = 0},
+"dump-aliases" => \$dump_aliases,
 );
 
 usage() if $help;
@@ -551,6 +557,11 @@ if (@alias_files and $aliasfiletype and defined 
$parse_alias{$aliasfiletype}) {
}
 }
 
+if ($dump_aliases) {
+print "$_ @{$aliases{$_}}\n" for (keys %aliases);
+exit(0);
+}
+
 # is_format_patch_arg($f) returns 0 if $f names a patch, or 1 if
 # $f is a revision list specification to be passed to format-patch.
 sub is_format_patch_arg {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 5b4a5ce06b94..bb537bb50b2b 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1579,6 +1579,24 @@ test_sendmail_aliases () {
'
 }
 
+test_sendmail_dump_aliases () {
+   msg="$1" && shift &&
+   expect="$@" &&
+   cat >.tmp-email-aliases &&
+
+   test_expect_success $PREREQ "$msg" '
+   clean_fake_sendmail && rm -fr outdir &&
+   git config --replace-all sendemail.aliasesfile \
+   "$(pwd)/.tmp-email-aliases" &&
+   git config sendemail.aliasfiletype sendmail &&
+   git send-email --dump-aliases 2>errors >out &&
+   for i in $expect
+   do
+   grep "$i" out || return 1
+   done
+   '
+}
+
 test_sendmail_aliases 'sendemail.aliasfiletype=sendmail' \
'awol@example\.com' \
'bob@example\.com' \
@@ -1593,6 +1611,21 @@ test_sendmail_aliases 'sendemail.aliasfiletype=sendmail' 
\
bcgrp: bob, chloe, Other 
EOF
 
+test_sendmail_dump_aliases '--dump-alias-names 
sendemail.aliasfiletype=sendmail' \
+   'alice' \
+   'bob' \
+   'chloe' \
+   'abgroup' \
+   'bcgrp' <<-\EOF
+   alice: Alice W Land 
+   bob: Robert Bobbyton 
+   # this is a comment
+  # this is also a comment
+   chloe: ch...@example.com
+   abgroup: alice, bob
+   bcgrp: bob, chloe, Other 
+   EOF
+
 test_sendmail_aliases 'sendmail aliases line folding' \
alice1 \
bob1 bob2 \
-- 
2.6.3.491.g3e3f6ce

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

[PATCH v3 2/2] completion: add support for completing email aliases

[Jacob Keller]

From: Jacob Keller 

Using the new --dump-alias-names option from git-send-email, add completion
for --to, --cc, and --bcc with the available configured aliases.

Signed-off-by: Jacob Keller 
---

Notes:
- v2
* Use git-send-email for parsing instead of re-implementing it in awk

- v3
* update for change to git-send-email
* add support for "--from"

 contrib/completion/git-completion.bash | 24 
 1 file changed, 24 insertions(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 482ca84b451b..1de8c0e6fc96 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -10,6 +10,7 @@
 #*) local and remote tag names
 #*) .git/remotes file names
 #*) git 'subcommands'
+#*) git email aliases for git-send-email
 #*) tree paths within 'ref:path/to/file' expressions
 #*) file paths within current working directory and index
 #*) common --long-options
@@ -1709,8 +1710,25 @@ _git_reflog ()
 __git_send_email_confirm_options="always never auto cc compose"
 __git_send_email_suppresscc_options="author self cc bodycc sob cccmd body all"
 
+__git_get_email_aliases ()
+{
+   git --git-dir="$(__gitdir)" send-email --dump-aliases 2>/dev/null | 
while read alias expansion
+   do
+   echo $alias
+   done
+}
+
 _git_send_email ()
 {
+   case "$prev" in
+   --to|--cc|--bcc|--from)
+   __gitcomp "
+   $(__git_get_email_aliases)
+   " "" ""
+   return
+   ;;
+   esac
+
case "$cur" in
--confirm=*)
__gitcomp "
@@ -1735,6 +1753,12 @@ _git_send_email ()
" "" "${cur##--thread=}"
return
;;
+   --to=*|--cc=*|--bcc=*|--from=*)
+   __gitcomp "
+   $(__git_get_email_aliases)
+   " "" "${cur#--*=}"
+   return
+   ;;
--*)
__gitcomp "--annotate --bcc --cc --cc-cmd --chain-reply-to
--compose --confirm= --dry-run --envelope-sender
-- 
2.6.3.491.g3e3f6ce

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

Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

[Keller, Jacob E]

On Mon, 2015-11-16 at 18:30 -0500, Eric Sunshine wrote:
> On Sun, Nov 15, 2015 at 3:22 PM, Jacob Keller  om> wrote:
> > Add an option "list-aliases" which changes the default behavior of
> > git-send-email. This mode will simply read the alias files
> > configured by
> > sendemail.aliasesfile and sendemail.aliasfiletype and print a list
> > of
> > all known aliases. The intended usecase for this option is the
> > bash-completion script which will use it to autocomplete aliases on
> > the
> > options which take addresses.
> 
> As this is primarily a plumbing option, I wonder if --dump-aliases
> might be a more suitable name.
> 

Sure that would be reasonable.

> Also, is it possible that some consumer down the road might want
> richer output which includes the expansion of each alias? For
> instance, it could emit the alias name as the first token on each
> line
> and the expansion as the remainder. Consumers interested in only the
> alias name would grab the first token on the line and ignore
> everything else.
> 

Maybe? The problem with printing the full address is that it may not be
quoted or similar, and it makes the bash completion require an extra
parameter.. I am not sure how valuable the alias expansion would be for
use? The main concern I have is we'd need to use another process on top
to extract only alias names.

> > Signed-off-by: Jacob Keller 
> > ---
> > diff --git a/git-send-email.perl b/git-send-email.perl
> > @@ -101,6 +102,9 @@ git send-email [options]  > rev-list options >
> >   `git format-patch` ones.
> >  --force* Send even if safety checks
> > would prevent it.
> > 
> > +  Information:
> > +--list-aliases * read the aliases from
> > configured alias files
> 
> This description is odd. It seems to imply that aliases will be
> loaded
> (and used) only if this option is given, and says nothing about its
> actual purpose of dumping the aliases.
> 

I can re-word this.

> Also, with one exception, all the other option descriptions are
> capitalized. This probably ought to follow suit.
> 
> > +if ($list_aliases) {
> > +print $_,"\n" for (keys %aliases);
> > +exit(0);
> > +}
> 
> New test(s) seem to be missing.
> 

I had removed the tests from the old version because they weren't
necessary anymore. New ones wouldn't hurt here either, though.. I'll
work on that.

Regards,
Jake

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

Re: [PATCH] check-ignore: correct documentation about output

[Stefan Beller]

+cc Jeff

On Mon, Nov 16, 2015 at 6:13 AM, Dennis Kaarsemaker
 wrote:
> Ping.

Junio is on vacation, so Jeff is our interim maintainer,
and it seems he isn't up to full power as Junio. :)
--
To unsubscribe from this list: send the line "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] Optimize usage of grep by passing -q

[Mikael Magnusson]

On Mon, Nov 16, 2015 at 10:43 PM, Stefan Beller  wrote:
> Instead of redirecting all grep output to /dev/null, we can just
> pass in -q instead. This preserves the exit code behavior, but is faster.
> As grep returns true if it finds at least one match, grep can exit promptly
> after finding the first line and doesn't need to find more occurrences
> which would be redirected to /dev/null anyways.
>
> This is true for the gnu version of grep. I am not sure if all
> versions of grep support this optimization. In case it is not,
> we'd revert this patch.

POSIX specifies -q, so you should be fine.
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/grep.html

-- 
Mikael Magnusson
--
To unsubscribe from this list: send the line "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] Optimize usage of grep by passing -q

[Stefan Beller]

+cc Andrey Rybak, who I credit for finding the reasoning below (he
sent to me privately,
without cc'ing the list)

On Mon, Nov 16, 2015 at 4:59 PM, Mikael Magnusson  wrote:
> On Mon, Nov 16, 2015 at 10:43 PM, Stefan Beller  wrote:
>> Instead of redirecting all grep output to /dev/null, we can just
>> pass in -q instead. This preserves the exit code behavior, but is faster.
>> As grep returns true if it finds at least one match, grep can exit promptly
>> after finding the first line and doesn't need to find more occurrences
>> which would be redirected to /dev/null anyways.
>>
>> This is true for the gnu version of grep. I am not sure if all
>> versions of grep support this optimization. In case it is not,
>> we'd revert this patch.
>
> POSIX specifies -q, so you should be fine.
> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/grep.html
>

>From http://www.gnu.org/software/grep/manual/grep.html :
-q
--quiet
--silent
Quiet; do not write anything to standard output. Exit immediately with
zero status if any match is found, even if an error was detected. Also
see the -s or --no-messages option. (-q is specified by POSIX.)
-s
--no-messages
Suppress error messages about nonexistent or unreadable files.
Portability note: unlike GNU grep, 7th Edition Unix grep did not
conform to POSIX, because it lacked -q and its -s option behaved like
GNU grep's -q option.1
USG-style grep also lacked -q but its -s option behaved like GNU
grep's. Portable shell scripts should avoid both -q and -s and should
redirect standard and error output to /dev/null instead. (-s is
specified by POSIX.)

Reading that in full, I think my patch is a bad idea.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1] blame: avoid checking if a file exists on the working tree if a revision is provided

[Edmundo Carmona Antoranz]

If a file has been deleted/renamed, blame refused to display
blame content even if the path provided does match an existing
blob on said revision.

$ git status
On branch hide
Changes not staged for commit:
  (use "git add/rm ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

deleted:testfile1.txt

no changes added to commit (use "git add" and/or "git commit -a")

Before:
$ git blame testfile1.txt
fatal: cannot stat path 'testfile1.txt': No such file or directory
$ git blame testfile1.txt HEAD
fatal: cannot stat path 'testfile1.txt': No such file or directory

After:
$ git blame testfile1.txt
fatal: Cannot lstat 'testfile1.txt': No such file or directory
$ git blame testfile1.txt HEAD
^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 1) 
testfile 2
^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 2)
^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 3) 
Some content

Signed-off-by: Edmundo Carmona Antoranz 
---
 builtin/blame.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 83612f5..db430d5 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2683,8 +2683,6 @@ parse_done:
argv[argc - 1] = "--";
 
setup_work_tree();
-   if (!file_exists(path))
-   die_errno("cannot stat path '%s'", path);
}
 
revs.disable_stdin = 1;
-- 
2.6.2

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

Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

[Eric Sunshine]

On Sun, Nov 15, 2015 at 3:22 PM, Jacob Keller  wrote:
> Add an option "list-aliases" which changes the default behavior of
> git-send-email. This mode will simply read the alias files configured by
> sendemail.aliasesfile and sendemail.aliasfiletype and print a list of
> all known aliases. The intended usecase for this option is the
> bash-completion script which will use it to autocomplete aliases on the
> options which take addresses.

As this is primarily a plumbing option, I wonder if --dump-aliases
might be a more suitable name.

Also, is it possible that some consumer down the road might want
richer output which includes the expansion of each alias? For
instance, it could emit the alias name as the first token on each line
and the expansion as the remainder. Consumers interested in only the
alias name would grab the first token on the line and ignore
everything else.

> Signed-off-by: Jacob Keller 
> ---
> diff --git a/git-send-email.perl b/git-send-email.perl
> @@ -101,6 +102,9 @@ git send-email [options]  options >
>   `git format-patch` ones.
>  --force* Send even if safety checks would 
> prevent it.
>
> +  Information:
> +--list-aliases * read the aliases from configured alias 
> files

This description is odd. It seems to imply that aliases will be loaded
(and used) only if this option is given, and says nothing about its
actual purpose of dumping the aliases.

Also, with one exception, all the other option descriptions are
capitalized. This probably ought to follow suit.

> +if ($list_aliases) {
> +print $_,"\n" for (keys %aliases);
> +exit(0);
> +}

New test(s) seem to be missing.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] sendemail: teach git-send-email to list aliases

[Eric Sunshine]

On Mon, Nov 16, 2015 at 6:50 PM, Eric Sunshine  wrote:
> Also, shouldn't --list-aliases (or --dump-aliases) be mutually
> exclusive with many of the other options? New tests would check such
> exclusivity as well.

In fact, while I agree with Szeder that it makes sense to re-use
send-email's aliases parsing functionality (and was going to suggest
the same, but he beat me to it), this new option is awfully orthogonal
to the overall purpose of send-email, thus, doesn't really fit in well
and almost cries out for a command of its own which would be used by
send-email and bash completion (though I'm not convinced that it's
worth going that route for this one minor use-case).
--
To unsubscribe from this list: send the line "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] blame: avoid checking if a file exists on the working tree if a revision is provided

[Edmundo Carmona Antoranz]

If a file has been deleted/renamed, blame refused to display
blame content even if the path provided does match an existing
blob on said revision.

$ git status
On branch hide
Changes not staged for commit:
  (use "git add/rm ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

deleted:testfile1.txt

no changes added to commit (use "git add" and/or "git commit -a")

Before:
$ git blame testfile1.txt
fatal: cannot stat path 'testfile1.txt': No such file or directory
$ git blame testfile1.txt HEAD
fatal: cannot stat path 'testfile1.txt': No such file or directory

After:
$ git blame testfile1.txt
fatal: Cannot lstat 'testfile1.txt': No such file or directory
$ git blame testfile1.txt HEAD
^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 1) 
testfile 2
^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 2)
^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 3) 
Some content

Signed-off-by: Edmundo Carmona Antoranz 
---
 builtin/blame.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 83612f5..856971a 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2683,12 +2683,13 @@ parse_done:
argv[argc - 1] = "--";
 
setup_work_tree();
-   if (!file_exists(path))
-   die_errno("cannot stat path '%s'", path);
}
 
revs.disable_stdin = 1;
setup_revisions(argc, argv, , NULL);
+   if (!revs.pending.nr && !file_exists(path))
+   die_errno("cannot stat path '%s'", path);
+
memset(, 0, sizeof(sb));
 
sb.revs = 
-- 
2.6.2

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

Re: [PATCH v1] blame: avoid checking if a file exists on the working tree if a revision is provided

[Edmundo Carmona Antoranz]

On Mon, Nov 16, 2015 at 7:07 PM, Edmundo Carmona Antoranz
 wrote:
> -   if (!file_exists(path))
> -   die_errno("cannot stat path '%s'", path);

Two things:
1
What I would love to do is check if revisions were provided. Something like:

if (no_revisions() && !file_exists(path))
die_errno("cannot stat path '%s'", path);

_but_ revisions are set up a little bit later. I don't know right now
if I could just move it up (I don't think it would be that simple
because I see there's some messing up with argv and argc in that 'if'
that encloses the lines I'm removing). Maybe it would make sense to
move the check for file existence to _after_ revisions have been set
up? Even without the check for revisions, it's behaving kind of the
way I mean it to. But I'm sure it'd be more elegant if I checked if
revisions were provided.

2 It makes sense to create test cases for this patch, right?

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

Re: [PATCH v1] blame: avoid checking if a file exists on the working tree if a revision is provided

[Edmundo Carmona Antoranz]

Ok... I think I got how to check revisions / file existence after
revisions have been set up.

Sending next patch version

On Mon, Nov 16, 2015 at 7:16 PM, Edmundo Carmona Antoranz
 wrote:
> On Mon, Nov 16, 2015 at 7:07 PM, Edmundo Carmona Antoranz
>  wrote:
>> -   if (!file_exists(path))
>> -   die_errno("cannot stat path '%s'", path);
>
> Two things:
> 1
> What I would love to do is check if revisions were provided. Something like:
>
> if (no_revisions() && !file_exists(path))
> die_errno("cannot stat path '%s'", path);
>
> _but_ revisions are set up a little bit later. I don't know right now
> if I could just move it up (I don't think it would be that simple
> because I see there's some messing up with argv and argc in that 'if'
> that encloses the lines I'm removing). Maybe it would make sense to
> move the check for file existence to _after_ revisions have been set
> up? Even without the check for revisions, it's behaving kind of the
> way I mean it to. But I'm sure it'd be more elegant if I checked if
> revisions were provided.
>
> 2 It makes sense to create test cases for this patch, right?
>
> Looking forward to your comments. Thanks in advance.
--
To unsubscribe from this list: send the line "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] blame: avoid checking if a file exists on the working tree if a revision is provided

[Eric Sunshine]

On Mon, Nov 16, 2015 at 8:29 PM, Edmundo Carmona Antoranz
 wrote:
> blame: avoid checking if a file exists on the working tree
> if a revision is provided

This subject is a bit long; try to keep it to about 72 characters or less.

More importantly, though, it doesn't give us a high level overview of
the purpose of the patch, which is that it is fixing blame to work on
a file at a given revision even if the file no longer exists in the
worktree.

> If a file has been deleted/renamed, blame refused to display

Imperative: s/refused/refuses/

> blame content even if the path provided does match an existing
> blob on said revision.

git-blame documentation does not advertise "blame  " as a
valid invocation. It does advertise "blame  -- ", and this
case already works correctly even when  does not exist in the
worktree.

git-annotate documentation, on the other hand, does advertise
"annotate  ", and it fails to work when  is absent
from the worktree, so perhaps you want to sell this patch as fixing
git-annotate instead?

> $ git status
> On branch hide
> Changes not staged for commit:
>   (use "git add/rm ..." to update what will be committed)
>   (use "git checkout -- ..." to discard changes in working directory)
>
> deleted:testfile1.txt
>
> no changes added to commit (use "git add" and/or "git commit -a")
>
> Before:
> $ git blame testfile1.txt
> fatal: cannot stat path 'testfile1.txt': No such file or directory
> $ git blame testfile1.txt HEAD
> fatal: cannot stat path 'testfile1.txt': No such file or directory
>
> After:
> $ git blame testfile1.txt
> fatal: Cannot lstat 'testfile1.txt': No such file or directory
> $ git blame testfile1.txt HEAD
> ^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 1) 
> testfile 2
> ^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 2)
> ^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 3) 
> Some content

This example is certainly illustrative, but an even more common case
may be trying to blame/annotate a file which existed in an older
revision but doesn't exist anymore at HEAD, thus is absent from the
worktree. Such a case could likely be described in a sentence or two
without resorting to verbose examples (though, not a big deal if you
keep the example).

A new test or two would be welcome, possibly in t/annotate-tests.sh if
you consider this also fixing git-blame, or in t8001-annotate.sh if
you're selling it only as a fix for git-annotate.

> Signed-off-by: Edmundo Carmona Antoranz 
> ---
> diff --git a/builtin/blame.c b/builtin/blame.c
> @@ -2683,12 +2683,13 @@ parse_done:
> argv[argc - 1] = "--";
>
> setup_work_tree();
> -   if (!file_exists(path))
> -   die_errno("cannot stat path '%s'", path);
> }
>
> revs.disable_stdin = 1;
> setup_revisions(argc, argv, , NULL);
> +   if (!revs.pending.nr && !file_exists(path))
> +   die_errno("cannot stat path '%s'", path);
> +
> memset(, 0, sizeof(sb));
>
> sb.revs = 
> --
> 2.6.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] diff-highlight: match up lines before highlighting

[Jonathan Lebon]

On Tue, Nov 3, 2015 at 5:03 PM, Jeff King  wrote:
> Your is _much_ slower. I get:
>
>   real0m25.538s
>   user0m25.420s
>   sys 0m0.120s
>
> for the old versus:
>
>   real2m3.580s
>   user2m3.548s
>   sys 0m0.156s

Thanks for investigating and trying it out. I got the same results
here as well.

> for your series. In an interactive setting, the latency may not be that
> noticeable, but if you are digging far into history (e.g., "git log -p",
> then using "/" in less to search for a commit or some test), I suspect
> it would be very noticeable.

Agreed.

> I was thinking there was some low-hanging fruit in memoizing the
> calculations, but even the prefix/suffix computation is pairwise. I'm
> not really sure how to make this much faster.

I gave memoization a try to see if it could improve the situation. I also
lowered maxhunksize to 10. Doing `git log -p` on git.git went from 2m31
to 2m11. So I think it would require a whole other approach overall.

> As for the output itself, the diff between the two looks promising. The
> first several cases I looked at ar strict improvements. Some of them are
> kind of weird, especially in English text.

Yes, I'm very happy with the improvements and run with these patches all
the time for now.

> In the other thread I mentioned earlier, the solution I cooked up was
> dropping highlighting entirely for hunks over a certain percentage of
> highlighting. I wonder if we could do something similar here (e.g.,
> don't match lines where more than 50% of the line would be highlighted).

I looked over but haven't tested the patches in the other thread yet. But
overall, the LCS definitely looks promising. I'm hoping to find some time
to have a more serious go at it and maybe pick it up where you left off.

>
> -Peff

Thanks again for reviewing these patches and apologies for the delayed
reply.

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 v5 3/6] git-p4: retry kill/cleanup operations in tests with timeout

[Luke Diamand]

On 15/11/15 13:08, larsxschnei...@gmail.com wrote:

From: Lars Schneider 

In rare cases kill/cleanup operations in tests fail. Retry these
operations with a timeout to make the test less flaky.


Should there be a sleep in that retry_until_success loop so that it 
doesn't spin sending signals to p4d?


Do we need to worry about the time offset being updated (e.g. NTP) while 
this is running?




Signed-off-by: Lars Schneider 
---
  t/lib-git-p4.sh | 31 +++
  1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 7548225..8d6b48f 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -6,6 +6,10 @@
  # a subdirectory called "$git"
  TEST_NO_CREATE_REPO=NoThanks

+# Some operations require multiple attempts to be successful. Define
+# here the maximal retry timeout in seconds.
+RETRY_TIMEOUT=60
+
  . ./test-lib.sh

  if ! test_have_prereq PYTHON
@@ -121,22 +125,33 @@ p4_add_user() {
EOF
  }

+retry_until_success() {
+timeout=$(($(date +%s) + $RETRY_TIMEOUT))
+until "$@" 2>/dev/null || test $(date +%s) -gt $timeout
+do :


 sleep here?


+done
+}
+
+retry_until_fail() {
+timeout=$(($(date +%s) + $RETRY_TIMEOUT))
+until ! "$@" 2>/dev/null || test $(date +%s) -gt $timeout
+do :


 sleep here?


+done
+}
+
  kill_p4d() {
pid=$(cat "$pidfile")
-   # it had better exist for the first kill
-   kill $pid &&
-   for i in 1 2 3 4 5 ; do
-   kill $pid >/dev/null 2>&1 || break
-   sleep 1
-   done &&
+   retry_until_fail kill $pid
+   retry_until_fail kill -9 $pid
# complain if it would not die
test_must_fail kill $pid >/dev/null 2>&1 &&
rm -rf "$db" "$cli" "$pidfile"
  }

  cleanup_git() {
-   rm -rf "$git" &&
-   mkdir "$git"
+   retry_until_success rm -r "$git"
+   test_must_fail test -d "$git" &&
+   retry_until_success mkdir "$git"
  }

  marshal_dump() {



--
To unsubscribe from this list: send the line "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 4/6] git-p4: add p4d timeout in tests

[Luke Diamand]

On 15/11/15 13:08, larsxschnei...@gmail.com wrote:

From: Lars Schneider 

In rare cases p4d seems to hang. This watchdog will kill the p4d
process after 300s in any case. That means each individual git p4 test
needs to finish before 300s or it will fail.


Looks good to me.




Signed-off-by: Lars Schneider 
---
  t/lib-git-p4.sh | 20 +++-
  1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 8d6b48f..f2a009c 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -10,6 +10,10 @@ TEST_NO_CREATE_REPO=NoThanks
  # here the maximal retry timeout in seconds.
  RETRY_TIMEOUT=60

+# Sometimes p4d seems to hang. Terminate the p4d process automatically after
+# the defined timeout in seconds.
+P4D_TIMEOUT=300
+
  . ./test-lib.sh

  if ! test_have_prereq PYTHON
@@ -85,6 +89,19 @@ start_p4d() {
# will be caught with the "kill -0" check below.
i=${P4D_START_PATIENCE:-300}
pid=$(cat "$pidfile")
+
+   timeout=$(($(date +%s) + $P4D_TIMEOUT))
+   while true
+   do
+   if test $(date +%s) -gt $timeout
+   then
+   kill -9 $pid
+   exit 1
+   fi
+   sleep 1
+   done &
+   watchdog_pid=$!
+
ready=
while test $i -gt 0
do
@@ -145,7 +162,8 @@ kill_p4d() {
retry_until_fail kill -9 $pid
# complain if it would not die
test_must_fail kill $pid >/dev/null 2>&1 &&
-   rm -rf "$db" "$cli" "$pidfile"
+   rm -rf "$db" "$cli" "$pidfile" &&
+   retry_until_fail kill -9 $watchdog_pid
  }

  cleanup_git() {



--
To unsubscribe from this list: send the line "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 5/6] git-p4: add trap to kill p4d on test exit

[Luke Diamand]

On 15/11/15 13:08, larsxschnei...@gmail.com wrote:

From: Lars Schneider 

Sometimes the "prove" test runner hangs on test exit because p4d is
still running. Add a trap to always kill "p4d" on test exit.


With this change, I've started seeing this when running the tests:

cat: /home/lgd/git/git/t/trash 
directory.t9819-git-p4-case-folding/p4d.pid: No such file or directory


Probably just needs the obvious "test -f" adding.

Other than, all looks good to me. Particularly nice that I can now do:

$ make T=t98* -j10

and it actually works!







You can reproduce the problem by commenting "P4D_TIMEOUT" in
"lib-git-p4.sh" and running "prove ./t9800-git-p4-basic.sh".
---
  t/lib-git-p4.sh | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index f2a009c..f9c68d4 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -65,6 +65,12 @@ cli="$TRASH_DIRECTORY/cli"
  git="$TRASH_DIRECTORY/git"
  pidfile="$TRASH_DIRECTORY/p4d.pid"

+# Sometimes "prove" seems to hang on exit because p4d is still running
+cleanup() {
+   kill -9 $(cat "$pidfile") 2>/dev/null && exit 255
+}
+trap cleanup EXIT
+
  # git p4 submit generates a temp file, which will
  # not get cleaned up if the submission fails.  Don't
  # clutter up /tmp on the test machine.



--
To unsubscribe from this list: send the line "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: opening an editor from git-gui on a file

[stefan.naewe]

Am 16.11.2015 um 08:58 schrieb Adam GROSZER:
> Hi there,
> 
> Hopefully sending the question/idea to the right list...
> 
> I'm missing the feature to open a (configurable) editor with the
> currently selected file in git gui.
> 
> E.g. Looking at
> 
> https://cdn.tutsplus.com/net/uploads/legacy/2081_gitwin/git-gui-stage.jpg
> 
> I'd like to open my editor with the "request.php".
> 
> Any chance to have that? Or do I miss something?

I have this in my .gitconfig:

[guitool "Edit/with GVim"]
cmd = gvim --remote-tab-silent $FILENAME
noconsole = yes
needsfile = yes

Works for me.

HTH,
  Stefan
-- 

/dev/random says: Why did Kamakazie pilots wear helmets???
python -c "print 
'73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"
 
GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9  9666 829B 49C5 9221 27AF

Re: opening an editor from git-gui on a file

[Adam GROSZER]

Thank you!

Works fine. Tho I don't find any docs/references for a hotkey...

On 11/16/2015 11:38 AM, stefan.na...@atlas-elektronik.com wrote:

Am 16.11.2015 um 08:58 schrieb Adam GROSZER:

Hi there,

Hopefully sending the question/idea to the right list...

I'm missing the feature to open a (configurable) editor with the
currently selected file in git gui.

E.g. Looking at

https://cdn.tutsplus.com/net/uploads/legacy/2081_gitwin/git-gui-stage.jpg

I'd like to open my editor with the "request.php".

Any chance to have that? Or do I miss something?


I have this in my .gitconfig:

[guitool "Edit/with GVim"]
 cmd = gvim --remote-tab-silent $FILENAME
 noconsole = yes
 needsfile = yes

Works for me.

HTH,
   Stefan



--
Best regards,
 Adam GROSZER
--
Quote of the day:
Rudin's Law: In a crisis that forces a choice to be made among 
alternative courses of action, most people will choose the worst one 
possible.

--
To unsubscribe from this list: send the line "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] allow hooks to ignore their standard input stream

[Clemens Buchacher]

Hi Junio,

I believe we have finalized the discussion on this patch. Please apply

On Fri, Nov 13, 2015 at 06:23:20PM -0500, Jeff King wrote:
> 
> > +test_expect_success 'filling pipe buffer does not cause failure' '
> > +   git push parent1 "refs/heads/b/*:refs/heads/b/*" &&
> > +   test_cmp expected actual
> > +'
> 
> It actually _does_ read all of the input, but I guess is making sure we
> call write() in a loop. I don't know if this is even worth keeping.
> 
> Can you think of a good reason that it is checking something
> interesting?

No, I also cannot think of a good reason to keep it. Here is the patch
with the test above removed.

Best regards,
Clemens

--o<--
Since ec7dbd145 (receive-pack: allow hooks to ignore its standard input stream)
the pre-receive and post-receive hooks ignore SIGPIPE. Do the same for the
remaining hooks pre-push and post-rewrite, which read from standard input. The
same arguments for ignoring SIGPIPE apply.

Include test by Jeff King which checks that SIGPIPE does not cause
pre-push hook failure. With the use of git update-ref --stdin it is fast
enough to be enabled by default.

Signed-off-by: Clemens Buchacher 
---
 builtin/commit.c |  3 +++
 t/t5571-pre-push-hook.sh | 33 +++--
 transport.c  | 11 +--
 3 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index dca09e2..f2a8b78 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -32,6 +32,7 @@
 #include "sequencer.h"
 #include "notes-utils.h"
 #include "mailmap.h"
+#include "sigchain.h"
 
 static const char * const builtin_commit_usage[] = {
N_("git commit [] [--] ..."),
@@ -1537,8 +1538,10 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
return code;
n = snprintf(buf, sizeof(buf), "%s %s\n",
 sha1_to_hex(oldsha1), sha1_to_hex(newsha1));
+   sigchain_push(SIGPIPE, SIG_IGN);
write_in_full(proc.in, buf, n);
close(proc.in);
+   sigchain_pop(SIGPIPE);
return finish_command();
 }
 
diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh
index 6f9916a..ba975bb 100755
--- a/t/t5571-pre-push-hook.sh
+++ b/t/t5571-pre-push-hook.sh
@@ -109,23 +109,20 @@ test_expect_success 'push to URL' '
diff expected actual
 '
 
-# Test that filling pipe buffers doesn't cause failure
-# Too slow to leave enabled for general use
-if false
-then
-   printf 'parent1\nrepo1\n' >expected
-   nr=1000
-   while test $nr -lt 2000
-   do
-   nr=$(( $nr + 1 ))
-   git branch b/$nr $COMMIT3
-   echo "refs/heads/b/$nr $COMMIT3 refs/heads/b/$nr $_z40" 
>>expected
-   done
-
-   test_expect_success 'push many refs' '
-   git push parent1 "refs/heads/b/*:refs/heads/b/*" &&
-   diff expected actual
-   '
-fi
+test_expect_success 'set up many-ref tests' '
+   {
+   nr=1000
+   while test $nr -lt 2000
+   do
+   nr=$(( $nr + 1 ))
+   echo "create refs/heads/b/$nr $COMMIT3"
+   done
+   } | git update-ref --stdin
+'
+
+test_expect_success 'sigpipe does not cause pre-push hook failure' '
+   echo "exit 0" | write_script "$HOOK" &&
+   git push parent1 "refs/heads/b/*:refs/heads/b/*"
+'
 
 test_done
diff --git a/transport.c b/transport.c
index 23b2ed6..e34ab92 100644
--- a/transport.c
+++ b/transport.c
@@ -15,6 +15,7 @@
 #include "submodule.h"
 #include "string-list.h"
 #include "sha1-array.h"
+#include "sigchain.h"
 
 /* rsync support */
 
@@ -1127,6 +1128,8 @@ static int run_pre_push_hook(struct transport *transport,
return -1;
}
 
+   sigchain_push(SIGPIPE, SIG_IGN);
+
strbuf_init(, 256);
 
for (r = remote_refs; r; r = r->next) {
@@ -1140,8 +1143,10 @@ static int run_pre_push_hook(struct transport *transport,
 r->peer_ref->name, sha1_to_hex(r->new_sha1),
 r->name, sha1_to_hex(r->old_sha1));
 
-   if (write_in_full(proc.in, buf.buf, buf.len) != buf.len) {
-   ret = -1;
+   if (write_in_full(proc.in, buf.buf, buf.len) < 0) {
+   /* We do not mind if a hook does not read all refs. */
+   if (errno != EPIPE)
+   ret = -1;
break;
}
}
@@ -1152,6 +1157,8 @@ static int run_pre_push_hook(struct transport *transport,
if (!ret)
ret = x;
 
+   sigchain_pop(SIGPIPE);
+
x = finish_command();
if (!ret)
ret = x;
-- 
1.9.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 1/2] sendemail: teach git-send-email to list aliases

[Eric Sunshine]

On Tue, Nov 17, 2015 at 12:10:35AM +, Keller, Jacob E wrote:
> On Mon, 2015-11-16 at 18:50 -0500, Eric Sunshine wrote:
> > It should be possible to extract the alias within the shell itself
> > without a separate process. For instance:
> > 
> > read alias rest
> > 
> > will leave the first token in $alias and the remainder of the line in
> > $rest, and it's all done within the shell process.
> 
> I'll look into this :)

My reason for asking is concern about scripts possibly breaking if
someone comes along and wants to "fix" --dump-aliases to also dump
the alias expansions. One possibility is just to punt today and say
that when that feature is needed in the future, then that someone can
add a --verbose option to complement --dump-aliases which would emit
the alias expansions as well. One nice thing about punting at this
point is that we don't (today) have to define a format for the output
of the expansions. If we did want to think about it, one verbose,
non-ambiguous format would be to show the alias name on a line by
itself, and each expansion value on a line by itself indented by a
tab. For instance:

managers
bob
fred
devs
jane
john

> > Also, shouldn't --list-aliases (or --dump-aliases) be mutually
> > exclusive with many of the other options? New tests would check such
> > exclusivity as well.
> 
> I am at a loss for how to do that correctly in the perl. Help would be
> appreciated here.

Since git-send-email.perl already configures GetOpt::Long with the
'pass_through' option, one possibility would be to invoke
GetOptions() once for --list-aliases (or --dump-aliases), and then
again for the normal options. Doing so may be a bit ugly; on the
other hand, it does indicate pretty clearly that --list-aliases is a
distinct "mode" of operation. On top of your patch, it might look
something like this:

--- 8< ---
diff --git a/git-send-email.perl b/git-send-email.perl
index ee14894..cada5ea 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -296,6 +296,12 @@ $SIG{INT}  = \_handler;
 
 my $help;
 my $rc = GetOptions("h" => \$help,
+   "list-aliases" => \$list_aliases);
+usage() unless $rc;
+die "--list-aliases incompatible with other options\n"
+   if !$help and $list_aliases and @ARGV;
+
+$rc = GetOptions(
"sender|from=s" => \$sender,
 "in-reply-to=s" => \$initial_reply_to,
"subject=s" => \$initial_subject,
@@ -349,7 +355,6 @@ my $rc = GetOptions("h" => \$help,
"force" => \$force,
"xmailer!" => \$use_xmailer,
"no-xmailer" => sub {$use_xmailer = 0},
-"list-aliases" => \$list_aliases,
 );
 
 usage() if $help;
--- 8< ---

Though, it may be overkill for this minor use-case.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html