Re: [PATCH v3 07/25] reflog: avoid constructing .lock path with git_path
Duy Nguyen pclo...@gmail.com writes: On Wed, Feb 26, 2014 at 5:44 AM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: git_path() soon understands the path given to it. Some paths abc may become def while other ghi may become ijk. We don't want git_path() to interfere with .lock path construction. Concatenate .lock after the path has been resolved by git_path() so if abc becomes def, we'll have def.lock, not ijk. Hmph. I am not sure if the above is readable (or if I am reading it correctly). Definitely not now that I have had my break from the series and reread it. If abc becomes def, it would take deliberate work to make abc.lock into ijk, and it would be natural to expect that abc.lock would become def.lock without any fancy trick, no? A better explanation may be, while many paths are not converted by git_path() (abc - abc), some of them will be based on the given path (def - ghi). Giving path def.lock to git_path() may confuse it and make it believe def.lock should not be converted because the signature is def.lock not def. But we want the lock file to have the same base name with the locked file (e.g. ghi.lock, not def.lock). So it's best to append .lock after git_path() has done its conversion. The alternative is teach git_path about .lock, but I don't really want to put more logic down there. I think your attempt to sound generic by using abc, def, etc. backfired and made the description only obscure. It would have been immediately understandable if it were more like this: Among pathnames in $GIT_DIR, e.g. index or packed-refs, we want to automatically and silently map some of them to the $GIT_DIR of the repository we are borrowing from via $GIT_COMMON_DIR mechanism. When we formulate the pathname for its lockfile, we want it to be in the same location as its final destination. index is not shared and needs to remain in the borrowing repository, while packed-refs is shared and needs to go to the borrowed repository. git_path() could be taught about the .lock suffix and map index.lock and packed-refs.lock the same way their basenames are mapped, but instead the caller can help by asking where the basename (e.g. index) is mapped to git_path() and then appending .lock after the mapping is done. Thanks for an explanation. With that understanding, the patch makes sense. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/25] reflog: avoid constructing .lock path with git_path
On Wed, Feb 26, 2014 at 5:44 AM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: git_path() soon understands the path given to it. Some paths abc may become def while other ghi may become ijk. We don't want git_path() to interfere with .lock path construction. Concatenate .lock after the path has been resolved by git_path() so if abc becomes def, we'll have def.lock, not ijk. Hmph. I am not sure if the above is readable (or if I am reading it correctly). Definitely not now that I have had my break from the series and reread it. If abc becomes def, it would take deliberate work to make abc.lock into ijk, and it would be natural to expect that abc.lock would become def.lock without any fancy trick, no? A better explanation may be, while many paths are not converted by git_path() (abc - abc), some of them will be based on the given path (def - ghi). Giving path def.lock to git_path() may confuse it and make it believe def.lock should not be converted because the signature is def.lock not def. But we want the lock file to have the same base name with the locked file (e.g. ghi.lock, not def.lock). So it's best to append .lock after git_path() has done its conversion. The alternative is teach git_path about .lock, but I don't really want to put more logic down there. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/25] reflog: avoid constructing .lock path with git_path
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
git_path() soon understands the path given to it. Some paths abc may
become def while other ghi may become ijk. We don't want
git_path() to interfere with .lock path construction. Concatenate
.lock after the path has been resolved by git_path() so if abc
becomes def, we'll have def.lock, not ijk.
Hmph. I am not sure if the above is readable (or if I am reading it
correctly).
If abc becomes def, it would take deliberate work to make
abc.lock into ijk, and it would be natural to expect that
abc.lock would become def.lock without any fancy trick, no?
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/reflog.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 852cff6..ccf2cf6 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -372,7 +372,7 @@ static int expire_reflog(const char *ref, const unsigned
char *sha1, int unused,
if (!file_exists(log_file))
goto finish;
if (!cmd-dry_run) {
- newlog_path = git_pathdup(logs/%s.lock, ref);
+ newlog_path = mkpathdup(%s.lock, log_file);
cb.newlog = fopen(newlog_path, w);
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 07/25] reflog: avoid constructing .lock path with git_path
git_path() soon understands the path given to it. Some paths abc may
become def while other ghi may become ijk. We don't want
git_path() to interfere with .lock path construction. Concatenate
.lock after the path has been resolved by git_path() so if abc
becomes def, we'll have def.lock, not ijk.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/reflog.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 852cff6..ccf2cf6 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -372,7 +372,7 @@ static int expire_reflog(const char *ref, const unsigned
char *sha1, int unused,
if (!file_exists(log_file))
goto finish;
if (!cmd-dry_run) {
- newlog_path = git_pathdup(logs/%s.lock, ref);
+ newlog_path = mkpathdup(%s.lock, log_file);
cb.newlog = fopen(newlog_path, w);
}
--
1.8.5.2.240.g8478abd
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
