Git development
 help / color / mirror / Atom feed
* Re: [PATCH 02/17] dir: convert create_simplify to use the pathspec struct interface
From: Duy Nguyen @ 2016-12-09 13:08 UTC (permalink / raw)
  To: Brandon Williams; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <20161208181957.GP116201@google.com>

On Fri, Dec 9, 2016 at 1:19 AM, Brandon Williams <bmwill@google.com> wrote:
> On 12/08, Duy Nguyen wrote:
>> On Thu, Dec 8, 2016 at 7:03 AM, Brandon Williams <bmwill@google.com> wrote:
>> > On 12/07, Duy Nguyen wrote:
>> >> On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:
>> >> > Convert 'create_simplify()' to use the pathspec struct interface from
>> >> > using the '_raw' entry in the pathspec.
>> >>
>> >> It would be even better to kill this create_simplify() and let
>> >> simplify_away() handle struct pathspec directly.
>> >>
>> >> There is a bug in this code, that might have been found if we
>> >> simpify_away() handled pathspec directly: the memcmp() in
>> >> simplify_away() will not play well with :(icase) magic. My bad. If
>> >> :(icase) is used, the easiest/safe way is simplify nothing. Later on
>> >> maybe we can teach simplify_away() to do strncasecmp instead. We could
>> >> ignore exclude patterns there too (although not excluding is not a
>> >> bug).
>> >
>> > So are you implying that the simplify struct needs to be killed?  That
>> > way the pathspec struct itself is being passed around instead?
>>
>> Yes. simplify struct was a thing when pathspec was an array of char *.
>> At this point I think it can retire (when we have time to retire it)
>
> Alright, then for now I can leave this change as is and have a follow up
> series that kills the simplify struct.

Do let me know if you decide to drop it, so I can put it back in my backlog.
-- 
Duy

^ permalink raw reply

* Re: [PATCHv6 4/7] worktree: get worktrees from submodules
From: Duy Nguyen @ 2016-12-09 12:46 UTC (permalink / raw)
  To: Stefan Beller; +Cc: Brandon Williams, Git Mailing List, Junio C Hamano
In-Reply-To: <CAGZ79kYtEUvuTX09sJm3C0rG0-BrBz4bN0FCs6E5d2jHhtKN6w@mail.gmail.com>

On Fri, Dec 9, 2016 at 1:55 AM, Stefan Beller <sbeller@google.com> wrote:
> On Thu, Dec 8, 2016 at 2:09 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Thu, Dec 8, 2016 at 8:46 AM, Stefan Beller <sbeller@google.com> wrote:
>>>
>>>         worktree = xcalloc(1, sizeof(*worktree));
>>>         worktree->path = strbuf_detach(&worktree_path, NULL);
>>> @@ -101,7 +101,8 @@ static struct worktree *get_main_worktree(void)
>>
>> All the good stuff is outside context lines again.. Somewhere between
>> here we call add_head_info() which calls resolve_ref_unsafe(), which
>> always uses data from current repo, not the submodule we want it to
>> look at.
>
> Unrelated side question: What would you think of "variable context line
> configuration" ? e.g. you could configure it to include anything from
> up that line
> that is currently shown after the @@ which is the function signature line.

Hmm.. no idea. I once dreamt of writing "Diff-Options: -U10" in the
commit message and let git-log and everybody use that option
automatically, though. I guess it's unrelated to your question :D

> As to the add_head_info/resolve_ref_unsafe what impact does that have?
> It produces a wrong head info but AFAICT it will never die(), such that for the
> purposes of this series (which only wants to know if a submodule uses the
> worktree feature) it should be fine.
>
> It is highly misleading though for others to build upon this.
> So maybe I'll only add the functionality internally in worktree.c
> and document why the values are wrong, and only expose the
> "int submodule_uses_worktrees(const char *path)" ?

Yeah for submodule use then it should be ok. But people may start
using it for something else, not realizing that it does not work as
expected.
-- 
Duy

^ permalink raw reply

* Re: [PATCH/RFC 0/7] Pie-in-the-sky attempt to fix the early config
From: Duy Nguyen @ 2016-12-09 12:42 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git Mailing List, Junio C Hamano, Jeff King
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>

On Thu, Dec 8, 2016 at 10:35 PM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> Hopefully these patches will lead to something that we can integrate,
> and that eventually will make Git's startup sequence much less
> surprising.

What did it surprise you with? Just curious. I can see that I
disrespect the ceiling directory setting, perhaps that's that.
-- 
Duy

^ permalink raw reply

* Re: Feature request: read git config from parent directory
From: Duy Nguyen @ 2016-12-09 12:38 UTC (permalink / raw)
  To: dod; +Cc: Git Mailing List
In-Reply-To: <3881793.6JIRvg1BPW@ylum>

On Fri, Dec 9, 2016 at 2:49 AM, Dominique Dumont <dod@debian.org> wrote:
> Hello
>
> I use the same machine for work and open-source contribution. In both cases, I
> deal with a lot of repositories. Depending on whether I commit for work or
> open-source activities, I must use a different mail address. I used to setup
> work address for each work repo in git local config, but this is no longer
> practical.

Sounds like the same problem I have (and the reason I came up with
conditional include [1]). Would that work for you (check out the
example part in that patch)?

It's not supported yet. I'll need to address a few comments in the
future reroll.

[1] http://public-inbox.org/git/20160712164216.24072-1-pclouds@gmail.com/

> Since I use different directories for work and open-source, would it be
> possible for git to read irs config also from parent directories ? I.e. setup
> git to read config from ./.git/config then ../.gitconfig, ../../gitconfig until
> global ~/.gitconfig.
-- 
Duy

^ permalink raw reply

* Re: [PATCH v2 0/4] road to reentrant real_path
From: Duy Nguyen @ 2016-12-09 12:33 UTC (permalink / raw)
  To: Brandon Williams
  Cc: Git Mailing List, Stefan Beller, Jeff King, Jacob Keller,
	Junio C Hamano, Ramsay Jones, Torsten Bögershausen,
	Johannes Sixt
In-Reply-To: <1481241494-6861-1-git-send-email-bmwill@google.com>

On Fri, Dec 9, 2016 at 6:58 AM, Brandon Williams <bmwill@google.com> wrote:
> diff --git a/setup.c b/setup.c
> index fe572b8..0d9fdd0 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -254,10 +254,12 @@ int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)
>                 if (!is_absolute_path(data.buf))
>                         strbuf_addf(&path, "%s/", gitdir);
>                 strbuf_addbuf(&path, &data);
> -               strbuf_addstr(sb, real_path(path.buf));
> +               strbuf_realpath(sb, path.buf, 1);

This is not the same because of this hunk in strbuf_realpath()

> @@ -81,17 +73,18 @@ static const char *real_path_internal(const char *path, int die_on_error)
>                         goto error_out;
>         }
>
> -       strbuf_reset(&resolved);
> +       strbuf_reset(resolved);
>
>         if (is_absolute_path(path)) {

But if you you remove that, then all (old) callers of
strbuf_realpath() must do a strbuf_reset() in advance if needed
(probably just real_path does) which sounds reasonable to me. You're
probably want to be careful about the strbuf_reset() at the end of the
function too.

Other than that, I think this diff looks nice.
-- 
Duy

^ permalink raw reply

* Re: [PATCH] doc: fill in omitted word
From: Kristoffer Haugsbakk @ 2016-12-09 12:31 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Kristoffer Haugsbakk, git
In-Reply-To: <20161110230605.pwgzhai6xhud7pnu@sigill.intra.peff.net>


I agree.  Just writing "... what the plumbing does ..." is clearer and
less redundant.

I'll probably be sending a patch series that includes your proposed fix
sometime soon.

-- 
Kristoffer Haugsbakk

^ permalink raw reply

* Re: [PATCH v2 14/16] pathspec: create strip submodule slash helpers
From: Duy Nguyen @ 2016-12-09 12:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Brandon Williams, Git Mailing List, Stefan Beller
In-Reply-To: <xmqqfulyrlmf.fsf@gitster.mtv.corp.google.com>

On Fri, Dec 9, 2016 at 7:28 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Brandon Williams <bmwill@google.com> writes:
>
>> +static void strip_submodule_slash_cheap(struct pathspec_item *item)
>> +{
>> +     int i;
>> +
>> +     if ((item->len >= 1 && item->match[item->len - 1] == '/') &&
>> +         (i = cache_name_pos(item->match, item->len - 1)) >= 0 &&
>> +         S_ISGITLINK(active_cache[i]->ce_mode)) {
>> +             item->len--;
>> +             item->match[item->len] = '\0';
>> +     }
>> +}
>
> I know that this is merely a moved code, but while I was reading
> this, it triggered "Do not make an assignment inside if condition"
> check.

Yeah with a function of its own, it's probably better to separate that
assignment.

> But more importantly, is the code even correct?  If the path
> for the submodule is unmerged, we would get a negative i that points
> at the conflicting entry; don't we want to do something about it, at
> least when we have a submodule entry at stage #2 (i.e. ours)?

In my defense I was simply moving (again!) the code from
strip_trailing_slash_from_submodules() in b69bb3f:builtin/ls-files.c.
Could be an improvement point for submodule people, I guess.
-- 
Duy

^ permalink raw reply

* Bug: git-sh-setup giving no such file or directory
From: Paul Boyle @ 2016-12-09 12:00 UTC (permalink / raw)
  To: git

Hi

There appears to be an issue with the latest master.

"git submodule init" is producing the following error:

/home/paul.boyle/bin/git/git-sh-setup: line 46:
/home/paul.boyle/libexec/git-core/git-sh-i18n: No such file or
directory

Broken sha: 8d7a455ed52e2a96debc080dfc011b6bb00db5d2

Checking out an older version works fine.

git checkout 'master@{2016-11-01 18:30:00}'

Sha: 3cdd5d19178a54d2e51b5098d43b57571241d0ab

This can be reproduced simply by:

make clean ; make ; git submodule init


I didn't track it down further than to a commit sometime in the last month.

Details of machine that this is happening on:

[paul.boyle@gonzo git]$ cat /etc/redhat-release

Scientific Linux release 6.8 (Carbon)


[paul.boyle@gonzo git]$ env

SSH_AGENT_PID=29474

HOSTNAME=gonzo

TERM=xterm

SHELL=/bin/bash

HISTSIZE=1000

SSH_CLIENT=

QTDIR=/usr/lib64/qt-3.3

QTINC=/usr/lib64/qt-3.3/include

SSH_TTY=/dev/pts/135

USER=paul.boyle

LS_COLORS=rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:mi=01;05;37;41:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arj=01;31:*.taz=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.zip=01;31:*.z=01;31:*.Z=01;31:*.dz=01;31:*.gz=01;31:*.lz=01;31:*.xz=01;31:*.bz2=01;31:*.tbz=01;31:*.tbz2=01;31:*.bz=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.rar=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.jpg=01;35:*.jpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.axv=01;35:*.anx=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=01;36:*.au=01;36:*.flac=01;36:*.mid=01;36:*.midi=01;36:*.mka=01;36:*.mp3=01;36:*.mpc=01;36:*.ogg=01;36:*.ra=01;36:*.wav=01;36:*.axa=01;36:*.oga=01;36:*.spx=01;36:*.xspf=01;36:

SSH_AUTH_SOCK=/tmp/ssh-oXONs29470/agent.29470

MAIL=/var/spool/mail/paul.boyle

PATH=/home/paul.boyle/bin/git:/home/paul.boyle/bin/tig:/home/paul.boyle/bin:/usr/lib64/qt-3.3/bin:/usr/local/bin:/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/sbin:/home/paul.boyle/bin

PWD=/home/paul.boyle/bin/git

LANG=en_IE.UTF-8

HISTCONTROL=ignoredups

SHLVL=1

HOME=/home/paul.boyle

LOGNAME=paul.boyle

QTLIB=/usr/lib64/qt-3.3/lib

CVS_RSH=ssh

SSH_CONNECTION=

LESSOPEN=||/usr/bin/lesspipe.sh %s

G_BROKEN_FILENAMES=1

OLDPWD=/home/paul.boyle/

_=/bin/env

^ permalink raw reply

* Re: [PATCHv7 4/6] worktree: have a function to check if worktrees are in use
From: Duy Nguyen @ 2016-12-09 12:00 UTC (permalink / raw)
  To: Stefan Beller; +Cc: bmwill, git, gitster
In-Reply-To: <20161208210329.12919-5-sbeller@google.com>

On Thu, Dec 08, 2016 at 01:03:27PM -0800, Stefan Beller wrote:
> +/*
> + * NEEDSWORK: The values in the returned worktrees are broken, e.g.
> + * the refs or path resolution is influenced by the current repository.
> + */
> +static struct worktree **get_submodule_worktrees(const char *path, unsigned flags)

I'm ok with this (at least we know the function is half-broken). But I
wonder if we could go simpler, with something like this. It's more
efficient as well. And we can replace its implementation with
get_worktrees() or something when we are able to.

As long as this function stays in worktree.c I think it's ok for it to
peek into "worktrees" directory directly. That's what all other
functions in this file do after all.

int submodule_uses_worktrees(const char *path)
{
	struct strbuf path = STRBUF_INIT;
	DIR *dir;
	struct dirent *d;
	int ret = 0;

	strbuf_addf(&path, "%s/worktrees", path);
	dir = opendir(path.buf);
	strbuf_release(&path);

	if (!dir)
		return 0;

	while ((d = readdir(dir)) != NULL) {
		if (is_dot_or_dotdot(d->d_name))
			continue;

		ret = 1;
		break;
	}
	closedir(dir);
	return ret;
}
--
Duy

^ permalink raw reply

* Resend: Gitk: memory consumption improvements
From: Markus Hitter @ 2016-12-09 11:51 UTC (permalink / raw)
  To: git@vger.kernel.org; +Cc: Paul Mackerras


It's a month now since I sent three patches to this list for reducing memory consumption of Gitk considerably:

https://public-inbox.org/git/de7cd593-0c10-4e93-1681-7e123504f5d5@jump-ing.de/
https://public-inbox.org/git/e09a5309-351d-d246-d272-f527f50ad444@jump-ing.de/
https://public-inbox.org/git/8e1c5923-d2a6-bc77-97ab-3f154b41d2ea@jump-ing.de/
https://public-inbox.org/git/2cb7f76f-0004-a5b6-79f1-9bb4f979cf14@jump-ing.de/

Everybody cheered, but apparently nobody picked these patches up and applied them to either the Git or Gitk repository. They don't appear in the regular "what's cooking" post either.

Anything missing? I'd like to put a 'done' checkmark behind this.


Markus

-- 
- - - - - - - - - - - - - - - - - - -
Dipl. Ing. (FH) Markus Hitter
http://www.jump-ing.de/

^ permalink raw reply

* [PATCH] revert, cherry-pick: rename --quit to be consistent with rebase
From: Nguyễn Thái Ngọc Duy @ 2016-12-09 11:34 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, s-beyer, christian.couder, szeder,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <20161209113427.6039-1-pclouds@gmail.com>

The old --quit remains supported, just hidden away.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-cherry-pick.txt      |  2 +-
 Documentation/git-revert.txt           |  2 +-
 Documentation/sequencer.txt            |  2 +-
 builtin/revert.c                       |  7 +++++--
 contrib/completion/git-completion.bash |  4 ++--
 sequencer.c                            |  2 +-
 t/t3510-cherry-pick-sequence.sh        | 14 +++++++-------
 t/t3511-cherry-pick-x.sh               |  2 +-
 8 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 6154e57..de878ff 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff]
 		  [-S[<keyid>]] <commit>...
 'git cherry-pick' --continue
-'git cherry-pick' --quit
+'git cherry-pick' --forget
 'git cherry-pick' --abort
 
 DESCRIPTION
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index 573616a..c21a43b 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git revert' [--[no-]edit] [-n] [-m parent-number] [-s] [-S[<keyid>]] <commit>...
 'git revert' --continue
-'git revert' --quit
+'git revert' --forget
 'git revert' --abort
 
 DESCRIPTION
diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt
index 5747f44..ddfaad6 100644
--- a/Documentation/sequencer.txt
+++ b/Documentation/sequencer.txt
@@ -3,7 +3,7 @@
 	'.git/sequencer'.  Can be used to continue after resolving
 	conflicts in a failed cherry-pick or revert.
 
---quit::
+--forget::
 	Forget about the current operation in progress.  Can be used
 	to clear the sequencer state after a failed cherry-pick or
 	revert.
diff --git a/builtin/revert.c b/builtin/revert.c
index 56a2c36..663eaf7 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -77,7 +77,10 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 	const char *me = action_name(opts);
 	int cmd = 0;
 	struct option options[] = {
-		OPT_CMDMODE(0, "quit", &cmd, N_("end revert or cherry-pick sequence"), 'q'),
+		OPT_CMDMODE(0, "forget", &cmd, N_("end revert or cherry-pick sequence"), 'q'),
+		{ OPTION_CMDMODE, 0, "quit", &cmd, NULL,
+		  N_("end revert or cherry-pick sequence"),
+		  PARSE_OPT_NOARG|PARSE_OPT_NONEG|PARSE_OPT_HIDDEN, NULL, 'q' },
 		OPT_CMDMODE(0, "continue", &cmd, N_("resume revert or cherry-pick sequence"), 'c'),
 		OPT_CMDMODE(0, "abort", &cmd, N_("cancel revert or cherry-pick sequence"), 'a'),
 		OPT_BOOL('n', "no-commit", &opts->no_commit, N_("don't automatically commit")),
@@ -134,7 +137,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 	if (opts->subcommand != REPLAY_NONE) {
 		char *this_operation;
 		if (opts->subcommand == REPLAY_REMOVE_STATE)
-			this_operation = "--quit";
+			this_operation = "--forget";
 		else if (opts->subcommand == REPLAY_CONTINUE)
 			this_operation = "--continue";
 		else {
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8159f28..d5c74e7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1047,7 +1047,7 @@ _git_cherry_pick ()
 {
 	local dir="$(__gitdir)"
 	if [ -f "$dir"/CHERRY_PICK_HEAD ]; then
-		__gitcomp "--continue --quit --abort"
+		__gitcomp "--continue --forget --abort"
 		return
 	fi
 	case "$cur" in
@@ -2303,7 +2303,7 @@ _git_revert ()
 {
 	local dir="$(__gitdir)"
 	if [ -f "$dir"/REVERT_HEAD ]; then
-		__gitcomp "--continue --quit --abort"
+		__gitcomp "--continue --forget --abort"
 		return
 	fi
 	case "$cur" in
diff --git a/sequencer.c b/sequencer.c
index e66f2fe..12d10d0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -812,7 +812,7 @@ static int create_seq_dir(void)
 {
 	if (file_exists(git_path_seq_dir())) {
 		error(_("a cherry-pick or revert is already in progress"));
-		advise(_("try \"git cherry-pick (--continue | --quit | --abort)\""));
+		advise(_("try \"git cherry-pick (--continue | --forget | --abort)\""));
 		return -1;
 	}
 	else if (mkdir(git_path_seq_dir(), 0777) < 0)
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 7b7a89d..d84fafa 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -18,7 +18,7 @@ test_description='Test cherry-pick continuation features
 _r10='\1\1\1\1\1\1\1\1\1\1'
 
 pristine_detach () {
-	git cherry-pick --quit &&
+	git cherry-pick --forget &&
 	git checkout -f "$1^0" &&
 	git read-tree -u --reset HEAD &&
 	git clean -d -f -f -q -x
@@ -89,9 +89,9 @@ test_expect_success 'cherry-pick cleans up sequencer state upon success' '
 	test_path_is_missing .git/sequencer
 '
 
-test_expect_success '--quit does not complain when no cherry-pick is in progress' '
+test_expect_success '--forget does not complain when no cherry-pick is in progress' '
 	pristine_detach initial &&
-	git cherry-pick --quit
+	git cherry-pick --forget
 '
 
 test_expect_success '--abort requires cherry-pick in progress' '
@@ -99,14 +99,14 @@ test_expect_success '--abort requires cherry-pick in progress' '
 	test_must_fail git cherry-pick --abort
 '
 
-test_expect_success '--quit cleans up sequencer state' '
+test_expect_success '--forget cleans up sequencer state' '
 	pristine_detach initial &&
 	test_expect_code 1 git cherry-pick base..picked &&
-	git cherry-pick --quit &&
+	git cherry-pick --forget &&
 	test_path_is_missing .git/sequencer
 '
 
-test_expect_success '--quit keeps HEAD and conflicted index intact' '
+test_expect_success '--forget keeps HEAD and conflicted index intact' '
 	pristine_detach initial &&
 	cat >expect <<-\EOF &&
 	OBJID
@@ -116,7 +116,7 @@ test_expect_success '--quit keeps HEAD and conflicted index intact' '
 	:000000 100644 OBJID OBJID A	unrelated
 	EOF
 	test_expect_code 1 git cherry-pick base..picked &&
-	git cherry-pick --quit &&
+	git cherry-pick --forget &&
 	test_path_is_missing .git/sequencer &&
 	test_must_fail git update-index --refresh &&
 	{
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index 9cce5ae..a56d48e 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -5,7 +5,7 @@ test_description='Test cherry-pick -x and -s'
 . ./test-lib.sh
 
 pristine_detach () {
-	git cherry-pick --quit &&
+	git cherry-pick --forget &&
 	git checkout -f "$1^0" &&
 	git read-tree -u --reset HEAD &&
 	git clean -d -f -f -q -x
-- 
2.8.2.524.g6ff3d78


^ permalink raw reply related

* [PATCH] rebase: rename --forget to be consistent with sequencer
From: Nguyễn Thái Ngọc Duy @ 2016-12-09 11:34 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, s-beyer, christian.couder, szeder,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <CACsJy8CX0HO=LxcEK3K+pCecgFY=40R+gpFoy7CGeN5zEJFJVQ@mail.gmail.com>

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-rebase.txt           | 4 ++--
 contrib/completion/git-completion.bash | 4 ++--
 git-rebase.sh                          | 6 +++---
 t/t3407-rebase-abort.sh                | 8 ++++----
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index e01d78e..f892458 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 	[<upstream> [<branch>]]
 'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
 	--root [<branch>]
-'git rebase' --continue | --skip | --abort | --forget | --edit-todo
+'git rebase' --continue | --skip | --abort | --quit | --edit-todo
 
 DESCRIPTION
 -----------
@@ -252,7 +252,7 @@ leave out at most one of A and B, in which case it defaults to HEAD.
 	will be reset to where it was when the rebase operation was
 	started.
 
---forget::
+--quit::
 	Abort the rebase operation but HEAD is not reset back to the
 	original branch. The index and working tree are also left
 	unchanged as a result.
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8159f28..2c134f8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1670,10 +1670,10 @@ _git_rebase ()
 {
 	local dir="$(__gitdir)"
 	if [ -f "$dir"/rebase-merge/interactive ]; then
-		__gitcomp "--continue --skip --abort --forget --edit-todo"
+		__gitcomp "--continue --skip --abort --quit --edit-todo"
 		return
 	elif [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
-		__gitcomp "--continue --skip --abort --forget"
+		__gitcomp "--continue --skip --abort --quit"
 		return
 	fi
 	__git_complete_strategy && return
diff --git a/git-rebase.sh b/git-rebase.sh
index f0de633..c62b178 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -43,7 +43,7 @@ continue!          continue
 abort!             abort and check out the original branch
 skip!              skip current patch and continue
 edit-todo!         edit the todo list during an interactive rebase
-forget!            abort but keep HEAD where it is
+quit!              abort but keep HEAD where it is
 "
 . git-sh-setup
 . git-sh-i18n
@@ -240,7 +240,7 @@ do
 	--verify)
 		ok_to_skip_pre_rebase=
 		;;
-	--continue|--skip|--abort|--forget|--edit-todo)
+	--continue|--skip|--abort|--quit|--edit-todo)
 		test $total_argc -eq 2 || usage
 		action=${1##--}
 		;;
@@ -403,7 +403,7 @@ abort)
 	finish_rebase
 	exit
 	;;
-forget)
+quit)
 	exec rm -rf "$state_dir"
 	;;
 edit-todo)
diff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh
index 6bc5e71..910f218 100755
--- a/t/t3407-rebase-abort.sh
+++ b/t/t3407-rebase-abort.sh
@@ -99,26 +99,26 @@ testrebase() {
 testrebase "" .git/rebase-apply
 testrebase " --merge" .git/rebase-merge
 
-test_expect_success 'rebase --forget' '
+test_expect_success 'rebase --quit' '
 	cd "$work_dir" &&
 	# Clean up the state from the previous one
 	git reset --hard pre-rebase &&
 	test_must_fail git rebase master &&
 	test_path_is_dir .git/rebase-apply &&
 	head_before=$(git rev-parse HEAD) &&
-	git rebase --forget &&
+	git rebase --quit &&
 	test $(git rev-parse HEAD) = $head_before &&
 	test ! -d .git/rebase-apply
 '
 
-test_expect_success 'rebase --merge --forget' '
+test_expect_success 'rebase --merge --quit' '
 	cd "$work_dir" &&
 	# Clean up the state from the previous one
 	git reset --hard pre-rebase &&
 	test_must_fail git rebase --merge master &&
 	test_path_is_dir .git/rebase-merge &&
 	head_before=$(git rev-parse HEAD) &&
-	git rebase --forget &&
+	git rebase --quit &&
 	test $(git rev-parse HEAD) = $head_before &&
 	test ! -d .git/rebase-merge
 '
-- 
2.8.2.524.g6ff3d78


^ permalink raw reply related

* Re: BUG: "cherry-pick A..B || git reset --hard OTHER"
From: Duy Nguyen @ 2016-12-09 11:33 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Stephan Beyer, Git Mailing List, Christian Couder,
	SZEDER Gábor
In-Reply-To: <xmqq8trry08k.fsf@gitster.mtv.corp.google.com>

On Thu, Dec 8, 2016 at 3:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Stephan Beyer <s-beyer@gmx.net> writes:
>
>> [1] By the way: git cherry-pick --quit, git rebase --forget ...
>> different wording for the same thing makes things unintuitive.
>
> It is not too late to STOP "--forget" from getting added to "rebase"
> and give it a better name.

Having the same operation with different names only increases git
reputation of bad/inconsistent UI. Either forget is renamed to quit,
or vice versa. I prefer forget, but the decision is yours and the
community's. So I'm sending two patches to rename in either direction.
You can pick one.
-- 
Duy

^ permalink raw reply

* Re: Any interest in 'git merge --continue' as a command
From: Jacob Keller @ 2016-12-09 10:37 UTC (permalink / raw)
  To: Jeff King, Chris Packham; +Cc: GIT
In-Reply-To: <20161209091127.sxxczhfslrqsqs3m@sigill.intra.peff.net>

On December 9, 2016 1:11:27 AM PST, Jeff King <peff@peff.net> wrote:
>On Fri, Dec 09, 2016 at 08:57:58PM +1300, Chris Packham wrote:
>
>> I hit this at $dayjob recently.
>> 
>> A developer had got themselves into a confused state when needing to
>> resolve a merge conflict.
>> 
>> They knew about git rebase --continue (and git am and git
>cherry-pick)
>> but they were unsure how to "continue" a merge (it didn't help that
>> the advice saying to use 'git commit' was scrolling off the top of
>the
>> terminal). I know that using 'git commit' has been the standard way
>to
>> complete a merge but given other commands have a --continue should
>> merge have it as well?
>
>It seems like that would be in line with 35d2fffdb (Provide 'git merge
>--abort' as a synonym to 'git reset --merge', 2010-11-09), whose stated
>goal was providing consistency with other multi-command operations.
>
>I assume it would _just_ run a vanilla "git commit", and not try to do
>any trickery with updating the index (which could be disastrous).
>
>-Peff

This makes sense to me.

Thanks,
Jake



^ permalink raw reply

* Re: Error after calling git difftool -d with
From: David Aguilar @ 2016-12-09  9:47 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: P. Duijst, git
In-Reply-To: <alpine.DEB.2.20.1612051142550.117539@virtualbox>

On Mon, Dec 05, 2016 at 11:56:31AM +0100, Johannes Schindelin wrote:
> Hi Peter,
> 
> On Mon, 5 Dec 2016, P. Duijst wrote:
> 
> > On 12/5/2016 06:15, David Aguilar wrote:
> > > On Fri, Dec 02, 2016 at 05:05:06PM +0100, Johannes Schindelin wrote:
> > > >
> > > > On Fri, 2 Dec 2016, P. Duijst wrote:
> > > >
> > > > > Incase filenames are used with a quote ' or a bracket [  (and
> > > > > maybe some more characters), git "diff" and "difftool -y" works
> > > > > fine, but git *difftool **-d* gives the next error message:
> > > > >
> > > > >     peter@scm_ws_10 MINGW64 /d/Dev/test (master)
> > > > >     $ git diff
> > > > >     diff --git a/Test ''inch.txt b/Test ''inch.txt
> > > > >     index dbff793..41f3257 100644
> > > > >     --- a/Test ''inch.txt
> > > > >     +++ b/Test ''inch.txt
> > > > >     @@ -1 +1,3 @@
> > > > >     +
> > > > >     +ddd
> > > > >       Test error in simple repository
> > > > >     warning: LF will be replaced by CRLF in Test ''inch.txt.
> > > > >     The file will have its original line endings in your working
> > > > >     directory.
> > > > >
> > > > >     peter@scm_ws_10 MINGW64 /d/Dev/test (master)
> > > > >     *$ git difftool -d*
> > > > >     *fatal: Cannot open '/d/Dev/test//Test ''inch.txt': No such file or
> > > > >     directory*
> > > > >     *hash-object /d/Dev/test//Test ''inch.txt: command returned error:
> > > > >     128*
> > > > >
> > > > >     peter@scm_ws_10 MINGW64 /d/Dev/test (master)
> > > > >     $
> > > > >
> > > > >
> > > > > This issue is inside V2.10.x and V2.11.0.
> > > > > V2.9.0 is working correctly...
> > > > You say v2.11.0, but did you also try the new, experimental builtin
> > > > difftool? You can test without reinstalling:
> > > >
> > > >  git -c difftool.useBuiltin=true difftool -d ...
> > >
> > > FWIW, I verified that this problem does not manifest itself on Linux,
> > > using the current scripted difftool.
> > >
> > > Peter, what actual diff tool are you using?
> > >
> > > Since these filenames work fine with "difftool -d" on Linux, it
> > > suggests that this is either a tool-specific issue, or an issue
> > > related to unix-to-windows path translation.
> > 
> > @Johannes: "git -c difftool.useBuiltin=true difftool -d" works OK :-), beyond
> > compare is launching with the diff's displayed
> 
> Perfect.
> 
> In that case, I think it is not worth fixing the scripted tool but focus
> on getting rid of it in favor of the builtin version.
> 
> It's not like it is the only problem with having difftool implemented
> as a script...

I just sent some patches[1] that makes it so that difftool always
operates from the top-level of the repo, particularly when
calling hash-object.  They also eliminate using paths with
embedded "//" in them, both of which may have caused this issue

Though we can side-step this specific issue with the new builtin
difftool, if our use of hash-object with double-slashed absolute
paths was not the problem reported above, then another
possibility is that there's a problem in the Git.pm Perl module,
which affects more than just difftool.

I'm curious to understand the root cause of the problem.

Does Git.pm go through a shell on Windows?

Why was hash-object complaining about the correct path,
but reported that it didn't exist?
Did having "//" in the path cause the problem?

Enlightenment from the GFW internals perspective is much
appreciated.

Since this reportedly worked in older versions, I'm led to
believe that 32b8c581ec (difftool: use Git::* functions instead
of passing around state), which first introduced the use of
paths with embedded "//", was the root cause.  If this is true
then the patches should fix the scripted difftool on Windows.

[1] http://public-inbox.org/git/20161209085848.10929-1-davvid@gmail.com/T/#t

cheers,
-- 
David

^ permalink raw reply

* Re: [REGRESSION 2.10.2] problematic "empty auth" changes
From: David Turner @ 2016-12-08 21:12 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1612081538260.23160@virtualbox>

On Thu, 2016-12-08 at 15:47 +0100, Johannes Schindelin wrote:
> Hi Dave,
> 
> I got a couple of bug reports that claim that 2.10.2 regressed on using
> network credentials. That is, users regularly hit Enter twice when being
> asked for user name and password while fetching via https://, and cURL
> automatically used to fall back to using the login credentials (i.e.
> authenticating via the Domain controller).
> 
> Turns out those claims are correct: hitting Enter twice (or using URLs
> with empty user name/password such as https://:tfs:8080/) work in 2.10.1
> and yield "Authentication failed" in 2.10.2.
> 
> I tracked this down to 5275c3081c (http: http.emptyauth should allow empty
> (not just NULL) usernames, 2016-10-04) which all of a sudden disallowed
> empty user names (and now only handles things correctly when
> http.emptyAuth is set to true specifically).
> 
> This smells like a real bad regression to me, certainly given the time I
> had to spend to figure this out (starting from not exactly helpful bug
> reports, due to being very specific to their setups being private).
> 
> I am *really* tempted to change the default of http.emptyAuth to true, *at
> least* for Windows (where it is quite common to use your login credentials
> to authenticate to corporate servers).
> 
> Before I do anything rash, though: Do you see any downside to that?

I know of no reason that shouldn't work.  Indeed, it's what we use do
internally.  So far, nobody has reported problems.  That said, we have
exactly three sets of git servers that most users talk to (two different
internal; and occasionally github.com for external stuff).  So our
coverage is not very broad.

If you're going to do it, tho, don't just do it for Windows users -- do
it for everyone.  Plenty of Unix clients connect to Windows-based auth
systems.

That said, I could imagine that there are cases where it would cause
failures for a different set of users.  I don't know of any off the top
of my head, but this is not my area of expertise.

We could move closer to the old behavior with something like:

        if (!http_auth.username || !*http_auth.username) {
                if (curl_empty_auth)
                        curl_easy_setopt(result, CURLOPT_USERPWD, ":");
                if (!http_auth.username)
                        return;
        }

This is very ad-hoc, in that I have not examined exactly when
http_auth.username would be null vs empty; it merely attempts to get as
close as possible to the old behavior.

[Side note: I was curious if 26a7b23429 would have been a better fix for
our problem.  It appears that the answer is no; using that patch but
minus my 5275c3081c does not work for us.]



^ permalink raw reply

* Re: Any interest in 'git merge --continue' as a command
From: Jeff King @ 2016-12-09  9:11 UTC (permalink / raw)
  To: Chris Packham; +Cc: GIT
In-Reply-To: <CAFOYHZDs5rBt5+4D_ViMYfV04foq3h_UrsSMA3FfyMzLh9QdwA@mail.gmail.com>

On Fri, Dec 09, 2016 at 08:57:58PM +1300, Chris Packham wrote:

> I hit this at $dayjob recently.
> 
> A developer had got themselves into a confused state when needing to
> resolve a merge conflict.
> 
> They knew about git rebase --continue (and git am and git cherry-pick)
> but they were unsure how to "continue" a merge (it didn't help that
> the advice saying to use 'git commit' was scrolling off the top of the
> terminal). I know that using 'git commit' has been the standard way to
> complete a merge but given other commands have a --continue should
> merge have it as well?

It seems like that would be in line with 35d2fffdb (Provide 'git merge
--abort' as a synonym to 'git reset --merge', 2010-11-09), whose stated
goal was providing consistency with other multi-command operations.

I assume it would _just_ run a vanilla "git commit", and not try to do
any trickery with updating the index (which could be disastrous).

-Peff

^ permalink raw reply

* [PATCH 2/3] difftool: chdir as early as possible
From: David Aguilar @ 2016-12-09  8:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git ML
In-Reply-To: <20161209085848.10929-1-davvid@gmail.com>

Make difftool chdir to the top-level of the repository as soon as it can
so that we can simplify how paths are handled.  Replace construction of
absolute paths via string concatenation with relative paths wherever
possible.  The bulk of the code no longer needs to use absolute paths.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-difftool.perl | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/git-difftool.perl b/git-difftool.perl
index 17c336321f..99b03949bf 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -59,14 +59,14 @@ sub exit_cleanup
 
 sub use_wt_file
 {
-	my ($workdir, $file, $sha1) = @_;
+	my ($file, $sha1) = @_;
 	my $null_sha1 = '0' x 40;
 
-	if (-l "$workdir/$file" || ! -e _) {
+	if (-l $file || ! -e _) {
 		return (0, $null_sha1);
 	}
 
-	my $wt_sha1 = Git::command_oneline('hash-object', "$workdir/$file");
+	my $wt_sha1 = Git::command_oneline('hash-object', $file);
 	my $use = ($sha1 eq $null_sha1) || ($sha1 eq $wt_sha1);
 	return ($use, $wt_sha1);
 }
@@ -105,6 +105,12 @@ sub setup_dir_diff
 	my $diffrtn = Git::command_oneline(@gitargs);
 	exit(0) unless defined($diffrtn);
 
+	# Go to the root of the worktree now that we've captured the list of
+	# changed files.  The paths returned by diff --raw are relative to the
+	# top-level of the repository, but we defer changing directories so
+	# that @ARGV can perform pathspec limiting in the current directory.
+	chdir($workdir);
+
 	# Build index info for left and right sides of the diff
 	my $submodule_mode = '160000';
 	my $symlink_mode = '120000';
@@ -172,7 +178,7 @@ EOF
 				next;
 			}
 			my ($use, $wt_sha1) =
-				use_wt_file($workdir, $dst_path, $rsha1);
+				use_wt_file($dst_path, $rsha1);
 			if ($use) {
 				push @working_tree, $dst_path;
 				$wtindex .= "$rmode $wt_sha1\t$dst_path\0";
@@ -182,10 +188,6 @@ EOF
 		}
 	}
 
-	# Go to the root of the worktree so that the left index files
-	# are properly setup -- the index is toplevel-relative.
-	chdir($workdir);
-
 	# Setup temp directories
 	my $tmpdir = tempdir('git-difftool.XXXXX', CLEANUP => 0, TMPDIR => 1);
 	my $ldir = "$tmpdir/left";
@@ -235,10 +237,10 @@ EOF
 			symlink("$workdir/$file", "$rdir/$file") or
 			exit_cleanup($tmpdir, 1);
 		} else {
-			copy("$workdir/$file", "$rdir/$file") or
+			copy($file, "$rdir/$file") or
 			exit_cleanup($tmpdir, 1);
 
-			my $mode = stat("$workdir/$file")->mode;
+			my $mode = stat($file)->mode;
 			chmod($mode, "$rdir/$file") or
 			exit_cleanup($tmpdir, 1);
 		}
@@ -430,10 +432,10 @@ sub dir_diff
 			$error = 1;
 		} elsif (exists $tmp_modified{$file}) {
 			my $mode = stat("$b/$file")->mode;
-			copy("$b/$file", "$workdir/$file") or
+			copy("$b/$file", $file) or
 			exit_cleanup($tmpdir, 1);
 
-			chmod($mode, "$workdir/$file") or
+			chmod($mode, $file) or
 			exit_cleanup($tmpdir, 1);
 		}
 	}
-- 
2.11.0.26.gb65c994


^ permalink raw reply related

* [PATCH 3/3] difftool: rename variables for consistency
From: David Aguilar @ 2016-12-09  8:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git ML
In-Reply-To: <20161209085848.10929-1-davvid@gmail.com>

Always call the list of files @files.
Always call the worktree $worktree.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-difftool.perl | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/git-difftool.perl b/git-difftool.perl
index 99b03949bf..4e4f5d8138 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -100,7 +100,7 @@ sub changed_files
 
 sub setup_dir_diff
 {
-	my ($workdir, $symlinks) = @_;
+	my ($worktree, $symlinks) = @_;
 	my @gitargs = ('diff', '--raw', '--no-abbrev', '-z', @ARGV);
 	my $diffrtn = Git::command_oneline(@gitargs);
 	exit(0) unless defined($diffrtn);
@@ -109,7 +109,7 @@ sub setup_dir_diff
 	# changed files.  The paths returned by diff --raw are relative to the
 	# top-level of the repository, but we defer changing directories so
 	# that @ARGV can perform pathspec limiting in the current directory.
-	chdir($workdir);
+	chdir($worktree);
 
 	# Build index info for left and right sides of the diff
 	my $submodule_mode = '160000';
@@ -121,7 +121,7 @@ sub setup_dir_diff
 	my $wtindex = '';
 	my %submodule;
 	my %symlink;
-	my @working_tree = ();
+	my @files = ();
 	my %working_tree_dups = ();
 	my @rawdiff = split('\0', $diffrtn);
 
@@ -173,14 +173,14 @@ EOF
 		}
 
 		if ($rmode ne $null_mode) {
-			# Avoid duplicate working_tree entries
+			# Avoid duplicate entries
 			if ($working_tree_dups{$dst_path}++) {
 				next;
 			}
 			my ($use, $wt_sha1) =
 				use_wt_file($dst_path, $rsha1);
 			if ($use) {
-				push @working_tree, $dst_path;
+				push @files, $dst_path;
 				$wtindex .= "$rmode $wt_sha1\t$dst_path\0";
 			} else {
 				$rindex .= "$rmode $rsha1\t$dst_path\0";
@@ -227,14 +227,14 @@ EOF
 
 	# Changes in the working tree need special treatment since they are
 	# not part of the index.
-	for my $file (@working_tree) {
+	for my $file (@files) {
 		my $dir = dirname($file);
 		unless (-d "$rdir/$dir") {
 			mkpath("$rdir/$dir") or
 			exit_cleanup($tmpdir, 1);
 		}
 		if ($symlinks) {
-			symlink("$workdir/$file", "$rdir/$file") or
+			symlink("$worktree/$file", "$rdir/$file") or
 			exit_cleanup($tmpdir, 1);
 		} else {
 			copy($file, "$rdir/$file") or
@@ -278,7 +278,7 @@ EOF
 		exit_cleanup($tmpdir, 1) if not $ok;
 	}
 
-	return ($ldir, $rdir, $tmpdir, @working_tree);
+	return ($ldir, $rdir, $tmpdir, @files);
 }
 
 sub write_to_file
@@ -388,9 +388,9 @@ sub dir_diff
 	my $error = 0;
 	my $repo = Git->repository();
 	my $repo_path = $repo->repo_path();
-	my $workdir = $repo->wc_path();
-	$workdir =~ s|/$||; # Avoid double slashes in symlink targets
-	my ($a, $b, $tmpdir, @worktree) = setup_dir_diff($workdir, $symlinks);
+	my $worktree = $repo->wc_path();
+	$worktree =~ s|/$||; # Avoid double slashes in symlink targets
+	my ($a, $b, $tmpdir, @files) = setup_dir_diff($worktree, $symlinks);
 
 	if (defined($extcmd)) {
 		$rc = system($extcmd, $a, $b);
@@ -411,13 +411,13 @@ sub dir_diff
 	my %tmp_modified;
 	my $indices_loaded = 0;
 
-	for my $file (@worktree) {
+	for my $file (@files) {
 		next if $symlinks && -l "$b/$file";
 		next if ! -f "$b/$file";
 
 		if (!$indices_loaded) {
 			%wt_modified = changed_files(
-				$repo_path, "$tmpdir/wtindex", $workdir);
+				$repo_path, "$tmpdir/wtindex", $worktree);
 			%tmp_modified = changed_files(
 				$repo_path, "$tmpdir/wtindex", $b);
 			$indices_loaded = 1;
@@ -425,7 +425,7 @@ sub dir_diff
 
 		if (exists $wt_modified{$file} and exists $tmp_modified{$file}) {
 			my $errmsg = "warning: Both files modified: ";
-			$errmsg .= "'$workdir/$file' and '$b/$file'.\n";
+			$errmsg .= "'$worktree/$file' and '$b/$file'.\n";
 			$errmsg .= "warning: Working tree file has been left.\n";
 			$errmsg .= "warning:\n";
 			warn $errmsg;
-- 
2.11.0.26.gb65c994


^ permalink raw reply related

* [PATCH 1/3] difftool: sanitize $workdir as early as possible
From: David Aguilar @ 2016-12-09  8:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git ML

The double-slash fixup on the $workdir variable was being
performed just-in-time to avoid double-slashes in symlink
targets, but the rest of the code was silently using paths with
embedded "//" in them.

A recent user-reported error message contained double-slashes.
Eliminate the issue by sanitizing inputs as soon as they arrive.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-difftool.perl | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/git-difftool.perl b/git-difftool.perl
index 959822d5f3..17c336321f 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -224,9 +224,7 @@ EOF
 	delete($ENV{GIT_INDEX_FILE});
 
 	# Changes in the working tree need special treatment since they are
-	# not part of the index. Remove any trailing slash from $workdir
-	# before starting to avoid double slashes in symlink targets.
-	$workdir =~ s|/$||;
+	# not part of the index.
 	for my $file (@working_tree) {
 		my $dir = dirname($file);
 		unless (-d "$rdir/$dir") {
@@ -389,6 +387,7 @@ sub dir_diff
 	my $repo = Git->repository();
 	my $repo_path = $repo->repo_path();
 	my $workdir = $repo->wc_path();
+	$workdir =~ s|/$||; # Avoid double slashes in symlink targets
 	my ($a, $b, $tmpdir, @worktree) = setup_dir_diff($workdir, $symlinks);
 
 	if (defined($extcmd)) {
-- 
2.11.0.26.gb65c994


^ permalink raw reply related

* Re: git bash error
From: Konstantin Khomoutov @ 2016-12-09  8:25 UTC (permalink / raw)
  To: git; +Cc: Karamjeet Singh
In-Reply-To: <58D2713C848141E88F0156A1BF3A8B19@Karamjeet>

On Fri, 9 Dec 2016 11:38:55 +0530
"Karamjeet Singh" <karamjeet.singh@netsutra.com> wrote:

> Dear git support,
> My app is crashing whenever i launch the git bash tool. I am
> attaching the error log file from the event viewer. Can you please
> let me know what the issue is with it.
> https://www.dropbox.com/sh/mhkmjn8bmh3x1oh/AABUKmhnn-HW2Kv5UVxdckN6a?dl=0

Hi!

Your report misses lots of information required to even approach it:

- What Git version are you using (the fact is: in each next version of
  a software package some bugs get fixed and others might creep in;
  so knowing an exact version is paramount).

- What OS? Version, flavor, architecture (32/64 bit).

- What software package (i.e. where did you get your Git install from)?

From the term "git bash", I gather you're talking about Git for Windows.
If so, that project has its own bug tracker on Github [1] -- because
it's still a project sort-of separate from the "vanilla" Git due to
the fact it maintains a set of changes not yet in the Git proper, and
they do packaging work, too.

Please use that issue tracker in two steps:

1) Search it for your issue.  Say, remove the "is:open" modifier from
   the search box in the tracker's web interface, put there the words
   "git", "bash" and "crash" and search.  I'm sure you'll get a hefty
   amount of reports.  Please see whether your issue is already
   reported.

2) If yes, and if (and only if) you have additional details about it,
   please summarise them in a comment.  Please try to write that in
   plain English (plain bad non-native English is okay :-)); try not to
   post links to pictures or videos.  They aren't indexed by search
   engines and require the maintainers to switch their context when
   reading your report/comment.  On some platforms (say, w/o proper
   full-blown web browser) they can even be plain hard to even see.

3) If no, write your report there -- by filling the offerred template.

Thanks.

1. https://github.com/git-for-windows/git/issues/

^ permalink raw reply

* Any interest in 'git merge --continue' as a command
From: Chris Packham @ 2016-12-09  7:57 UTC (permalink / raw)
  To: GIT

I hit this at $dayjob recently.

A developer had got themselves into a confused state when needing to
resolve a merge conflict.

They knew about git rebase --continue (and git am and git cherry-pick)
but they were unsure how to "continue" a merge (it didn't help that
the advice saying to use 'git commit' was scrolling off the top of the
terminal). I know that using 'git commit' has been the standard way to
complete a merge but given other commands have a --continue should
merge have it as well?

^ permalink raw reply

* git bash error
From: Karamjeet Singh @ 2016-12-09  6:08 UTC (permalink / raw)
  To: git

Dear git support,
My app is crashing whenever i launch the git bash tool. I am attaching the 
error log file from the event viewer. Can you please let me know what the 
issue is with it.
https://www.dropbox.com/sh/mhkmjn8bmh3x1oh/AABUKmhnn-HW2Kv5UVxdckN6a?dl=0

Thanks
Karamjeet


^ permalink raw reply

* Re: [BUG] regarding `git add -p` and files containing wildcard characters
From: Jeff King @ 2016-12-09  5:37 UTC (permalink / raw)
  To: unixway.drive; +Cc: git
In-Reply-To: <c9876671-6252-5dfa-18df-a6719dc6834c@gmail.com>

On Fri, Dec 09, 2016 at 04:46:49AM +0300, unixway.drive@gmail.com wrote:

> The problem is that `git-diff-files` does some globbing on the 'path'
> arguments on its own and has no option to disable that (and
> `git-add--interactive`'s `run_cmd_pipe` already handles all other sorts of
> unsafe characters like spaces and backticks well).

I think the option you're looking for is:

  git --literal-pathspecs diff-files ... -- 'Random *'

I don't know if there are other commands run by add--interactive that
would want the same treatment. It might actually make sense to set
GIT_LITERAL_PATHSPECS=1 in the environment right after it expands the
list via ls-files.

> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index ee3d812..358d877 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -2,3 +2,3 @@
> 
> -use 5.008;
> +use 5.014;
>  use strict;
> @@ -761,3 +761,5 @@ sub parse_diff {
>         }
> -       my @diff = run_cmd_pipe("git", @diff_cmd, "--", $path);
> +       my @diff = run_cmd_pipe("git", @diff_cmd, "--", (
> +               $path =~ s#[\[*?]#\\$&#gr
> +       ));

This callsite covers "-p". It looks like list_modified() probably needs
similar treatment. Maybe others; I didn't look exhaustively.

-Peff

^ permalink raw reply

* [PATCH] commit: remove 'Clever' message for --only --amend
From: Andreas Krey @ 2016-12-09  4:10 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <xmqq1sxiv051.fsf@gitster.mtv.corp.google.com>

That behavior is now documented, and we don't
need a reward afterwards.

Signed-off-by: Andreas Krey <a.krey@gmx.de>
---

> Sorry for making you send an extra round; let's queue the original,
> and if you still are interested, have the "Clever" removal as its
> own patch.

Here you go.

 builtin/commit.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 89b66816f..276c74034 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1208,8 +1208,6 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		die(_("Only one of --include/--only/--all/--interactive/--patch can be used."));
 	if (argc == 0 && (also || (only && !amend && !allow_empty)))
 		die(_("No paths with --include/--only does not make sense."));
-	if (argc == 0 && only && amend)
-		only_include_assumed = _("Clever... amending the last one with dirty index.");
 	if (argc > 0 && !also && !only)
 		only_include_assumed = _("Explicit paths specified without -i or -o; assuming --only paths...");
 	if (!cleanup_arg || !strcmp(cleanup_arg, "default"))
-- 
2.11.0.10.g1e1b186.dirty

^ permalink raw reply related


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