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