On Mon, Jul 14, 2014 at 4:02 PM, Ephrim Khong <dr.khong@xxxxxxxxx> wrote:
> When adding alternate object directories, we try not to add the
> directory of the current repository to avoid cycles. Unfortunately,
> that test was broken, since it compared an absolute with a relative
> path.
Not blaming anyone. I'm simply interested in code archeology. The
first introduction of this strcmp is from 1494e03 (sha1_file.c: make
sure packs in an alternate odb is named properly. - 2005-12-04). The
intention is good as described in the commit message. But I think it's
broken even back then because "objdir" (or get_object_directory())
will always be relative (unless someone sets it explicitly) and
ent->base back then is already absolute (but not normalized).
>
> Signed-off-by: Ephrim Khong <dr.khong@xxxxxxxxx>
> ---
> My first patch, so be harsh. I'm not sure about the filename of the test,
> the behavior is tested with repack, but it affects gc and others as well.
>
> sha1_file.c | 2 +-
> t/t7702-repack-cyclic-alternate.sh | 24 ++++++++++++++++++++++++
> 2 files changed, 25 insertions(+), 1 deletion(-)
> create mode 100755 t/t7702-repack-cyclic-alternate.sh
>
> diff --git a/sha1_file.c b/sha1_file.c
> index 34d527f..7e98e9e 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -320,7 +320,7 @@ static int link_alt_odb_entry(const char *entry, const
> char *relative_base, int
> return -1;
> }
> }
> - if (!strcmp(ent->base, objdir)) {
> + if (!strcmp(ent->base, absolute_path(objdir))) {
I think you want to normalize objdir in addition to making it
absolute, in case someone sets GIT_OBJECT_DIR to foo///bar. ent->base
is normalized already. Oh and maybe use strcmp_icase to be nice on
case-insensitive filesystems..
Kinda nit picking, but perhaps this path
absolute-ization/normalization should be done by the caller
link_alt_odb_entries so you don't have to redo it for every entry in
alternates file. I know there's a loop of memcmp() right above this
that may be more expensive than this micro-optimization when we have
lots of alternate entries.
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html