Re: [PATCH/RFC 1/3] Teach mv to move submodules together with their work trees

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

 



Ramkumar Ramachandra <artagnon@xxxxxxxxx> writes:

>> Then rename() will move the submodule work tree just
>> like it moves a file.
>
> What is this rename() function you're talking about?  I don't see it anywhere.

"man 2 rename"; it is called from a generic part of builtin/mv.c to
rename one path to another and can move both files and directories.

>> +                       if (first >= 0) {
>> +                               if (!S_ISGITLINK(active_cache[first]->ce_mode))
>> +                                       die (_("Huh? Directory %s is in index and no submodule?"), src);
>
> I didn't understand this.  Why does it have to be a gitlink if it is
> stored at index position >= 0?

The path is not in the middle of the conflict, but the index records
something that is not a gitlink.  E.g. you start from A (regular file)
in the index, "rm A && mkdir A" and make it the top of a working
tree of a separate project.  "git mv A elsewhere" will say "src_is_dir"
but the index still thinks A should be a regular file blob.

> I'm assuming the else-case has nothing to do with the actual moving
> but rather something specific to directories (enumerating entries in
> it?), which is why it doesn't get executed when we find a gitlink.

It wants to move all the paths in the directory to a new
destination, e.g. "git mv srcdir dstdir", and update 


>> +                       } else {
>> +                               const char *src_w_slash = add_slash(src);
>> +                               int last, len_w_slash = length + 1;
>> +
>> +                               modes[i] = WORKING_DIRECTORY;
>> +
>> +                               first = cache_name_pos(src_w_slash, len_w_slash);
>> +                               if (first >= 0)
>> +                                       die (_("Huh? %.*s is in index?"),
>> +                                                       len_w_slash, src_w_slash);
>> +
>> +                               first = -1 - first;
>> +                               for (last = first; last < active_nr; last++) {
>> +                                       const char *path = active_cache[last]->name;
>> +                                       if (strncmp(path, src_w_slash, len_w_slash))
>> +                                               break;
>>                                 }
>
> Mostly unchanged, but I didn't understand the line first = -1 - first
> in the original.  What is it doing?  So, I'm guessing first is the
> cache position of the directory itself, and last stores the index of
> the "last" entry in the cache?  What does that even mean?

cache_name_pos() either returns the position for a path at stage #0
that exists in the index, or the position the given path _would_ be
inserted if you were to add it to the index for a path that does not
exist in the index at stage #0 (I think this part of the code does
not consider that the given path is unmerged, either by being sloppy
or detecting that case much earlier---I didn't check), and when
doing the latter, it encodes the position by negating it and
offsetting it by 1 (otherwise you cannot tell "it would come at the
very beginning" and "it is at the very beginning", because negated
zero is still zero).  The "-1 - first" is an idiom used everywhere
by callers of cache_name_pos() to recover the latter from the
returned value.

If you start at a position "src/" would have been inserted, and
iterate over the index while the entry's path prefix-matches with
"src/", you will find where the entries in the "src/" directory
ends.

>> +                               if (last - first < 1)
>> +                                       bad = _("source directory is empty");
>
> This is exactly what was tripping us up earlier.  Can you explain what
> last - first < 1 means?

I think the above covers it.

Asking questions to learn the basic part of Git internals on this
list, e.g. "I found this existing code, and I do not understand what
it is doing. Can somebody shed a light on it?", is perfectly fine,
but can you do so outside the review discussion?

It clutters the review discussions when such a request for education
is labeled as if it were a review or mixed with a message with
genuine review comments.
--
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]