On Mon, 5 Feb 2018 15:54:46 -0800 Stefan Beller <sbel...@google.com> 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.
