Git development
 help / color / mirror / Atom feed
* Re: [PATCH 13/17] refs: convert each_reflog_ent_fn to struct object_id
From: Michael Haggerty @ 2017-01-02 15:07 UTC (permalink / raw)
  To: brian m. carlson, git; +Cc: Jeff King
In-Reply-To: <20170101191847.564741-14-sandals@crustytoothpaste.net>

On 01/01/2017 08:18 PM, brian m. carlson wrote:
> Make each_reflog_ent_fn take two struct object_id pointers instead of
> two pointers to unsigned char.  Convert the various callbacks to use
> struct object_id as well.  Also, rename fsck_handle_reflog_sha1 to
> fsck_handle_reflog_oid.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  builtin/fsck.c       | 16 ++++++++--------
>  builtin/merge-base.c |  6 +++---
>  builtin/reflog.c     |  2 +-
>  reflog-walk.c        |  6 +++---
>  refs.c               | 24 ++++++++++++------------
>  refs.h               |  2 +-
>  refs/files-backend.c | 24 ++++++++++++------------
>  revision.c           | 12 ++++++------
>  sha1_name.c          |  2 +-
>  wt-status.c          |  6 +++---
>  10 files changed, 50 insertions(+), 50 deletions(-)
> 
> [...]
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index f9023939d..3da3141ee 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -3113,15 +3113,15 @@ static int files_delete_reflog(struct ref_store *ref_store,
>  
>  static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *cb_data)
>  {
> -	unsigned char osha1[20], nsha1[20];
> +	struct object_id ooid, noid;
>  	char *email_end, *message;
>  	unsigned long timestamp;
>  	int tz;
>  
>  	/* old SP new SP name <email> SP time TAB msg LF */
>  	if (sb->len < 83 || sb->buf[sb->len - 1] != '\n' ||
> -	    get_sha1_hex(sb->buf, osha1) || sb->buf[40] != ' ' ||
> -	    get_sha1_hex(sb->buf + 41, nsha1) || sb->buf[81] != ' ' ||
> +	    get_oid_hex(sb->buf, &ooid) || sb->buf[40] != ' ' ||
> +	    get_oid_hex(sb->buf + 41, &noid) || sb->buf[81] != ' ' ||

Some magic numbers above could be converted to use constants.

>  	    !(email_end = strchr(sb->buf + 82, '>')) ||
>  	    email_end[1] != ' ' ||
>  	    !(timestamp = strtoul(email_end + 2, &message, 10)) ||
> @@ -3136,7 +3136,7 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c
>  		message += 6;
>  	else
>  		message += 7;
> -	return fn(osha1, nsha1, sb->buf + 82, timestamp, tz, message, cb_data);
> +	return fn(&ooid, &noid, sb->buf + 82, timestamp, tz, message, cb_data);

Here, too.

>  }
>  
>  static char *find_beginning_of_line(char *bob, char *scan)
> [...]
> @@ -4047,14 +4047,14 @@ static int files_reflog_expire(struct ref_store *ref_store,
>  		 */
>  		int update = (flags & EXPIRE_REFLOGS_UPDATE_REF) &&
>  			!(type & REF_ISSYMREF) &&
> -			!is_null_sha1(cb.last_kept_sha1);
> +			!is_null_oid(&cb.last_kept_oid);
>  
>  		if (close_lock_file(&reflog_lock)) {
>  			status |= error("couldn't write %s: %s", log_file,
>  					strerror(errno));
>  		} else if (update &&
>  			   (write_in_full(get_lock_file_fd(lock->lk),
> -				sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
> +				oid_to_hex(&cb.last_kept_oid), 40) != 40 ||

More magic numbers above.

>  			    write_str_in_full(get_lock_file_fd(lock->lk), "\n") != 1 ||
>  			    close_ref(lock) < 0)) {
>  			status |= error("couldn't write %s",
> [...]

I thought it would make sense to convert `struct read_ref_at_cb` in
`refs.c` to use `struct object_id` at the same time, but I see that
would require the interface to `read_ref_at()` to change. I guess it's
important to pick your battles in this campaign :-)

Michael


^ permalink raw reply

* Re: [PATCH v2 20/34] sequencer (rebase -i): copy commit notes at end
From: Johannes Schindelin @ 2017-01-02 14:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqqvaujr7ed.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Fri, 16 Dec 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > @@ -1750,6 +1797,17 @@ static int is_final_fixup(struct todo_list *todo_list)
> >  	return 1;
> >  }
> >  
> > +static enum todo_command peek_command(struct todo_list *todo_list, int offset)
> > +{
> > +	int i;
> > +
> > +	for (i = todo_list->current + offset; i < todo_list->nr; i++)
> > +		if (todo_list->items[i].command != TODO_NOOP)
> > +			return todo_list->items[i].command;
> 
> Makes me wonder, after having commented on 07/34 regarding the fact
> that in the end you would end up having three variants of no-op
> (i.e. NOOP, DROP and COMMENT), what definition of a "command" this
> function uses to return its result, when asked to "peek".

Well, it uses the todo_command idea of a "command"... ;-)

The only thing we do with this for now is to look whether the next command
is a fixup/squash (so that the user gets to edit the commit message just
once, for example, and also to record rewritten commits properly).

> I suspect that this will be updated in a later patch to do "< TODO_NOOP"
> instead?

Actually, no. I introduced a new function is_noop() and that is used now.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH v2 28/34] run_command_opt(): optionally hide stderr when the command succeeds
From: Johannes Schindelin @ 2017-01-02 14:38 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Jeff King, git, Junio C Hamano, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <f4a72743-3488-3466-5b9f-0dacec102a54@kdbg.org>

Hi Hannes,

On Wed, 14 Dec 2016, Johannes Sixt wrote:

> Am 14.12.2016 um 14:06 schrieb Jeff King:
> > On Wed, Dec 14, 2016 at 07:53:23AM -0500, Jeff King wrote:
> >
> > > I don't have a strong opinion on the patches under discussion, but
> > > here are a few pointers on the run-command interface:
> > > [...]
> >
> > And here is a patch representing my suggestions, on top of yours. Not
> > tested beyond "make test".
> 
> Thank you, that looks way better.
> 
> If there is agreement that this approach is preferable, I think we can
> have patches on top of the series; they would be orthogonal and do not
> have to take hostage of it. (And it looks like I won't be able to follow
> up until later this week[end].)

Seeing as the original intention was to do away with the
RUN_HIDE_STDERR_ON_SUCCESS flag, and that the sequencer-i branch *must*
include that functionality somehow, it is unfortunately not really
possible to do this on top of the patch series.

I say "unfortunately" because I feel pretty uncomfortable with replacing
something that has been tried and tested by something that still awaits
the test of time.

So the only possible course of action I see is to go the really long
route: incorporate the patches to use pipe_command() instead of
introducing a new RUN_* flag (which means basically munch up your patch
and Peff's and move it somewhere into the middle of the sequencer-i patch
series, which is exactly what I already did locally), cook the patches
beyond recognition in `next`, i.e. cook it really long to give it a really
good testing before moving the patches to `master`.

Ciao,
Johannes

^ permalink raw reply

* Re: [PATCH 09/17] builtin/merge: convert to struct object_id
From: Michael Haggerty @ 2017-01-02 14:34 UTC (permalink / raw)
  To: brian m. carlson, git; +Cc: Jeff King
In-Reply-To: <20170101191847.564741-10-sandals@crustytoothpaste.net>

On 01/01/2017 08:18 PM, brian m. carlson wrote:
> Additionally convert several uses of the constant 40 into
> GIT_SHA1_HEXSZ.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  builtin/merge.c | 136 ++++++++++++++++++++++++++++----------------------------
>  1 file changed, 68 insertions(+), 68 deletions(-)
> 
> [...]
> @@ -437,25 +437,25 @@ static void merge_name(const char *remote, struct strbuf *msg)
>  	strbuf_branchname(&bname, remote);
>  	remote = bname.buf;
>  
> -	memset(branch_head, 0, sizeof(branch_head));
> +	memset(&branch_head, 0, sizeof(branch_head));

I think this could be

        oidclr(&branch_head);

>  	remote_head = get_merge_parent(remote);
>  	if (!remote_head)
>  		die(_("'%s' does not point to a commit"), remote);
> [...]
> @@ -1113,9 +1113,9 @@ static struct commit_list *collect_parents(struct commit *head_commit,
>  
>  int cmd_merge(int argc, const char **argv, const char *prefix)
>  {
> -	unsigned char result_tree[20];
> -	unsigned char stash[20];
> -	unsigned char head_sha1[20];
> +	struct object_id result_tree;
> +	struct object_id stash;
> +	struct object_id head_oid;

These could comfortably be declared on a single line now.

>  	struct commit *head_commit;
>  	struct strbuf buf = STRBUF_INIT;
>  	const char *head_arg;
> [...]

Michael


^ permalink raw reply

* Re: [PATCH v3 20/21] Documentation/config: add splitIndex.sharedIndexExpire
From: Christian Couder @ 2017-01-02 14:33 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð Bjarmason,
	Christian Couder
In-Reply-To: <xmqqeg0t9oct.fsf@gitster.mtv.corp.google.com>

On Tue, Dec 27, 2016 at 8:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>>  Documentation/config.txt | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index e0f5a77980..24e771d22e 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -2786,6 +2786,17 @@ splitIndex.maxPercentChange::
>>       than 20 percent of the total number of entries.
>>       See linkgit:git-update-index[1].
>>
>> +splitIndex.sharedIndexExpire::
>> +     When the split index feature is used, shared index files with
>> +     a mtime older than this time will be removed when a new shared
>
> As end-user facing documentation, it would be much better if we can
> rephrase it for those who do not know what a 'mtime' is, and it
> would be even better if we can do so without losing precision.
>
> I think "shared index files that were not modified since the time
> this variable specifies will be removed" would be understandable and
> correct enough?

Yeah, I agree it is better for end users. I will use what you suggest.

>> +     index file is created. The value "now" expires all entries
>> +     immediately, and "never" suppresses expiration altogether.
>> +     The default value is "one.week.ago".
>> +     Note that each time a new split-index file is created, the
>> +     mtime of the related shared index file is updated to the
>> +     current time.
>
> To match the above suggestion, "Note that a shared index file is
> considered modified (for the purpose of expiration) each time a new
> split-index file is created based on it."?

Yeah, I also agree it is better and will use that.

^ permalink raw reply

* Re: [PATCH v3 14/21] read-cache: touch shared index files when used
From: Christian Couder @ 2017-01-02 14:09 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð Bjarmason,
	Christian Couder
In-Reply-To: <xmqqa8bhb32x.fsf@gitster.mtv.corp.google.com>

On Tue, Dec 27, 2016 at 8:10 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> +/*
>> + * Signal that the shared index is used by updating its mtime.
>> + *
>> + * This way, shared index can be removed if they have not been used
>> + * for some time. It's ok to fail to update the mtime if we are on a
>> + * read only file system.
>> + */
>> +void freshen_shared_index(char *base_sha1_hex)
>> +{
>> +     const char *shared_index = git_path("sharedindex.%s", base_sha1_hex);
>> +     check_and_freshen_file(shared_index, 1);
>
> What happens when this call fails?  The function returns 0 if the
> file did not even exist.  It also returns 0 if you cannot update its
> timestamp.

Yeah and I don't think it's a problem in either case.
If we cannot update its timestamp, it's not a problem, as we could be
on a read-only file system, and you said in a previous iteration that
we should not even warn in this case.
If the file does not exist, it could be because it has just been
deleted for a good reason, and anyway, if it is a problem that the
shared index file has been deleted, it is better addressed when we
will actually need the shared index file to read something into from
it, rather than just to update its mtime.

> Shouldn't the series be exposing freshen_file() instead _and_ taking
> its error return value seriously?

So what should we do if freshen_file() returns 0 which means that the
freshening failed?

>> +}

^ permalink raw reply

* Re: [PATCH v3 12/21] Documentation/config: add splitIndex.maxPercentChange
From: Christian Couder @ 2017-01-02 13:50 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð Bjarmason,
	Christian Couder
In-Reply-To: <xmqqlgv1b34y.fsf@gitster.mtv.corp.google.com>

On Tue, Dec 27, 2016 at 8:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>>  Documentation/config.txt | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index 221c5982c0..e0f5a77980 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -2773,6 +2773,19 @@ showbranch.default::
>>       The default set of branches for linkgit:git-show-branch[1].
>>       See linkgit:git-show-branch[1].
>>
>> +splitIndex.maxPercentChange::
>> +     When the split index feature is used, this specifies the
>> +     percent of entries the split index can contain compared to the
>> +     whole number of entries in both the split index and the shared
>
> s/whole/total/ to match the last sentence of this section, perhaps?
> "The number of all entries" would also be OK, but "the whole number
> of entries" sounds a bit strange.

Yeah "total" is better than "whole" here. I will use "total".

^ permalink raw reply

* Re: [PATCH] don't use test_must_fail with grep
From: Pranit Bauva @ 2017-01-02 13:40 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Luke Diamand, Git Users, Stefan Beller
In-Reply-To: <285ed013-5c59-0b98-7dc0-8f729587a313@kdbg.org>

Hey Johannes,

On Sun, Jan 1, 2017 at 8:20 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> which makes me wonder: Is the message that we do expect not to occur
> actually printed on stdout? It sounds much more like an error message, i.e.,
> text that is printed on stderr. Wouldn't we need this?
>
>         git p4 commit >actual 2>&1 &&
>         ! grep "git author.*does not match" actual &&
>
> -- Hannes

This seems better! Since I am at it, I can remove the traces of pipes
in an another patch.

Regards,
Pranit Bauva

^ permalink raw reply

* [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND
From: Johannes Schindelin @ 2017-01-02 12:09 UTC (permalink / raw)
  To: git; +Cc: Segev Finer, Junio C Hamano

From: Segev Finer <segev208@gmail.com>

Git for Windows has special support for the popular SSH client PuTTY:
when using PuTTY's non-interactive version ("plink.exe"), we use the -P
option to specify the port rather than OpenSSH's -p option. TortoiseGit
ships with its own, forked version of plink.exe, that adds support for
the -batch option, and for good measure we special-case that, too.

However, this special-casing of PuTTY only covers the case where the
user overrides the SSH command via the environment variable GIT_SSH
(which allows specifying the name of the executable), not
GIT_SSH_COMMAND (which allows specifying a full command, including
additional command-line options).

When users want to pass any additional arguments to (Tortoise-)Plink,
such as setting a private key, they are required to either use a shell
script named plink or tortoiseplink or duplicate the logic that is
already in Git for passing the correct style of command line arguments,
which can be difficult, error prone and annoying to get right.

This patch simply reuses the existing logic and expands it to cover
GIT_SSH_COMMAND, too.

Note: it may look a little heavy-handed to duplicate the entire
command-line and then split it, only to extract the name of the
executable. However, this is not a performance-critical code path, and
the code is much more readable this way.

Signed-off-by: Segev Finer <segev208@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/putty-w-args-v1
Fetch-It-Via: git fetch https://github.com/dscho/git putty-w-args-v1


	Original Pull Request:
	https://github.com/git-for-windows/git/pull/1006

 connect.c        | 23 ++++++++++++++++-------
 t/t5601-clone.sh | 15 +++++++++++++++
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/connect.c b/connect.c
index 8cb93b0720..c81f77001b 100644
--- a/connect.c
+++ b/connect.c
@@ -772,6 +772,7 @@ struct child_process *git_connect(int fd[2], const char *url,
 			int putty = 0, tortoiseplink = 0;
 			char *ssh_host = hostandport;
 			const char *port = NULL;
+			char *ssh_argv0 = NULL;
 			transport_check_allowed("ssh");
 			get_host_and_port(&ssh_host, &port);
 
@@ -792,10 +793,15 @@ struct child_process *git_connect(int fd[2], const char *url,
 			}
 
 			ssh = get_ssh_command();
-			if (!ssh) {
-				const char *base;
-				char *ssh_dup;
-
+			if (ssh) {
+				char *split_ssh = xstrdup(ssh);
+				const char **ssh_argv;
+
+				if (split_cmdline(split_ssh, &ssh_argv))
+					ssh_argv0 = xstrdup(ssh_argv[0]);
+				free(split_ssh);
+				free((void *)ssh_argv);
+			} else {
 				/*
 				 * GIT_SSH is the no-shell version of
 				 * GIT_SSH_COMMAND (and must remain so for
@@ -807,8 +813,11 @@ struct child_process *git_connect(int fd[2], const char *url,
 				if (!ssh)
 					ssh = "ssh";
 
-				ssh_dup = xstrdup(ssh);
-				base = basename(ssh_dup);
+				ssh_argv0 = xstrdup(ssh);
+			}
+
+			if (ssh_argv0) {
+				const char *base = basename(ssh_argv0);
 
 				tortoiseplink = !strcasecmp(base, "tortoiseplink") ||
 					!strcasecmp(base, "tortoiseplink.exe");
@@ -816,7 +825,7 @@ struct child_process *git_connect(int fd[2], const char *url,
 					!strcasecmp(base, "plink") ||
 					!strcasecmp(base, "plink.exe");
 
-				free(ssh_dup);
+				free(ssh_argv0);
 			}
 
 			argv_array_push(&conn->args, ssh);
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index a433394200..5b228e2675 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -386,6 +386,21 @@ test_expect_success 'tortoiseplink is like putty, with extra arguments' '
 	expect_ssh "-batch -P 123" myhost src
 '
 
+test_expect_success 'double quoted plink.exe in GIT_SSH_COMMAND' '
+	copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink.exe" &&
+	GIT_SSH_COMMAND="\"$TRASH_DIRECTORY/plink.exe\" -v" \
+		git clone "[myhost:123]:src" ssh-bracket-clone-plink-3 &&
+	expect_ssh "-v -P 123" myhost src
+'
+
+SQ="'"
+test_expect_success 'single quoted plink.exe in GIT_SSH_COMMAND' '
+	copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink.exe" &&
+	GIT_SSH_COMMAND="$SQ$TRASH_DIRECTORY/plink.exe$SQ -v" \
+		git clone "[myhost:123]:src" ssh-bracket-clone-plink-4 &&
+	expect_ssh "-v -P 123" myhost src
+'
+
 # Reset the GIT_SSH environment variable for clone tests.
 setup_ssh_wrapper
 

base-commit: e05806da9ec4aff8adfed142ab2a2b3b02e33c8c
-- 
2.11.0.rc3.windows.1

^ permalink raw reply related

* Re: [PATCH v3 10/21] read-cache: regenerate shared index if necessary
From: Christian Couder @ 2017-01-02 11:23 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð Bjarmason,
	Christian Couder
In-Reply-To: <xmqqk2al9ocv.fsf@gitster.mtv.corp.google.com>

On Tue, Dec 27, 2016 at 8:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> +     case 0:
>> +             return 1; /* 0% means always write a new shared index */
>> +     case 100:
>> +             return 0; /* 100% means never write a new shared index */
>> +     default:
>> +             ; /* do nothing: just use the configured value */
>> +     }
>
> Just like you did in 04/21, write "break" to avoid mistakes made in
> the future, i.e.
>
>         default:
>                 break; /* just use the configured value */

Ok, I will do that.

>> +
>> +     /* Count not shared entries */
>> +     for (i = 0; i < istate->cache_nr; i++) {
>> +             struct cache_entry *ce = istate->cache[i];
>> +             if (!ce->index)
>> +                     not_shared++;
>> +     }
>> +
>> +     return istate->cache_nr * max_split < not_shared * 100;
>
> On a 32-bit arch with 2G int and more than 20 million paths in the
> index, multiplying by max_split that can come close to 100 can
> theoretically cause integer overflow, but in practice it probably
> does not matter.  Or does it?

From a cursory look a "struct cache_entry" takes at least 80 bytes
without counting the "char name[FLEX_ARRAY]" on a 32 bit machine, so I
don't think it would be a good idea to work on a repo with 20 million
paths on a 32 bit machine, but maybe theoretically it could be a
problem.

To be safe I think I will use:

return (int64_t)istate->cache_nr * max_split < (int64_t)not_shared * 100;

^ permalink raw reply

* [RFC PATCH 3/5] error/warning framework framework: coccinelli rules
From: Michael J Gruber @ 2017-01-02 11:14 UTC (permalink / raw)
  To: git
In-Reply-To: <cover.1483354746.git.git@drmicha.warpmail.net>

Provide coccinelli rules which check for error(), warning() etc. with
localised argument and create a patch to replace them with error_(),
warning_() etc. in order to fully localize them.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 contrib/coccinelle/errorl10n.cocci | 47 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 contrib/coccinelle/errorl10n.cocci

diff --git a/contrib/coccinelle/errorl10n.cocci b/contrib/coccinelle/errorl10n.cocci
new file mode 100644
index 0000000000..d62a440644
--- /dev/null
+++ b/contrib/coccinelle/errorl10n.cocci
@@ -0,0 +1,47 @@
+@@
+expression E;
+@@
+- usage(_(E));
++ usage_(_(E));
+
+@@
+expression E;
+@@
+- usagef(_(E));
++ usagef_(_(E));
+
+@@
+expression E;
+@@
+- die(_(E));
++ die_(_(E));
+
+@@
+expression E;
+@@
+- error(_(E));
++ error_(_(E));
+
+@@
+expression E;
+@@
+- warning(_(E));
++ warning_(_(E));
+
+@@
+expression E;
+@@
+- die_errno(_(E));
++ die_errno_(_(E));
+
+@@
+expression E;
+@@
+- error_errno(_(E));
++ error_errno_(_(E));
+
+@@
+expression E;
+@@
+- warning_errno(_(E));
++ warning_errno_(_(E));
-- 
2.11.0.372.g2fcea0e476


^ permalink raw reply related

* [RFC PATCH 2/5] error/warn framework: provide localized variants
From: Michael J Gruber @ 2017-01-02 11:14 UTC (permalink / raw)
  To: git
In-Reply-To: <cover.1483354746.git.git@drmicha.warpmail.net>

Provide localized variants of error(), warning(), die() etc.
to go along with localized messages.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 git-compat-util.h |  8 ++++++
 usage.c           | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index f1bf0a6749..48209a6c67 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -405,13 +405,21 @@ struct strbuf;
 /* General helper functions */
 extern void vreportf(const char *prefix, const char *err, va_list params);
 extern NORETURN void usage(const char *err);
+extern NORETURN void usage_(const char *err);
 extern NORETURN void usagef(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern NORETURN void usagef_(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern NORETURN void die(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern NORETURN void die_(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern NORETURN void die_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern NORETURN void die_errno_(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern int error_(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern int error_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern int error_errno_(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern void warning_(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern void warning_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
+extern void warning_errno_(const char *err, ...) __attribute__((format (printf, 1, 2)));
 
 #ifndef NO_OPENSSL
 #ifdef APPLE_COMMON_CRYPTO
diff --git a/usage.c b/usage.c
index 4270b04bf9..d0cfb02a64 100644
--- a/usage.c
+++ b/usage.c
@@ -105,11 +105,25 @@ void NORETURN usagef(const char *err, ...)
 	va_end(params);
 }
 
+void NORETURN usagef_(const char *err, ...)
+{
+	va_list params;
+
+	va_start(params, err);
+	usage_routine(_("usage: "), err, params);
+	va_end(params);
+}
+
 void NORETURN usage(const char *err)
 {
 	usagef("%s", err);
 }
 
+void NORETURN usage_(const char *err)
+{
+	usagef_("%s", err);
+}
+
 void NORETURN die(const char *err, ...)
 {
 	va_list params;
@@ -124,6 +138,20 @@ void NORETURN die(const char *err, ...)
 	va_end(params);
 }
 
+void NORETURN die_(const char *err, ...)
+{
+	va_list params;
+
+	if (die_is_recursing()) {
+		fputs(_("fatal: recursion detected in die handler\n"), stderr);
+		exit(128);
+	}
+
+	va_start(params, err);
+	die_routine(_("fatal: "), err, params);
+	va_end(params);
+}
+
 static const char *fmt_with_err(char *buf, int n, const char *fmt)
 {
 	char str_error[256], *err;
@@ -163,6 +191,22 @@ void NORETURN die_errno(const char *fmt, ...)
 	va_end(params);
 }
 
+void NORETURN die_errno_(const char *fmt, ...)
+{
+	char buf[1024];
+	va_list params;
+
+	if (die_is_recursing()) {
+		fputs(_("fatal: recursion detected in die_errno handler\n"),
+			stderr);
+		exit(128);
+	}
+
+	va_start(params, fmt);
+	die_routine(_("fatal: "), fmt_with_err(buf, sizeof(buf), fmt), params);
+	va_end(params);
+}
+
 #undef error_errno
 int error_errno(const char *fmt, ...)
 {
@@ -175,6 +219,17 @@ int error_errno(const char *fmt, ...)
 	return -1;
 }
 
+int error_errno_(const char *fmt, ...)
+{
+	char buf[1024];
+	va_list params;
+
+	va_start(params, fmt);
+	error_routine(_("error: "), fmt_with_err(buf, sizeof(buf), fmt), params);
+	va_end(params);
+	return -1;
+}
+
 #undef error
 int error(const char *err, ...)
 {
@@ -186,6 +241,16 @@ int error(const char *err, ...)
 	return -1;
 }
 
+int error_(const char *err, ...)
+{
+	va_list params;
+
+	va_start(params, err);
+	error_routine(_("error: "), err, params);
+	va_end(params);
+	return -1;
+}
+
 void warning_errno(const char *warn, ...)
 {
 	char buf[1024];
@@ -204,3 +269,12 @@ void warning(const char *warn, ...)
 	warn_routine("warning: ", warn, params);
 	va_end(params);
 }
+
+void warning_(const char *warn, ...)
+{
+	va_list params;
+
+	va_start(params, warn);
+	warn_routine(_("warning: "), warn, params);
+	va_end(params);
+}
-- 
2.11.0.372.g2fcea0e476


^ permalink raw reply related

* [RFC PATCH 1/5] error/warning framework: prepare for l10n
From: Michael J Gruber @ 2017-01-02 11:14 UTC (permalink / raw)
  To: git
In-Reply-To: <cover.1483354746.git.git@drmicha.warpmail.net>

Currently, errors, warnings etc. are output with a fixed prefix "error: "
etc. that is not subject to l10n.

Change the call signatures of error_routine() etc. so that they receive
the prefix as first argument.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 apply.c           |  2 +-
 apply.h           |  4 ++--
 daemon.c          |  3 ++-
 fast-import.c     |  4 ++--
 git-compat-util.h | 10 +++++-----
 http-backend.c    |  4 ++--
 run-command.c     |  4 ++--
 usage.c           | 48 ++++++++++++++++++++++++------------------------
 8 files changed, 40 insertions(+), 39 deletions(-)

diff --git a/apply.c b/apply.c
index 2ed808d429..0f93792e1c 100644
--- a/apply.c
+++ b/apply.c
@@ -112,7 +112,7 @@ void clear_apply_state(struct apply_state *state)
 	/* &state->fn_table is cleared at the end of apply_patch() */
 }
 
-static void mute_routine(const char *msg, va_list params)
+static void mute_routine(const char *prefix, const char *msg, va_list params)
 {
 	/* do nothing */
 }
diff --git a/apply.h b/apply.h
index b3d6783d55..56b5622868 100644
--- a/apply.h
+++ b/apply.h
@@ -100,8 +100,8 @@ struct apply_state {
 	 * set_error_routine() or set_warn_routine() to install muting
 	 * routines when in verbosity_silent mode.
 	 */
-	void (*saved_error_routine)(const char *err, va_list params);
-	void (*saved_warn_routine)(const char *warn, va_list params);
+	void (*saved_error_routine)(const char *prefix, const char *err, va_list params);
+	void (*saved_warn_routine)(const char *prefix, const char *warn, va_list params);
 
 	/* These control whitespace errors */
 	enum apply_ws_error_action ws_error_action;
diff --git a/daemon.c b/daemon.c
index 473e6b6b63..cd52a25001 100644
--- a/daemon.c
+++ b/daemon.c
@@ -114,8 +114,9 @@ static void loginfo(const char *err, ...)
 	va_end(params);
 }
 
-static void NORETURN daemon_die(const char *err, va_list params)
+static void NORETURN daemon_die(const char *prefix, const char *err, va_list params)
 {
+	/* no need to pass down prefix */
 	logreport(LOG_ERR, err, params);
 	exit(1);
 }
diff --git a/fast-import.c b/fast-import.c
index f561ba833b..4576f12163 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -496,13 +496,13 @@ static void end_packfile(void);
 static void unkeep_all_packs(void);
 static void dump_marks(void);
 
-static NORETURN void die_nicely(const char *err, va_list params)
+static NORETURN void die_nicely(const char *prefix, const char *err, va_list params)
 {
 	static int zombie;
 	char message[2 * PATH_MAX];
 
 	vsnprintf(message, sizeof(message), err, params);
-	fputs("fatal: ", stderr);
+	fputs(prefix, stderr);
 	fputs(message, stderr);
 	fputc('\n', stderr);
 
diff --git a/git-compat-util.h b/git-compat-util.h
index 87237b092b..f1bf0a6749 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -439,11 +439,11 @@ static inline int const_error(void)
 #define error_errno(...) (error_errno(__VA_ARGS__), const_error())
 #endif
 
-extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
-extern void set_error_routine(void (*routine)(const char *err, va_list params));
-extern void (*get_error_routine(void))(const char *err, va_list params);
-extern void set_warn_routine(void (*routine)(const char *warn, va_list params));
-extern void (*get_warn_routine(void))(const char *warn, va_list params);
+extern void set_die_routine(NORETURN_PTR void (*routine)(const char *prefix, const char *err, va_list params));
+extern void set_error_routine(void (*routine)(const char *prefix, const char *err, va_list params));
+extern void (*get_error_routine(void))(const char *prefix, const char *err, va_list params);
+extern void set_warn_routine(void (*routine)(const char *prefix, const char *warn, va_list params));
+extern void (*get_warn_routine(void))(const char *prefix, const char *warn, va_list params);
 extern void set_die_is_recursing_routine(int (*routine)(void));
 extern void set_error_handle(FILE *);
 
diff --git a/http-backend.c b/http-backend.c
index eef0a361f4..5c235e8b52 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -577,12 +577,12 @@ static void service_rpc(struct strbuf *hdr, char *service_name)
 }
 
 static int dead;
-static NORETURN void die_webcgi(const char *err, va_list params)
+static NORETURN void die_webcgi(const char *prefix, const char *err, va_list params)
 {
 	if (dead <= 1) {
 		struct strbuf hdr = STRBUF_INIT;
 
-		vreportf("fatal: ", err, params);
+		vreportf(prefix, err, params);
 
 		http_status(&hdr, 500, "Internal Server Error");
 		hdr_nocache(&hdr);
diff --git a/run-command.c b/run-command.c
index ca905a9e80..3133bf5320 100644
--- a/run-command.c
+++ b/run-command.c
@@ -618,9 +618,9 @@ static void *run_thread(void *data)
 	return (void *)ret;
 }
 
-static NORETURN void die_async(const char *err, va_list params)
+static NORETURN void die_async(const char *prefix, const char *err, va_list params)
 {
-	vreportf("fatal: ", err, params);
+	vreportf(prefix, err, params);
 
 	if (in_async()) {
 		struct async *async = pthread_getspecific(async_key);
diff --git a/usage.c b/usage.c
index 17f52c1b5c..4270b04bf9 100644
--- a/usage.c
+++ b/usage.c
@@ -24,26 +24,26 @@ void vreportf(const char *prefix, const char *err, va_list params)
 	fputc('\n', fh);
 }
 
-static NORETURN void usage_builtin(const char *err, va_list params)
+static NORETURN void usage_builtin(const char *prefix, const char *err, va_list params)
 {
-	vreportf("usage: ", err, params);
+	vreportf(prefix, err, params);
 	exit(129);
 }
 
-static NORETURN void die_builtin(const char *err, va_list params)
+static NORETURN void die_builtin(const char *prefix, const char *err, va_list params)
 {
-	vreportf("fatal: ", err, params);
+	vreportf(prefix, err, params);
 	exit(128);
 }
 
-static void error_builtin(const char *err, va_list params)
+static void error_builtin(const char *prefix, const char *err, va_list params)
 {
-	vreportf("error: ", err, params);
+	vreportf(prefix, err, params);
 }
 
-static void warn_builtin(const char *warn, va_list params)
+static void warn_builtin(const char *prefix, const char *warn, va_list params)
 {
-	vreportf("warning: ", warn, params);
+	vreportf(prefix, warn, params);
 }
 
 static int die_is_recursing_builtin(void)
@@ -54,33 +54,33 @@ static int die_is_recursing_builtin(void)
 
 /* If we are in a dlopen()ed .so write to a global variable would segfault
  * (ugh), so keep things static. */
-static NORETURN_PTR void (*usage_routine)(const char *err, va_list params) = usage_builtin;
-static NORETURN_PTR void (*die_routine)(const char *err, va_list params) = die_builtin;
-static void (*error_routine)(const char *err, va_list params) = error_builtin;
-static void (*warn_routine)(const char *err, va_list params) = warn_builtin;
+static NORETURN_PTR void (*usage_routine)(const char *prefix, const char *err, va_list params) = usage_builtin;
+static NORETURN_PTR void (*die_routine)(const char *prefix, const char *err, va_list params) = die_builtin;
+static void (*error_routine)(const char *prefix, const char *err, va_list params) = error_builtin;
+static void (*warn_routine)(const char *prefix, const char *err, va_list params) = warn_builtin;
 static int (*die_is_recursing)(void) = die_is_recursing_builtin;
 
-void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params))
+void set_die_routine(NORETURN_PTR void (*routine)(const char *prefix, const char *err, va_list params))
 {
 	die_routine = routine;
 }
 
-void set_error_routine(void (*routine)(const char *err, va_list params))
+void set_error_routine(void (*routine)(const char *prefix, const char *err, va_list params))
 {
 	error_routine = routine;
 }
 
-void (*get_error_routine(void))(const char *err, va_list params)
+void (*get_error_routine(void))(const char *prefix, const char *err, va_list params)
 {
 	return error_routine;
 }
 
-void set_warn_routine(void (*routine)(const char *warn, va_list params))
+void set_warn_routine(void (*routine)(const char *prefix, const char *warn, va_list params))
 {
 	warn_routine = routine;
 }
 
-void (*get_warn_routine(void))(const char *warn, va_list params)
+void (*get_warn_routine(void))(const char *prefix, const char *warn, va_list params)
 {
 	return warn_routine;
 }
@@ -101,7 +101,7 @@ void NORETURN usagef(const char *err, ...)
 	va_list params;
 
 	va_start(params, err);
-	usage_routine(err, params);
+	usage_routine("usage: ", err, params);
 	va_end(params);
 }
 
@@ -120,7 +120,7 @@ void NORETURN die(const char *err, ...)
 	}
 
 	va_start(params, err);
-	die_routine(err, params);
+	die_routine("fatal: ", err, params);
 	va_end(params);
 }
 
@@ -159,7 +159,7 @@ void NORETURN die_errno(const char *fmt, ...)
 	}
 
 	va_start(params, fmt);
-	die_routine(fmt_with_err(buf, sizeof(buf), fmt), params);
+	die_routine("fatal: ", fmt_with_err(buf, sizeof(buf), fmt), params);
 	va_end(params);
 }
 
@@ -170,7 +170,7 @@ int error_errno(const char *fmt, ...)
 	va_list params;
 
 	va_start(params, fmt);
-	error_routine(fmt_with_err(buf, sizeof(buf), fmt), params);
+	error_routine("error: ", fmt_with_err(buf, sizeof(buf), fmt), params);
 	va_end(params);
 	return -1;
 }
@@ -181,7 +181,7 @@ int error(const char *err, ...)
 	va_list params;
 
 	va_start(params, err);
-	error_routine(err, params);
+	error_routine("error: ", err, params);
 	va_end(params);
 	return -1;
 }
@@ -192,7 +192,7 @@ void warning_errno(const char *warn, ...)
 	va_list params;
 
 	va_start(params, warn);
-	warn_routine(fmt_with_err(buf, sizeof(buf), warn), params);
+	warn_routine("warning: ", fmt_with_err(buf, sizeof(buf), warn), params);
 	va_end(params);
 }
 
@@ -201,6 +201,6 @@ void warning(const char *warn, ...)
 	va_list params;
 
 	va_start(params, warn);
-	warn_routine(warn, params);
+	warn_routine("warning: ", warn, params);
 	va_end(params);
 }
-- 
2.11.0.372.g2fcea0e476


^ permalink raw reply related

* [RFC PATCH 0/5] Localise error headers
From: Michael J Gruber @ 2017-01-02 11:14 UTC (permalink / raw)
  To: git

Currently, the headers "error: ", "warning: " etc. - generated by die(),
warning() etc. - are not localized, but we feed many localized error messages
into these functions so that we produce error messages with mixed localisation.

This series introduces variants of die() etc. that use localised variants of
the headers, i.e. _("error: ") etc., and are to be fed localized messages. So,
instead of die(_("not workee")), which would produce a mixed localisation (such
as "error: geht ned"), one should use die_(_("not workee")) (resulting in
"Fehler: geht ned").

In this implementation, the gettext call for the header and the body are done
in different places (error function vs. caller) but this call pattern seems to
be the easiest variant for the caller, because the message body has to be marked
for localisation in any case, and N_() requires more letters than _(), an extra
argument to die() etc. even more than the extra "_" in the function name.

1/5 prepares the error machinery
2/5 provides new variants error_() etc.
3/5 has coccinelli rules error(_(E)) -> error_(_(E)) etc.
4/5 applies the coccinelli patches

5/5 is not to be applied to the main tree, but helps you try out the feature:
it has changes to de.po and git.pot so that e.g. "git branch" has fully localised
error messages (see the recipe in the commit message).

Michael J Gruber (5):
  error/warn framework: prepare for l10n
  error/warn framework: provide localized variants
  error/warn framework framework: coccinelli rules
  error/warn framework: localize
  WIP: Feature demonstration

 advice.c                           |   16 +-
 apply.c                            |   10 +-
 apply.h                            |    4 +-
 archive.c                          |    6 +-
 attr.c                             |    3 +-
 bisect.c                           |    2 +-
 branch.c                           |    4 +-
 builtin/add.c                      |   20 +-
 builtin/am.c                       |   27 +-
 builtin/archive.c                  |   10 +-
 builtin/blame.c                    |   12 +-
 builtin/branch.c                   |   45 +-
 builtin/bundle.c                   |    4 +-
 builtin/check-ignore.c             |   14 +-
 builtin/check-mailmap.c            |    2 +-
 builtin/checkout-index.c           |    2 +-
 builtin/checkout.c                 |   27 +-
 builtin/clean.c                    |   10 +-
 builtin/clone.c                    |   39 +-
 builtin/column.c                   |    2 +-
 builtin/commit.c                   |   87 +-
 builtin/config.c                   |    2 +-
 builtin/describe.c                 |    6 +-
 builtin/diff.c                     |    2 +-
 builtin/fetch.c                    |   21 +-
 builtin/gc.c                       |    3 +-
 builtin/grep.c                     |   14 +-
 builtin/help.c                     |    4 +-
 builtin/index-pack.c               |   42 +-
 builtin/interpret-trailers.c       |    2 +-
 builtin/log.c                      |   48 +-
 builtin/merge-recursive.c          |    2 +-
 builtin/merge.c                    |   53 +-
 builtin/mv.c                       |    6 +-
 builtin/notes.c                    |   40 +-
 builtin/pack-objects.c             |    4 +-
 builtin/prune.c                    |    2 +-
 builtin/pull.c                     |   10 +-
 builtin/push.c                     |   22 +-
 builtin/remote.c                   |   10 +-
 builtin/repack.c                   |    4 +-
 builtin/replace.c                  |    2 +-
 builtin/reset.c                    |   10 +-
 builtin/rev-list.c                 |    2 +-
 builtin/rev-parse.c                |    2 +-
 builtin/revert.c                   |    4 +-
 builtin/rm.c                       |    6 +-
 builtin/shortlog.c                 |    2 +-
 builtin/show-branch.c              |    7 +-
 builtin/submodule--helper.c        |    9 +-
 builtin/tag.c                      |   20 +-
 builtin/unpack-objects.c           |    2 +-
 builtin/update-index.c             |    8 +-
 builtin/worktree.c                 |    6 +-
 bundle.c                           |    4 +-
 config.c                           |    4 +-
 connect.c                          |    6 +-
 connected.c                        |    2 +-
 contrib/coccinelle/errorl10n.cocci |   47 +
 daemon.c                           |    3 +-
 diff.c                             |    8 +-
 dir.c                              |    4 +-
 fast-import.c                      |    4 +-
 fetch-pack.c                       |   26 +-
 git-compat-util.h                  |   18 +-
 help.c                             |    2 +-
 http-backend.c                     |    4 +-
 http.c                             |    4 +-
 merge.c                            |    2 +-
 notes-utils.c                      |    2 +-
 pathspec.c                         |   13 +-
 po/de.po                           | 2922 ++++++++++++++++++++++++------------
 po/git.pot                         | 2781 ++++++++++++++++++++++------------
 pretty.c                           |    2 +-
 ref-filter.c                       |   20 +-
 remote.c                           |    2 +-
 revision.c                         |    4 +-
 run-command.c                      |    6 +-
 send-pack.c                        |   12 +-
 sequencer.c                        |    7 +-
 setup.c                            |    6 +-
 sha1_file.c                        |    2 +-
 submodule.c                        |    8 +-
 trailer.c                          |    4 +-
 transport.c                        |    2 +-
 tree-walk.c                        |    2 +-
 usage.c                            |  122 +-
 wrapper.c                          |    2 +-
 wt-status.c                        |    2 +-
 89 files changed, 4408 insertions(+), 2374 deletions(-)
 create mode 100644 contrib/coccinelle/errorl10n.cocci

-- 
2.11.0.372.g2fcea0e476


^ permalink raw reply

* Re: Error: could not fork child process: Resource temporarily unavailable (-1)
From: Johannes Schindelin @ 2017-01-02 10:58 UTC (permalink / raw)
  To: M. Abuhena Sobuj; +Cc: git
In-Reply-To: <CACkJq_gQQURdyUf5HvSqvw0Ee_yjCnFZ0DXJxZt42JOwEOd2tg@mail.gmail.com>

Hi,

On Mon, 2 Jan 2017, M. Abuhena Sobuj wrote:

> My Git Bash was perfect but suddenly It stooped work for me.
> 
> I just got message
> "Error: could not fork child process: Resource temporarily unavailable (-1).
> DLL rebasing may be required. See 'rebaseall / rebase --help'."

Is this a 32-bit system? If so, simply try to reinstall Git for Windows,
that should make it work again [*1*].

If that does not solve the problem, you will want to open a ticket at
https://github.com/git-for-windows/git/issues/new

Ciao,
Johannes

Footnote *1*: https://github.com/git-for-windows/git/wiki/32-bit-issues

^ permalink raw reply

* [PATCH v2] mingw: add a regression test for pushing to UNC paths
From: Johannes Schindelin @ 2017-01-02 10:40 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Sixt
In-Reply-To: <9fb8a9f405b19db087379ea5bbad80c3fbac8e4e.1482513055.git.johannes.schindelin@gmx.de>

On Windows, there are "UNC paths" to access network (AKA shared)
folders, of the form \\server\sharename\directory. This provides a
convenient way for Windows developers to share their Git repositories
without having to have a dedicated server.

Git for Windows v2.11.0 introduced a regression where pushing to said
UNC paths no longer works, although fetching and cloning still does, as
reported here: https://github.com/git-for-windows/git/issues/979

This regression was fixed in 7814fbe3f1 (normalize_path_copy(): fix
pushing to //server/share/dir on Windows, 2016-12-14).

Let's make sure that it does not regress again, by introducing a test
that uses so-called "administrative shares": disk volumes are
automatically shared under certain circumstances, e.g.  the C: drive is
shared as \\localhost\c$. The test needs to be skipped if the current
directory is inaccessible via said administrative share, of course.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/unc-paths-test-v2
Fetch-It-Via: git fetch https://github.com/dscho/git unc-paths-test-v2
Interdiff vs v1:

 diff --git a/t/t5580-clone-push-unc.sh b/t/t5580-clone-push-unc.sh
 index e06d230724..b195f71ea9 100755
 --- a/t/t5580-clone-push-unc.sh
 +++ b/t/t5580-clone-push-unc.sh
 @@ -40,7 +40,9 @@ test_expect_success push '
  		git checkout -b to-push &&
  		test_commit to-push &&
  		git push origin HEAD
 -	)
 +	) &&
 +	rev="$(git -C clone rev-parse --verify refs/heads/to-push)" &&
 +	test "$rev" = "$(git rev-parse --verify refs/heads/to-push)"
  '
  
  test_done


 t/t5580-clone-push-unc.sh | 48 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100755 t/t5580-clone-push-unc.sh

diff --git a/t/t5580-clone-push-unc.sh b/t/t5580-clone-push-unc.sh
new file mode 100755
index 0000000000..b195f71ea9
--- /dev/null
+++ b/t/t5580-clone-push-unc.sh
@@ -0,0 +1,48 @@
+#!/bin/sh
+
+test_description='various UNC path tests (Windows-only)'
+. ./test-lib.sh
+
+if ! test_have_prereq MINGW; then
+	skip_all='skipping UNC path tests, requires Windows'
+	test_done
+fi
+
+UNCPATH="$(pwd)"
+case "$UNCPATH" in
+[A-Z]:*)
+	# Use administrative share e.g. \\localhost\C$\git-sdk-64\usr\src\git
+	# (we use forward slashes here because MSYS2 and Git accept them, and
+	# they are easier on the eyes)
+	UNCPATH="//localhost/${UNCPATH%%:*}\$/${UNCPATH#?:}"
+	test -d "$UNCPATH" || {
+		skip_all='could not access administrative share; skipping'
+		test_done
+	}
+	;;
+*)
+	skip_all='skipping UNC path tests, cannot determine current path as UNC'
+	test_done
+	;;
+esac
+
+test_expect_success setup '
+	test_commit initial
+'
+
+test_expect_success clone '
+	git clone "file://$UNCPATH" clone
+'
+
+test_expect_success push '
+	(
+		cd clone &&
+		git checkout -b to-push &&
+		test_commit to-push &&
+		git push origin HEAD
+	) &&
+	rev="$(git -C clone rev-parse --verify refs/heads/to-push)" &&
+	test "$rev" = "$(git rev-parse --verify refs/heads/to-push)"
+'
+
+test_done

base-commit: c69c2f50cfc0dcd4bcd014c7fd56e344a7c5522f
-- 
2.11.0.rc3.windows.1

^ permalink raw reply related

* Re: [PATCH] mingw: add a regression test for pushing to UNC paths
From: Johannes Schindelin @ 2017-01-02 10:39 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, Junio C Hamano
In-Reply-To: <6d69b529-a42c-9f93-f342-7c6c19170285@kdbg.org>

Hi Hannes,

On Fri, 23 Dec 2016, Johannes Sixt wrote:

> Am 23.12.2016 um 18:11 schrieb Johannes Schindelin:
> 
> > +test_expect_success setup '
> > +	test_commit initial
> > +'
> > +
> > +test_expect_success clone '
> > +	git clone "file://$UNCPATH" clone
> > +'
> > +
> > +test_expect_success push '
> > +	(
> > +		cd clone &&
> > +		git checkout -b to-push &&
> > +		test_commit to-push &&
> > +		git push origin HEAD
> > +	)
> > +'
> > +
> > +test_done
> 
> Wouldn't at a minimum
> 
> test_expect_success 'check push result' '
> 	git rev-parse to-push
> '
> 
> be a good idea to make sure that the pushed commit actually arrived?

Sure.

Ciao,
Dscho

^ permalink raw reply

* Error: could not fork child process: Resource temporarily unavailable (-1)
From: M. Abuhena Sobuj @ 2017-01-02 10:36 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 317 bytes --]

Hello sir,
Good day.

My Git Bash was perfect but suddenly It stooped work for me.

I just got message
"Error: could not fork child process: Resource temporarily unavailable (-1).
DLL rebasing may be required. See 'rebaseall / rebase --help'."

Please help me to resolve this issue.

Thank you.

I'm Windows 10 User.

[-- Attachment #2: Screenshot_2017.01.02_16h34m44s_001.png --]
[-- Type: image/png, Size: 12133 bytes --]

^ permalink raw reply

* Re: [PATCH v3 08/21] Documentation/git-update-index: talk about core.splitIndex config var
From: Christian Couder @ 2017-01-02  9:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð Bjarmason,
	Christian Couder
In-Reply-To: <xmqqpokd9ocw.fsf@gitster.mtv.corp.google.com>

On Tue, Dec 27, 2016 at 8:07 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>>  Documentation/git-update-index.txt | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
>> index 7386c93162..e091b2a409 100644
>> --- a/Documentation/git-update-index.txt
>> +++ b/Documentation/git-update-index.txt
>> @@ -171,6 +171,12 @@ may not support it yet.
>>       given again, all changes in $GIT_DIR/index are pushed back to
>>       the shared index file. This mode is designed for very large
>>       indexes that take a significant amount of time to read or write.
>> ++
>> +These options take effect whatever the value of the `core.splitIndex`
>> +configuration variable (see linkgit:git-config[1]).
>
> Doesn't the "whatever..." clause in this sentence lack its verb
> (presumably, "is", right after "variable")?

I think that it is ok to have no verb here. My preferred online
dictionary (http://www.wordreference.com/enfr/whatever) gives "We'll
try to free the hostage whatever the cost." as an example with no
verb, though I am not a native speaker.

Also note that I mostly copied what I had already written for
--(no-)untracked-cache:

--untracked-cache::
--no-untracked-cache::
    Enable or disable untracked cache feature. Please use
    `--test-untracked-cache` before enabling it.
+
These options take effect whatever the value of the `core.untrackedCache`
configuration variable (see linkgit:git-config[1]). But a warning is
emitted when the change goes against the configured value, as the
configured value will take effect next time the index is read and this
will remove the intended effect of the option.

>> +emitted when the change goes against the configured value, as the
>> +configured value will take effect next time the index is read and this
>> +will remove the intended effect of the option.
>
> It feels somewhat strange to see a warning in this case.

We already discussed the warning issue for --(no-)untracked-cache, and
I am just following the conclusions of that previous discussion about
--(no-)untracked-cache. See the end of this message where you
suggested the warning:

https://public-inbox.org/git/xmqqlh8cg9y2.fsf@gitster.mtv.corp.google.com/

> If the user configures the system to usually do X, and issues a
> command that explicitly does Y (or "not-X"), we do not warn for the
> command invocation if it is a single-shot operation.  That's the
> usual "command line overrides configuration" an end-user expects.
>
> I think you made this deviate from the usual end-user expectation
> because it is unpredictable when the configuration kicks in the next
> time to undo the effect of this command line request.  And I agree
> that it is a very un-nice thing to do to the users if we did not
> warn here, if we are to keep that unpredictableness.

I also think it would be strange and user unfriendly for
--untracked-cache and --split-index to behave differently, and I am
reluctant at this point to rework the way it works for
--untracked-cache.

> But stepping back a bit, we know the user's intention is that with
> "--split-index" the user wants to use the split index, even though
> the user may have configuration not to use the split index.  Don't
> we want to honor that intention?  In order to honor that intention
> without getting confused by the configured variable to tell us not
> to split, another way is to destroy that unpredictableness.  For
> that, shouldn't we automatically remove the configuration that says
> "do not split" (by perhaps set it to "do split")?  Doing so still
> may need a warning that says "we disabled your previously configured
> setting while at it", but it would be much better than a warning
> message that essentially says "we do it once, but we will disobey
> you at an unpredictable time", I would think.

I wanted to do that for --untracked-cache around one year ago but in
the following message:

https://public-inbox.org/git/xmqqsi3ckadi.fsf@gitster.mtv.corp.google.com/

you wrote the following:

"Why is it a good idea to allow an explicit operation from the
command line to muck with the configuration?  If the user wants to
change the configuration permanently, "git config" has been there
for the purpose for all the configuration variables to do so for
almost forever.  Why is it a good idea to make this variable so
special that the user does not have to use "git config" but disable
or enable it in some other way?"

It feels strange that when I do things one way, you suggest another
way, and the next time in a similar situation when I do things the way
you suggested previously, then you suggest the way I did it initially
the first time...

^ permalink raw reply

* [PATCH] completion: complete git submodule subcommands
From: Denton Liu @ 2017-01-02  8:58 UTC (permalink / raw)
  To: git; +Cc: spearce, gitster

Allow git submodule subcommands to be completed. This allows the
'--remote' in the command 'git submodule update --remote', for example,
to be fully completed.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
Hi Shawn, sorry this is my first contribution to a mailing-list based
project. If I've done anything wrong, please let me know.

Thanks,

Denton Liu

---
 contrib/completion/git-completion.bash | 46 ++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 21016bf8d..941fbdfe2 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2556,17 +2556,41 @@ _git_submodule ()
 	__git_has_doubledash && return
 
 	local subcommands="add status init deinit update summary foreach sync"
-	if [ -z "$(__git_find_on_cmdline "$subcommands")" ]; then
-		case "$cur" in
-		--*)
-			__gitcomp "--quiet --cached"
-			;;
-		*)
-			__gitcomp "$subcommands"
-			;;
-		esac
-		return
-	fi
+	local subcommand="$(__git_find_on_cmdline "$subcommands")"
+
+	case "$subcommand,$cur" in
+	,--*)
+		__gitcomp "--quiet"
+		;;
+	,*)
+		__gitcomp "$subcommands --quiet"
+		;;
+	add,--*)
+		__gitcomp "--force --name --reference --depth"
+		;;
+	status,--*)
+		__gitcomp "--cached --recursive"
+		;;
+	deinit,--*)
+		__gitcomp "--force --all"
+		;;
+	update,--*)
+		__gitcomp "
+			--init --remote --no-fetch --no-recommended-shallow
+			--recommended-shallow --force --rebase --merge --reference
+			--depth --recursive --jobs
+			"
+		;;
+	summary,--*)
+		__gitcomp "--cached --files --summary-limit"
+		;;
+	summary,*)
+		__gitcomp_nl "$(__git_refs)"
+		;;
+	foreach,--*|sync,--*)
+		__gitcomp "--recursive"
+		;;
+	esac
 }
 
 _git_svn ()
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH v3 06/21] t1700: add tests for core.splitIndex
From: Christian Couder @ 2017-01-02  8:29 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð Bjarmason,
	Christian Couder
In-Reply-To: <xmqqvau59ocy.fsf@gitster.mtv.corp.google.com>

On Tue, Dec 27, 2016 at 8:04 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> +test_expect_success 'set core.splitIndex config variable to true' '
>> +     git config core.splitIndex true &&
>> +     : >three &&
>> +     git update-index --add three &&
>> +     git ls-files --stage >ls-files.actual &&
>> +     cat >ls-files.expect <<EOF &&
>> +100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0    one
>> +100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0    three
>> +100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0    two
>> +EOF
>> +     test_cmp ls-files.expect ls-files.actual &&
>
> It does not add much value to follow the "existing" outdated style
> like this when you are only adding new tests.  Write these like
>
>         cat >ls-files.expect <<-\EOF &&
>         100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       one
>         100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       three
>         100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0       two
>         EOF
>
> which would give incentive to others (or yourself) to update the
> style of the existing mess ;-).

Ok, I will add a patch to update the style of the existing tests at
the beginning of the series and then use the same new style in the
tests I add in later patches.

^ permalink raw reply

* Re: Test failures when Git is built with libpcre and grep is built without it
From: Jeff King @ 2017-01-02  6:53 UTC (permalink / raw)
  To: A. Wilcox; +Cc: git
In-Reply-To: <58688C9F.4000605@adelielinux.org>

On Sat, Dec 31, 2016 at 10:59:11PM -0600, A. Wilcox wrote:

> I'm attempting to package Git for our new Linux distribution and I
> have run in to a failure on our PowerPC builder while running the test
> suite.
> 
> The PowerPC builder runs a tiny version of grep(1) that was not built
> with PCRE.  As such, grep -P returns 2 and prints:
> 
> grep: support for the -P option is not compiled into this
> - --disable-perl-regexp binary
> 
> However, our Git build *does* link against libpcre.  This causes a
> tests numbered 142 and 143 to fail in t7810-grep.sh.

If we are using "grep -P" in our test suite, it should definitely be
marked with a prerequisite that is independent of the LIBPCRE one.

But I can't find any such place in our test suite. Grepping for
"grep.*-P" doesn't turn up any hits, and dropping this into my PATH as
"grep":

    #!/bin/sh

    case "$*" in
    *-P*|*perl-regex*)
      echo >&2 "Pretending not to understand -P"
      exit 1
    esac

    exec /bin/grep "$@"

doesn't break anything. We do call "git grep -P", of course, but that
should be using the internal libpcre (once upon a time we would invoke
an external grep, but that feature has been gone for years).

Can you show us the output of "./t7810-grep.sh -v -i"?

-Peff

^ permalink raw reply

* Re: [PATCH] contrib: remove gitview
From: Aneesh Kumar K.V @ 2017-01-02  6:46 UTC (permalink / raw)
  To: Jeff King, Stefan Beller; +Cc: git, jvoss, Aneesh Kumar K.V
In-Reply-To: <20161229015918.jyiqd42z4htjibul@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Wed, Dec 28, 2016 at 09:28:37AM -0800, Stefan Beller wrote:
>
>> gitview did not have meaningful contributions since 2007, which gives the
>> impression it is either a mature or dead project.
>> 
>> In both cases we should not carry it in git.git as the README for contrib
>> states we only want to carry experimental things to give early exposure.
>> 
>> Recently a security vulnerability was reported by Javantea, so the decision
>> to either fix the issue or remove the code in question becomes a bit
>> more urgent.
>> 
>> Reported-by: Javantea <jvoss@altsci.com>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>>  contrib/gitview/gitview     | 1305 -------------------------------------------
>>  contrib/gitview/gitview.txt |   57 --
>>  2 files changed, 1362 deletions(-)
>>  delete mode 100755 contrib/gitview/gitview
>>  delete mode 100644 contrib/gitview/gitview.txt
>
> Thanks for assembling the patch. This seems reasonable to me, though I'd
> like to get an Ack from Aneesh if we can.

Acked-by: Aneesh Kumar K.V <aneesh.kumar@gmail.com>

-aneesh


^ permalink raw reply

* Re: Rebasing multiple branches at once
From: Jeff King @ 2017-01-02  6:42 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git, Junio C Hamano
In-Reply-To: <xmqqmvfb4i5a.fsf@gitster.mtv.corp.google.com>

On Sat, Dec 31, 2016 at 06:40:33PM -0800, Junio C Hamano wrote:

> What people seem to do is to teach the branch that ends with F that
> its upstream is the local branch that ends with E, so that they can
> be lazy when rebasing a branch that knows its upstream.  I suspect
> that you would end up with
> 
> A---G---B'--C'--D'--E'--F'
> 
> instead if it is done naively, but if you really care that the
> branch that ends with F does not have E, you presumably want to have
> the branch that ends at D its own identity, so
> 
>  (1) 'master' or whatever that used to end at A and now its tip is
>      at G;
> 
>  (2) the branch that ends at D whose upstream is 'master';
> 
>  (3) the branch that ends at E whose upstream is (2); and
> 
>  (4) the branch that ends at F whose upstream is (2).
> 
> I personally do not do that, though, because you'd need to remember
> the order in which these three branches must be rebased (i.e. (2)
> must be done first before rebasing (3) and (4) in any order).

I do occasionally have dependent topics, and use a topological sort to
order my rebases, which solves the problem. The code I use is in:

  https://github.com/peff/git/blob/meta/rebase

-Peff

^ permalink raw reply

* Re: [PATCH v3 00/23] Delete directories left empty after ref deletion
From: Jeff King @ 2017-01-02  4:19 UTC (permalink / raw)
  To: Jacob Keller
  Cc: Philip Oakley, Junio C Hamano, Michael Haggerty, Git mailing list,
	David Turner
In-Reply-To: <CA+P7+xqxSpV4yOjE+Lv0kw19Kq6UAbcN_-7O3U_EeBoT7AOtfw@mail.gmail.com>

On Sun, Jan 01, 2017 at 12:36:11PM -0800, Jacob Keller wrote:

> But how likely is it to end up with differing binaries running on the
> exact same repository concurrently? Basically, I am trying to see
> whether or not we could accidentally end up causing problems by trying
> to race with other git processes that haven't yet been made safe
> against race? Is the worst case only that some git operation would
> fail and you would have to retry?

Yes, I think that is the worst case.

A more likely scenario might be something like a server accepting pushes
or other ref updates from both JGit and regular git (or maybe libgit2
and regular git).

IMHO it's not really worth worrying about too much. Certain esoteric
setups might have a slightly higher chance of a pretty obscure race
condition happening on a very busy repository. I hate to say "eh, ship
it, we'll see if anybody complains". But I'd be surprised to get a
single report about this.

-Peff

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox