Johannes Sixt <johannes.sixt@xxxxxxxxxx> writes:
> Some file systems that can host git repositories and their working copies
> do not support symbolic links. But then if the repository contains a symbolic
> link, it is impossible to check out the working copy.
Overall the patch looks good. Thanks.
> Of course, this does not magically make symbolic links work on such defective
> file systems; hence, this solution does not help if the working copy relies
> on that an entry is a real symbolic link.
Probably defective is too strong a word here, but I'll apply
your patch as-is anyway. We may need a few fix-ups, but it is
easier to deal with such small updates on top of it once it is
in-tree.
> --- a/Documentation/git-update-index.txt
> +++ b/Documentation/git-update-index.txt
> @@ -295,6 +295,11 @@ in the index and the file mode on the filesystem if they differ only on
> executable bit. On such an unfortunate filesystem, you may
> need to use `git-update-index --chmod=`.
>
> +Quite similarly, if `core.symlinks` configuration variable is set
> +to 'false' (see gitlink:git-config[1]), symbolic links are checked out
> +as plain files, and this command does not modify a recorded file mode
> +from symbolic link to regular file.
> +
> The command looks at `core.ignorestat` configuration variable. See
> 'Using "assume unchanged" bit' section above.
This is off-topic, but I am having this nagging feeling that we
may want to revisit the ignorestat stuff to ignore more than
what it currently does. For example, if we make it to not even
care if the path is missing from the working tree, I think it
would help working in a narrowly checked out repository.
> diff --git a/cache.h b/cache.h
> index 04f8e63..6f932fe 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -108,7 +108,10 @@ static inline unsigned int create_ce_mode(unsigned int mode)
> }
> static inline unsigned int ce_mode_from_stat(struct cache_entry *ce, unsigned int mode)
> {
> - extern int trust_executable_bit;
> + extern int trust_executable_bit, has_symlinks;
> + if (!has_symlinks && S_ISREG(mode) &&
> + ce && S_ISLNK(ntohl(ce->ce_mode)))
> + return ce->ce_mode;
> if (!trust_executable_bit && S_ISREG(mode)) {
> if (ce && S_ISREG(ntohl(ce->ce_mode)))
> return ce->ce_mode;
This code (I am the guilty one before your change above) always
confused me. How about doing something like this instead?
static inline unsigned int ce_mode_from_stat(struct cache_entry *ce,...
{
/*
* A regular file that appears on the filesystem can have
* a "wrong" st_mode information. A few repository config
* variables can tell us to trust the mode recorded in the
* index more than what we get from the filesystem.
*/
if (ce && S_ISREG(mode)) {
extern int trust_executable_bit, has_symlinks;
if (!has_symlinks && S_ISLNK(ntohl(ce->ce_mode)))
return ce->ce_mode;
if (!trust_executable_bit && S_ISREG(ntohl(ce->ce_mode)))
return ce->ce_mode;
return create_ce_mode(0666);
}
return create_ce_mode(mode);
}
> diff --git a/diff-lib.c b/diff-lib.c
> index 60c0fa6..2c121d2 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -134,6 +134,9 @@ int run_diff_files(struct rev_info *revs, int silent_on_removed)
> S_ISREG(newmode) && S_ISREG(oldmode) &&
> ((newmode ^ oldmode) == 0111))
> newmode = oldmode;
> + else if (!has_symlinks &&
> + S_ISREG(newmode) && S_ISLNK(oldmode))
> + newmode = oldmode;
> diff_change(&revs->diffopt, oldmode, newmode,
> ce->sha1, (changed ? null_sha1 : ce->sha1),
> ce->name, NULL);
I suspect that this part becomes clearer if we rewrite it like
this:
changed = ce_match_stat(ce, &st, 0);
if (!changed && !revs->diffopt.find_copies_harder)
continue;
oldmode = ntohl(ce->ce_mode);
-
- newmode = canon_mode(st.st_mode);
- if (!trust_executable_bit &&
- S_ISREG(newmode) && S_ISREG(oldmode) &&
- ((newmode ^ oldmode) == 0111))
- newmode = oldmode;
- else if (!has_symlinks &&
- S_ISREG(newmode) && S_ISLNK(oldmode))
- newmode = oldmode;
+ newmode = ntohl(ce_mode_from_stat(ce, st.st_mode));
diff_change(&revs->diffopt, oldmode, newmode,
ce->sha1, (changed ? null_sha1 : ce->sha1),
ce->name, NULL);
-
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