Re: [PATCH v8 15/15] bugreport: summarize contents of alternates file
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Feb 20, 2020, 14:08 UTC
- Message-ID
- <nycvar.QRO.7.76.6.2002201458120.46@tvgsbejvaqbjf.bet>
- In-Reply-To
- <20200220015858.181086-16-emilyshaffer@google.com>
Hi Emily,
On Wed, 19 Feb 2020, Emily Shaffer wrote:
Show 12 quoted lines
> 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.
Show 12 quoted lines
> diff --git a/bugreport.c b/bugreport.c
> index 1d61e0f642..1640a71086 100644
> --- a/bugreport.c
> +++ b/bugreport.c
> @@ -255,6 +255,48 @@ static void get_object_info_summary(struct strbuf *obj_info, int nongit)
> strbuf_release(&dirpath);
> }
>
> +static void get_alternates_summary(struct strbuf *alternates_info, int nongit)
> +{
> + struct strbuf alternates_path = STRBUF_INIT;
> + struct strbuf alternate = STRBUF_INIT;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.
Show 22 quoted lines
> + 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.
Show 7 quoted lines
> + 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 > + exists + broken, > + exists, > + broken); > + > + fclose(file); > + strbuf_release(&alternate); > + strbuf_release(&alternates_path); > +} > + > static const char * const bugreport_usage[] = { > N_("git bugreport [-o|--output-directory <file>] [-s|--suffix <format>]"), > NULL > @@ -355,6 +397,9 @@ int cmd_main(int argc, const char **argv) > get_header(&buffer, "Object Info Summary"); > get_object_info_summary(&buffer, nongit_ok); > > + get_header(&buffer, "Alternates"); > + get_alternates_summary(&buffer, nongit_ok); > + > report = fopen_for_writing(report_path.buf); > > if (report == NULL) { > -- > 2.25.0.265.gbab2e86ba0-goog > >