Re: [PATCH] remote-hg: add shared repo upgrade

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Antoine Pelisse <apelisse@xxxxxxxxx> writes:

> On Mon, Aug 5, 2013 at 11:02 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Antoine Pelisse <apelisse@xxxxxxxxx> writes:
>> Is the untold
>> and obvious-to-those-who-are-familiar-with-this-codepath assumption
>> that it is guaranteed that there is at most one "*/clone/.hg" under
>> shared_path?
>
> No, there is no such assumption.
> That is why we create a repository just below if it doesn't exist (no
> copy was found).
> That's also why I don't see how we could split the patch.
>
> We could improve that part of the commit message:
>
>     It's trivial to upgrade to the new organization by copying the Mercurial
>     repo from one of the remotes (e.g. 'origin'), so let's do so. If
>     we can't find
>     any existing repo, we create an empty one.

That is fine, and I do not (yet) have an opinion on this patch
needing to be further split.

Quoting that part I was asking about again:

> +        # check and upgrade old organization
> +        hg_path = os.path.join(shared_path, '.hg')
> +        if os.path.exists(shared_path) and not os.path.exists(hg_path):
> +            repos = os.listdir(shared_path)
> +            for x in repos:
> +                local_hg = os.path.join(shared_path, x, 'clone', '.hg')
> +                if not os.path.exists(local_hg):
> +                    continue
> +                shutil.copytree(local_hg, hg_path)

if you can have more than one 'x' such that

    local_hg = os.path.join(shared_path, x, 'clone', '.hg')

exists, that means in repos[], there are two (or more) x1,and x2,
and in this loop you will run

	shutil.copytree(local_hg, hg_path)

twice, once for local_hg derived from x1 and another time from x2,
both to the same hg_path directory that does not change inside the
loop.  shutil.copytree(src, dst) however creates leading paths down
to dst and it would barf when dst already exists, no?

That is what I was puzzled about the code.  The log message says "we
can copy from one of them if exists, so let's do so", which makes
sense, and a code structure that may match would have looked like
so:

	for x in repos:
        	'''pick one at random, copy it and leave'''
                copytree()
                break
	else:
        	'''nothing to be copied, do it the hard way by cloning'''

but that is not what I saw so that is where my confusion came from.
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]