David Aguilar <davvid@xxxxxxxxx> writes:
> + # Do not copy back files when symlinks are used
> + if ($symlinks) {
> + exit(0);
> + }
> +
Isn't this a bit risky, depending on the behaviour of the tool that
eventually lead the user to invoke his favorite editor to muck with
the files in the temporary directory? I think most sane people and
their editors would follow symlinks and update the file the symlink
points at when writing out the modified contents, but it should not
be too much trouble to detect the case in which the editor unlinked
the symlink and recreated a regular file in its place, and copy the
file back when that happened, to make it even safer, no?
The most lazy solution would be to just remove the above block, and
let the compare() compare the symlink $b/$file and the working tree
file $workdir/$file that is pointed by it. We will find data losing
case where the editor unlinks and creates that way automatically.
Optionally, you can update
if (-e "$b/$file" && compare("$b/$file", "$workdir/$file")) {
with
if (! -l "$b/$file" && -f _ && compare("$b/$file", "$workdir/$file")) {
to avoid the cost of comparison.
> # If the diff including working copy files and those
> # files were modified during the diff, then the changes
> # should be copied back to the working tree
> +
> for my $file (@working_tree) {
> if (-e "$b/$file" && compare("$b/$file", "$workdir/$file")) {
> copy("$b/$file", "$workdir/$file") or die $!;
> - chmod(stat("$b/$file")->mode, "$workdir/$file") or die $!;
> + my $mode = stat("$b/$file")->mode;
> + chmod($mode, "$workdir/$file") or die $!;
> }
> }
> + exit(0);
> }
Other than that, the series looked well thought-out.
Thanks.
--
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