Particularly for the "alternates" file, if one will be created, we
want a path that doesn't depend on the current directory, but we want
to retain any symlinks in the path as given and any in the user's view
of the current directory when the path was given.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
Lightly tested; I haven't put together a test cases for the problem that
started this, though.
builtin-clone.c | 4 ++--
cache.h | 1 +
path.c | 34 ++++++++++++++++++++++++++++++++++
3 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/builtin-clone.c b/builtin-clone.c
index f4accbe..7190952 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -76,7 +76,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)
path = mkpath("%s%s", repo, suffix[i]);
if (!stat(path, &st) && S_ISDIR(st.st_mode)) {
*is_bundle = 0;
- return xstrdup(make_absolute_path(path));
+ return xstrdup(make_nonrelative_path(path));
}
}
@@ -85,7 +85,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)
path = mkpath("%s%s", repo, bundle_suffix[i]);
if (!stat(path, &st) && S_ISREG(st.st_mode)) {
*is_bundle = 1;
- return xstrdup(make_absolute_path(path));
+ return xstrdup(make_nonrelative_path(path));
}
}
diff --git a/cache.h b/cache.h
index 092a997..0a63c0e 100644
--- a/cache.h
+++ b/cache.h
@@ -524,6 +524,7 @@ static inline int is_absolute_path(const char *path)
return path[0] == '/';
}
const char *make_absolute_path(const char *path);
+const char *make_nonrelative_path(const char *path);
/* Read and unpack a sha1 file into memory, write memory to a sha1 file */
extern int sha1_object_info(const unsigned char *, unsigned long *);
diff --git a/path.c b/path.c
index b7c24a2..60b55ae 100644
--- a/path.c
+++ b/path.c
@@ -291,6 +291,40 @@ int adjust_shared_perm(const char *path)
return 0;
}
+static const char *get_pwd_cwd(void)
+{
+ static char cwd[PATH_MAX + ...Hi, I have to say that I do not follow why the symlinks should be trusted any more than the absolute paths. Ciao, Dscho --
It's not so much a case of trusting symlinks more, but trusting the version of the path that the user used more. This came up because kernel.org has HTTP service arranged such that /pub/... is served at http://kernel.org/pub/..., but /pub is a symlink to /home/ftp/pub/, and http://kernel.org/home/ftp/pub/... doesn't work. Clever people like Greg use "git clone -s /pub/.../linux-2.6.git" in order to share Linus's object storage, and this generated an HTTP-clonable repo with git-clone.sh, because it didn't expand symlinks, but the builtin version, without this patch, would expand them into the thing that doesn't work. If there's a difference between using the symlink and using the thing it links to, probably the user picked the right thing, or wouldn't be too surprised that git doesn't come up with something better. -Daniel *This .sig left intentionally blank* --
Thanks, Daniel. I am obviously in favor of fixing this regression before 1.5.6, and the introduction of make_nonrelative_path() for the use of clone alone is probably the least impact and safe solution in the short term after -rc1. In the longer term, we would inevitably face "when should one use nonrelative and when should one use absolute?" and we would eventually have to answer it. It may turn out that many current users of "absolute" are better off using "nonrelative", but I suspect we won't get rid of "absolute" completely, because one of the reasons it avoids symlinks at great lengths is so that it can check the containment relationships between paths reliably (e.g. "is this path outside the repository, in which case we should refuse to add it to the index, and we use --no-index without being asked when running "diff""). But using "absolute" for containment comparison is one thing. Storing the result of "absolute" is quite another. We've already learned to love a similar crazyness somebody's system has on this list. If you want to check if the user string is equivalent to a path you stored in the filesystem, you compare them after normalizing and that is a sensible approach. Storing path after normalizing, which would be different form than what the user originally used to create, is not so sane and leads to all sorts of pain. Ending up storing the output from "absolute" in info/alternates is just like giving normalized sequence back from readdir(3) on such a system. --
Hi, The easy way would be to add an option to make_absolute_path(), say "resolve_symlinks". Ciao, Dscho --
I am afraid that it does not solve anything. Be they two separate functions, or a one function that has two different semantics depending on an option, the API documentation needs to answer the "when should I use one and when should I use the other" question. And the hard part is figuring out which of the current "absolute" callers need to be fixed in a way similar to how Daniel fixed git-clone, and which of them stay the same. Perhaps all of the "chdir then getpwd" patterns need to be looked at and some of them need to be restructured to honor $PWD better. I dunno. --
