Git development
 help / color / mirror / Atom feed
* Re: difftool -d not populating left correctly when not in git root
From: David Aguilar @ 2016-12-04 22:31 UTC (permalink / raw)
  To: Frank Becker; +Cc: git, John Keeping, Johannes Schindelin, Junio C Hamano
In-Reply-To: <68f49f5e-4e71-fd52-cd6f-64e92face962@mooflu.com>

On Fri, Dec 02, 2016 at 03:04:01PM -0800, Frank Becker wrote:
> Hi,
> 
> looks like this broke between 2.9.2 and 2.9.3
> 
> cat ~/.gitconfig
> [difftool "diff"]
>     cmd = ls -l ${LOCAL}/* ${REMOTE}/*
>     #cmd = diff -r ${LOCAL} ${REMOTE} | less
> 
> ~/stuff/gittest> ls -l *
> d1:
> total 8
> -rw-r--r--  1 frank  staff  16  2 Dec 14:30 test.txt
> 
> d2:
> total 8
> -rw-r--r--  1 frank  staff  18  2 Dec 14:30 anothertest.tst
> 
> 
> ~/stuff/gittest> git status
> On branch master
> Changes not staged for commit:
>   (use "git add <file>..." to update what will be committed)
>   (use "git checkout -- <file>..." to discard changes in working directory)
> 
>     modified:   d1/test.txt
>     modified:   d2/anothertest.tst
> 
> 
> ~/stuff/gittest> ~/stuff/git_tmp/bin/git --version
> git version 2.11.0
> 
> ~/stuff/gittest> ~/stuff/git_tmp/bin/git difftool -d -t diff
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/left/d1:
> total 8
> -rw-r--r--  1 frank  staff  6  2 Dec 14:52 test.txt
> 
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/left/d2:
> total 8
> -rw-r--r--  1 frank  staff  7  2 Dec 14:52 anothertest.tst
> 
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/right/d1:
> total 8
> lrwxr-xr-x  1 frank  staff  38  2 Dec 14:52 test.txt ->
> /Users/frank/stuff/gittest/d1/test.txt
> 
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.0oGRF/right/d2:
> total 8
> lrwxr-xr-x  1 frank  staff  45  2 Dec 14:52 anothertest.tst ->
> /Users/frank/stuff/gittest/d2/anothertest.tst
> 
> 
> cd d2
> ~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git difftool -d -t diff
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.eRXhB/left/d2:
> total 8
> -rw-r--r--  1 frank  staff  7  2 Dec 14:52 anothertest.tst
> 
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.eRXhB/right/d1:
> total 8
> lrwxr-xr-x  1 frank  staff  38  2 Dec 14:52 test.txt ->
> /Users/frank/stuff/gittest/d1/test.txt
> 
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.eRXhB/right/d2:
> total 8
> lrwxr-xr-x  1 frank  staff  45  2 Dec 14:52 anothertest.tst ->
> /Users/frank/stuff/gittest/d2/anothertest.tst
> 
> 
> Note that left does not contain d1
> 
> 
> 
> ~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git --version
> git version 2.9.2
> ~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git difftool -d -t diff
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/left/d1:
> total 8
> -rw-r--r--  1 frank  staff  6  2 Dec 15:02 test.txt
> 
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/left/d2:
> total 8
> -rw-r--r--  1 frank  staff  7  2 Dec 15:02 anothertest.tst
> 
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/right/d1:
> total 8
> lrwxr-xr-x  1 frank  staff  38  2 Dec 15:02 test.txt ->
> /Users/frank/stuff/gittest/d1/test.txt
> 
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.YxtVw/right/d2:
> total 8
> lrwxr-xr-x  1 frank  staff  45  2 Dec 15:02 anothertest.tst ->
> /Users/frank/stuff/gittest/d2/anothertest.tst
> 
> 
> 
> ~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git --version
> git version 2.9.3
> ~/stuff/gittest/d2> ~/stuff/git_tmp/bin/git difftool -d -t diff
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.TpJ5u/left/d2:
> total 8
> -rw-r--r--  1 frank  staff  7  2 Dec 15:01 anothertest.tst
> 
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.TpJ5u/right/d1:
> total 8
> lrwxr-xr-x  1 frank  staff  38  2 Dec 15:01 test.txt ->
> /Users/frank/stuff/gittest/d1/test.txt
> 
> /var/folders/0j/3pk3pdsx7rzb9_njdpyjwm000000gn/T/git-difftool.TpJ5u/right/d2:
> total 8
> lrwxr-xr-x  1 frank  staff  45  2 Dec 15:01 anothertest.tst ->
> /Users/frank/stuff/gittest/d2/anothertest.tst

This regression was not caught by our test suite.

This looks like it's an edge case not handled by:
9ec26e797781 "difftool: fix argument handling in subdirs"
The current "rewrite difftool in C" topic may need a
similar adjustment.

The problem:

When preparing the right-side of the diff we only construct the
parts that changed.  When constructing the left side we
construct a full index, but we were constructing it relative to
the subdirectory, and thus it ends up empty because we are in a
subdirectory and the paths are incorrect.

The fix seems simple -- when preparing the index files we need
to chdir to the toplevel to ensure that the index construction
steps find the correct toplevel-relative paths.

Thanks for the heads-up,
David

--- 8< ---
Date: Sun, 4 Dec 2016 14:27:17 -0800
Subject: [PATCH] difftool: properly handle being launched from a subdirectory

9ec26e797781239b36ebccb87c590e5778358007 corrected how path arguments
are handled in a subdirectory, but it introduced a regression in how
entries outside of the subdirectory are handled by the dir-diff.

When preparing the right-side of the diff we only construct the parts
that changed.

When constructing the left side we construct an index, but we were
constructing it relative to the subdirectory, and thus it ends up empty
because we are in a subdirectory and the paths are incorrect.

Teach difftool to chdir to the toplevel of the repository before
preparing its temporary indexes.  This ensures that all of the
toplevel-relative paths are valid.

Add a test case to exercise this use case.

Reported-by: Frank Becker <fb@mooflu.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-difftool.perl   | 4 ++++
 t/t7800-difftool.sh | 7 +++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/git-difftool.perl b/git-difftool.perl
index a5790d03a..959822d5f 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -182,6 +182,10 @@ 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";
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 70a2de461..caab4b5ca 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -413,8 +413,11 @@ run_dir_diff_test 'difftool --dir-diff from subdirectory' '
 	(
 		cd sub &&
 		git difftool --dir-diff $symlinks --extcmd ls branch >output &&
-		grep sub output &&
-		grep file output
+		# "sub" must only exist in "right"
+		# "file" and "file2" must be listed in both "left" and "right"
+		test "1" = $(grep sub output | wc -l) &&
+		test "2" = $(grep file"$" output | wc -l) &&
+		test "2" = $(grep file2 output | wc -l)
 	)
 '
 
-- 
2.11.0.dirty

^ permalink raw reply related

* [PATCH] clone,fetch: explain the shallow-clone option a little more clearly
From: Alex Henrie @ 2016-12-04 22:03 UTC (permalink / raw)
  To: pclouds, gitster, git; +Cc: Alex Henrie

"deepen by excluding" does not make sense because excluding a revision
does not deepen a repository; it makes the repository more shallow.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 builtin/clone.c | 2 +-
 builtin/fetch.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 6c76a6e..e3cb808 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -99,7 +99,7 @@ static struct option builtin_clone_options[] = {
 	OPT_STRING(0, "shallow-since", &option_since, N_("time"),
 		    N_("create a shallow clone since a specific time")),
 	OPT_STRING_LIST(0, "shallow-exclude", &option_not, N_("revision"),
-			N_("deepen history of shallow clone by excluding rev")),
+			N_("deepen history of shallow clone, excluding rev")),
 	OPT_BOOL(0, "single-branch", &option_single_branch,
 		    N_("clone only one branch, HEAD or --branch")),
 	OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
diff --git a/builtin/fetch.c b/builtin/fetch.c
index b6a5597..fc74c84 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -122,7 +122,7 @@ static struct option builtin_fetch_options[] = {
 	OPT_STRING(0, "shallow-since", &deepen_since, N_("time"),
 		   N_("deepen history of shallow repository based on time")),
 	OPT_STRING_LIST(0, "shallow-exclude", &deepen_not, N_("revision"),
-			N_("deepen history of shallow clone by excluding rev")),
+			N_("deepen history of shallow clone, excluding rev")),
 	OPT_INTEGER(0, "deepen", &deepen_relative,
 		    N_("deepen history of shallow clone")),
 	{ OPTION_SET_INT, 0, "unshallow", &unshallow, NULL,
-- 
2.10.2


^ permalink raw reply related

* [PATCH] receive-pack: improve English grammar of denyCurrentBranch message
From: Alex Henrie @ 2016-12-04 22:04 UTC (permalink / raw)
  To: gitster, git; +Cc: Alex Henrie

The article "the" is required here.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 builtin/receive-pack.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e6b3879..6b97cbd 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -795,8 +795,8 @@ static char *refuse_unconfigured_deny_msg =
 	   "with what you pushed, and will require 'git reset --hard' to match\n"
 	   "the work tree to HEAD.\n"
 	   "\n"
-	   "You can set 'receive.denyCurrentBranch' configuration variable to\n"
-	   "'ignore' or 'warn' in the remote repository to allow pushing into\n"
+	   "You can set the 'receive.denyCurrentBranch' configuration variable\n"
+	   "to 'ignore' or 'warn' in the remote repository to allow pushing into\n"
 	   "its current branch; however, this is not recommended unless you\n"
 	   "arranged to update its work tree to match what you pushed in some\n"
 	   "other way.\n"
-- 
2.10.2


^ permalink raw reply related

* [PATCH] bisect: improve English grammar of not-ancestors message
From: Alex Henrie @ 2016-12-04 22:04 UTC (permalink / raw)
  To: pclouds, gitster, git; +Cc: Alex Henrie

Multiple revisions cannot be a single ancestor.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 bisect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bisect.c b/bisect.c
index 21bc6da..8e63c40 100644
--- a/bisect.c
+++ b/bisect.c
@@ -747,7 +747,7 @@ static void handle_bad_merge_base(void)
 		exit(3);
 	}
 
-	fprintf(stderr, _("Some %s revs are not ancestor of the %s rev.\n"
+	fprintf(stderr, _("Some %s revs are not ancestors of the %s rev.\n"
 		"git bisect cannot work properly in this case.\n"
 		"Maybe you mistook %s and %s revs?\n"),
 		term_good, term_bad, term_good, term_bad);
-- 
2.10.2


^ permalink raw reply related

* [RFC PATCH] GIT-VERSION-GEN: set --abbrev=9 to match auto-scaling
From: Ramsay Jones @ 2016-12-04 20:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: GIT Mailing-list


Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---

Hi Junio,

I recently noticed that:

    $ make >pout 2>&1
    $ ./git version
    git version 2.11.0.286.g109e8a9
    $ git describe
    v2.11.0-286-g109e8a99d
    $

... for non-release builds, the commit part of the version
string was still using an --abbrev=7.

I don't know that it actually matters too much (since it will
show as many as necessary, thus the RFC), but it caused me to
look twice. ;-)

ATB,
Ramsay Jones

 GIT-VERSION-GEN | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 520d6e66e..05601f753 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -12,7 +12,7 @@ if test -f version
 then
 	VN=$(cat version) || VN="$DEF_VER"
 elif test -d ${GIT_DIR:-.git} -o -f .git &&
-	VN=$(git describe --match "v[0-9]*" --abbrev=7 HEAD 2>/dev/null) &&
+	VN=$(git describe --match "v[0-9]*" --abbrev=9 HEAD 2>/dev/null) &&
 	case "$VN" in
 	*$LF*) (exit 1) ;;
 	v[0-9]*)
-- 
2.11.0

^ permalink raw reply related

* Re: [BUG] git gui can't commit multiple files
From: David Aguilar @ 2016-12-04 20:40 UTC (permalink / raw)
  To: Timon; +Cc: git
In-Reply-To: <CANtxn9J9O+PADxpWa0JCcgwwk_tC5DuJGUruULN2fGP3knZ-Sw@mail.gmail.com>

On Sun, Dec 04, 2016 at 05:36:46PM +0100, Timon wrote:
> This is a regression in git 2.11.0 (version 2.10.2 is fine).
> 
> In git-gui I select multiple files in the Unstaged Changes (using
> shift+click) and press ctrl+t to stage them. Then only one files gets
> staged instead of all of the selected files.
> The same happens when unstaging files.
> 
> Git-cola also exhibits the same behavior. Although there I could stage
> multiple files if I used a popup menu instead of the keyboard shortcut
> (I'm guessing it goes through a different code path?).

Can you elaborate a bit?

I just tested git-cola with Git 2.11 and it worked fine for me.
I selected several files and used the Ctrl+s hotkey to stage the
selected files.  They all got staged.

If you have a test repo, or reproduction recipe, I'd be curious
to try it out.
-- 
David

^ permalink raw reply

* [PATCH] diff: fix up SHA-1 abbreviations outside of repository
From: Jack Bates @ 2016-12-04 19:47 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Jack Bates

The three cases where "git diff" operates outside of a repository are 1)
when we run it outside of a repository, 2) when one of the files we're
comparing is outside of the repository we're in, and 3) the --no-index
option. Commit 4f03666 ("diff: handle sha1 abbreviations outside of
repository", 2016-10-20) only worked in the first case.
---
 builtin/diff.c                  |  4 +++-
 t/t4063-diff-no-index-abbrev.sh | 50 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100755 t/t4063-diff-no-index-abbrev.sh

diff --git a/builtin/diff.c b/builtin/diff.c
index 7f91f6d..ec7c432 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -342,9 +342,11 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 		       "--no-index" : "[--no-index]");
 
 	}
-	if (no_index)
+	if (no_index) {
 		/* If this is a no-index diff, just run it and exit there. */
+		startup_info->have_repository = 0;
 		diff_no_index(&rev, argc, argv);
+	}
 
 	/* Otherwise, we are doing the usual "git" diff */
 	rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
diff --git a/t/t4063-diff-no-index-abbrev.sh b/t/t4063-diff-no-index-abbrev.sh
new file mode 100755
index 0000000..d1d6302
--- /dev/null
+++ b/t/t4063-diff-no-index-abbrev.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='don'\'' peek into .git when operating outside of a repository
+
+When abbreviating SHA-1s, if another object in the repository has the
+same abbreviation, we normally lengthen the abbreviation until it'\''s
+unique. Commit 4f03666 ("diff: handle sha1 abbreviations outside of
+repository", 2016-10-20) addressed the case of abbreviating SHA-1s
+outside the context of a repository. In that case we shouldn'\''t peek
+into a .git directory to make an abbreviation unique.
+
+To check that we don'\''t, create an blob with a SHA-1 that starts with
+0000. (Outside of a repository, SHA-1s are all zeros.) Then make an
+abbreviation and check that Git doesn'\''t lengthen it.
+
+The three cases where "git diff" operates outside of a repository are
+1) when we run it outside of a repository, 2) when one of the files
+we'\''re comparing is outside of the repository we'\''re in,
+and 3) the --no-index option.
+'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	echo 1 >a &&
+	echo 2 >b &&
+	git init repo &&
+	(
+		cd repo &&
+
+		# Create a blob
+		# 00002e907f44c3881822c473d8842405cfd96362
+		echo 119132 >collision &&
+		git add collision
+	)
+'
+
+cat >expect <<EOF
+:100644 100644 0000... 0000... M	../a
+EOF
+
+test_expect_success 'don'\''t peek into .git when operating outside of a repository' '
+	(
+		cd repo &&
+		test_must_fail git diff --raw --abbrev=4 ../a ../b >actual &&
+		test_cmp ../expect actual
+	)
+'
+
+test_done
-- 
2.10.2

^ permalink raw reply related

* Re: git reset --hard should not irretrievably destroy new files
From: Junio C Hamano @ 2016-12-04 19:08 UTC (permalink / raw)
  To: Julian de Bhal; +Cc: git
In-Reply-To: <CAJZCeG1Eu+5DfaxavX_WGUCa+SY+yepDWZhPXxiFcV__h0xjrw@mail.gmail.com>

Julian de Bhal <julian.debhal@gmail.com> writes:

> The behaviour that would make the most sense to me (personally) would be
> for a hard reset to unstage new files,...

I think _sometimes_ that may be useful.  I haven't thought things
through yet to arrive the final decision, but one thing that must be
kept in mind by anybody who wants to move this topic forward is that
a path that does not exist in the HEAD commit MUST be removed from
the index and the working tree upon "git reset --hard" when the path
resulted from a mergy operation.  I.e. in this sequence:

    $ git merge other-branch ;# or git cherry-pick one-commit
    ... try to resolve conflicts, make a mess, decide
    ... to try it again from scratch
    $ git reset --hard
    $ git merge other-branch ;# or git cherry-pick one-commit

the "reset --hard" step MUST remove new paths that existed only on
the other-branch (or in one-commit), which by definition would have
auto-resolved cleanly, from the index and the working tree.  There
are other commands (e.g. "git am -3", "git apply -3", "git rebase")
that are "mergy" and their intermediate states must be handled the
same way.

If a very simple to explain and understand rule can be used to tell
if a new path (i.e. a path that exists in the index and in the
working tree, but is not in the HEAD commit) is what was created
manually by the end user without any other copy (i.e. "create
newfile && edit newfile && git add newfile") and is not a result of
a mergy operation being abandoned, then I think it is OK to allow
"reset --hard" to leave the working tree files untracked, but if the
rule becomes anything complex to understand for the users, I think
it would make the behaviour of "reset --hard" hard to explain,
understand AND anticipate---and at that point, we would be better
off keeping the "You said 'hard', we clean 'hard' to match HEAD"
behaviour of "reset --hard" and EDUCATE users not to say 'hard' too
casually. There may be a room for new option that unconditionally
leave the new working tree files untracked so that users can choose
between the two, if we end up going that route.




^ permalink raw reply

* [BUG] git gui can't commit multiple files
From: Timon @ 2016-12-04 16:36 UTC (permalink / raw)
  To: git

This is a regression in git 2.11.0 (version 2.10.2 is fine).

In git-gui I select multiple files in the Unstaged Changes (using
shift+click) and press ctrl+t to stage them. Then only one files gets
staged instead of all of the selected files.
The same happens when unstaging files.

Git-cola also exhibits the same behavior. Although there I could stage
multiple files if I used a popup menu instead of the keyboard shortcut
(I'm guessing it goes through a different code path?).

Note that I tested by reverting back to 2.10.2 and verified that
everything works, so I'm quite certain that this is a regression in
git.

^ permalink raw reply

* [PATCH v1] git-p4: fix empty file processing for large file system backend GitLFS
From: larsxschneider @ 2016-12-04 16:03 UTC (permalink / raw)
  To: git; +Cc: luke, Lars Schneider, Lars Schneider

From: Lars Schneider <lars.schneider@autodesk.com>

If git-p4 tried to store an empty file in GitLFS then it crashed while
parsing the pointer file:

  oid = re.search(r'^oid \w+:(\w+)', pointerFile, re.MULTILINE).group(1)
  AttributeError: 'NoneType' object has no attribute 'group'

This happens because GitLFS does not create a pointer file for an empty
file. Teach git-p4 this behavior to fix the problem and add a test case.

Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---

Notes:
    Base Commit: 454cb6b (v2.11.0)
    Diff on Web: https://github.com/git/git/compare/454cb6b...larsxschneider:b717fde
    Checkout:    git fetch https://github.com/larsxschneider/git git-p4/empty-files-v1 && git checkout b717fde

 git-p4.py                 | 29 +++++++++++++++++------------
 t/t9824-git-p4-git-lfs.sh |  2 ++
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index fd5ca52462..ccfb68105f 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1005,18 +1005,20 @@ class LargeFileSystem(object):
            steps."""
         if self.exceedsLargeFileThreshold(relPath, contents) or self.hasLargeFileExtension(relPath):
             contentTempFile = self.generateTempFile(contents)
-            (git_mode, contents, localLargeFile) = self.generatePointer(contentTempFile)
-
-            # Move temp file to final location in large file system
-            largeFileDir = os.path.dirname(localLargeFile)
-            if not os.path.isdir(largeFileDir):
-                os.makedirs(largeFileDir)
-            shutil.move(contentTempFile, localLargeFile)
-            self.addLargeFile(relPath)
-            if gitConfigBool('git-p4.largeFilePush'):
-                self.pushFile(localLargeFile)
-            if verbose:
-                sys.stderr.write("%s moved to large file system (%s)\n" % (relPath, localLargeFile))
+            (pointer_git_mode, contents, localLargeFile) = self.generatePointer(contentTempFile)
+            if pointer_git_mode:
+                git_mode = pointer_git_mode
+            if localLargeFile:
+                # Move temp file to final location in large file system
+                largeFileDir = os.path.dirname(localLargeFile)
+                if not os.path.isdir(largeFileDir):
+                    os.makedirs(largeFileDir)
+                shutil.move(contentTempFile, localLargeFile)
+                self.addLargeFile(relPath)
+                if gitConfigBool('git-p4.largeFilePush'):
+                    self.pushFile(localLargeFile)
+                if verbose:
+                    sys.stderr.write("%s moved to large file system (%s)\n" % (relPath, localLargeFile))
         return (git_mode, contents)
 
 class MockLFS(LargeFileSystem):
@@ -1056,6 +1058,9 @@ class GitLFS(LargeFileSystem):
            the actual content. Return also the new location of the actual
            content.
            """
+        if os.path.getsize(contentFile) == 0:
+            return (None, '', None)
+
         pointerProcess = subprocess.Popen(
             ['git', 'lfs', 'pointer', '--file=' + contentFile],
             stdout=subprocess.PIPE
diff --git a/t/t9824-git-p4-git-lfs.sh b/t/t9824-git-p4-git-lfs.sh
index 110a7e7924..734b8db4cb 100755
--- a/t/t9824-git-p4-git-lfs.sh
+++ b/t/t9824-git-p4-git-lfs.sh
@@ -42,6 +42,8 @@ test_expect_success 'Create repo with binary files' '
 	(
 		cd "$cli" &&
 
+		>file0.dat &&
+		p4 add file0.dat &&
 		echo "content 1 txt 23 bytes" >file1.txt &&
 		p4 add file1.txt &&
 		echo "content 2-3 bin 25 bytes" >file2.dat &&
-- 
2.11.0


^ permalink raw reply related

* [PATCH] Completion: Add support for --submodule=diff
From: peterjclaw @ 2016-12-04 14:41 UTC (permalink / raw)
  To: git; +Cc: jacob.keller, gitster, szeder, Peter Law

From: Peter Law <PeterJCLaw@gmail.com>

Teach git-completion.bash about the 'diff' option to 'git diff
--submodule=', which was added in Git 2.11.

Signed-off-by: Peter Law <PeterJCLaw@gmail.com>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 21016bf8d..ab11e7371 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1206,7 +1206,7 @@ _git_describe ()
 
 __git_diff_algorithms="myers minimal patience histogram"
 
-__git_diff_submodule_formats="log short"
+__git_diff_submodule_formats="diff log short"
 
 __git_diff_common_options="--stat --numstat --shortstat --summary
 			--patch-with-stat --name-only --name-status --color
-- 
2.11.0


^ permalink raw reply related

* [PATCH v1] git-p4: add config to retry p4 commands; retry 3 times by default
From: larsxschneider @ 2016-12-04 14:03 UTC (permalink / raw)
  To: git; +Cc: luke, orirawlings, Lars Schneider, Lars Schneider

From: Lars Schneider <lars.schneider@autodesk.com>

P4 commands can fail due to random network issues. P4 users can counter
these issues by using a retry flag supported by all p4 commands [1].

Add an integer Git config value `git-p4.retries` to define the number of
retries for all p4 invocations. If the config is not defined then set
the default retry count to 3.

[1] https://www.perforce.com/perforce/doc.current/manuals/cmdref/global.options.html

Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---

Notes:
    Base Commit: 454cb6b (v2.11.0)
    Diff on Web: https://github.com/git/git/compare/454cb6b...larsxschneider:654c727
    Checkout:    git fetch https://github.com/larsxschneider/git git-p4/retries-v1 && git checkout 654c727

 Documentation/git-p4.txt | 4 ++++
 git-p4.py                | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index c83aaf39c3..656587248c 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -467,6 +467,10 @@ git-p4.client::
 	Client specified as an option to all p4 commands, with
 	'-c <client>', including the client spec.
 
+git-p4.retries::
+	Specifies the number of times to retry a p4 command (notably,
+	'p4 sync') if the network times out. The default value is 3.
+
 Clone and sync variables
 ~~~~~~~~~~~~~~~~~~~~~~~~
 git-p4.syncFromOrigin::
diff --git a/git-p4.py b/git-p4.py
index fd5ca52462..2422178210 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -78,6 +78,11 @@ def p4_build_cmd(cmd):
     if len(client) > 0:
         real_cmd += ["-c", client]
 
+    retries = gitConfigInt("git-p4.retries")
+    if retries is None:
+        # Perform 3 retries by default
+        retries = 3
+    real_cmd += ["-r", str(retries)]
 
     if isinstance(cmd,basestring):
         real_cmd = ' '.join(real_cmd) + ' ' + cmd
-- 
2.11.0


^ permalink raw reply related

* [PATCH v1] travis-ci: update P4 to 16.2 and GitLFS to 1.5.2 in Linux build
From: larsxschneider @ 2016-12-04 13:52 UTC (permalink / raw)
  To: git; +Cc: Lars Schneider

From: Lars Schneider <larsxschneider@gmail.com>

Update Travis-CI dependencies to the latest available versions in
Linux build.

Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---

Notes:
    Base Commit: 454cb6b (v2.11.0)
    Diff on Web: https://git.io/test_ls_travisci_dep-update-v1.diff
    Checkout:    git fetch https://github.com/larsxschneider/git travisci/dep-update-v1 && git checkout 421365c

 .travis.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 0b2ea5c3e2..3843967a69 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -27,8 +27,8 @@ env:
     # The Linux build installs the defined dependency versions below.
     # The OS X build installs the latest available versions. Keep that
     # in mind when you encounter a broken OS X build!
-    - LINUX_P4_VERSION="16.1"
-    - LINUX_GIT_LFS_VERSION="1.2.0"
+    - LINUX_P4_VERSION="16.2"
+    - LINUX_GIT_LFS_VERSION="1.5.2"
     - DEFAULT_TEST_TARGET=prove
     - GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
     - GIT_TEST_OPTS="--verbose-log"
-- 
2.11.0


^ permalink raw reply related

* [PATCH v1] t0021: minor filter process test cleanup
From: larsxschneider @ 2016-12-04 13:37 UTC (permalink / raw)
  To: git; +Cc: Lars Schneider

From: Lars Schneider <larsxschneider@gmail.com>

Remove superfluous .gitignore pattern and invalid '.' in `git commit`
calls.

Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---

Notes:
    Base Commit: 454cb6b (v2.11.0)
    Diff on Web: https://git.io/test_ls_filter-process_cleanup-test-v1.diff
    Checkout:    git fetch https://github.com/larsxschneider/git filter-process/cleanup-test-v1 && git checkout c052a3d

 t/t0021-conversion.sh | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 4ea534e9fa..34891c4b1a 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -350,10 +350,9 @@ test_expect_success PERL 'required process filter should filter data' '
 		cd repo &&
 		git init &&
 
-		echo "git-stderr.log" >.gitignore &&
 		echo "*.r filter=protocol" >.gitattributes &&
 		git add . &&
-		git commit . -m "test commit 1" &&
+		git commit -m "test commit 1" &&
 		git branch empty-branch &&
 
 		cp "$TEST_ROOT/test.o" test.r &&
@@ -378,7 +377,7 @@ test_expect_success PERL 'required process filter should filter data' '
 		EOF
 		test_cmp_count expected.log rot13-filter.log &&
 
-		filter_git commit . -m "test commit 2" &&
+		filter_git commit -m "test commit 2" &&
 		cat >expected.log <<-EOF &&
 			START
 			init handshake complete
-- 
2.11.0


^ permalink raw reply related

* Re: Git v2.11.0 breaks max depth nested alternates
From: Philip Oakley @ 2016-12-04 11:22 UTC (permalink / raw)
  To: Kyle J. McKay, Jeff King, Junio C Hamano; +Cc: Git mailing list
In-Reply-To: <fe33de5b5f0b3da68b249cc4a49a6d7@3c843fe6ba8f3c586a21345a2783aa0>

From: "Kyle J. McKay" <mackyle@gmail.com>
Sent: Sunday, December 04, 2016 12:24 AM
> The recent addition of pre-receive quarantining breaks nested
> alternates that are already at the maximum alternates nesting depth.
>
> In the file sha1_file.c in the function link_alt_odb_entries we have
> this:
>
> > if (depth > 5) {
> >         error("%s: ignoring alternate object stores, nesting too deep.",
> >                         relative_base);
> >         return;
> > }
>
> When the incoming quarantine takes place the current objects directory
> is demoted to an alternate thereby increasing its depth (and any
> alternates it references) by one and causing any object store that was
> previously at the maximum nesting depth to be ignored courtesy of the
> above hard-coded maximum depth.
>
> If the incoming push happens to need access to some of those objects
> to perhaps "--fix-thin" its pack it will crash and burn.
>
> Originally I was not going to include a patch to fix this, but simply
> suggest that the expeditious fix is to just allow one additional
> alternates nesting depth level during quarantine operations.
>
> However, it was so simple, I have included the patch below :)
>
> I have verified that where a push with Git v2.10.2 succeeds and a push
> with Git v2.11.0 to the same repository fails because of this problem
> that the below patch does indeed correct the issue and allow the push
> to succeed.
>
> Cheers,
>
> Kyle
>
> -- 8< --
> Subject: [PATCH] receive-pack: increase max alternates depth during 
> quarantine
>
> Ever since 722ff7f876 (receive-pack: quarantine objects until
> pre-receive accepts, 2016-10-03, v2.11.0), Git has been quarantining
> objects and packs received during an incoming push into a separate
> objects directory and using the alternates mechanism to make them
> available until they are either accepted and moved into the main
> objects directory or rejected and discarded.

Is there a step here that after the accepted/rejected stage, it should then 
decrement the limit back to its original value. The problem description 
suggests that might be the case.
--
Philip

>
> Unfortunately this has the side effect of increasing the alternates
> nesting depth level by one for all pre-existing alternates.
>
> If a repository is already at the maximum alternates nesting depth,
> then this quarantining operation can temporarily push it over making
> the incoming push fail.
>
> To prevent the failure we simply increase the allowed alternates
> nesting depth by one whenever a quarantine operation is in effect.
>
> Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
> ---
>
> Notes:
>    Some alternates nesting depth background:
>
>    If base/fork0/fork1/fork2/fork3/fork4/fork5 represents
>    seven git repositories where base.git has no alternates,
>    fork0.git has base.git as an alternate, fork1.git has
>    fork0.git as an alternate and so on where fork5.git has
>    only fork4.git as an alternate, then fork5.git is at
>    the maximum allowed depth of 5.  git fsck --strict --full
>    works without complaint on fork5.git.
>
>    However, in base/fork0/fork1/fork2/fork3/fork4/fork5/fork6,
>    an fsck --strict --full of fork6.git will generate complaints
>    and any objects/packs present in base.git will be ignored.
>
> cache.h       | 1 +
> common-main.c | 3 +++
> environment.c | 1 +
> sha1_file.c   | 2 +-
> 4 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/cache.h b/cache.h
> index a50a61a1..25c17c29 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -676,6 +676,7 @@ extern size_t packed_git_limit;
> extern size_t delta_base_cache_limit;
> extern unsigned long big_file_threshold;
> extern unsigned long pack_size_limit_cfg;
> +extern int alt_odb_max_depth;
>
> /*
>  * Accessors for the core.sharedrepository config which lazy-load the 
> value
> diff --git a/common-main.c b/common-main.c
> index c654f955..9f747491 100644
> --- a/common-main.c
> +++ b/common-main.c
> @@ -37,5 +37,8 @@ int main(int argc, const char **argv)
>
>  restore_sigpipe_to_default();
>
> + if (getenv(GIT_QUARANTINE_ENVIRONMENT))
> + alt_odb_max_depth++;
> +
>  return cmd_main(argc, argv);
> }
> diff --git a/environment.c b/environment.c
> index 0935ec69..32e11f70 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -64,6 +64,7 @@ int merge_log_config = -1;
> int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
> unsigned long pack_size_limit_cfg;
> enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;
> +int alt_odb_max_depth = 5;
>
> #ifndef PROTECT_HFS_DEFAULT
> #define PROTECT_HFS_DEFAULT 0
> diff --git a/sha1_file.c b/sha1_file.c
> index 9c86d192..15b8432e 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -337,7 +337,7 @@ static void link_alt_odb_entries(const char *alt, int 
> len, int sep,
>  int i;
>  struct strbuf objdirbuf = STRBUF_INIT;
>
> - if (depth > 5) {
> + if (depth > alt_odb_max_depth) {
>  error("%s: ignoring alternate object stores, nesting too deep.",
>  relative_base);
>  return;
> ---
> 


^ permalink raw reply

* Re: git reset --hard should not irretrievably destroy new files
From: Christian Couder @ 2016-12-04 10:47 UTC (permalink / raw)
  To: Julian de Bhal; +Cc: git
In-Reply-To: <CAJZCeG0p5UrqM4oSOJ1ALKqNG8SyYh8cexKaN9R6RYYzPsMfxQ@mail.gmail.com>

On Sun, Dec 4, 2016 at 1:57 AM, Julian de Bhal <julian.debhal@gmail.com> wrote:
> On Sat, Dec 3, 2016 at 6:11 PM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> On Sat, Dec 3, 2016 at 6:04 AM, Julian de Bhal <julian.debhal@gmail.com> wrote:
>>> but I'd be nearly as happy if a
>>> commit was added to the reflog when the reset happens (I can probably make
>>> that happen with some configuration now that I've been bitten).
>>
>> Not sure if this has been proposed. Perhaps it would be simpler to
>> just output the sha1, and maybe the filenames too, of the blobs, that
>> are no more referenced from the trees, somewhere (in a bloblog?).
>
> Yeah, after doing a bit more reading around the issue, this seems like
> a smaller part of destroying local changes with a hard reset, and I'm
> one of the lucky ones where it is recoverable.

Yeah, but not everyone knows it is recoverable and using fsck to
recover is not nice and easy for the user.
So having a bloblog for example in .git/logs/blobs/, like the reflogs
we already have, but for blobs, could help even if (first) it's just
about writing the filenames and sha1s related to the blobs we stop
referencing.

> Has anyone discussed having `git reset --hard` create objects for the
> current state of anything it's about to destroy, specifically so they
> end up in the --lost-found?

Well, when we start talking about creating new objects, then someone
usually says that it is what "git stash" is about. So the discussion
then often turns to how can we make people more aware of "git stash",
or incite them to create an alias or a shell function that does a "git
stash" before "git reset --hard ...", or teach them to use "git reset
--keep ..." when it does what they want and is safer...

> I think this is what you're suggesting, only without checking for
> references, so that tree & blob objects exist that make any hard reset
> reversible.

I suggest we start with just logging blobs that we have already
created (when they have been "git add"ed) but that we are
dereferencing.
If we can agree on that, it will already help and not be very costly
performance wise. After that we could then start thinking about
creating blobs for all the content we discard, which could be done
only in a beginner mode (at least at first) to make sure it has no
performance impact if people rely on "git reset --hard" being fast.

^ permalink raw reply

* Re: Git v2.11.0 breaks max depth nested alternates
From: Kyle J. McKay @ 2016-12-04  9:37 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Git mailing list
In-Reply-To: <20161204045554.advzvylytdmt2bh2@sigill.intra.peff.net>

On Dec 3, 2016, at 20:55, Jeff King wrote:

> So I do think this is worth dealing with, but I'm also curious why
> you're hitting the depth-5 limit. I'm guessing it has to do with  
> hosting
> a hierarchy of related repos. But is your system then always in danger
> of busting the 5-limit if people create too deep a repository  
> hierarchy?

No we check for the limit.  Anything at the limit gets broken by the  
quarantine change though.

> Specifically, I'm wondering if it would be sufficient to just bump  
> it to
> 6. Or 100.

Well, if we left the current limit in place, but as you say:

> Of course any static bump runs into the funny case where a repo
> _usually_ works, but fails when pushed to. Which is kind of nasty and
> unintuitive. And your patch fixes that,

Yes.  That's not nice, hence the patch.  Without the fix, pushing  
might work sometimes until you actually need to access cut-off objects  
at pre-receive time.  So you might be able to push sometimes and  
sometimes it breaks.

> and we can leave the idea of
> bumping the static depth number as an orthogonal issue (that  
> personally,
> I do not care about much about either way).

The patch is a step on that road.  It doesn't go that far but all it  
would take is connecting the introduced variable to a config item.   
But you still need to bump it by 1 during quarantine operations.  Such  
support would even allow alternates to be disallowed (except during  
quarantine).  I wonder if there's an opportunity for further pack  
operation optimizations in such a case (you know there are no  
alternates because they're not allowed)?

>> diff --git a/common-main.c b/common-main.c
>> index c654f955..9f747491 100644
>> --- a/common-main.c
>> +++ b/common-main.c
>> @@ -37,5 +37,8 @@ int main(int argc, const char **argv)
>>
>> 	restore_sigpipe_to_default();
>>
>> +	if (getenv(GIT_QUARANTINE_ENVIRONMENT))
>> +		alt_odb_max_depth++;
>> +
>> 	return cmd_main(argc, argv);
>
> After reading your problem description, my initial thought was to
> increment the counter when we allocate the tmp-objdir, and decrement
> when it is destroyed. Because the parent receive-pack process adds  
> it to
> its alternates, too. But:
>
>  1. Receive-pack doesn't care; it adds the tmp-objdir as an alternate,
>     rather than adding it as its main object dir and bumping down the
>     main one.
>
>  2. There would have to be some way of communicating to sub-processes
>     that they should bump their max-depth by one.

All true.  And I had similar thoughts.  Perhaps we should add your  
comments to the patch description?  There seems to be a trend towards  
having longer patch descriptions these days... ;)

> You've basically used the quarantine-path variable as the
> inter-process flag for (2). Which feels a little funny, because its
> value is unrelated to the alt-odb setup. But it is a reliable  
> signal, so
> there's a certain elegance. It's probably the best option, given that
> the alternative is a specific variable to say "hey, bump your
> max-alt-odb-depth by one". That's pretty ugly, too. :)

You took the words right out of my mouth...   I guess I need to work  
on doing a better job of dumping my stream-of-thoughts that go into a  
patch into the emails to the list.

Most all of your comments could be dumped into the patch description  
as-is to pimp it out some.  I have no objection to that, even adding  
an "Additional-analysis-by:" (or similar) credit line too.  :)

--Kyle

^ permalink raw reply

* Re: git 2.11.0 error when pushing to remote located on a windows share
From: Torsten Bögershausen @ 2016-12-04  8:09 UTC (permalink / raw)
  To: Jeff King; +Cc: thomas.attwood, git
In-Reply-To: <20161202223749.2n7wa37e5w6446uv@sigill.intra.peff.net>

On Fri, Dec 02, 2016 at 05:37:50PM -0500, Jeff King wrote:
> On Fri, Dec 02, 2016 at 06:02:16PM +0000, thomas.attwood@stfc.ac.uk wrote:
> 
> > After updating git from 2.10.0 to 2.11.0 when trying to push any
> > changes to a repo located in a windows share, the following error
> > occurs:
> > 
> > $ git push origin test
> > Counting objects: 2, done.
> > Delta compression using up to 8 threads.
> > Compressing objects: 100% (2/2), done.
> > Writing objects: 100% (2/2), 284 bytes | 0 bytes/s, done.
> > Total 2 (delta 1), reused 1 (delta 0)
> > remote: error: object directory /path/to/dir/objects does not exist; check .git/objects/info/alternates.
> > remote: fatal: unresolved deltas left after unpacking
> > error: unpack failed: unpack-objects abnormal exit
> > To //path/to/dir
> >  ! [remote rejected] test -> test (unpacker error)
> > error: failed to push some refs to '//path/to/dir'
> 
> Hmm. This is probably related to the quarantine-push change in v2.11;
> the receiving end will write the objects into a temporary directory but
> point to the original via GIT_ALTERNATE_OBJECT_DIRECTORIES. That pointer
> isn't working for some reason, so the receiver can't resolve the deltas
> it needs.
> 
> As you noted, the extra "/" is missing in the error message, and that
> sounds like a plausible cause for what you're seeing. I'm not sure where
> the slash is getting dropped, though. The value in the environment comes
> from calling absolute_path(get_object_directory()), so I suspect the
> real problem is not in the quarantine code, but it's just triggering a
> latent bug elsewhere (either in absolute_path(), or in the code which
> generates the objdir path).
> 
> > No error occurs if pushing to the same repo (a direct copy into a local directory) using 2.11.0.
> > 
> > $ git push local_test test
> > Counting objects: 2, done.
> > Delta compression using up to 8 threads.
> > Compressing objects: 100% (2/2), done.
> > Writing objects: 100% (2/2), 284 bytes | 0 bytes/s, done.
> > Total 2 (delta 1), reused 1 (delta 0)
> > To C:/path/to/dir
> >  * [new branch]      test -> test
> 
> The fact that it works using the non-UNC path reinforces my feeling that
> something is normalizing the absolute path incorrectly.
> 
> > Using `git fsck --full` in both 2.11.0 and 2.10.0, it doesn't reveal any additional problems.
> 
> Yeah, I don't think there is anything wrong with your repo. It's just a
> path-building issue internal to the receiving process.
> 

There seems to be another issue, which may or may not being related:
https://github.com/git-for-windows/git/issues/979

This is pure speculation:
Could it be that a '/' is lost because of a change in the underlying
Msys2 between 2.10 and 2.11 ?

Dscho, (or anybody else) any ideas? 


^ permalink raw reply

* Re: [PATCH v4 1/3] update-unicode.sh: automatically download newer definition files
From: Torsten Bögershausen @ 2016-12-04  7:58 UTC (permalink / raw)
  To: Beat Bolli; +Cc: git
In-Reply-To: <1480798849-13907-1-git-send-email-dev+git@drbeat.li>

On Sat, Dec 03, 2016 at 10:00:47PM +0100, Beat Bolli wrote:
> Checking just for the unicode data files' existence is not sufficient;
> we should also download them if a newer version exists on the Unicode
> consortium's servers. Option -N of wget does this nicely for us.
> 
> Reviewed-by: Torsten Boegershausen <tboegi@web.de>

Minor remark (Not sure if this motivates v5, may be Junio can fix it locally?)
s/oe/ö/

Beside this: Thanks again (and I learned about the -N option of wget)

^ permalink raw reply

* Re: Git v2.11.0 breaks max depth nested alternates
From: Jeff King @ 2016-12-04  4:55 UTC (permalink / raw)
  To: Kyle J. McKay; +Cc: Junio C Hamano, Git mailing list
In-Reply-To: <fe33de5b5f0b3da68b249cc4a49a6d7@3c843fe6ba8f3c586a21345a2783aa0>

On Sat, Dec 03, 2016 at 04:24:02PM -0800, Kyle J. McKay wrote:

> When the incoming quarantine takes place the current objects directory  
> is demoted to an alternate thereby increasing its depth (and any  
> alternates it references) by one and causing any object store that was  
> previously at the maximum nesting depth to be ignored courtesy of the  
> above hard-coded maximum depth.
> 
> If the incoming push happens to need access to some of those objects  
> to perhaps "--fix-thin" its pack it will crash and burn.

Yep, that makes sense. I didn't really worry about this because the
existing "5" is totally arbitrary, and meant to be so high that nobody
reaches it (it's just there to break cycles).

So I do think this is worth dealing with, but I'm also curious why
you're hitting the depth-5 limit. I'm guessing it has to do with hosting
a hierarchy of related repos. But is your system then always in danger
of busting the 5-limit if people create too deep a repository hierarchy?

Specifically, I'm wondering if it would be sufficient to just bump it to
6. Or 100.

Of course any static bump runs into the funny case where a repo
_usually_ works, but fails when pushed to. Which is kind of nasty and
unintuitive. And your patch fixes that, and we can leave the idea of
bumping the static depth number as an orthogonal issue (that personally,
I do not care about much about either way).

> diff --git a/common-main.c b/common-main.c
> index c654f955..9f747491 100644
> --- a/common-main.c
> +++ b/common-main.c
> @@ -37,5 +37,8 @@ int main(int argc, const char **argv)
>  
>  	restore_sigpipe_to_default();
>  
> +	if (getenv(GIT_QUARANTINE_ENVIRONMENT))
> +		alt_odb_max_depth++;
> +
>  	return cmd_main(argc, argv);

After reading your problem description, my initial thought was to
increment the counter when we allocate the tmp-objdir, and decrement
when it is destroyed. Because the parent receive-pack process adds it to
its alternates, too. But:

  1. Receive-pack doesn't care; it adds the tmp-objdir as an alternate,
     rather than adding it as its main object dir and bumping down the
     main one.

  2. There would have to be some way of communicating to sub-processes
     that they should bump their max-depth by one.

You've basically used the quarantine-path variable as the
inter-process flag for (2). Which feels a little funny, because its
value is unrelated to the alt-odb setup. But it is a reliable signal, so
there's a certain elegance. It's probably the best option, given that
the alternative is a specific variable to say "hey, bump your
max-alt-odb-depth by one". That's pretty ugly, too. :)

-Peff

^ permalink raw reply

* [PATCH v2] tag, branch, for-each-ref: add --ignore-case for sorting and filtering
From: Nguyễn Thái Ngọc Duy @ 2016-12-04  2:52 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, karthik.188,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <20161130123502.12973-1-pclouds@gmail.com>

This options makes sorting ignore case, which is great when you have
branches named bug-12-do-something, Bug-12-do-some-more and
BUG-12-do-what and want to group them together. Sorting externally may
not be an option because we lose coloring and column layout from
git-branch and git-tag.

The same could be said for filtering, but it's probably less important
because you can always go with the ugly pattern [bB][uU][gG]-* if you're
desperate.

You can't have case-sensitive filtering and case-insensitive sorting (or
the other way around) with this though. For branch and tag, that should
be no problem. for-each-ref, as a plumbing, might want finer control.
But we can always add --{filter,sort}-ignore-case when there is a need
for it.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Changes are in tests only:

    diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
    index fad79e8..52283df 100755
    --- a/t/t3203-branch-output.sh
    +++ b/t/t3203-branch-output.sh
    @@ -208,6 +208,13 @@ test_expect_success 'sort branches, ignore case' '
     		test_commit initial &&
     		git branch branch-one &&
     		git branch BRANCH-two &&
    +		git branch --list | awk "{print \$NF}" >actual &&
    +		cat >expected <<-\EOF &&
    +		BRANCH-two
    +		branch-one
    +		master
    +		EOF
    +		test_cmp expected actual &&
     		git branch --list -i | awk "{print \$NF}" >actual &&
     		cat >expected <<-\EOF &&
     		branch-one
    diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
    index 2d9cae3..07869b0 100755
    --- a/t/t7004-tag.sh
    +++ b/t/t7004-tag.sh
    @@ -34,6 +34,13 @@ test_expect_success 'sort tags, ignore case' '
     		test_commit initial &&
     		git tag tag-one &&
     		git tag TAG-two &&
    +		git tag -l >actual &&
    +		cat >expected <<-\EOF &&
    +		TAG-two
    +		initial
    +		tag-one
    +		EOF
    +		test_cmp expected actual &&
     		git tag -l -i >actual &&
     		cat >expected <<-\EOF &&
     		initial
    @@ -98,8 +105,8 @@ test_expect_success 'listing all tags if one exists should output that tag' '
     test_expect_success 'listing a tag using a matching pattern should succeed' \
     	'git tag -l mytag'
     
    -test_expect_success 'listing a tag using a matching pattern should succeed' \
    -	'git tag -l --ignore-case MYTAG'
    +test_expect_success 'listing a tag with --ignore-case' \
    +	'test $(git tag -l --ignore-case MYTAG) = mytag'
     
     test_expect_success \
     	'listing a tag using a matching pattern should output that tag' \

 Documentation/git-branch.txt       |  4 ++++
 Documentation/git-for-each-ref.txt |  3 +++
 Documentation/git-tag.txt          |  4 ++++
 builtin/branch.c                   | 23 ++++++++++++++---------
 builtin/for-each-ref.c             |  5 ++++-
 builtin/tag.c                      |  4 ++++
 ref-filter.c                       | 28 +++++++++++++++++++++-------
 ref-filter.h                       |  2 ++
 t/t3203-branch-output.sh           | 29 +++++++++++++++++++++++++++++
 t/t7004-tag.sh                     | 27 +++++++++++++++++++++++++++
 10 files changed, 112 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 1fe7344..5516a47 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -118,6 +118,10 @@ OPTIONS
 	default to color output.
 	Same as `--color=never`.
 
+-i::
+--ignore-case::
+	Sorting and filtering branches are case insensitive.
+
 --column[=<options>]::
 --no-column::
 	Display branch listing in columns. See configuration variable
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index f57e69b..6d22974 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -79,6 +79,9 @@ OPTIONS
 	Only list refs which contain the specified commit (HEAD if not
 	specified).
 
+--ignore-case::
+	Sorting and filtering refs are case insensitive.
+
 FIELD NAMES
 -----------
 
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 80019c5..76cfe40 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -108,6 +108,10 @@ OPTIONS
 	variable if it exists, or lexicographic order otherwise. See
 	linkgit:git-config[1].
 
+-i::
+--ignore-case::
+	Sorting and filtering tags are case insensitive.
+
 --column[=<options>]::
 --no-column::
 	Display tag listing in columns. See configuration variable
diff --git a/builtin/branch.c b/builtin/branch.c
index 60cc5c8..36e0a21 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -512,15 +512,6 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
 	if (filter->verbose)
 		maxwidth = calc_maxwidth(&array, strlen(remote_prefix));
 
-	/*
-	 * If no sorting parameter is given then we default to sorting
-	 * by 'refname'. This would give us an alphabetically sorted
-	 * array with the 'HEAD' ref at the beginning followed by
-	 * local branches 'refs/heads/...' and finally remote-tacking
-	 * branches 'refs/remotes/...'.
-	 */
-	if (!sorting)
-		sorting = ref_default_sorting();
 	ref_array_sort(sorting, &array);
 
 	for (i = 0; i < array.nr; i++)
@@ -645,6 +636,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	const char *new_upstream = NULL;
 	enum branch_track track;
 	struct ref_filter filter;
+	int icase = 0;
 	static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
 
 	struct option options[] = {
@@ -686,6 +678,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"),
 			N_("print only branches of the object"), 0, parse_opt_object_name
 		},
+		OPT_BOOL('i', "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
 		OPT_END(),
 	};
 
@@ -723,6 +716,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 	if (filter.abbrev == -1)
 		filter.abbrev = DEFAULT_ABBREV;
+	filter.ignore_case = icase;
+
 	finalize_colopts(&colopts, -1);
 	if (filter.verbose) {
 		if (explicitly_enable_column(colopts))
@@ -744,6 +739,16 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)
 			filter.kind |= FILTER_REFS_DETACHED_HEAD;
 		filter.name_patterns = argv;
+		/*
+		 * If no sorting parameter is given then we default to sorting
+		 * by 'refname'. This would give us an alphabetically sorted
+		 * array with the 'HEAD' ref at the beginning followed by
+		 * local branches 'refs/heads/...' and finally remote-tacking
+		 * branches 'refs/remotes/...'.
+		 */
+		if (!sorting)
+			sorting = ref_default_sorting();
+		sorting->ignore_case = icase;
 		print_ref_list(&filter, sorting);
 		print_columns(&output, colopts, NULL);
 		string_list_clear(&output, 0);
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 4e9f6c2..df41fa0 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -18,7 +18,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	int i;
 	const char *format = "%(objectname) %(objecttype)\t%(refname)";
 	struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
-	int maxcount = 0, quote_style = 0;
+	int maxcount = 0, quote_style = 0, icase = 0;
 	struct ref_array array;
 	struct ref_filter filter;
 
@@ -43,6 +43,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_MERGED(&filter, N_("print only refs that are merged")),
 		OPT_NO_MERGED(&filter, N_("print only refs that are not merged")),
 		OPT_CONTAINS(&filter.with_commit, N_("print only refs which contain the commit")),
+		OPT_BOOL(0, "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
 		OPT_END(),
 	};
 
@@ -63,6 +64,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 
 	if (!sorting)
 		sorting = ref_default_sorting();
+	sorting->ignore_case = icase;
+	filter.ignore_case = icase;
 
 	/* for warn_ambiguous_refs */
 	git_config(git_default_config, NULL);
diff --git a/builtin/tag.c b/builtin/tag.c
index 50e4ae5..73df728 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -335,6 +335,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	struct ref_filter filter;
 	static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
 	const char *format = NULL;
+	int icase = 0;
 	struct option options[] = {
 		OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
 		{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
@@ -370,6 +371,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 			N_("print only tags of the object"), 0, parse_opt_object_name
 		},
 		OPT_STRING(  0 , "format", &format, N_("format"), N_("format to use for the output")),
+		OPT_BOOL('i', "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
 		OPT_END()
 	};
 
@@ -401,6 +403,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	}
 	if (!sorting)
 		sorting = ref_default_sorting();
+	sorting->ignore_case = icase;
+	filter.ignore_case = icase;
 	if (cmdmode == 'l') {
 		int ret;
 		if (column_active(colopts)) {
diff --git a/ref-filter.c b/ref-filter.c
index f5f7a70..bd98010 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1231,8 +1231,14 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)
  * matches a pattern "refs/heads/mas") or a wildcard (e.g. the same ref
  * matches "refs/heads/mas*", too).
  */
-static int match_pattern(const char **patterns, const char *refname)
+static int match_pattern(const struct ref_filter *filter, const char *refname)
 {
+	const char **patterns = filter->name_patterns;
+	unsigned flags = 0;
+
+	if (filter->ignore_case)
+		flags |= WM_CASEFOLD;
+
 	/*
 	 * When no '--format' option is given we need to skip the prefix
 	 * for matching refs of tags and branches.
@@ -1243,7 +1249,7 @@ static int match_pattern(const char **patterns, const char *refname)
 	       skip_prefix(refname, "refs/", &refname));
 
 	for (; *patterns; patterns++) {
-		if (!wildmatch(*patterns, refname, 0, NULL))
+		if (!wildmatch(*patterns, refname, flags, NULL))
 			return 1;
 	}
 	return 0;
@@ -1255,9 +1261,15 @@ static int match_pattern(const char **patterns, const char *refname)
  * matches a pattern "refs/heads/" but not "refs/heads/m") or a
  * wildcard (e.g. the same ref matches "refs/heads/m*", too).
  */
-static int match_name_as_path(const char **pattern, const char *refname)
+static int match_name_as_path(const struct ref_filter *filter, const char *refname)
 {
+	const char **pattern = filter->name_patterns;
 	int namelen = strlen(refname);
+	unsigned flags = WM_PATHNAME;
+
+	if (filter->ignore_case)
+		flags |= WM_CASEFOLD;
+
 	for (; *pattern; pattern++) {
 		const char *p = *pattern;
 		int plen = strlen(p);
@@ -1280,8 +1292,8 @@ static int filter_pattern_match(struct ref_filter *filter, const char *refname)
 	if (!*filter->name_patterns)
 		return 1; /* No pattern always matches */
 	if (filter->match_as_path)
-		return match_name_as_path(filter->name_patterns, refname);
-	return match_pattern(filter->name_patterns, refname);
+		return match_name_as_path(filter, refname);
+	return match_pattern(filter, refname);
 }
 
 /*
@@ -1536,18 +1548,20 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru
 	struct atom_value *va, *vb;
 	int cmp;
 	cmp_type cmp_type = used_atom[s->atom].type;
+	int (*cmp_fn)(const char *, const char *);
 
 	get_ref_atom_value(a, s->atom, &va);
 	get_ref_atom_value(b, s->atom, &vb);
+	cmp_fn = s->ignore_case ? strcasecmp : strcmp;
 	if (s->version)
 		cmp = versioncmp(va->s, vb->s);
 	else if (cmp_type == FIELD_STR)
-		cmp = strcmp(va->s, vb->s);
+		cmp = cmp_fn(va->s, vb->s);
 	else {
 		if (va->ul < vb->ul)
 			cmp = -1;
 		else if (va->ul == vb->ul)
-			cmp = strcmp(a->refname, b->refname);
+			cmp = cmp_fn(a->refname, b->refname);
 		else
 			cmp = 1;
 	}
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..fc55fa3 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -29,6 +29,7 @@ struct ref_sorting {
 	struct ref_sorting *next;
 	int atom; /* index into used_atom array (internal) */
 	unsigned reverse : 1,
+		ignore_case : 1,
 		version : 1;
 };
 
@@ -62,6 +63,7 @@ struct ref_filter {
 
 	unsigned int with_commit_tag_algo : 1,
 		match_as_path : 1,
+		ignore_case : 1,
 		detached : 1;
 	unsigned int kind,
 		lines;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index c6a3ccb..52283df 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -89,6 +89,11 @@ test_expect_success 'git branch --list -v pattern shows branch summaries' '
 	awk "{print \$NF}" <tmp >actual &&
 	test_cmp expect actual
 '
+test_expect_success 'git branch --ignore-case --list -v pattern shows branch summaries' '
+	git branch --list --ignore-case -v BRANCH* >tmp &&
+	awk "{print \$NF}" <tmp >actual &&
+	test_cmp expect actual
+'
 
 test_expect_success 'git branch -v pattern does not show branch summaries' '
 	test_must_fail git branch -v branch*
@@ -196,4 +201,28 @@ test_expect_success 'local-branch symrefs shortened properly' '
 	test_cmp expect actual
 '
 
+test_expect_success 'sort branches, ignore case' '
+	(
+		git init sort-icase &&
+		cd sort-icase &&
+		test_commit initial &&
+		git branch branch-one &&
+		git branch BRANCH-two &&
+		git branch --list | awk "{print \$NF}" >actual &&
+		cat >expected <<-\EOF &&
+		BRANCH-two
+		branch-one
+		master
+		EOF
+		test_cmp expected actual &&
+		git branch --list -i | awk "{print \$NF}" >actual &&
+		cat >expected <<-\EOF &&
+		branch-one
+		BRANCH-two
+		master
+		EOF
+		test_cmp expected actual
+	)
+'
+
 test_done
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 8b0f71a..07869b0 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -27,6 +27,30 @@ test_expect_success 'listing all tags in an empty tree should output nothing' '
 	test $(git tag | wc -l) -eq 0
 '
 
+test_expect_success 'sort tags, ignore case' '
+	(
+		git init sort &&
+		cd sort &&
+		test_commit initial &&
+		git tag tag-one &&
+		git tag TAG-two &&
+		git tag -l >actual &&
+		cat >expected <<-\EOF &&
+		TAG-two
+		initial
+		tag-one
+		EOF
+		test_cmp expected actual &&
+		git tag -l -i >actual &&
+		cat >expected <<-\EOF &&
+		initial
+		tag-one
+		TAG-two
+		EOF
+		test_cmp expected actual
+	)
+'
+
 test_expect_success 'looking for a tag in an empty tree should fail' \
 	'! (tag_exists mytag)'
 
@@ -81,6 +105,9 @@ test_expect_success 'listing all tags if one exists should output that tag' '
 test_expect_success 'listing a tag using a matching pattern should succeed' \
 	'git tag -l mytag'
 
+test_expect_success 'listing a tag with --ignore-case' \
+	'test $(git tag -l --ignore-case MYTAG) = mytag'
+
 test_expect_success \
 	'listing a tag using a matching pattern should output that tag' \
 	'test $(git tag -l mytag) = mytag'
-- 
2.8.2.524.g6ff3d78


^ permalink raw reply related

* Re: git reset --hard should not irretrievably destroy new files
From: Julian de Bhal @ 2016-12-04  0:57 UTC (permalink / raw)
  To: Christian Couder; +Cc: git
In-Reply-To: <CAP8UFD0ipS_4p+njfbbDGpYSDJhp43e9XDP69MOruZz9c136ew@mail.gmail.com>

On Sat, Dec 3, 2016 at 6:11 PM, Christian Couder
<christian.couder@gmail.com> wrote:
> On Sat, Dec 3, 2016 at 6:04 AM, Julian de Bhal <julian.debhal@gmail.com> wrote:
>> but I'd be nearly as happy if a
>> commit was added to the reflog when the reset happens (I can probably make
>> that happen with some configuration now that I've been bitten).
>
> Not sure if this has been proposed. Perhaps it would be simpler to
> just output the sha1, and maybe the filenames too, of the blobs, that
> are no more referenced from the trees, somewhere (in a bloblog?).

Yeah, after doing a bit more reading around the issue, this seems like
a smaller part of destroying local changes with a hard reset, and I'm
one of the lucky ones where it is recoverable.

Has anyone discussed having `git reset --hard` create objects for the
current state of anything it's about to destroy, specifically so they
end up in the --lost-found?

I think this is what you're suggesting, only without checking for
references, so that tree & blob objects exist that make any hard reset
reversible.

Cheers

Jules

P.s. Thank you for such a warm welcome while I blunder through
unfamiliar protocols.

^ permalink raw reply

* Git v2.11.0 breaks max depth nested alternates
From: Kyle J. McKay @ 2016-12-04  0:24 UTC (permalink / raw)
  To: Jeff King, Junio C Hamano; +Cc: Git mailing list

The recent addition of pre-receive quarantining breaks nested  
alternates that are already at the maximum alternates nesting depth.

In the file sha1_file.c in the function link_alt_odb_entries we have  
this:

 > if (depth > 5) {
 >         error("%s: ignoring alternate object stores, nesting too deep.",
 >                         relative_base);
 >         return;
 > }

When the incoming quarantine takes place the current objects directory  
is demoted to an alternate thereby increasing its depth (and any  
alternates it references) by one and causing any object store that was  
previously at the maximum nesting depth to be ignored courtesy of the  
above hard-coded maximum depth.

If the incoming push happens to need access to some of those objects  
to perhaps "--fix-thin" its pack it will crash and burn.

Originally I was not going to include a patch to fix this, but simply  
suggest that the expeditious fix is to just allow one additional  
alternates nesting depth level during quarantine operations.

However, it was so simple, I have included the patch below :)

I have verified that where a push with Git v2.10.2 succeeds and a push  
with Git v2.11.0 to the same repository fails because of this problem  
that the below patch does indeed correct the issue and allow the push  
to succeed.

Cheers,

Kyle

-- 8< --
Subject: [PATCH] receive-pack: increase max alternates depth during quarantine

Ever since 722ff7f876 (receive-pack: quarantine objects until
pre-receive accepts, 2016-10-03, v2.11.0), Git has been quarantining
objects and packs received during an incoming push into a separate
objects directory and using the alternates mechanism to make them
available until they are either accepted and moved into the main
objects directory or rejected and discarded.

Unfortunately this has the side effect of increasing the alternates
nesting depth level by one for all pre-existing alternates.

If a repository is already at the maximum alternates nesting depth,
then this quarantining operation can temporarily push it over making
the incoming push fail.

To prevent the failure we simply increase the allowed alternates
nesting depth by one whenever a quarantine operation is in effect.

Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
---

Notes:
    Some alternates nesting depth background:
    
    If base/fork0/fork1/fork2/fork3/fork4/fork5 represents
    seven git repositories where base.git has no alternates,
    fork0.git has base.git as an alternate, fork1.git has
    fork0.git as an alternate and so on where fork5.git has
    only fork4.git as an alternate, then fork5.git is at
    the maximum allowed depth of 5.  git fsck --strict --full
    works without complaint on fork5.git.
    
    However, in base/fork0/fork1/fork2/fork3/fork4/fork5/fork6,
    an fsck --strict --full of fork6.git will generate complaints
    and any objects/packs present in base.git will be ignored.

 cache.h       | 1 +
 common-main.c | 3 +++
 environment.c | 1 +
 sha1_file.c   | 2 +-
 4 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index a50a61a1..25c17c29 100644
--- a/cache.h
+++ b/cache.h
@@ -676,6 +676,7 @@ extern size_t packed_git_limit;
 extern size_t delta_base_cache_limit;
 extern unsigned long big_file_threshold;
 extern unsigned long pack_size_limit_cfg;
+extern int alt_odb_max_depth;
 
 /*
  * Accessors for the core.sharedrepository config which lazy-load the value
diff --git a/common-main.c b/common-main.c
index c654f955..9f747491 100644
--- a/common-main.c
+++ b/common-main.c
@@ -37,5 +37,8 @@ int main(int argc, const char **argv)
 
 	restore_sigpipe_to_default();
 
+	if (getenv(GIT_QUARANTINE_ENVIRONMENT))
+		alt_odb_max_depth++;
+
 	return cmd_main(argc, argv);
 }
diff --git a/environment.c b/environment.c
index 0935ec69..32e11f70 100644
--- a/environment.c
+++ b/environment.c
@@ -64,6 +64,7 @@ int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
 unsigned long pack_size_limit_cfg;
 enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;
+int alt_odb_max_depth = 5;
 
 #ifndef PROTECT_HFS_DEFAULT
 #define PROTECT_HFS_DEFAULT 0
diff --git a/sha1_file.c b/sha1_file.c
index 9c86d192..15b8432e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -337,7 +337,7 @@ static void link_alt_odb_entries(const char *alt, int len, int sep,
 	int i;
 	struct strbuf objdirbuf = STRBUF_INIT;
 
-	if (depth > 5) {
+	if (depth > alt_odb_max_depth) {
 		error("%s: ignoring alternate object stores, nesting too deep.",
 				relative_base);
 		return;
---

^ permalink raw reply related

* Re: git reset --hard should not irretrievably destroy new files
From: Julian de Bhal @ 2016-12-04  0:14 UTC (permalink / raw)
  Cc: git
In-Reply-To: <f2f8c7c9-cbe0-870d-3c13-fc928dd91dd1@kdbg.org>

On Sat, Dec 3, 2016 at 5:49 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 03.12.2016 um 06:04 schrieb Julian de Bhal:
>>
>> If you `git add new_file; git reset --hard`, new_file is gone forever.
>
> AFAIC, this is a feature ;-) I occasionally use it to remove a file when I
> already have git-gui in front of me. Then it's often less convenient to type
> the path in a shell, or to pointy-click around in a file browser.

Yeah, I'm conscious that it would be a change in behaviour and would
almost certainly break things in the wild.

On the other hand, `rm` deletes perfectly well, but there's no good
way to recover the lost files after the fact. You can take some
precautions after you've been bitten, but git usually means never
saying "you should have".

>> git add new_file
>> [...]
>> git reset --hard                 # decided copy from backed up diff
>> # boom. new_file is gone forever
>
> ... it is not. The file is still among the dangling blobs in the repository
> until you clean it up with 'git gc'. Use 'git fsck --lost-found':

Thank you so much! Super glad to be wrong here.

Cheers,

Jules

On Sat, Dec 3, 2016 at 5:49 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 03.12.2016 um 06:04 schrieb Julian de Bhal:
>>
>> If you `git add new_file; git reset --hard`, new_file is gone forever.
>
>
> AFAIC, this is a feature ;-) I occasionally use it to remove a file when I
> already have git-gui in front of me. Then it's often less convenient to type
> the path in a shell, or to pointy-click around in a file browser.
>
>> git add new_file
>
>
> Because of this ...
>
>> git add -p                       # also not necessary, but distracting
>> git reset --hard                 # decided copy from backed up diff
>> # boom. new_file is gone forever
>
>
> ... it is not. The file is still among the dangling blobs in the repository
> until you clean it up with 'git gc'. Use 'git fsck --lost-found':
>
> --lost-found
>
>     Write dangling objects into .git/lost-found/commit/ or
> .git/lost-found/other/, depending on type. If the object is a blob, the
> contents are written into the file, rather than its object name.
>
> -- Hannes
>

^ permalink raw reply

* [PATCH v4 1/3] update-unicode.sh: automatically download newer definition files
From: Beat Bolli @ 2016-12-03 21:00 UTC (permalink / raw)
  To: git; +Cc: Beat Bolli
In-Reply-To: <835c0328-e812-1cb7-c49e-714ff0e9ffb3@drbeat.li>

Checking just for the unicode data files' existence is not sufficient;
we should also download them if a newer version exists on the Unicode
consortium's servers. Option -N of wget does this nicely for us.

Reviewed-by: Torsten Boegershausen <tboegi@web.de>
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
Diff to v3:
  - change the Cc: into Reviewed-by: on Thorsten's request
  - include the old reroll diffs

Diff to v2:
  - reorder the commits: fix all of update-unicode.sh first, then
    regenerate unicode_width.h only once

Diff to v1:
  - reword the commit message
  - add Thorsten's Cc:

 update_unicode.sh | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/update_unicode.sh b/update_unicode.sh
index 27af77c..3c84270 100755
--- a/update_unicode.sh
+++ b/update_unicode.sh
@@ -10,12 +10,8 @@ if ! test -d unicode; then
 	mkdir unicode
 fi &&
 ( cd unicode &&
-	if ! test -f UnicodeData.txt; then
-		wget http://www.unicode.org/Public/UCD/latest/ucd/UnicodeData.txt
-	fi &&
-	if ! test -f EastAsianWidth.txt; then
-		wget http://www.unicode.org/Public/UCD/latest/ucd/EastAsianWidth.txt
-	fi &&
+	wget -N http://www.unicode.org/Public/UCD/latest/ucd/UnicodeData.txt \
+		http://www.unicode.org/Public/UCD/latest/ucd/EastAsianWidth.txt &&
 	if ! test -d uniset; then
 		git clone https://github.com/depp/uniset.git
 	fi &&
-- 
2.7.2

^ 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