Re: [PATCH 025/194] object-store: allow prepare_alt_odb to handle arbitrary repositories
- Date: Wed, 7 Feb 2018 14:06:38 -0800
- From: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
- Subject: Re: [PATCH 025/194] object-store: allow prepare_alt_odb to handle arbitrary repositories
On Mon, 5 Feb 2018 15:54:46 -0800
Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> + /*
> + * Path to the alternate object database, relative to the
> + * current working directory.
> + */
> char path[FLEX_ARRAY];
I would prefer this to be commented:
Path to the alternative object store. If this is a relative path, it
is relative to the current working directory.
to show that it is not necessarily relative, but the current version is
> + /*
> + * Paths in alt are relative to the cwd. We ignore environment
> + * settings like this for all repositories except for
> + * the_repository, so we don't have to worry about transforming
> + * the path to be relative to another repository.
> + */
> + link_alt_odb_entries(r, alt, PATH_SEP, NULL, 0);
I find the comment confusing - it makes more sense for the reason for us
not worrying about transforming the path is that the paths as stored in
struct alternate_object_database are relative to the CWD, not that we
ignore environment variables for certain repositories.
I think it's best to remove this comment, and instead add a comment to
read_info_alternates() before its call to link_alt_odb_entries(),
explaining that paths in the alternates file are relative to
"info/alternates", not to the CWD (since that is the exceptional case).
All the patches prior to this look good. Thanks especially for the
consistent naming convention of the patch titles.