Git development
 help / color / mirror / Atom feed

* [PATCH v2 5/6] pack-objects: create pack.useSparse setting
From: Derrick Stolee via GitGitGadget @ 2018-11-29 14:24 UTC (permalink / raw)
  To: git; +Cc: peff, avarab, jrnieder, Junio C Hamano, Derrick Stolee
In-Reply-To: <pull.89.v2.git.gitgitgadget@gmail.com>

From: Derrick Stolee <dstolee@microsoft.com>

The '--sparse' flag in 'git pack-objects' changes the algorithm
used to enumerate objects to one that is faster for individual
users pushing new objects that change only a small cone of the
working directory. The sparse algorithm is not recommended for a
server, which likely sends new objects that appear across the
entire working directory.

Create a 'pack.useSparse' setting that enables this new algorithm.
This allows 'git push' to use this algorithm without passing a
'--sparse' flag all the way through four levels of run_command()
calls.

If the '--no-sparse' flag is set, then this config setting is
overridden.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 builtin/pack-objects.c         |  4 ++++
 t/t5322-pack-objects-sparse.sh | 15 +++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 7d5b0735e3..124b1bafc4 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2711,6 +2711,10 @@ static int git_pack_config(const char *k, const char *v, void *cb)
 		use_bitmap_index_default = git_config_bool(k, v);
 		return 0;
 	}
+	if (!strcmp(k, "pack.usesparse")) {
+		sparse = git_config_bool(k, v);
+		return 0;
+	}
 	if (!strcmp(k, "pack.threads")) {
 		delta_search_threads = git_config_int(k, v);
 		if (delta_search_threads < 0)
diff --git a/t/t5322-pack-objects-sparse.sh b/t/t5322-pack-objects-sparse.sh
index 45dba6e014..8f5699bd91 100755
--- a/t/t5322-pack-objects-sparse.sh
+++ b/t/t5322-pack-objects-sparse.sh
@@ -121,4 +121,19 @@ test_expect_success 'sparse pack-objects' '
 	test_cmp expect_sparse_objects.txt sparse_objects.txt
 '
 
+test_expect_success 'pack.useSparse enables algorithm' '
+	git config pack.useSparse true &&
+	git pack-objects --stdout --revs <packinput.txt >sparse.pack &&
+	git index-pack -o sparse.idx sparse.pack &&
+	git show-index <sparse.idx | awk "{print \$2}" >sparse_objects.txt &&
+	test_cmp expect_sparse_objects.txt sparse_objects.txt
+'
+
+test_expect_success 'pack.useSparse overridden' '
+	git pack-objects --stdout --revs --no-sparse <packinput.txt >sparse.pack &&
+	git index-pack -o sparse.idx sparse.pack &&
+	git show-index <sparse.idx | awk "{print \$2}" >sparse_objects.txt &&
+	test_cmp expect_objects.txt sparse_objects.txt
+'
+
 test_done
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 2/6] list-objects: consume sparse tree walk
From: Derrick Stolee via GitGitGadget @ 2018-11-29 14:24 UTC (permalink / raw)
  To: git; +Cc: peff, avarab, jrnieder, Junio C Hamano, Derrick Stolee
In-Reply-To: <pull.89.v2.git.gitgitgadget@gmail.com>

From: Derrick Stolee <dstolee@microsoft.com>

When creating a pack-file using 'git pack-objects --revs' we provide
a list of interesting and uninteresting commits. For example, a push
operation would make the local topic branch be interesting and the
known remote refs as uninteresting. We want to discover the set of
new objects to send to the server as a thin pack.

We walk these commits until we discover a frontier of commits such
that every commit walk starting at interesting commits ends in a root
commit or unintersting commit. We then need to discover which
non-commit objects are reachable from  uninteresting commits.

The mark_edges_unintersting() method in list-objects.c iterates on
the commit list and does the following:

* If the commit is UNINTERSTING, then mark its root tree and every
  object it can reach as UNINTERESTING.

* If the commit is interesting, then mark the root tree of every
  UNINTERSTING parent (and all objects that tree can reach) as
  UNINTERSTING.

At the very end, we repeat the process on every commit directly
given to the revision walk from stdin. This helps ensure we properly
cover shallow commits that otherwise were not included in the
frontier.

The logic to recursively follow trees is in the
mark_tree_uninteresting() method in revision.c. The algorithm avoids
duplicate work by not recursing into trees that are already marked
UNINTERSTING.

Add a new 'sparse' option to the mark_edges_uninteresting() method
that performs this logic in a slightly new way. As we iterate over
the commits, we add all of the root trees to an oidset. Then, call
mark_trees_uninteresting_sparse() on that oidset. Note that we
include interesting trees in this process. The current implementation
of mark_trees_unintersting_sparse() will walk the same trees as
the old logic, but this will be replaced in a later change.

The sparse option is not used by any callers at the moment, but
will be wired to 'git pack-objects' in the next change.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 bisect.c               |  2 +-
 builtin/pack-objects.c |  2 +-
 builtin/rev-list.c     |  2 +-
 http-push.c            |  2 +-
 list-objects.c         | 55 +++++++++++++++++++++++++++++++++++++++---
 list-objects.h         |  4 ++-
 revision.c             |  3 +++
 7 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/bisect.c b/bisect.c
index 487675c672..842f8b4b8f 100644
--- a/bisect.c
+++ b/bisect.c
@@ -656,7 +656,7 @@ static void bisect_common(struct rev_info *revs)
 	if (prepare_revision_walk(revs))
 		die("revision walk setup failed");
 	if (revs->tree_objects)
-		mark_edges_uninteresting(revs, NULL);
+		mark_edges_uninteresting(revs, NULL, 0);
 }
 
 static void exit_if_skipped_commits(struct commit_list *tried,
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 411aefd687..5f70d840a7 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3135,7 +3135,7 @@ static void get_object_list(int ac, const char **av)
 
 	if (prepare_revision_walk(&revs))
 		die(_("revision walk setup failed"));
-	mark_edges_uninteresting(&revs, show_edge);
+	mark_edges_uninteresting(&revs, show_edge, 0);
 
 	if (!fn_show_object)
 		fn_show_object = show_object;
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 2880ed37e3..9663cbfae0 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -543,7 +543,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	if (prepare_revision_walk(&revs))
 		die("revision walk setup failed");
 	if (revs.tree_objects)
-		mark_edges_uninteresting(&revs, show_edge);
+		mark_edges_uninteresting(&revs, show_edge, 0);
 
 	if (bisect_list) {
 		int reaches, all;
diff --git a/http-push.c b/http-push.c
index cd48590912..ea52d6f9f6 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1933,7 +1933,7 @@ int cmd_main(int argc, const char **argv)
 		pushing = 0;
 		if (prepare_revision_walk(&revs))
 			die("revision walk setup failed");
-		mark_edges_uninteresting(&revs, NULL);
+		mark_edges_uninteresting(&revs, NULL, 0);
 		objects_to_send = get_delta(&revs, ref_lock);
 		finish_all_active_slots();
 
diff --git a/list-objects.c b/list-objects.c
index c41cc80db5..4fbdeca0a4 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -222,25 +222,72 @@ static void mark_edge_parents_uninteresting(struct commit *commit,
 	}
 }
 
-void mark_edges_uninteresting(struct rev_info *revs, show_edge_fn show_edge)
+static void add_edge_parents(struct commit *commit,
+			     struct rev_info *revs,
+			     show_edge_fn show_edge,
+			     struct oidset *set)
+{
+	struct commit_list *parents;
+
+	for (parents = commit->parents; parents; parents = parents->next) {
+		struct commit *parent = parents->item;
+		struct tree *tree = get_commit_tree(parent);
+
+		if (!tree)
+			continue;
+
+		oidset_insert(set, &tree->object.oid);
+
+		if (!(parent->object.flags & UNINTERESTING))
+			continue;
+		tree->object.flags |= UNINTERESTING;
+
+		if (revs->edge_hint && !(parent->object.flags & SHOWN)) {
+			parent->object.flags |= SHOWN;
+			show_edge(parent);
+		}
+	}
+}
+
+void mark_edges_uninteresting(struct rev_info *revs,
+			      show_edge_fn show_edge,
+			      int sparse)
 {
 	struct commit_list *list;
+	struct oidset set;
 	int i;
 
+	if (sparse)
+		oidset_init(&set, 16);
+
 	for (list = revs->commits; list; list = list->next) {
 		struct commit *commit = list->item;
 
-		if (commit->object.flags & UNINTERESTING) {
+		if (sparse) {
+			struct tree *tree = get_commit_tree(commit);
+
+			if (commit->object.flags & UNINTERESTING)
+				tree->object.flags |= UNINTERESTING;
+
+			oidset_insert(&set, &tree->object.oid);
+			add_edge_parents(commit, revs, show_edge, &set);
+		} else if (commit->object.flags & UNINTERESTING) {
 			mark_tree_uninteresting(revs->repo,
 						get_commit_tree(commit));
 			if (revs->edge_hint_aggressive && !(commit->object.flags & SHOWN)) {
 				commit->object.flags |= SHOWN;
 				show_edge(commit);
 			}
-			continue;
+		} else {
+			mark_edge_parents_uninteresting(commit, revs, show_edge);
 		}
-		mark_edge_parents_uninteresting(commit, revs, show_edge);
 	}
+
+	if (sparse) {
+		mark_trees_uninteresting_sparse(revs->repo, &set);
+		oidset_clear(&set);
+	}
+
 	if (revs->edge_hint_aggressive) {
 		for (i = 0; i < revs->cmdline.nr; i++) {
 			struct object *obj = revs->cmdline.rev[i].item;
diff --git a/list-objects.h b/list-objects.h
index ad40762926..a952680e46 100644
--- a/list-objects.h
+++ b/list-objects.h
@@ -10,7 +10,9 @@ typedef void (*show_object_fn)(struct object *, const char *, void *);
 void traverse_commit_list(struct rev_info *, show_commit_fn, show_object_fn, void *);
 
 typedef void (*show_edge_fn)(struct commit *);
-void mark_edges_uninteresting(struct rev_info *, show_edge_fn);
+void mark_edges_uninteresting(struct rev_info *revs,
+			      show_edge_fn show_edge,
+			      int sparse);
 
 struct oidset;
 struct list_objects_filter_options;
diff --git a/revision.c b/revision.c
index 3a62c7c187..f9eb6400f1 100644
--- a/revision.c
+++ b/revision.c
@@ -109,6 +109,9 @@ void mark_trees_uninteresting_sparse(struct repository *r,
 	while ((oid = oidset_iter_next(&iter))) {
 		struct tree *tree = lookup_tree(r, oid);
 
+		if (!tree)
+			continue;
+
 		if (tree->object.flags & UNINTERESTING) {
 			/*
 			 * Remove the flag so the next call
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 1/6] revision: add mark_tree_uninteresting_sparse
From: Derrick Stolee via GitGitGadget @ 2018-11-29 14:24 UTC (permalink / raw)
  To: git; +Cc: peff, avarab, jrnieder, Junio C Hamano, Derrick Stolee
In-Reply-To: <pull.89.v2.git.gitgitgadget@gmail.com>

From: Derrick Stolee <dstolee@microsoft.com>

In preparation for a new algorithm that walks fewer trees when
creating a pack from a set of revisions, create a method that
takes an oidset of tree oids and marks reachable objects as
UNINTERESTING.

The current implementation uses the existing
mark_tree_uninteresting to recursively walk the trees and blobs.
This will walk the same number of trees as the old mechanism.

There is one new assumption in this approach: we are also given
the oids of the interesting trees. This implementation does not
use those trees at the moment, but we will use them in a later
rewrite of this method.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 revision.c | 22 ++++++++++++++++++++++
 revision.h |  2 ++
 2 files changed, 24 insertions(+)

diff --git a/revision.c b/revision.c
index 13e0519c02..3a62c7c187 100644
--- a/revision.c
+++ b/revision.c
@@ -99,6 +99,28 @@ void mark_tree_uninteresting(struct repository *r, struct tree *tree)
 	mark_tree_contents_uninteresting(r, tree);
 }
 
+void mark_trees_uninteresting_sparse(struct repository *r,
+				     struct oidset *set)
+{
+	struct object_id *oid;
+	struct oidset_iter iter;
+
+	oidset_iter_init(set, &iter);
+	while ((oid = oidset_iter_next(&iter))) {
+		struct tree *tree = lookup_tree(r, oid);
+
+		if (tree->object.flags & UNINTERESTING) {
+			/*
+			 * Remove the flag so the next call
+			 * is not a no-op. The flag is added
+			 * in mark_tree_unintersting().
+			 */
+			tree->object.flags ^= UNINTERESTING;
+			mark_tree_uninteresting(r, tree);
+		}
+	}
+}
+
 struct commit_stack {
 	struct commit **items;
 	size_t nr, alloc;
diff --git a/revision.h b/revision.h
index 7987bfcd2e..f828e91ae9 100644
--- a/revision.h
+++ b/revision.h
@@ -67,6 +67,7 @@ struct rev_cmdline_info {
 #define REVISION_WALK_NO_WALK_SORTED 1
 #define REVISION_WALK_NO_WALK_UNSORTED 2
 
+struct oidset;
 struct topo_walk_info;
 
 struct rev_info {
@@ -327,6 +328,7 @@ void put_revision_mark(const struct rev_info *revs,
 
 void mark_parents_uninteresting(struct commit *commit);
 void mark_tree_uninteresting(struct repository *r, struct tree *tree);
+void mark_trees_uninteresting_sparse(struct repository *r, struct oidset *set);
 
 void show_object_with_name(FILE *, struct object *, const char *);
 
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 0/6] Add a new "sparse" tree walk algorithm
From: Derrick Stolee via GitGitGadget @ 2018-11-29 14:24 UTC (permalink / raw)
  To: git; +Cc: peff, avarab, jrnieder, Junio C Hamano
In-Reply-To: <pull.89.git.gitgitgadget@gmail.com>

One of the biggest remaining pain points for users of very large
repositories is the time it takes to run 'git push'. We inspected some slow
pushes by our developers and found that the "Enumerating Objects" phase of a
push was very slow. This is unsurprising, because this is why reachability
bitmaps exist. However, reachability bitmaps are not available to us because
of the single pack-file requirement. The bitmap approach is intended for
servers anyway, and clients have a much different behavior pattern.

Specifically, clients are normally pushing a very small number of objects
compared to the entire working directory. A typical user changes only a
small cone of the working directory, so let's use that to our benefit.

Create a new "sparse" mode for 'git pack-objects' that uses the paths that
introduce new objects to direct our search into the reachable trees. By
collecting trees at each path, we can then recurse into a path only when
there are uninteresting and interesting trees at that path. This gains a
significant performance boost for small topics while presenting a
possibility of packing extra objects.

The main algorithm change is in patch 4, but is set up a little bit in
patches 1 and 2.

As demonstrated in the included test script, we see that the existing
algorithm can send extra objects due to the way we specify the "frontier".
But we can send even more objects if a user copies objects from one folder
to another. I say "copy" because a rename would (usually) change the
original folder and trigger a walk into that path, discovering the objects.

In order to benefit from this approach, the user can opt-in using the
pack.useSparse config setting. This setting can be overridden using the
'--no-sparse' option.

Update in V2: 

 * Added GIT_TEST_PACK_SPARSE test option.
 * Fixed test breakages when GIT_TEST_PACK_SPARSE is enabled by adding null
   checks.

Derrick Stolee (6):
  revision: add mark_tree_uninteresting_sparse
  list-objects: consume sparse tree walk
  pack-objects: add --sparse option
  revision: implement sparse algorithm
  pack-objects: create pack.useSparse setting
  pack-objects: create GIT_TEST_PACK_SPARSE

 Documentation/git-pack-objects.txt |   9 +-
 bisect.c                           |   2 +-
 builtin/pack-objects.c             |  10 ++-
 builtin/rev-list.c                 |   2 +-
 http-push.c                        |   2 +-
 list-objects.c                     |  55 +++++++++++-
 list-objects.h                     |   4 +-
 revision.c                         | 121 +++++++++++++++++++++++++
 revision.h                         |   2 +
 t/README                           |   4 +
 t/t5322-pack-objects-sparse.sh     | 139 +++++++++++++++++++++++++++++
 11 files changed, 340 insertions(+), 10 deletions(-)
 create mode 100755 t/t5322-pack-objects-sparse.sh


base-commit: a1598010f775d82b5adf12c29d0f5bc9b41434c6
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-89%2Fderrickstolee%2Fpush%2Fsparse-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-89/derrickstolee/push/sparse-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/89

Range-diff vs v1:

 1:  b73b8de98c = 1:  60617681f7 revision: add mark_tree_uninteresting_sparse
 2:  9bf04c748b ! 2:  4527addacb list-objects: consume sparse tree walk
     @@ -116,6 +116,10 @@
      +	for (parents = commit->parents; parents; parents = parents->next) {
      +		struct commit *parent = parents->item;
      +		struct tree *tree = get_commit_tree(parent);
     ++
     ++		if (!tree)
     ++			continue;
     ++
      +		oidset_insert(set, &tree->object.oid);
      +
      +		if (!(parent->object.flags & UNINTERESTING))
     @@ -142,14 +146,14 @@
      +
       	for (list = revs->commits; list; list = list->next) {
       		struct commit *commit = list->item;
     -+		
     + 
     +-		if (commit->object.flags & UNINTERESTING) {
      +		if (sparse) {
      +			struct tree *tree = get_commit_tree(commit);
     -+			
     ++
      +			if (commit->object.flags & UNINTERESTING)
      +				tree->object.flags |= UNINTERESTING;
     - 
     --		if (commit->object.flags & UNINTERESTING) {
     ++
      +			oidset_insert(&set, &tree->object.oid);
      +			add_edge_parents(commit, revs, show_edge, &set);
      +		} else if (commit->object.flags & UNINTERESTING) {
     @@ -189,3 +193,17 @@
       
       struct oidset;
       struct list_objects_filter_options;
     +
     +diff --git a/revision.c b/revision.c
     +--- a/revision.c
     ++++ b/revision.c
     +@@
     + 	while ((oid = oidset_iter_next(&iter))) {
     + 		struct tree *tree = lookup_tree(r, oid);
     + 
     ++		if (!tree)
     ++			continue;
     ++
     + 		if (tree->object.flags & UNINTERESTING) {
     + 			/*
     + 			 * Remove the flag so the next call
 3:  9d6b8f6d06 = 3:  9644f6ff04 pack-objects: add --sparse option
 4:  0725aac4bb ! 4:  c99957d06f revision: implement sparse algorithm
     @@ -157,6 +157,9 @@
      +	struct tree_desc desc;
      +	struct name_entry entry;
      +
     ++	if (!tree)
     ++		return;
     ++
      +	if (parse_tree_gently(tree, 1) < 0)
      +		return;
      +
     @@ -168,13 +171,15 @@
      +
      +			if (tree->object.flags & UNINTERESTING) {
      +				struct tree *child = lookup_tree(r, entry.oid);
     -+				child->object.flags |= UNINTERESTING;
     ++				if (child)
     ++					child->object.flags |= UNINTERESTING;
      +			}
      +			break;
      +		case OBJ_BLOB:
      +			if (tree->object.flags & UNINTERESTING) {
      +				struct blob *child = lookup_blob(r, entry.oid);
     -+				child->object.flags |= UNINTERESTING;
     ++				if (child)
     ++					child->object.flags |= UNINTERESTING;
      +			}
      +			break;
      +		default:
     @@ -201,6 +206,9 @@
      +	       (oid = oidset_iter_next(&iter))) {
       		struct tree *tree = lookup_tree(r, oid);
       
     + 		if (!tree)
     + 			continue;
     + 
      -		if (tree->object.flags & UNINTERESTING) {
      -			/*
      -			 * Remove the flag so the next call
 5:  bbc3f78182 = 5:  d6912188be pack-objects: create pack.useSparse setting
 -:  ---------- > 6:  3d394a9136 pack-objects: create GIT_TEST_PACK_SPARSE

-- 
gitgitgadget

^ permalink raw reply

* Re: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)
From: Ian Jackson @ 2018-11-29 14:30 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Jonathan Nieder, Junio C Hamano,
	Ævar Arnfjörð Bjarmason, git
In-Reply-To: <nycvar.QRO.7.76.6.1811291516540.41@tvgsbejvaqbjf.bet>

Johannes Schindelin writes ("Re: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)"):
> if you could pry more information (or better information) out of that bug
> reporter, that would be nice. Apparently my email address is blacklisted
> by his mail provider, so he is unlikely to have received my previous mail
> (nor will he receive this one, I am sure).

(I did receive this mail.  Sorry for the inconvenience, which sadly is
inevitable occasionally in the modern email world.  FTR in future feel
free to send the bounce to postmaster@chiark and I will make a
you-shaped hole in my spamfilter.  Also with Debian bugs you can
launder your messages by, eg, emailing 914695-submitter@bugs.)

> > > At https://bugs.debian.org/914695 is a report of a test regression in
> > > an outside project that is very likely to have been triggered by the
> > > new faster rebase code.

As I wrote in the bug report last night:

 https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=914695#15

 I have investigated and the bug seems to be that git-rebase --onto now
 fails to honour GIT_REFLOG_ACTION for the initial checkout.

 In a successful run with older git I get a reflog like this:

   4833d74 HEAD@{0}: rebase finished: returning to refs/heads/with-preexisting
   4833d74 HEAD@{1}: debrebase new-upstream 2.1-1: rebase: Add another new upstream file
   cabd5ec HEAD@{2}: debrebase new-upstream 2.1-1: rebase: Edit the .c file
   0b362ce HEAD@{3}: debrebase new-upstream 2.1-1: rebase: Add a new upstream file
   29653e5 HEAD@{4}: debrebase new-upstream 2.1-1: rebase: checkout 29653e5a17bee4ac23a68bba3e12bc1f52858ac3
   85e0c46 HEAD@{5}: debrebase: launder for new upstream

 With a newer git I get this:

   6d3fb91 HEAD@{0}: rebase finished: returning to refs/heads/master
   6d3fb91 HEAD@{1}: debrebase new-upstream 2.1-1: rebase: Add another new upstream file
   86c0721 HEAD@{2}: debrebase new-upstream 2.1-1: rebase: Edit the .c file
   50ba56c HEAD@{3}: debrebase new-upstream 2.1-1: rebase: Add a new upstream file
   8272825 HEAD@{4}: rebase: checkout 8272825bb4ff6eba89afa936e32b6460f963a36a
   c78db55 HEAD@{5}: debrebase: launder for new upstream

 This breaks the test because my test suite is checking that I set
 GIT_REFLOG_ACTION appropriately.

 If you want I can provide a minimal test case but this should suffice
 to see the bug I hope...

Regards
Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

^ permalink raw reply

* How de-duplicate similar repositories with alternates
From: Ævar Arnfjörð Bjarmason @ 2018-11-29 14:59 UTC (permalink / raw)
  To: git, Git for human beings; +Cc: Christian Couder

A co-worker asked me today how space could be saved when you have
multiple checkouts of the same repository (at different revs) on the
same machine. I said since these won't block-level de-duplicate well[1]
one way to do this is with alternates.

However, once you have an existing clone I didn't know how to get the
gains without a full re-clone, but I hadn't looked deeply into it. As it
turns out I'm wrong about that, which I found when writing the following
test-case which shows that it works:

    (
        cd /tmp &&
        rm -rf /tmp/git-{master,pu,pu-alt}.git &&

        # Normal clones
        git clone --bare --no-tags --single-branch --branch master https://github.com/git/git.git /tmp/git-master.git &&
        git clone --bare --no-tags --single-branch --branch pu https://github.com/git/git.git /tmp/git-pu.git &&

        # An 'alternate' clone using 'master' objects from another repo
        git --bare init /tmp/git-pu-alt.git &&
        for git in git-pu.git git-pu-alt.git
        do
            echo /tmp/git-master.git/objects >/tmp/$git/objects/info/alternates
        done &&
        git -C git-pu-alt.git fetch --no-tags https://github.com/git/git.git pu:pu

        # Respective sizes, 'alternate' clone much smaller
        du -shc /tmp/git-*.git &&

        # GC them all. Compacts the git-pu.git to git-pu-alt.git's size
        for repo in git-*.git
        do
            git -C $repo gc
        done &&
        du -shc /tmp/git-*.git

        # Add another big history (GFW) to git-{pu,master}.git (in that order!)
        for repo in $(ls -d /tmp/git-*.git | sort -r)
        do
            git -C $repo fetch --no-tags https://github.com/git-for-windows/git master:master-gfw
        done &&
        du -shc /tmp/git-*.git &&

        # Another GC. The objects now in git-master.git will be de-duped by all
        for repo in git-*.git
        do
            git -C $repo gc
        done &&
        du -shc /tmp/git-*.git
    )

This shows a scenario where we clone git.git at "master" and "pu" in
different places. After clone the relevant sizes are:

    108M    /tmp/git-master.git
    3.2M    /tmp/git-pu-alt.git
    109M    /tmp/git-pu.git
    219M    total

I.e. git-pu-alt.git is much smaller since it points via alternates to
git-master.git, and the history of "pu" shares most of the objects with
"master". But then how do you get those gains for git-pu.git? Turns out
you just "git gc"

    111M    /tmp/git-master.git
    2.1M    /tmp/git-pu-alt.git
    2.1M    /tmp/git-pu.git
    115M    total

This is the thing I was wrong about, in retrospect probably because I'd
been putting PATH_TO_REPO in objects/info/alternates, but we actually
need PATH_TO_REPO/objects, and "git gc" won't warn about this (or "git
fsck"). Probably a good idea to patch that at some point, i.e. whine
about paths in alternates that don't have objects, or at the very least
those that don't exist. #leftoverbits

Then when we fetch git-for-windows:master to all the repos they all grow
by the amount git-for-windows has diverged:

    144M    /tmp/git-master.git
    36M     /tmp/git-pu-alt.git
    36M     /tmp/git-pu.git
    214M    total

Note that the "sort -r" is critical here. If we fetched git-master.git
first (at this point the alternate for git-pu*.git) we wouldn't get the
duplication in the first place, but instead:

    144M    /tmp/git-master.git
    2.1M    /tmp/git-pu-alt.git
    2.1M    /tmp/git-pu.git
    148M    total

This shows the importance of keeping such an 'alternate' repo
up-to-date, i.e. we don't get the duplication in the first place, but
regardless (this from a run with sort -r) a "git gc" will coalesce them:

    131M    /tmp/git-master.git
    2.1M    /tmp/git-pu-alt.git
    2.2M    /tmp/git-pu.git
    135M    total

If you find this interesting make sure to read my
https://public-inbox.org/git/87k1s3bomt.fsf@evledraar.gmail.com/ and
https://public-inbox.org/git/87in7nbi5b.fsf@evledraar.gmail.com/ for the
caveats, i.e. if this is something intended for users then no ref in the
alternate can ever be rewound, that'll potentially result in repository
corruption.

1. https://public-inbox.org/git/87bmhiykvw.fsf@evledraar.gmail.com/

^ permalink raw reply

* Re: Simple git push --tags deleted all branches
From: Ævar Arnfjörð Bjarmason @ 2018-11-29 15:03 UTC (permalink / raw)
  To: Mateusz Loskot; +Cc: git
In-Reply-To: <CABUeae-e33Jc4s4MezjhpVsEJ_0sO8QZ1mpa1z_79ZGuQWM-Xw@mail.gmail.com>


On Wed, Nov 28 2018, Mateusz Loskot wrote:

> Hi,
>
> (using git version 2.19.2.windows.1)
>
> I've just encountered one of those WTH moments.
>
> I have a bare repository
>
> core.git (BARE:master) $ git branch
>   1.0
>   2.0
> * master
>
> core.git (BARE:master) $ git tag
> 1.0.1651
> 1.0.766
> 2.0.1103
> 2.0.1200
>
> I published the repo using: git push --all --follow-tags
>
> This succeeded, but there seem to be no tags pushed, just branches.
> So, I followed with
>
> core.git (BARE:master) $ git push --tags
> To XXX
>  - [deleted]               1.0
>  - [deleted]               2.0
>  ! [remote rejected]       master (refusing to delete the current
> branch: refs/heads/master)
> error: failed to push some refs to 'XXX'
>
> And, I've found out that all branches and tags have been
> wiped in both, local repo and remote :)
>
> I restored the repo and tried out
>
> git push origin 1.0
> git push origin --tags
>
> and this time both succeeded, without wiping out any refs.
>
> Could anyone help me to understand why remote-less
>
> git push --tags
>
> is/was so dangerous and unforgiving?!

Since nobody's replied yet, I can't see what's going on here from the
info you've provided. My guess is that you have something "mirror" set
on the remote.

It seems you can't share the repo or its URL, but could you share the
scrubbed output of 'git config -l --show-origin' when run inside this
repository?

^ permalink raw reply

* Re: Git Tags
From: Ævar Arnfjörð Bjarmason @ 2018-11-29 15:09 UTC (permalink / raw)
  To: Stefanie Leisestreichler; +Cc: git
In-Reply-To: <c8fc0da2-c3ff-4985-e4a2-a066a3a6f2af@peter-speer.de>


On Thu, Nov 29 2018, Stefanie Leisestreichler wrote:

> Hi.
>
> I have done this (on box A):
>
> git commit -m "Message"
> git tag -a 0.9.0
> git push origin master
>
> In my local repository, when I run "git tag" it is showing me "0.9.0".
>
> Then I did (on box B)
> git clone ssh://user@host:/path/project.git
> cd project
> git tag
>
> Now git tag is showing nothing.
>
> Why is the tag only available in my local repository?
>
> Also when I try to
> git clone --branch 0.9.0 ssh://user@host:/path/project.git
> it tells me: fatal:remote branch not found in upstream repository origin

Because --branch <name> means get refs/heads/<name>, tags are not
branches. However, because we're apparently quite loose about this in
the clone/fetch code this does give you the tag if it exists, but
probably not in the way you expect.

We interpret the argument as a branch, and will get not only this tag
but "follow" (see --no-tags in git-fetch(1)) the tag as though it were a
branch and give you all tags leading up to that one. This would give you
a single tag:

    git clone --no-tags --branch v2.19.0 --single-branch https://github.com/git/git.git

But this is a more direct way to do it:

    git init git; git -C git fetch --no-tags https://github.com/git/git.git tag v2.19.0

Which'll since you said it failed that's because you haven't pushed the
tag. Try 'git ls-remote <url>' to see if it's there (it's not).

^ permalink raw reply

* Re: Simple git push --tags deleted all branches
From: Mateusz Loskot @ 2018-11-29 15:18 UTC (permalink / raw)
  To: git
In-Reply-To: <87y39cx6wt.fsf@evledraar.gmail.com>

On Thu, 29 Nov 2018 at 16:03, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> On Wed, Nov 28 2018, Mateusz Loskot wrote:
> >
> > (using git version 2.19.2.windows.1)
> >
> > I've just encountered one of those WTH moments.
> >
> > I have a bare repository
> >
> > core.git (BARE:master) $ git branch
> >   1.0
> >   2.0
> > * master
> >
> > core.git (BARE:master) $ git tag
> > 1.0.1651
> > 1.0.766
> > 2.0.1103
> > 2.0.1200
> >
> > I published the repo using: git push --all --follow-tags
> >
> > This succeeded, but there seem to be no tags pushed, just branches.
> > So, I followed with
> >
> > core.git (BARE:master) $ git push --tags
> > To XXX
> >  - [deleted]               1.0
> >  - [deleted]               2.0
> >  ! [remote rejected]       master (refusing to delete the current
> > branch: refs/heads/master)
> > error: failed to push some refs to 'XXX'
> >
> > And, I've found out that all branches and tags have been
> > wiped in both, local repo and remote :)
> >
> > I restored the repo and tried out
> >
> > git push origin 1.0
> > git push origin --tags
> >
> > and this time both succeeded, without wiping out any refs.
> >
> > Could anyone help me to understand why remote-less
> >
> > git push --tags
> >
> > is/was so dangerous and unforgiving?!
>
> Since nobody's replied yet, I can't see what's going on here from the
> info you've provided. My guess is that you have something "mirror" set
> on the remote.

Thank you for responding.

The git push --tags hugely surprised me, and here is genuine screenshot
https://twitter.com/mloskot/status/1068072285846859776

> It seems you can't share the repo or its URL, but could you share the
> scrubbed output of 'git config -l --show-origin' when run inside this
> repository?

Here is complete output. I have not stripped the basics like aliases,
just in case.

```
core-external-metadata.git (BARE:master) $ git config -l --show-origin
file:"C:\\ProgramData/Git/config"       core.symlinks=false
file:"C:\\ProgramData/Git/config"       core.autocrlf=true
file:"C:\\ProgramData/Git/config"       color.diff=auto
file:"C:\\ProgramData/Git/config"       color.status=auto
file:"C:\\ProgramData/Git/config"       color.branch=auto
file:"C:\\ProgramData/Git/config"       color.interactive=true
file:"C:\\ProgramData/Git/config"       help.format=html
file:"C:\\ProgramData/Git/config"       http.sslcainfo=C:/Program
Files/Git/mingw64/ssl/certs/ca-bundle.crt
file:"C:\\ProgramData/Git/config"       diff.astextplain.textconv=astextplain
file:"C:\\ProgramData/Git/config"       rebase.autosquash=true
file:C:/Program Files/Git/mingw64/etc/gitconfig
http.sslcainfo=C:/Program Files/Git/mingw64/ssl/certs/ca-bundle.crt
file:C:/Program Files/Git/mingw64/etc/gitconfig http.sslbackend=openssl
file:C:/Program Files/Git/mingw64/etc/gitconfig
diff.astextplain.textconv=astextplain
file:C:/Program Files/Git/mingw64/etc/gitconfig
filter.lfs.clean=git-lfs clean -- %f
file:C:/Program Files/Git/mingw64/etc/gitconfig
filter.lfs.smudge=git-lfs smudge -- %f
file:C:/Program Files/Git/mingw64/etc/gitconfig
filter.lfs.process=git-lfs filter-process
file:C:/Program Files/Git/mingw64/etc/gitconfig filter.lfs.required=true
file:C:/Program Files/Git/mingw64/etc/gitconfig credential.helper=manager
file:C:/Users/mateuszl/.gitconfig       user.name=Mateusz Łoskot
file:C:/Users/mateuszl/.gitconfig       user.email=mateusz@loskot.net
file:C:/Users/mateuszl/.gitconfig       github.user=mloskot
file:C:/Users/mateuszl/.gitconfig       core.editor=vim
file:C:/Users/mateuszl/.gitconfig       color.branch=auto
file:C:/Users/mateuszl/.gitconfig       color.diff=auto
file:C:/Users/mateuszl/.gitconfig       color.status=auto
file:C:/Users/mateuszl/.gitconfig       color.ui=auto
file:C:/Users/mateuszl/.gitconfig       alias.br=branch
file:C:/Users/mateuszl/.gitconfig       alias.bv=branch -vv
file:C:/Users/mateuszl/.gitconfig       alias.brv=branch -vv
file:C:/Users/mateuszl/.gitconfig       alias.bra=branch -a
file:C:/Users/mateuszl/.gitconfig       alias.ci=commit --verbose
file:C:/Users/mateuszl/.gitconfig       alias.cia=commit --verbose --amend
file:C:/Users/mateuszl/.gitconfig       alias.ciae=commit --verbose
--amend --no-edit
file:C:/Users/mateuszl/.gitconfig       alias.co=checkout
file:C:/Users/mateuszl/.gitconfig       alias.dc=diff --cached
file:C:/Users/mateuszl/.gitconfig       alias.df=diff
file:C:/Users/mateuszl/.gitconfig       alias.gf=flow
file:C:/Users/mateuszl/.gitconfig       alias.lg=log --color --graph
--pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr)
%C(bold blue)<%an>%Creset' --abbrev-commit
file:C:/Users/mateuszl/.gitconfig       alias.lol=log --graph
--decorate --pretty=oneline --abbrev-commit
file:C:/Users/mateuszl/.gitconfig       alias.lola=log --graph
--decorate --pretty=oneline --abbrev-commit --all
file:C:/Users/mateuszl/.gitconfig       alias.ls=ls-files
file:C:/Users/mateuszl/.gitconfig       alias.lst=lfs status
file:C:/Users/mateuszl/.gitconfig       alias.rt=remote
file:C:/Users/mateuszl/.gitconfig       alias.rtv=remote -v
file:C:/Users/mateuszl/.gitconfig       alias.st=status
file:C:/Users/mateuszl/.gitconfig       alias.ghclm=!sh -c 'git log
$1...$2  --pretty=format:"* [view](http://github.com/$3/$4/commit/%H)
- %s"' -
file:C:/Users/mateuszl/.gitconfig       alias.ign=ls-files -o -i
--exclude-standard
file:C:/Users/mateuszl/.gitconfig
alias.prune-branches-origin=!git remote prune origin && git branch -vv
| grep ': gone]' | awk '{print $1}' | xargs -r git branch -D
file:C:/Users/mateuszl/.gitconfig
alias.prune-branches-mloskot=!git remote prune mloskot && git branch
-vv | grep ': gone]' | awk '{print $1}' | xargs -r git branch -D
file:C:/Users/mateuszl/.gitconfig       alias.hist=log
--pretty=format:'%h %ad | %s%d [%an]' --graph --date=short
file:C:/Users/mateuszl/.gitconfig       commit.template=~/.gitmessage
file:C:/Users/mateuszl/.gitconfig       credential.helper=manager
file:C:/Users/mateuszl/.gitconfig       filter.lfs.clean=git-lfs clean -- %f
file:C:/Users/mateuszl/.gitconfig       filter.lfs.smudge=git-lfs
smudge --skip -- %f
file:C:/Users/mateuszl/.gitconfig       filter.lfs.process=git-lfs
filter-process --skip
file:C:/Users/mateuszl/.gitconfig       filter.lfs.required=true
file:config     core.repositoryformatversion=0
file:config     core.filemode=false
file:config     core.bare=true
file:config     core.symlinks=false
file:config     core.ignorecase=true
file:config     remote.origin.url=https://xxx.com/core-external-metadata.git
file:config     remote.origin.fetch=+refs/*:refs/*
file:config     remote.origin.mirror=true
file:config
lfs.https://xxx.com/core-external-metadata.git/info/lfs.access=basic
```

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net

^ permalink raw reply

* Re: [PATCH/RFC v2 0/7] Introduce new commands switch-branch and checkout-files
From: Duy Nguyen @ 2018-11-29 15:33 UTC (permalink / raw)
  To: Stefan Beller
  Cc: Ævar Arnfjörð Bjarmason, Git Mailing List,
	Junio C Hamano, Thomas Gummerer
In-Reply-To: <CAGZ79kYiMObOHAuf+01r0-YVWHBk-6NpceXh9Z25dx9JZsP62Q@mail.gmail.com>

On Wed, Nov 28, 2018 at 9:30 PM Stefan Beller <sbeller@google.com> wrote:
>
> On Wed, Nov 28, 2018 at 12:09 PM Duy Nguyen <pclouds@gmail.com> wrote:
> >
> > On Wed, Nov 28, 2018 at 9:01 PM Duy Nguyen <pclouds@gmail.com> wrote:
> > > should we do
> > > something about detached HEAD in this switch-branch command (or
> > > whatever its name will be)?
> > >
> > > This is usually a confusing concept to new users
> >
> > And it just occurred to me that perhaps we should call this "unnamed
> > branch" (at least at high UI level) instead of detached HEAD. It is
> > technically not as accurate, but much better to understand.
>
> or 'direct' branch?

makes me think, what is an indirect branch?

> I mean 'detached HEAD' itself is also not correct
> as the HEAD points to a valid commit/tag usually, so it is attached to
> that content. The detachment comes from the implicit "from a branch".

Yeah I guess it's short for "HEAD that is detached from a branch"
-- 
Duy

^ permalink raw reply

* Re: [PATCH v2 6/7] checkout: split into switch-branch and checkout-files
From: Duy Nguyen @ 2018-11-29 15:36 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Stefan Xenos, Ævar Arnfjörð Bjarmason,
	Git Mailing List, Stefan Beller, Thomas Gummerer
In-Reply-To: <xmqqd0qopgpn.fsf@gitster-ct.c.googlers.com>

On Thu, Nov 29, 2018 at 6:59 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Stefan Xenos <sxenos@google.com> writes:
>
> > Although I have no problem with "switch-branch" as a command name,
> > some alternative names we might consider for switch-branch might be:
> >
> > chbranch
> > swbranch
>
> Please never go in that direction.  So far, we made a conscious
> effort to keep the names of most frequently used subcommand to
> proper words that can be understood by coders (IOW, I expect they
> know what 'grep' is, even though that may not be a 'proper word').
>
> > switch
> > branch change (as a subcommand for the "branch" command)
>
> It is more like moving to the branch to work on.  I think 'switch'
> is something SVN veterans may find familiar.  Both are much better
> than ch/swbranch that are overly cryptic.

OK I'll go with switch-branch and restore-files in the next round. And
perhaps consider just 'switch' and 'restore' later.
-- 
Duy

^ permalink raw reply

* Re: [PATCH v1] mem_pool: add GIT_TRACE_MEMPOOL support
From: Johannes Schindelin @ 2018-11-29 15:37 UTC (permalink / raw)
  To: Ben Peart; +Cc: git, benpeart
In-Reply-To: <9ce2df67-d698-0372-4770-32659668ab7e@gmail.com>

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

Hi Ben,

On Thu, 29 Nov 2018, Ben Peart wrote:

> On 11/28/2018 4:37 AM, Johannes Schindelin wrote:
> > Hi Ben,
> >
> > On Tue, 27 Nov 2018, Ben Peart wrote:
> >
> > > From: Ben Peart <benpeart@microsoft.com>
> > >
> > > Add tracing around initializing and discarding mempools. In discard report
> > > on the amount of memory unused in the current block to help tune setting
> > > the initial_size.
> > >
> > > Signed-off-by: Ben Peart <benpeart@microsoft.com>
> > > ---
> > Looks good.
> >
> > My only question: should we also trace calls to _alloc(), _calloc() and
> > _combine()?
> 
> I was trying to tune the initial size in my use of the mem_pool and so found
> this tracing useful to see how much memory was actually being used.  I'm
> inclined to only add tracing as it is needed rather that proactively because
> we think it _might_ be needed.  I suspect _alloc() and _calloc() would get
> very noisy and not add much value.

In other words, YAGNI. Makes sense.

Thanks,
Johannes

> 
> >
> > Ciao,
> > Johannes
> >
> > > Notes:
> > >      Base Ref: * git-trace-mempool
> > >      Web-Diff: https://github.com/benpeart/git/commit/9ac84bbca2
> > >      Checkout: git fetch https://github.com/benpeart/git
> > >      git-trace-mempool-v1 && git checkout 9ac84bbca2
> > >
> > >   mem-pool.c | 5 +++++
> > >   1 file changed, 5 insertions(+)
> > >
> > > diff --git a/mem-pool.c b/mem-pool.c
> > > index a2841a4a9a..065389aaec 100644
> > > --- a/mem-pool.c
> > > +++ b/mem-pool.c
> > > @@ -5,6 +5,7 @@
> > >   #include "cache.h"
> > >   #include "mem-pool.h"
> > >   
> > > +static struct trace_key trace_mem_pool = TRACE_KEY_INIT(MEMPOOL);
> > >   #define BLOCK_GROWTH_SIZE 1024*1024 - sizeof(struct mp_block);
> > >   
> > >   /*
> > > @@ -48,12 +49,16 @@ void mem_pool_init(struct mem_pool **mem_pool, size_t
> > > initial_size)
> > >     mem_pool_alloc_block(pool, initial_size, NULL);
> > >   
> > >   	*mem_pool = pool;
> > > +	trace_printf_key(&trace_mem_pool, "mem_pool (%p): init (%"PRIuMAX")
> > > initial size\n",
> > > +		pool, (uintmax_t)initial_size);
> > >   }
> > >   
> > >   void mem_pool_discard(struct mem_pool *mem_pool, int invalidate_memory)
> > >   {
> > >    struct mp_block *block, *block_to_free;
> > >   +	trace_printf_key(&trace_mem_pool, "mem_pool (%p): discard (%"PRIuMAX")
> > > unused\n",
> > > +		mem_pool, (uintmax_t)(mem_pool->mp_block->end -
> > > mem_pool->mp_block->next_free));
> > >    block = mem_pool->mp_block;
> > >    while (block)
> > >    {
> > >
> > > base-commit: bb75be6cb916297f271c846f2f9caa3daaaec718
> > > -- 
> > > 2.18.0.windows.1
> > >
> > >
> 
> 

^ permalink raw reply

* Re: Simple git push --tags deleted all branches
From: Ævar Arnfjörð Bjarmason @ 2018-11-29 15:39 UTC (permalink / raw)
  To: Mateusz Loskot; +Cc: git
In-Reply-To: <CABUeae_VVtbj0JCRyUuqf=uaPFXkmHwHpYyapH4H5A_cQSQsdA@mail.gmail.com>


On Thu, Nov 29 2018, Mateusz Loskot wrote:

> On Thu, 29 Nov 2018 at 16:03, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> On Wed, Nov 28 2018, Mateusz Loskot wrote:
>> >
>> > (using git version 2.19.2.windows.1)
>> >
>> > I've just encountered one of those WTH moments.
>> >
>> > I have a bare repository
>> >
>> > core.git (BARE:master) $ git branch
>> >   1.0
>> >   2.0
>> > * master
>> >
>> > core.git (BARE:master) $ git tag
>> > 1.0.1651
>> > 1.0.766
>> > 2.0.1103
>> > 2.0.1200
>> >
>> > I published the repo using: git push --all --follow-tags
>> >
>> > This succeeded, but there seem to be no tags pushed, just branches.
>> > So, I followed with
>> >
>> > core.git (BARE:master) $ git push --tags
>> > To XXX
>> >  - [deleted]               1.0
>> >  - [deleted]               2.0
>> >  ! [remote rejected]       master (refusing to delete the current
>> > branch: refs/heads/master)
>> > error: failed to push some refs to 'XXX'
>> >
>> > And, I've found out that all branches and tags have been
>> > wiped in both, local repo and remote :)
>> >
>> > I restored the repo and tried out
>> >
>> > git push origin 1.0
>> > git push origin --tags
>> >
>> > and this time both succeeded, without wiping out any refs.
>> >
>> > Could anyone help me to understand why remote-less
>> >
>> > git push --tags
>> >
>> > is/was so dangerous and unforgiving?!
>>
>> Since nobody's replied yet, I can't see what's going on here from the
>> info you've provided. My guess is that you have something "mirror" set
>> on the remote.
>
> Thank you for responding.
>
> The git push --tags hugely surprised me, and here is genuine screenshot
> https://twitter.com/mloskot/status/1068072285846859776
>
>> It seems you can't share the repo or its URL, but could you share the
>> scrubbed output of 'git config -l --show-origin' when run inside this
>> repository?
>
> Here is complete output. I have not stripped the basics like aliases,
> just in case.

Right, it's because you used --mirror, the important bit:

> file:config     remote.origin.url=https://xxx.com/core-external-metadata.git
> file:config     remote.origin.fetch=+refs/*:refs/*
> file:config     remote.origin.mirror=true
> file:config

I.e. you have cloned with the --mirror flag, this is what it's supposed
to do: https://git-scm.com/docs/git-clone#git-clone---mirror
https://git-scm.com/docs/git-fetch#git-fetch---prune

I.e. you push and git tries to mirror the refs you have locally to the
remote, including pruning stuff in the remote.

This is useful, but not what you wanted here. It's used for e.g. making
an up-to-date copy of a repo from server A to server B in HA setups
where you'd like to fail over to server B and get the same refs you had
on A.

^ permalink raw reply

* Re: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)
From: Johannes Schindelin @ 2018-11-29 15:39 UTC (permalink / raw)
  To: Ian Jackson
  Cc: Jonathan Nieder, Junio C Hamano,
	Ævar Arnfjörð Bjarmason, git
In-Reply-To: <23551.63504.876084.449440@chiark.greenend.org.uk>

Hi Ian,

On Thu, 29 Nov 2018, Ian Jackson wrote:

> Johannes Schindelin writes ("Re: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)"):
> > if you could pry more information (or better information) out of that bug
> > reporter, that would be nice. Apparently my email address is blacklisted
> > by his mail provider, so he is unlikely to have received my previous mail
> > (nor will he receive this one, I am sure).
> 
> (I did receive this mail.  Sorry for the inconvenience, which sadly is
> inevitable occasionally in the modern email world.  FTR in future feel
> free to send the bounce to postmaster@chiark and I will make a
> you-shaped hole in my spamfilter.  Also with Debian bugs you can
> launder your messages by, eg, emailing 914695-submitter@bugs.)

Right. I myself have plenty of email-related problems that seem to crop up
this year in particular.

> > > > At https://bugs.debian.org/914695 is a report of a test regression in
> > > > an outside project that is very likely to have been triggered by the
> > > > new faster rebase code.
> 
> As I wrote in the bug report last night:
> 
>  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=914695#15
> 
>  I have investigated and the bug seems to be that git-rebase --onto now
>  fails to honour GIT_REFLOG_ACTION for the initial checkout.
> 
>  In a successful run with older git I get a reflog like this:
> 
>    4833d74 HEAD@{0}: rebase finished: returning to refs/heads/with-preexisting
>    4833d74 HEAD@{1}: debrebase new-upstream 2.1-1: rebase: Add another new upstream file
>    cabd5ec HEAD@{2}: debrebase new-upstream 2.1-1: rebase: Edit the .c file
>    0b362ce HEAD@{3}: debrebase new-upstream 2.1-1: rebase: Add a new upstream file
>    29653e5 HEAD@{4}: debrebase new-upstream 2.1-1: rebase: checkout 29653e5a17bee4ac23a68bba3e12bc1f52858ac3
>    85e0c46 HEAD@{5}: debrebase: launder for new upstream
> 
>  With a newer git I get this:
> 
>    6d3fb91 HEAD@{0}: rebase finished: returning to refs/heads/master
>    6d3fb91 HEAD@{1}: debrebase new-upstream 2.1-1: rebase: Add another new upstream file
>    86c0721 HEAD@{2}: debrebase new-upstream 2.1-1: rebase: Edit the .c file
>    50ba56c HEAD@{3}: debrebase new-upstream 2.1-1: rebase: Add a new upstream file
>    8272825 HEAD@{4}: rebase: checkout 8272825bb4ff6eba89afa936e32b6460f963a36a
>    c78db55 HEAD@{5}: debrebase: launder for new upstream
> 
>  This breaks the test because my test suite is checking that I set
>  GIT_REFLOG_ACTION appropriately.
> 
>  If you want I can provide a minimal test case but this should suffice
>  to see the bug I hope...

This should be plenty for me to get going. Thank you!

Ciao,
Johannes

> 
> Regards
> Ian.
> 
> -- 
> Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.
> 
> If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
> a private address which bypasses my fierce spamfilter.
> 

^ permalink raw reply

* Re: [PATCH 2/2] format-patch: allow for independent diff & range-diff options
From: Johannes Schindelin @ 2018-11-29 15:41 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, Junio C Hamano, Eric Sunshine
In-Reply-To: <871s74yms3.fsf@evledraar.gmail.com>

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

Hi Ævar,

On Thu, 29 Nov 2018, Ævar Arnfjörð Bjarmason wrote:

> On Thu, Nov 29 2018, Johannes Schindelin wrote:
> 
> > On Thu, 29 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
> >
> >> On Thu, Nov 29 2018, Johannes Schindelin wrote:
> >>
> >> > On Wed, 28 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
> >> >
> >> >> Change the semantics of the "--range-diff" option so that the regular
> >> >> diff options can be provided separately for the range-diff and the
> >> >> patch. This allows for supplying e.g. --range-diff-U0 and -U1 to
> >> >> "format-patch" to provide different context for the range-diff and the
> >> >> patch. This wasn't possible before.
> >> >
> >> > I really, really dislike the `--range-diff-<random-thing>`. We have
> >> > precedent for passing optional arguments that are passed to some other
> >> > command, so a much more logical and consistent convention would be to use
> >> > `--range-diff[=<diff-option>..]`, allowing all of the diff options that
> >> > you might want to pass to the outer diff in one go rather than having a
> >> > lengthy string of `--range-diff-this` and `--range-diff-that` options.
> >>
> >> Where do we pass those sorts of arguments?
> >>
> >> Reasons I did it this way:
> >>
> >>  a) Passing it as one option will require the user to double-quote those
> >>     options that take quoted arguments (e.g. --word-diff-regex), which I
> >>     thought sucked more than the prefix. On the implementation side we
> >>     couldn't leave the parsing of the command-line to the shell anymore.
> >>
> >>  b) I think people will want to tweak this very rarely, much more rarely
> >>     than e.g. -U10 in format-patch itself, so having something long-ish
> >>     doesn't sound bad.
> >
> > Hmm. I still don't like it. It sets a precedent, and we simply do not do
> > it that way in other circumstances (most obvious would be the -X merge
> > options). The more divergent user interfaces for the same sort of thing
> > are, the more brain cycles you force users to spend on navigating said
> > interfaces.
> 
> Yeah it sucks, I just think it sucks less than the alternative :)
> I.e. I'm not picky about --range-diff-* prefix the name, but I think
> doing our own shell parsing would be nasty.

What prevents you from using `sq_dequote_to_argv()`?

> >> > I only had time to skim the patch, and I have to wonder why you pass
> >> > around full-blown `rev_info` structs for range diff (and with that really
> >> > awful name `rd_rev`) rather than just the `diff_options` that you
> >> > *actually* care about?
> >>
> >> Because setup_revisions() which does all the command-line parsing needs
> >> a rev_info, so even if we only need the diffopt in the end we need to
> >> initiate the whole thing.
> >>
> >> Suggestions for a better varibale name most welcome.
> >
> > `range_diff_revs`
> >
> > And you do not need to pass around the whole thing. You can easily pass
> > `&range_diff_revs.diffopt`.
> >
> > Don't pass around what you do not need to pass around.
> 
> Ah, you mean internally in log.c, yes that makes sense. I thought you
> meant just pass diffopt to setup_revisions() (which needs the containing
> struct). Willdo.

Thanks,
Dscho

> 
> > Ciao,
> > Dscho
> >
> >>
> >> > Ciao,
> >> > Dscho
> >> >
> >> >>
> >> >> Ever since the "--range-diff" option was added in
> >> >> 31e2617a5f ("format-patch: add --range-diff option to embed diff in
> >> >> cover letter", 2018-07-22) the "rev->diffopt" we pass down to the diff
> >> >> machinery has been the one we get from "format-patch"'s own
> >> >> setup_revisions().
> >> >>
> >> >> This sort of thing is unique among the log-like commands in
> >> >> builtin/log.c, no command than format-patch will embed the output of
> >> >> another log-like command. Since the "rev->diffopt" is reused we need
> >> >> to munge it before we pass it to show_range_diff(). See
> >> >> 43dafc4172 ("format-patch: don't include --stat with --range-diff
> >> >> output", 2018-11-22) for a related regression fix which is being
> >> >> mostly reverted here.
> >> >>
> >> >> Implementation notes: 1) We're not bothering with the full teardown
> >> >> around die() and will leak memory, but it's too much boilerplate to do
> >> >> all the frees with/without the die() and not worth it. 2) We call
> >> >> repo_init_revisions() for "rd_rev" even though we could get away with
> >> >> a shallow copy like the code we're replacing (and which
> >> >> show_range_diff() itself does). This is to make this code more easily
> >> >> understood.
> >> >>
> >> >> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> >> >> ---
> >> >>  Documentation/git-format-patch.txt | 10 ++++++-
> >> >>  builtin/log.c                      | 42 +++++++++++++++++++++++-------
> >> >>  t/t3206-range-diff.sh              | 41 +++++++++++++++++++++++++++++
> >> >>  3 files changed, 82 insertions(+), 11 deletions(-)
> >> >>
> >> >> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> >> >> index aba4c5febe..6c048f415f 100644
> >> >> --- a/Documentation/git-format-patch.txt
> >> >> +++ b/Documentation/git-format-patch.txt
> >> >> @@ -24,7 +24,8 @@ SYNOPSIS
> >> >>  		   [--to=<email>] [--cc=<email>]
> >> >>  		   [--[no-]cover-letter] [--quiet] [--notes[=<ref>]]
> >> >>  		   [--interdiff=<previous>]
> >> >> -		   [--range-diff=<previous> [--creation-factor=<percent>]]
> >> >> +		   [--range-diff=<previous> [--creation-factor=<percent>]
> >> >> +		      [--range-diff<common diff option>]]
> >> >>  		   [--progress]
> >> >>  		   [<common diff options>]
> >> >>  		   [ <since> | <revision range> ]
> >> >> @@ -257,6 +258,13 @@ feeding the result to `git send-email`.
> >> >>  	creation/deletion cost fudge factor. See linkgit:git-range-diff[1])
> >> >>  	for details.
> >> >>
> >> >> +--range-diff<common diff option>::
> >> >> +	Other options prefixed with `--range-diff` are stripped of
> >> >> +	that prefix and passed as-is to the diff machinery used to
> >> >> +	generate the range-diff, e.g. `--range-diff-U0` and
> >> >> +	`--range-diff--no-color`. This allows for adjusting the format
> >> >> +	of the range-diff independently from the patch itself.
> >> >> +
> >> >>  --notes[=<ref>]::
> >> >>  	Append the notes (see linkgit:git-notes[1]) for the commit
> >> >>  	after the three-dash line.
> >> >> diff --git a/builtin/log.c b/builtin/log.c
> >> >> index 02d88fa233..7658e56ecc 100644
> >> >> --- a/builtin/log.c
> >> >> +++ b/builtin/log.c
> >> >> @@ -1023,7 +1023,8 @@ static void show_diffstat(struct rev_info *rev,
> >> >>  	fprintf(rev->diffopt.file, "\n");
> >> >>  }
> >> >>
> >> >> -static void make_cover_letter(struct rev_info *rev, int use_stdout,
> >> >> +static void make_cover_letter(struct rev_info *rev, struct rev_info *rd_rev,
> >> >> +			      int use_stdout,
> >> >>  			      struct commit *origin,
> >> >>  			      int nr, struct commit **list,
> >> >>  			      const char *branch_name,
> >> >> @@ -1095,13 +1096,9 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
> >> >>  	}
> >> >>
> >> >>  	if (rev->rdiff1) {
> >> >> -		struct diff_options opts;
> >> >> -		memcpy(&opts, &rev->diffopt, sizeof(opts));
> >> >> -		opts.output_format &= ~(DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY);
> >> >> -
> >> >>  		fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
> >> >>  		show_range_diff(rev->rdiff1, rev->rdiff2,
> >> >> -				rev->creation_factor, 1, &opts);
> >> >> +				rev->creation_factor, 1, &rd_rev->diffopt);
> >> >>  	}
> >> >>  }
> >> >>
> >> >> @@ -1485,6 +1482,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
> >> >>  	struct commit *commit;
> >> >>  	struct commit **list = NULL;
> >> >>  	struct rev_info rev;
> >> >> +	struct rev_info rd_rev;
> >> >>  	struct setup_revision_opt s_r_opt;
> >> >>  	int nr = 0, total, i;
> >> >>  	int use_stdout = 0;
> >> >> @@ -1603,6 +1601,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
> >> >>  	init_log_defaults();
> >> >>  	git_config(git_format_config, NULL);
> >> >>  	repo_init_revisions(the_repository, &rev, prefix);
> >> >> +	repo_init_revisions(the_repository, &rd_rev, prefix);
> >> >>  	rev.commit_format = CMIT_FMT_EMAIL;
> >> >>  	rev.expand_tabs_in_log_default = 0;
> >> >>  	rev.verbose_header = 1;
> >> >> @@ -1689,8 +1688,32 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
> >> >>  	rev.preserve_subject = keep_subject;
> >> >>
> >> >>  	argc = setup_revisions(argc, argv, &rev, &s_r_opt);
> >> >> -	if (argc > 1)
> >> >> -		die(_("unrecognized argument: %s"), argv[1]);
> >> >> +	if (argc > 1) {
> >> >> +		struct argv_array args = ARGV_ARRAY_INIT;
> >> >> +		const char *prefix = "--range-diff";
> >> >> +		int have_prefix = 0;
> >> >> +
> >> >> +		for (i = 0; i < argc; i++) {
> >> >> +			struct strbuf sb = STRBUF_INIT;
> >> >> +			char *str;
> >> >> +
> >> >> +			strbuf_addstr(&sb, argv[i]);
> >> >> +			if (starts_with(argv[i], prefix)) {
> >> >> +				have_prefix = 1;
> >> >> +				strbuf_remove(&sb, 0, strlen(prefix));
> >> >> +			}
> >> >> +			str = strbuf_detach(&sb, NULL);
> >> >> +			strbuf_release(&sb);
> >> >> +
> >> >> +			argv_array_push(&args, str);
> >> >> +		}
> >> >> +
> >> >> +		if (!have_prefix)
> >> >> +			die(_("unrecognized argument: %s"), argv[1]);
> >> >> +		argc = setup_revisions(args.argc, args.argv, &rd_rev, NULL);
> >> >> +		if (argc > 1)
> >> >> +			die(_("unrecognized argument: %s"), argv[1]);
> >> >> +	}
> >> >>
> >> >>  	if (rev.diffopt.output_format & DIFF_FORMAT_NAME)
> >> >>  		die(_("--name-only does not make sense"));
> >> >> @@ -1702,7 +1725,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
> >> >>  	if (!use_patch_format &&
> >> >>  		(!rev.diffopt.output_format ||
> >> >>  		 rev.diffopt.output_format == DIFF_FORMAT_PATCH))
> >> >> -		/* Needs to be mirrored in show_range_diff() invocation */
> >> >>  		rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY;
> >> >>  	if (!rev.diffopt.stat_width)
> >> >>  		rev.diffopt.stat_width = MAIL_DEFAULT_WRAP;
> >> >> @@ -1877,7 +1899,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
> >> >>  	if (cover_letter) {
> >> >>  		if (thread)
> >> >>  			gen_message_id(&rev, "cover");
> >> >> -		make_cover_letter(&rev, use_stdout,
> >> >> +		make_cover_letter(&rev, &rd_rev, use_stdout,
> >> >>  				  origin, nr, list, branch_name, quiet);
> >> >>  		print_bases(&bases, rev.diffopt.file);
> >> >>  		print_signature(rev.diffopt.file);
> >> >> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
> >> >> index bc5facc1cd..6916103888 100755
> >> >> --- a/t/t3206-range-diff.sh
> >> >> +++ b/t/t3206-range-diff.sh
> >> >> @@ -308,6 +308,35 @@ test_expect_success 'format-patch with <common diff option>' '
> >> >>  		--range-diff=topic~..topic changed~..changed >actual.raw &&
> >> >>  	sed -ne "/^1:/,/^--/p" <actual.raw >actual.range-diff &&
> >> >>  	sed -e "s|:$||" >expect <<-\EOF &&
> >> >> +	1:  a63e992 ! 1:  d966c5c s/12/B/
> >> >> +	    @@ -8,7 +8,7 @@
> >> >> +	     @@
> >> >> +	      9
> >> >> +	      10
> >> >> +	    - B
> >> >> +	    + BB
> >> >> +	     -12
> >> >> +	     +B
> >> >> +	      13
> >> >> +	-- :
> >> >> +	EOF
> >> >> +	test_cmp expect actual.range-diff &&
> >> >> +	sed -ne "/^--- /,/^--/p" <actual.raw >actual.diff &&
> >> >> +	sed -e "s|:$||" >expect <<-\EOF &&
> >> >> +	--- a/file
> >> >> +	+++ b/file
> >> >> +	@@ -12 +12 @@ BB
> >> >> +	-12
> >> >> +	+B
> >> >> +	-- :
> >> >> +	EOF
> >> >> +	test_cmp expect actual.diff &&
> >> >> +
> >> >> +	# -U0 & --range-diff-U0
> >> >> +	git format-patch --cover-letter --stdout -U0 --range-diff-U0 \
> >> >> +		--range-diff=topic~..topic changed~..changed >actual.raw &&
> >> >> +	sed -ne "/^1:/,/^--/p" <actual.raw >actual.range-diff &&
> >> >> +	sed -e "s|:$||" >expect <<-\EOF &&
> >> >>  	1:  a63e992 ! 1:  d966c5c s/12/B/
> >> >>  	    @@ -11 +11 @@
> >> >>  	    - B
> >> >> @@ -327,4 +356,16 @@ test_expect_success 'format-patch with <common diff option>' '
> >> >>  	test_cmp expect actual.diff
> >> >>  '
> >> >>
> >> >> +test_expect_success 'format-patch option parsing with --range-diff-*' '
> >> >> +	test_must_fail git format-patch --stdout --unknown \
> >> >> +		master..unmodified 2>stderr &&
> >> >> +	test_i18ngrep "unrecognized argument: --unknown" stderr &&
> >> >> +	test_must_fail git format-patch --stdout --range-diff-unknown \
> >> >> +		master..unmodified 2>stderr &&
> >> >> +	test_i18ngrep "unrecognized argument: --range-diff-unknown" stderr &&
> >> >> +	test_must_fail git format-patch --stdout --unknown --range-diff-unknown \
> >> >> +		master..unmodified 2>stderr &&
> >> >> +	test_i18ngrep "unrecognized argument: --unknown" stderr
> >> >> +'
> >> >> +
> >> >>  test_done
> >> >> --
> >> >> 2.20.0.rc1.387.gf8505762e3
> >> >>
> >> >>
> >>
> 

^ permalink raw reply

* Re: [PATCH v2 6/7] checkout: split into switch-branch and checkout-files
From: Duy Nguyen @ 2018-11-29 15:46 UTC (permalink / raw)
  To: Stefan Xenos
  Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
	Git Mailing List, Stefan Beller, Thomas Gummerer
In-Reply-To: <CAPL8ZiuaEW5tp8ZMOZtZcb5oi3L-pDF6ajcA7b5wnH3=7Ls7Tg@mail.gmail.com>

On Thu, Nov 29, 2018 at 12:22 AM Stefan Xenos <sxenos@google.com> wrote:
> Some behaviors I'd expect to see from these commands (I haven't yet
> checked to see if you've already done this):
>
> git checkout-files <tree-ish>
> should reset all the files in the repository regardless of the current
> directory - it should produce the same effect as "git reset --hard
> <tree-ish> && git reset HEAD@{1}". It should also delete
> locally-created files that aren't present in <tree-ish>, such that the
> final working tree is exactly identical to what was committed in that
> tree-ish.
>
> git checkout-files foo -- myfile.txt
> should delete myfile.txt if it is present locally but not present in foo.
>
> git checkout-files foo -- .
> should recursively checkout all files in the current folder and all
> subfolders, and delete any locally-created files if they're not
> present in foo.

I think all these are the same as the non-overlay mode Thomas
mentioned [1]. Once he implements that in git-checkout, we can make it
default in checkout-files.

Two things though. I plan to get rid of "--" in checkout-files. The
main use case (I think) is reset from index, so you can just write

 git checkout-files somefiles

and if you want to get it from the tree(-ish) "foo", you do

 git checkout-files --from=foo somefiles

This form is easier to read (and even guess before you read man pages)
and leaves no room for ambiguation.

The second thing is, I plan to forbid "git checkout-files" without
arguments. If you want to reset the entire worktree, do

 git checkout-files :/

or just current dir

 git checkout-files .

Which brings us back to your "git checkout-files <tree-ish>" use case
above. It should be treat the same way in my opinion, so we either do

 git checkout-files --from=tree-ish :/

or

 git checkout-files --from=tree-ish .

But "git checkout-files --from=tree-ish" alone is rejected.


[1] https://public-inbox.org/git/xmqqwoowo1fk.fsf@gitster-ct.c.googlers.com/T/#mdb076d178ccf0ae3dba0fd63143f99278047da93

> Suggestion:
> If git checkout-files overwrites or deletes any locally-modified files
> from the workspace or index, those files could be auto-stashed. That
> would make it easy to restore them in the event of a mistyped command.
> Auto-stashing could be suppressed with a command-line argument (with
> alternate behaviors being fail-if-modified or always-overwrite).

Stashing I think is not the right tool for this. When you stash, you
plan to retrieve it back later but here you should rarely ever need to
unstash until the accident. For recovery from accidents like this, I
have another thing in queue to achieve the same (I'm calling it
"backup log" now). So we will have the same functionality, just with
different means.

> Idea:
> If git checkout-files modifies the submodules file, it could also
> auto-update the submodules. (For example, with something like "git
> submodule update --init --recursive --progress").

This one is tricky because we should deal with submodule autoupdate
consistently across all porcelain commands (or at least common ones),
not just checkout-files. I'd prefer to delay this until later. Once we
figure out what to do with other commands, then we can still change
defaults for checkout-files.
-- 
Duy

^ permalink raw reply

* Re: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)
From: Ian Jackson @ 2018-11-29 15:50 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Jonathan Nieder, Junio C Hamano,
	Ævar Arnfjörð Bjarmason, git
In-Reply-To: <nycvar.QRO.7.76.6.1811291638400.41@tvgsbejvaqbjf.bet>

Johannes Schindelin writes ("Re: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)"):
> >  In a successful run with older git I get a reflog like this:
> > 
> >    4833d74 HEAD@{0}: rebase finished: returning to refs/heads/with-preexisting
> >    4833d74 HEAD@{1}: debrebase new-upstream 2.1-1: rebase: Add another new upstream file
> >    cabd5ec HEAD@{2}: debrebase new-upstream 2.1-1: rebase: Edit the .c file
> >    0b362ce HEAD@{3}: debrebase new-upstream 2.1-1: rebase: Add a new upstream file
> >    29653e5 HEAD@{4}: debrebase new-upstream 2.1-1: rebase: checkout 29653e5a17bee4ac23a68bba3e12bc1f52858ac3
> >    85e0c46 HEAD@{5}: debrebase: launder for new upstream
...
> >  This breaks the test because my test suite is checking that I set
> >  GIT_REFLOG_ACTION appropriately.
> > 
> >  If you want I can provide a minimal test case but this should suffice
> >  to see the bug I hope...
> 
> This should be plenty for me to get going. Thank you!

Happy hunting.

While you're looking at this, I observe that the fact that the `rebase
finished' message also does not honour GIT_REFLOG_ACTION appears to be
a pre-existing bug.

(In general one often can't rely on GIT_REFLOG_ACTION still being set
because the rebase might have been interrupted and restarted, which I
think is why my test case looks for it in the initial `checkout'
message.)

Regards,
Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

^ permalink raw reply

* Re: [PATCH 2/2] format-patch: allow for independent diff & range-diff options
From: Ævar Arnfjörð Bjarmason @ 2018-11-29 16:03 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Junio C Hamano, Eric Sunshine
In-Reply-To: <nycvar.QRO.7.76.6.1811291641090.41@tvgsbejvaqbjf.bet>


On Thu, Nov 29 2018, Johannes Schindelin wrote:

> Hi Ævar,
>
> On Thu, 29 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
>
>> On Thu, Nov 29 2018, Johannes Schindelin wrote:
>>
>> > On Thu, 29 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
>> >
>> >> On Thu, Nov 29 2018, Johannes Schindelin wrote:
>> >>
>> >> > On Wed, 28 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
>> >> >
>> >> >> Change the semantics of the "--range-diff" option so that the regular
>> >> >> diff options can be provided separately for the range-diff and the
>> >> >> patch. This allows for supplying e.g. --range-diff-U0 and -U1 to
>> >> >> "format-patch" to provide different context for the range-diff and the
>> >> >> patch. This wasn't possible before.
>> >> >
>> >> > I really, really dislike the `--range-diff-<random-thing>`. We have
>> >> > precedent for passing optional arguments that are passed to some other
>> >> > command, so a much more logical and consistent convention would be to use
>> >> > `--range-diff[=<diff-option>..]`, allowing all of the diff options that
>> >> > you might want to pass to the outer diff in one go rather than having a
>> >> > lengthy string of `--range-diff-this` and `--range-diff-that` options.
>> >>
>> >> Where do we pass those sorts of arguments?
>> >>
>> >> Reasons I did it this way:
>> >>
>> >>  a) Passing it as one option will require the user to double-quote those
>> >>     options that take quoted arguments (e.g. --word-diff-regex), which I
>> >>     thought sucked more than the prefix. On the implementation side we
>> >>     couldn't leave the parsing of the command-line to the shell anymore.
>> >>
>> >>  b) I think people will want to tweak this very rarely, much more rarely
>> >>     than e.g. -U10 in format-patch itself, so having something long-ish
>> >>     doesn't sound bad.
>> >
>> > Hmm. I still don't like it. It sets a precedent, and we simply do not do
>> > it that way in other circumstances (most obvious would be the -X merge
>> > options). The more divergent user interfaces for the same sort of thing
>> > are, the more brain cycles you force users to spend on navigating said
>> > interfaces.
>>
>> Yeah it sucks, I just think it sucks less than the alternative :)
>> I.e. I'm not picky about --range-diff-* prefix the name, but I think
>> doing our own shell parsing would be nasty.
>
> What prevents you from using `sq_dequote_to_argv()`?

I mean not just nasty in terms of implementation, yeah we could do it,
but also a nasty UX for things like --word-diff-regex. I.e. instead of:

    --range-diff-word-diff-regex='[0-9"]'

You need:

    --range-diff-opts="--word-diff-regex='[0-9\"]'"

Now admittedly that in itself isn't very painful *in this case*, but in
terms of precedent I really dislike that option, i.e. git having some
mode where I need to work to escape input to pass to another command.

Not saying that this --range-diff-* thing is what we should go for, but
surely we can find some way to do deal with this that doesn't involve
the user needing to escape stuff like this.

It also has other downstream effects in the UI, e.g. it's presumably
easy to teach the bash completion that a --foo=XYZ option is also called
--some-prefix--foo=XYZ and to enable completion for that, less so for
making it smart enough to complete "--some-prefix-opts="--foo=<TAB>".

>> >> > I only had time to skim the patch, and I have to wonder why you pass
>> >> > around full-blown `rev_info` structs for range diff (and with that really
>> >> > awful name `rd_rev`) rather than just the `diff_options` that you
>> >> > *actually* care about?
>> >>
>> >> Because setup_revisions() which does all the command-line parsing needs
>> >> a rev_info, so even if we only need the diffopt in the end we need to
>> >> initiate the whole thing.
>> >>
>> >> Suggestions for a better varibale name most welcome.
>> >
>> > `range_diff_revs`
>> >
>> > And you do not need to pass around the whole thing. You can easily pass
>> > `&range_diff_revs.diffopt`.
>> >
>> > Don't pass around what you do not need to pass around.
>>
>> Ah, you mean internally in log.c, yes that makes sense. I thought you
>> meant just pass diffopt to setup_revisions() (which needs the containing
>> struct). Willdo.
>
> Thanks,
> Dscho
>
>>
>> > Ciao,
>> > Dscho
>> >
>> >>
>> >> > Ciao,
>> >> > Dscho
>> >> >
>> >> >>
>> >> >> Ever since the "--range-diff" option was added in
>> >> >> 31e2617a5f ("format-patch: add --range-diff option to embed diff in
>> >> >> cover letter", 2018-07-22) the "rev->diffopt" we pass down to the diff
>> >> >> machinery has been the one we get from "format-patch"'s own
>> >> >> setup_revisions().
>> >> >>
>> >> >> This sort of thing is unique among the log-like commands in
>> >> >> builtin/log.c, no command than format-patch will embed the output of
>> >> >> another log-like command. Since the "rev->diffopt" is reused we need
>> >> >> to munge it before we pass it to show_range_diff(). See
>> >> >> 43dafc4172 ("format-patch: don't include --stat with --range-diff
>> >> >> output", 2018-11-22) for a related regression fix which is being
>> >> >> mostly reverted here.
>> >> >>
>> >> >> Implementation notes: 1) We're not bothering with the full teardown
>> >> >> around die() and will leak memory, but it's too much boilerplate to do
>> >> >> all the frees with/without the die() and not worth it. 2) We call
>> >> >> repo_init_revisions() for "rd_rev" even though we could get away with
>> >> >> a shallow copy like the code we're replacing (and which
>> >> >> show_range_diff() itself does). This is to make this code more easily
>> >> >> understood.
>> >> >>
>> >> >> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> >> >> ---
>> >> >>  Documentation/git-format-patch.txt | 10 ++++++-
>> >> >>  builtin/log.c                      | 42 +++++++++++++++++++++++-------
>> >> >>  t/t3206-range-diff.sh              | 41 +++++++++++++++++++++++++++++
>> >> >>  3 files changed, 82 insertions(+), 11 deletions(-)
>> >> >>
>> >> >> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
>> >> >> index aba4c5febe..6c048f415f 100644
>> >> >> --- a/Documentation/git-format-patch.txt
>> >> >> +++ b/Documentation/git-format-patch.txt
>> >> >> @@ -24,7 +24,8 @@ SYNOPSIS
>> >> >>  		   [--to=<email>] [--cc=<email>]
>> >> >>  		   [--[no-]cover-letter] [--quiet] [--notes[=<ref>]]
>> >> >>  		   [--interdiff=<previous>]
>> >> >> -		   [--range-diff=<previous> [--creation-factor=<percent>]]
>> >> >> +		   [--range-diff=<previous> [--creation-factor=<percent>]
>> >> >> +		      [--range-diff<common diff option>]]
>> >> >>  		   [--progress]
>> >> >>  		   [<common diff options>]
>> >> >>  		   [ <since> | <revision range> ]
>> >> >> @@ -257,6 +258,13 @@ feeding the result to `git send-email`.
>> >> >>  	creation/deletion cost fudge factor. See linkgit:git-range-diff[1])
>> >> >>  	for details.
>> >> >>
>> >> >> +--range-diff<common diff option>::
>> >> >> +	Other options prefixed with `--range-diff` are stripped of
>> >> >> +	that prefix and passed as-is to the diff machinery used to
>> >> >> +	generate the range-diff, e.g. `--range-diff-U0` and
>> >> >> +	`--range-diff--no-color`. This allows for adjusting the format
>> >> >> +	of the range-diff independently from the patch itself.
>> >> >> +
>> >> >>  --notes[=<ref>]::
>> >> >>  	Append the notes (see linkgit:git-notes[1]) for the commit
>> >> >>  	after the three-dash line.
>> >> >> diff --git a/builtin/log.c b/builtin/log.c
>> >> >> index 02d88fa233..7658e56ecc 100644
>> >> >> --- a/builtin/log.c
>> >> >> +++ b/builtin/log.c
>> >> >> @@ -1023,7 +1023,8 @@ static void show_diffstat(struct rev_info *rev,
>> >> >>  	fprintf(rev->diffopt.file, "\n");
>> >> >>  }
>> >> >>
>> >> >> -static void make_cover_letter(struct rev_info *rev, int use_stdout,
>> >> >> +static void make_cover_letter(struct rev_info *rev, struct rev_info *rd_rev,
>> >> >> +			      int use_stdout,
>> >> >>  			      struct commit *origin,
>> >> >>  			      int nr, struct commit **list,
>> >> >>  			      const char *branch_name,
>> >> >> @@ -1095,13 +1096,9 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
>> >> >>  	}
>> >> >>
>> >> >>  	if (rev->rdiff1) {
>> >> >> -		struct diff_options opts;
>> >> >> -		memcpy(&opts, &rev->diffopt, sizeof(opts));
>> >> >> -		opts.output_format &= ~(DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY);
>> >> >> -
>> >> >>  		fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
>> >> >>  		show_range_diff(rev->rdiff1, rev->rdiff2,
>> >> >> -				rev->creation_factor, 1, &opts);
>> >> >> +				rev->creation_factor, 1, &rd_rev->diffopt);
>> >> >>  	}
>> >> >>  }
>> >> >>
>> >> >> @@ -1485,6 +1482,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>> >> >>  	struct commit *commit;
>> >> >>  	struct commit **list = NULL;
>> >> >>  	struct rev_info rev;
>> >> >> +	struct rev_info rd_rev;
>> >> >>  	struct setup_revision_opt s_r_opt;
>> >> >>  	int nr = 0, total, i;
>> >> >>  	int use_stdout = 0;
>> >> >> @@ -1603,6 +1601,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>> >> >>  	init_log_defaults();
>> >> >>  	git_config(git_format_config, NULL);
>> >> >>  	repo_init_revisions(the_repository, &rev, prefix);
>> >> >> +	repo_init_revisions(the_repository, &rd_rev, prefix);
>> >> >>  	rev.commit_format = CMIT_FMT_EMAIL;
>> >> >>  	rev.expand_tabs_in_log_default = 0;
>> >> >>  	rev.verbose_header = 1;
>> >> >> @@ -1689,8 +1688,32 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>> >> >>  	rev.preserve_subject = keep_subject;
>> >> >>
>> >> >>  	argc = setup_revisions(argc, argv, &rev, &s_r_opt);
>> >> >> -	if (argc > 1)
>> >> >> -		die(_("unrecognized argument: %s"), argv[1]);
>> >> >> +	if (argc > 1) {
>> >> >> +		struct argv_array args = ARGV_ARRAY_INIT;
>> >> >> +		const char *prefix = "--range-diff";
>> >> >> +		int have_prefix = 0;
>> >> >> +
>> >> >> +		for (i = 0; i < argc; i++) {
>> >> >> +			struct strbuf sb = STRBUF_INIT;
>> >> >> +			char *str;
>> >> >> +
>> >> >> +			strbuf_addstr(&sb, argv[i]);
>> >> >> +			if (starts_with(argv[i], prefix)) {
>> >> >> +				have_prefix = 1;
>> >> >> +				strbuf_remove(&sb, 0, strlen(prefix));
>> >> >> +			}
>> >> >> +			str = strbuf_detach(&sb, NULL);
>> >> >> +			strbuf_release(&sb);
>> >> >> +
>> >> >> +			argv_array_push(&args, str);
>> >> >> +		}
>> >> >> +
>> >> >> +		if (!have_prefix)
>> >> >> +			die(_("unrecognized argument: %s"), argv[1]);
>> >> >> +		argc = setup_revisions(args.argc, args.argv, &rd_rev, NULL);
>> >> >> +		if (argc > 1)
>> >> >> +			die(_("unrecognized argument: %s"), argv[1]);
>> >> >> +	}
>> >> >>
>> >> >>  	if (rev.diffopt.output_format & DIFF_FORMAT_NAME)
>> >> >>  		die(_("--name-only does not make sense"));
>> >> >> @@ -1702,7 +1725,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>> >> >>  	if (!use_patch_format &&
>> >> >>  		(!rev.diffopt.output_format ||
>> >> >>  		 rev.diffopt.output_format == DIFF_FORMAT_PATCH))
>> >> >> -		/* Needs to be mirrored in show_range_diff() invocation */
>> >> >>  		rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY;
>> >> >>  	if (!rev.diffopt.stat_width)
>> >> >>  		rev.diffopt.stat_width = MAIL_DEFAULT_WRAP;
>> >> >> @@ -1877,7 +1899,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>> >> >>  	if (cover_letter) {
>> >> >>  		if (thread)
>> >> >>  			gen_message_id(&rev, "cover");
>> >> >> -		make_cover_letter(&rev, use_stdout,
>> >> >> +		make_cover_letter(&rev, &rd_rev, use_stdout,
>> >> >>  				  origin, nr, list, branch_name, quiet);
>> >> >>  		print_bases(&bases, rev.diffopt.file);
>> >> >>  		print_signature(rev.diffopt.file);
>> >> >> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
>> >> >> index bc5facc1cd..6916103888 100755
>> >> >> --- a/t/t3206-range-diff.sh
>> >> >> +++ b/t/t3206-range-diff.sh
>> >> >> @@ -308,6 +308,35 @@ test_expect_success 'format-patch with <common diff option>' '
>> >> >>  		--range-diff=topic~..topic changed~..changed >actual.raw &&
>> >> >>  	sed -ne "/^1:/,/^--/p" <actual.raw >actual.range-diff &&
>> >> >>  	sed -e "s|:$||" >expect <<-\EOF &&
>> >> >> +	1:  a63e992 ! 1:  d966c5c s/12/B/
>> >> >> +	    @@ -8,7 +8,7 @@
>> >> >> +	     @@
>> >> >> +	      9
>> >> >> +	      10
>> >> >> +	    - B
>> >> >> +	    + BB
>> >> >> +	     -12
>> >> >> +	     +B
>> >> >> +	      13
>> >> >> +	-- :
>> >> >> +	EOF
>> >> >> +	test_cmp expect actual.range-diff &&
>> >> >> +	sed -ne "/^--- /,/^--/p" <actual.raw >actual.diff &&
>> >> >> +	sed -e "s|:$||" >expect <<-\EOF &&
>> >> >> +	--- a/file
>> >> >> +	+++ b/file
>> >> >> +	@@ -12 +12 @@ BB
>> >> >> +	-12
>> >> >> +	+B
>> >> >> +	-- :
>> >> >> +	EOF
>> >> >> +	test_cmp expect actual.diff &&
>> >> >> +
>> >> >> +	# -U0 & --range-diff-U0
>> >> >> +	git format-patch --cover-letter --stdout -U0 --range-diff-U0 \
>> >> >> +		--range-diff=topic~..topic changed~..changed >actual.raw &&
>> >> >> +	sed -ne "/^1:/,/^--/p" <actual.raw >actual.range-diff &&
>> >> >> +	sed -e "s|:$||" >expect <<-\EOF &&
>> >> >>  	1:  a63e992 ! 1:  d966c5c s/12/B/
>> >> >>  	    @@ -11 +11 @@
>> >> >>  	    - B
>> >> >> @@ -327,4 +356,16 @@ test_expect_success 'format-patch with <common diff option>' '
>> >> >>  	test_cmp expect actual.diff
>> >> >>  '
>> >> >>
>> >> >> +test_expect_success 'format-patch option parsing with --range-diff-*' '
>> >> >> +	test_must_fail git format-patch --stdout --unknown \
>> >> >> +		master..unmodified 2>stderr &&
>> >> >> +	test_i18ngrep "unrecognized argument: --unknown" stderr &&
>> >> >> +	test_must_fail git format-patch --stdout --range-diff-unknown \
>> >> >> +		master..unmodified 2>stderr &&
>> >> >> +	test_i18ngrep "unrecognized argument: --range-diff-unknown" stderr &&
>> >> >> +	test_must_fail git format-patch --stdout --unknown --range-diff-unknown \
>> >> >> +		master..unmodified 2>stderr &&
>> >> >> +	test_i18ngrep "unrecognized argument: --unknown" stderr
>> >> >> +'
>> >> >> +
>> >> >>  test_done
>> >> >> --
>> >> >> 2.20.0.rc1.387.gf8505762e3
>> >> >>
>> >> >>
>> >>
>>

^ permalink raw reply

* Re: How de-duplicate similar repositories with alternates
From: Ævar Arnfjörð Bjarmason @ 2018-11-29 16:09 UTC (permalink / raw)
  To: git, Git for human beings; +Cc: Christian Couder, Duy Nguyen
In-Reply-To: <87zhtsx73l.fsf@evledraar.gmail.com>


On Thu, Nov 29 2018, Ævar Arnfjörð Bjarmason wrote:

> A co-worker asked me today how space could be saved when you have
> multiple checkouts of the same repository (at different revs) on the
> same machine. I said since these won't block-level de-duplicate well[1]
> one way to do this is with alternates.
>
> However, once you have an existing clone I didn't know how to get the
> gains without a full re-clone, but I hadn't looked deeply into it. As it
> turns out I'm wrong about that, which I found when writing the following
> test-case which shows that it works:
>
>     (
>         cd /tmp &&
>         rm -rf /tmp/git-{master,pu,pu-alt}.git &&
>
>         # Normal clones
>         git clone --bare --no-tags --single-branch --branch master https://github.com/git/git.git /tmp/git-master.git &&
>         git clone --bare --no-tags --single-branch --branch pu https://github.com/git/git.git /tmp/git-pu.git &&
>
>         # An 'alternate' clone using 'master' objects from another repo
>         git --bare init /tmp/git-pu-alt.git &&
>         for git in git-pu.git git-pu-alt.git
>         do
>             echo /tmp/git-master.git/objects >/tmp/$git/objects/info/alternates
>         done &&
>         git -C git-pu-alt.git fetch --no-tags https://github.com/git/git.git pu:pu
>
>         # Respective sizes, 'alternate' clone much smaller
>         du -shc /tmp/git-*.git &&
>
>         # GC them all. Compacts the git-pu.git to git-pu-alt.git's size
>         for repo in git-*.git
>         do
>             git -C $repo gc
>         done &&
>         du -shc /tmp/git-*.git
>
>         # Add another big history (GFW) to git-{pu,master}.git (in that order!)
>         for repo in $(ls -d /tmp/git-*.git | sort -r)
>         do
>             git -C $repo fetch --no-tags https://github.com/git-for-windows/git master:master-gfw
>         done &&
>         du -shc /tmp/git-*.git &&
>
>         # Another GC. The objects now in git-master.git will be de-duped by all
>         for repo in git-*.git
>         do
>             git -C $repo gc
>         done &&
>         du -shc /tmp/git-*.git
>     )
>
> This shows a scenario where we clone git.git at "master" and "pu" in
> different places. After clone the relevant sizes are:
>
>     108M    /tmp/git-master.git
>     3.2M    /tmp/git-pu-alt.git
>     109M    /tmp/git-pu.git
>     219M    total
>
> I.e. git-pu-alt.git is much smaller since it points via alternates to
> git-master.git, and the history of "pu" shares most of the objects with
> "master". But then how do you get those gains for git-pu.git? Turns out
> you just "git gc"
>
>     111M    /tmp/git-master.git
>     2.1M    /tmp/git-pu-alt.git
>     2.1M    /tmp/git-pu.git
>     115M    total
>
> This is the thing I was wrong about, in retrospect probably because I'd
> been putting PATH_TO_REPO in objects/info/alternates, but we actually
> need PATH_TO_REPO/objects, and "git gc" won't warn about this (or "git
> fsck"). Probably a good idea to patch that at some point, i.e. whine
> about paths in alternates that don't have objects, or at the very least
> those that don't exist. #leftoverbits

Actually looking at this again the thing that may have stumped me last
time is that this has a bad interaction with gc.bigPackThreshold. If you
have an alternate that would otherwise house most of your objects *and*
you have a pack that's larger than the gc.bigPackThreshold your mostly
redundant pack won't be removed.

That's understandable in terms of implementation, but unfortunate. It
would be nice if we learned some way to detect this, i.e. "I have this
10GB pack, but with this alternate I can extract this 100MB out of it
and throw it away". Now we just keep the 10GB pack even if it's mostly
redundant to what's in the alternate.

> Then when we fetch git-for-windows:master to all the repos they all grow
> by the amount git-for-windows has diverged:
>
>     144M    /tmp/git-master.git
>     36M     /tmp/git-pu-alt.git
>     36M     /tmp/git-pu.git
>     214M    total
>
> Note that the "sort -r" is critical here. If we fetched git-master.git
> first (at this point the alternate for git-pu*.git) we wouldn't get the
> duplication in the first place, but instead:
>
>     144M    /tmp/git-master.git
>     2.1M    /tmp/git-pu-alt.git
>     2.1M    /tmp/git-pu.git
>     148M    total
>
> This shows the importance of keeping such an 'alternate' repo
> up-to-date, i.e. we don't get the duplication in the first place, but
> regardless (this from a run with sort -r) a "git gc" will coalesce them:
>
>     131M    /tmp/git-master.git
>     2.1M    /tmp/git-pu-alt.git
>     2.2M    /tmp/git-pu.git
>     135M    total
>
> If you find this interesting make sure to read my
> https://public-inbox.org/git/87k1s3bomt.fsf@evledraar.gmail.com/ and
> https://public-inbox.org/git/87in7nbi5b.fsf@evledraar.gmail.com/ for the
> caveats, i.e. if this is something intended for users then no ref in the
> alternate can ever be rewound, that'll potentially result in repository
> corruption.
>
> 1. https://public-inbox.org/git/87bmhiykvw.fsf@evledraar.gmail.com/

^ permalink raw reply

* Re: [PATCH] pack-protocol.txt: accept error packets in any context
From: Masaya Suzuki @ 2018-11-29 16:10 UTC (permalink / raw)
  To: gitster; +Cc: git
In-Reply-To: <xmqqwoowmfzt.fsf@gitster-ct.c.googlers.com>

On Thu, Nov 29, 2018 at 3:42 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Masaya Suzuki <masayasuzuki@google.com> writes:
>
> > In the Git pack protocol definition, an error packet may appear only in
> > a certain context. However, servers can face a runtime error (e.g. I/O
> > error) at an arbitrary timing. This patch changes the protocol to allow
> > an error packet to be sent instead of any packet.
> >
> > Following this protocol spec change, the error packet handling code is
> > moved to pkt-line.c.
> >
> > Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
> > ---
>
> Have you run tests with this applied?  I noticed 5703.9 now has
> stale expectations, but there may be others.

Yes, I did. And it also didn't end up in a build error. Do I have a
different build option...?

^ permalink raw reply

* Re: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)
From: Johannes Schindelin @ 2018-11-29 16:14 UTC (permalink / raw)
  To: Ian Jackson
  Cc: Jonathan Nieder, Junio C Hamano,
	Ævar Arnfjörð Bjarmason, git
In-Reply-To: <23552.2773.538601.372248@chiark.greenend.org.uk>

Hi Ian,

On Thu, 29 Nov 2018, Ian Jackson wrote:

> Johannes Schindelin writes ("Re: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)"):
> > >  In a successful run with older git I get a reflog like this:
> > > 
> > >    4833d74 HEAD@{0}: rebase finished: returning to refs/heads/with-preexisting
> > >    4833d74 HEAD@{1}: debrebase new-upstream 2.1-1: rebase: Add another new upstream file
> > >    cabd5ec HEAD@{2}: debrebase new-upstream 2.1-1: rebase: Edit the .c file
> > >    0b362ce HEAD@{3}: debrebase new-upstream 2.1-1: rebase: Add a new upstream file
> > >    29653e5 HEAD@{4}: debrebase new-upstream 2.1-1: rebase: checkout 29653e5a17bee4ac23a68bba3e12bc1f52858ac3
> > >    85e0c46 HEAD@{5}: debrebase: launder for new upstream
> ...
> > >  This breaks the test because my test suite is checking that I set
> > >  GIT_REFLOG_ACTION appropriately.
> > > 
> > >  If you want I can provide a minimal test case but this should suffice
> > >  to see the bug I hope...
> > 
> > This should be plenty for me to get going. Thank you!
> 
> Happy hunting.

I'll have to take a (lengthy) dinner break now, but this is what I have so
far: a regression test that verifies the breakage (see the
`fix-reflog-action` branch at https://github.com/dscho/git). I'll continue
after dinner and am confident that this bug will be fixed within the next
four hours.

> While you're looking at this, I observe that the fact that the `rebase
> finished' message also does not honour GIT_REFLOG_ACTION appears to be
> a pre-existing bug.

I noticed that, too, but at this point I am only fixing regressions. We
can try to fix this long-standing bug in the v2.20 cycle.

Ciao,
Johannes

> (In general one often can't rely on GIT_REFLOG_ACTION still being set
> because the rebase might have been interrupted and restarted, which I
> think is why my test case looks for it in the initial `checkout'
> message.)
> 
> Regards,
> Ian.
> 
> -- 
> Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.
> 
> If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
> a private address which bypasses my fierce spamfilter.
> 

^ permalink raw reply

* Re: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)
From: Ian Jackson @ 2018-11-29 16:26 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Jonathan Nieder, Junio C Hamano,
	Ævar Arnfjörð Bjarmason, git
In-Reply-To: <nycvar.QRO.7.76.6.1811291713030.41@tvgsbejvaqbjf.bet>

Johannes Schindelin writes ("Re: [PATCH] rebase: mark the C reimplementation as an experimental opt-in feature (was Re: [ANNOUNCE] Git v2.20.0-rc1)"):
> I'll have to take a (lengthy) dinner break now, but this is what I have so
> far: a regression test that verifies the breakage (see the
> `fix-reflog-action` branch at https://github.com/dscho/git). I'll continue
> after dinner and am confident that this bug will be fixed within the next
> four hours.

That seems super speedy to me!

When you have a fix I will leave it up to the Debian git maintainers
to decide whether they want to cherry pick your fix into their
package, or await an updated upstream branch with rc, or what.

> [Ian:]
> > While you're looking at this, I observe that the fact that the `rebase
> > finished' message also does not honour GIT_REFLOG_ACTION appears to be
> > a pre-existing bug.
> 
> I noticed that, too, but at this point I am only fixing regressions. We
> can try to fix this long-standing bug in the v2.20 cycle.

Right.

Thanks,
Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

^ permalink raw reply

* Re: [PATCH v2 6/7] checkout: split into switch-branch and checkout-files
From: Stefan Beller @ 2018-11-29 18:14 UTC (permalink / raw)
  To: Duy Nguyen
  Cc: Stefan Xenos, Junio C Hamano,
	Ævar Arnfjörð Bjarmason, git, Thomas Gummerer
In-Reply-To: <CACsJy8An2n5yah1UTCJZoC5ucSpCoM0vrXtEXnjg-di7jQZwLA@mail.gmail.com>

> > Idea:
> > If git checkout-files modifies the submodules file, it could also
> > auto-update the submodules. (For example, with something like "git
> > submodule update --init --recursive --progress").
>
> This one is tricky because we should deal with submodule autoupdate
> consistently across all porcelain commands (or at least common ones),
> not just checkout-files. I'd prefer to delay this until later. Once we
> figure out what to do with other commands, then we can still change
> defaults for checkout-files.

checkout/reset are respecting the submodule.recurse setting for this
already, and as your patches only change the UX frontend

    git -c submodule.recurse checkout-files <pathsspec>

would also touch submodules. Given that deep down in
the submodules it's all about files again, we could think
checkout-files is still a good name.

I think Stefan X. is asking for making submodule.recurse
to default to true, which is indeed unrelated to this.

Stefan

^ permalink raw reply

* Re: [PATCH v2 6/7] checkout: split into switch-branch and checkout-files
From: Duy Nguyen @ 2018-11-29 18:30 UTC (permalink / raw)
  To: Stefan Beller
  Cc: Stefan Xenos, Junio C Hamano,
	Ævar Arnfjörð Bjarmason, Git Mailing List,
	Thomas Gummerer
In-Reply-To: <CAGZ79kY2Fu_3b8MnO-yV_JhevZRgx7=6ndX-N_R2HdJGByciHA@mail.gmail.com>

On Thu, Nov 29, 2018 at 7:14 PM Stefan Beller <sbeller@google.com> wrote:
>
> > > Idea:
> > > If git checkout-files modifies the submodules file, it could also
> > > auto-update the submodules. (For example, with something like "git
> > > submodule update --init --recursive --progress").
> >
> > This one is tricky because we should deal with submodule autoupdate
> > consistently across all porcelain commands (or at least common ones),
> > not just checkout-files. I'd prefer to delay this until later. Once we
> > figure out what to do with other commands, then we can still change
> > defaults for checkout-files.
>
> checkout/reset are respecting the submodule.recurse setting for this
> already, and as your patches only change the UX frontend
>
>     git -c submodule.recurse checkout-files <pathsspec>
>
> would also touch submodules. Given that deep down in
> the submodules it's all about files again, we could think
> checkout-files is still a good name.
>
> I think Stefan X. is asking for making submodule.recurse
> to default to true, which is indeed unrelated to this.

Yes and I'm concerned that checkout-files now recurses into submodules
this by default but grep for example does not. That just adds more
confusion.
-- 
Duy

^ permalink raw reply

* Re: How de-duplicate similar repositories with alternates
From: Stefan Beller @ 2018-11-29 18:55 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, git-users, Christian Couder
In-Reply-To: <87zhtsx73l.fsf@evledraar.gmail.com>

On Thu, Nov 29, 2018 at 7:00 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> A co-worker asked me today how space could be saved when you have
> multiple checkouts of the same repository (at different revs) on the
> same machine. I said since these won't block-level de-duplicate well[1]
> one way to do this is with alternates.

Another way is to use git-worktree, which would solve the gc issues
mentioned below?

I view alternates as a historic artefact as the deduping
of objects client side can be done using worktrees, and on the
serverside - I think - most of the git hosters use namespaces
and put a fork network into the same repository and use pack islands.

Can you elaborate on why worktrees would not solve the problem?
(I initially was hesitant to use them as I liked going into .git and tempering
with files such as the config directly. But now I cannot `cd .git` any more;
it turns out the advantages outweigh this corner case that I was attached to)

^ permalink raw reply

page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]

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