Miklos Vajna <vmiklos@xxxxxxxxxxxxxx> writes:
> -int delete_ref(const char *refname, const unsigned char *sha1)
> +int delete_ref(const char *refname, const unsigned char *sha1, int flags)
> {
> struct ref_lock *lock;
> - int err, i, ret = 0, flag = 0;
> + int err, i = 0, ret = 0, flag = 0;
> + char *path;
>
> lock = lock_ref_sha1_basic(refname, sha1, 0, &flag);
> if (!lock)
> return 1;
> if (!(flag & REF_ISPACKED)) {
Two variables flag vs flags is a bit confusing, isn't it? How about
naming the new one "delopt" or something?
The new variable "char *path" at the toplevel can be confined in the scope
of this if () {} block and probably can become "const char *", right?
> @@ -964,10 +971,15 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
> struct ref_lock *lock;
> struct stat loginfo;
> int log = !lstat(git_path("logs/%s", oldref), &loginfo);
> + const char *symref = NULL;
> + int is_symref = 0;
>
> if (S_ISLNK(loginfo.st_mode))
> return error("reflog for %s is a symlink", oldref);
Possible bug in the context. When there is no reflog for the ref being
renamed, lstat would fail; it doesn't feel right to have this S_ISLNK()
before checking the result of the lstat which is in "log".
> + symref = resolve_ref(oldref, orig_sha1, 0, &flag);
> + if (flag & REF_ISSYMREF)
> + is_symref = 1;
> if (!resolve_ref(oldref, orig_sha1, 1, &flag))
> return error("refname %s not found", oldref);
Do we really need two calls to resolve_ref()? Your new call calls it
without must-exist bit --- why? Immediately after that, the existing call
will barf if it does not exist anyway.
I agree it is good to have symref aware delete_ref(), but I am not sure
supporting symref in rename_ref() is either needed or necessarily a good
idea. You also need to worry about a symref pointing at a branch yet to
be born.
In the meantime, I think we should just check (flag & REF_ISSYMREF) after
the existing resolve_ref() we can see in the context above, and error out
saying you cannot rename a symref, and do nothing else.
--
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
