Web lists-archives.com

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
fine too.

> +		/*
> +		 * 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.