Hi Emily,

On Wed, 19 Feb 2020, Emily Shaffer wrote:

In some cases, it could be that the user is having a problem with an
object which isn't present in their normal object directory. We can get
a hint that that might be the case by examining the list of alternates
where their object may be stored instead. Since paths to alternates may
be sensitive, we'll instead count how many alternates have been
specified and note how many of them exist or are broken.

While object-cache.h describes a function "foreach_alt_odb()", this
function does not provide information on broken alternates, which are
skipped over in "link_alt_odb_entry()". Since the goal is to identify
missing alternates, we can gather the contents of
.git/objects/info/alternates manually.

Makes sense.

I am not sure that those variables and the parameter need to repeat
"alternate", I find it rather distracting. If it were me, I would rename
the parameter to `info`, the first strbuf to `path` and the second to
`line`. This function is so specific to read the alternates file that it
is quite obvious what their roles are.

+	FILE *file;
+	size_t exists = 0, broken = 0;
+
+	if (nongit) {
+		strbuf_addstr(alternates_info,
+			"not run from a git repository - alternates unavailable\n");
+		return;
+	}
+
+	strbuf_addstr(&alternates_path, get_object_directory());
+	strbuf_complete(&alternates_path, '/');
+	strbuf_addstr(&alternates_path, "info/alternates");
+
+	file = fopen(alternates_path.buf, "r");
+	if (!file) {
+		strbuf_addstr(alternates_info, "No alternates file found.\n");
+		strbuf_release(&alternates_path);
+		return;
+	}
+
+	while (strbuf_getline(&alternate, file) != EOF) {
+		if (!access(alternate.buf, F_OK))

Should we make sure that the alternate is actually valid objects directory
here? Like, look whether it is a directory, not a file or a (possibly
dangling) symbolic link. This seems to be the only check
`alt_odb_usable()` performs, so that should probably be good enough here,
too.

+			exists++;
+		else
+			broken++;
+	}
+
+	strbuf_addf(alternates_info,
+		    "%zd alternates found (%zd working, %zd broken)\n",

Sadly, `%zd` is not portable. Therefore, `pu` (and `es/bugreport`) do not
even _build_ on Windows. I need this to make it work:

-- snip --

diff --git a/bugreport.c b/bugreport.c
index 3770aa73fae..5033668e22f 100644
--- a/bugreport.c
+++ b/bugreport.c

@@ -287,10 +287,11 @@ static void get_alternates_summary(struct strbuf *alternates_info, int nongit)
 	}

 	strbuf_addf(alternates_info,
-		    "%zd alternates found (%zd working, %zd broken)\n",
-		    exists + broken,
-		    exists,
-		    broken);
+		    "%"PRIuMAX" alternates found "
+		    "(%"PRIuMAX" working, %"PRIuMAX" broken)\n",
+		    (uintmax_t)(exists + broken),
+		    (uintmax_t)exists,
+		    (uintmax_t)broken);

 	fclose(file);
 	strbuf_release(&alternate);
-- snap --

Could you incorporate that into the next iteration, please?

Thanks,
Dscho