Two last-minute changes before 2.46.0 (#678)

These two changes would be nice to have before rebasing onto 2.46.0.

* Revert b7d6f23 due to the regression
it introduced to Git 2.45.0 and the `incremental-repack` maintenance
step. Since `git pack-objects --stdin-packs` uses a notion of "included"
and "excluded" packs, it separates the choice to repack an object from
whether or not the multi-pack-index points to the copy in the selected
pack-files. Thus, `.scalarCache`s are filling up with pack-files with
fewer than a dozen reference objects, but they still can't be expired
with `git multi-pack-index expire`. This revert fixes the problem but
will need more justification, testing, and consideration of the
server-side experience when moving upstream.

* Add an advice message around the sparse index expanding. This commit
is already in `next` in upstream, but just missed the `v2.46.0` release
window. This advice will be valuable for `microsoft/git` customers,
especially those using the Office monorepo. The commit was cherry-picked
from `next`, but requires an additional change on top to make some
`microsoft/git`-specific tests pass due to comparing `stderr`.

Documentation/config/advice.txt

@@ -117,6 +117,10 @@ advice.*::
 	skippedCherryPicks::
 		Shown when linkgit:git-rebase[1] skips a commit that has already
 		been cherry-picked onto the upstream branch.
+	sparseIndexExpanded::
+		Shown when a sparse index is expanded to a full index, which is likely
+		due to an unexpected set of files existing outside of the
+		sparse-checkout.
 	statusAheadBehind::
 		Shown when linkgit:git-status[1] computes the ahead/behind
 		counts for a local ref compared to its remote tracking ref,

advice.c

@@ -77,6 +77,7 @@ static struct {
 	[ADVICE_SEQUENCER_IN_USE]			= { "sequencerInUse" },
 	[ADVICE_SET_UPSTREAM_FAILURE]			= { "setUpstreamFailure" },
 	[ADVICE_SKIPPED_CHERRY_PICKS]			= { "skippedCherryPicks" },
+	[ADVICE_SPARSE_INDEX_EXPANDED]			= { "sparseIndexExpanded" },
 	[ADVICE_STATUS_AHEAD_BEHIND_WARNING]		= { "statusAheadBehindWarning" },
 	[ADVICE_STATUS_HINTS]				= { "statusHints" },
 	[ADVICE_STATUS_U_OPTION]			= { "statusUoption" },

advice.h

@@ -45,6 +45,7 @@ enum advice_type {
 	ADVICE_SEQUENCER_IN_USE,
 	ADVICE_SET_UPSTREAM_FAILURE,
 	ADVICE_SKIPPED_CHERRY_PICKS,
+	ADVICE_SPARSE_INDEX_EXPANDED,
 	ADVICE_STATUS_AHEAD_BEHIND_WARNING,
 	ADVICE_STATUS_HINTS,
 	ADVICE_STATUS_U_OPTION,

midx-write.c

@@ -1474,8 +1474,7 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,
 	repo_config_get_bool(r, "repack.usedeltabaseoffset", &delta_base_offset);
 	repo_config_get_bool(r, "repack.usedeltaislands", &use_delta_islands);
 
-	strvec_pushl(&cmd.args, "pack-objects", "--stdin-packs", "--non-empty",
-		     NULL);
+	strvec_push(&cmd.args, "pack-objects");
 
 	strvec_pushf(&cmd.args, "%s/pack/pack", object_dir);
 
@@ -1499,15 +1498,16 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,
 	}
 
 	cmd_in = xfdopen(cmd.in, "w");
-	for (i = 0; i < m->num_packs; i++) {
-		struct packed_git *p = m->packs[i];
-		if (!p)
+
+	for (i = 0; i < m->num_objects; i++) {
+		struct object_id oid;
+		uint32_t pack_int_id = nth_midxed_pack_int_id(m, i);
+
+		if (!include_pack[pack_int_id])
 			continue;
 
-		if (include_pack[i])
-			fprintf(cmd_in, "%s\n", pack_basename(p));
-		else
-			fprintf(cmd_in, "^%s\n", pack_basename(p));
+		nth_midxed_object_oid(&oid, m, i);
+		fprintf(cmd_in, "%s\n", oid_to_hex(&oid));
 	}
 	fclose(cmd_in);
 

sparse-index.c

@@ -12,6 +12,22 @@
 #include "config.h"
 #include "dir.h"
 #include "fsmonitor-ll.h"
+#include "advice.h"
+
+/**
+ * This global is used by expand_index() to determine if we should give the
+ * advice for advice.sparseIndexExpanded when expanding a sparse index to a full
+ * one. However, this is sometimes done on purpose, such as in the sparse-checkout
+ * builtin, even when index.sparse=false. This may be disabled in
+ * convert_to_sparse().
+ */
+static int give_advice_on_expansion = 1;
+#define ADVICE_MSG \
+	"The sparse index is expanding to a full index, a slow operation.\n"   \
+	"Your working directory likely has contents that are outside of\n"     \
+	"your sparse-checkout patterns. Use 'git sparse-checkout list' to\n"   \
+	"see your sparse-checkout definition and compare it to your working\n" \
+	"directory contents. Running 'git clean' may assist in this cleanup."
 
 struct modify_index_context {
 	struct index_state *write;
@@ -183,6 +199,12 @@ int convert_to_sparse(struct index_state *istate, int flags)
 	    !is_sparse_index_allowed(istate, flags))
 		return 0;
 
+	/*
+	 * If we are purposefully collapsing a full index, then don't give
+	 * advice when it is expanded later.
+	 */
+	give_advice_on_expansion = 0;
+
 	/*
 	 * NEEDSWORK: If we have unmerged entries, then stay full.
 	 * Unmerged entries prevent the cache-tree extension from working.
@@ -328,6 +350,12 @@ void expand_index(struct index_state *istate, struct pattern_list *pl)
 			pl = NULL;
 	}
 
+	if (!pl && give_advice_on_expansion) {
+		give_advice_on_expansion = 0;
+		advise_if_enabled(ADVICE_SPARSE_INDEX_EXPANDED,
+				  _(ADVICE_MSG));
+	}
+
 	/*
 	 * A NULL pattern set indicates we are expanding a full index, so
 	 * we use a special region name that indicates the full expansion.

t/t1091-sparse-checkout-builtin.sh

@@ -701,6 +701,7 @@ test_expect_success 'pattern-checks: contained glob characters' '
 
 test_expect_success BSLASHPSPEC 'pattern-checks: escaped characters' '
 	git clone repo escaped &&
+	git -C escaped config advice.sparseIndexExpanded false &&
 	TREEOID=$(git -C escaped rev-parse HEAD:folder1) &&
 	NEWTREE=$(git -C escaped mktree <<-EOF
 	$(git -C escaped ls-tree HEAD)

t/t1092-sparse-checkout-compatibility.sh

@@ -160,7 +160,10 @@ init_repos () {
 	git -C sparse-checkout sparse-checkout set deep &&
 	git -C sparse-index sparse-checkout init --cone --sparse-index &&
 	test_cmp_config -C sparse-index true index.sparse &&
-	git -C sparse-index sparse-checkout set deep
+	git -C sparse-index sparse-checkout set deep &&
+
+	# Disable this message to keep stderr the same.
+	git -C sparse-index config advice.sparseIndexExpanded false
 }
 
 init_repos_as_submodules () {
@@ -537,6 +540,8 @@ test_expect_success 'diff --cached' '
 test_expect_success 'diff partially-staged' '
 	init_repos &&
 
+	git -C full-checkout config advice.sparseIndexExpanded false &&
+
 	write_script edit-contents <<-\EOF &&
 	echo text >>$1
 	EOF
@@ -2432,4 +2437,15 @@ test_expect_success 'sparse-index is not expanded: check-attr' '
 	ensure_not_expanded check-attr -a --cached -- folder1/a
 '
 
+test_expect_success 'advice.sparseIndexExpanded' '
+	init_repos &&
+
+	git -C sparse-index config --unset advice.sparseIndexExpanded &&
+	git -C sparse-index sparse-checkout set deep/deeper1 &&
+	mkdir -p sparse-index/deep/deeper2/deepest &&
+	touch sparse-index/deep/deeper2/deepest/bogus &&
+	git -C sparse-index status 2>err &&
+	grep "The sparse index is expanding to a full index" err
+'
+
 test_done

t/t7002-mv-sparse-checkout.sh

@@ -155,6 +155,9 @@ test_expect_success 'mv refuses to move sparse-to-non-sparse' '
 
 test_expect_success 'recursive mv refuses to move (possible) sparse' '
 	test_when_finished rm -rf b c e sub2 &&
+
+	git config advice.sparseIndexExpanded false &&
+
 	git reset --hard &&
 	# Without cone mode, "sub" and "sub2" do not match
 	git sparse-checkout set sub/dir sub2/dir &&