log_ref_setup(): separate code for create vs non-create

The behavior of this function (especially how it handles errors) is
quite different depending on whether we are willing to create the reflog
vs. whether we are only trying to open an existing reflog. So separate
the code paths.

This also simplifies the next steps.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs/files-backend.c b/refs/files-backend.c
index fd8a751e10d0a273bb8881f77b9fcacc9569782f..c8f6d82e6d87307ac727d6e546910db1dc983df9 100644 (file)
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2718,45 +2718,64 @@ static int commit_ref(struct ref_lock *lock)
  */
 static int log_ref_setup(const char *refname, struct strbuf *logfile, struct strbuf *err, int force_create)
 {
  */
 static int log_ref_setup(const char *refname, struct strbuf *logfile, struct strbuf *err, int force_create)
 {

-       int logfd, oflags = O_APPEND | O_WRONLY;
+       int logfd;

 
        strbuf_git_path(logfile, "logs/%s", refname);
 
        strbuf_git_path(logfile, "logs/%s", refname);

+

        if (force_create || should_autocreate_reflog(refname)) {
                if (safe_create_leading_directories(logfile->buf) < 0) {
                        strbuf_addf(err, "unable to create directory for '%s': "
                                    "%s", logfile->buf, strerror(errno));
                        return -1;
                }
        if (force_create || should_autocreate_reflog(refname)) {
                if (safe_create_leading_directories(logfile->buf) < 0) {
                        strbuf_addf(err, "unable to create directory for '%s': "
                                    "%s", logfile->buf, strerror(errno));
                        return -1;
                }

-               oflags |= O_CREAT;
-       }
-
-       logfd = open(logfile->buf, oflags, 0666);
-       if (logfd < 0) {
-               if (!(oflags & O_CREAT) && (errno == ENOENT || errno == EISDIR))
-                       return 0;
+               logfd = open(logfile->buf, O_APPEND | O_WRONLY | O_CREAT, 0666);
+               if (logfd < 0) {
+                       if (errno == EISDIR) {
+                               /*
+                                * The directory that is in the way might be
+                                * empty. Try to remove it.
+                                */
+                               if (remove_empty_directories(logfile)) {
+                                       strbuf_addf(err, "there are still logs under "
+                                                   "'%s'", logfile->buf);
+                                       return -1;
+                               }
+                               logfd = open(logfile->buf, O_APPEND | O_WRONLY | O_CREAT, 0666);
+                       }

 
 

-               if (errno == EISDIR) {
-                       if (remove_empty_directories(logfile)) {
-                               strbuf_addf(err, "there are still logs under "
-                                           "'%s'", logfile->buf);
+                       if (logfd < 0) {
+                               strbuf_addf(err, "unable to append to '%s': %s",
+                                           logfile->buf, strerror(errno));

                                return -1;
                        }
                                return -1;
                        }

-                       logfd = open(logfile->buf, oflags, 0666);

                }
                }

-
+       } else {
+               logfd = open(logfile->buf, O_APPEND | O_WRONLY, 0666);

                if (logfd < 0) {
                if (logfd < 0) {

-                       strbuf_addf(err, "unable to append to '%s': %s",
-                                   logfile->buf, strerror(errno));
-                       return -1;
+                       if (errno == ENOENT || errno == EISDIR) {
+                               /*
+                                * The logfile doesn't already exist,
+                                * but that is not an error; it only
+                                * means that we won't write log
+                                * entries to it.
+                                */
+                               ;
+                       } else {
+                               strbuf_addf(err, "unable to append to '%s': %s",
+                                           logfile->buf, strerror(errno));
+                               return -1;
+                       }

                }
        }
 
                }
        }
 

-       adjust_shared_perm(logfile->buf);
-       close(logfd);
+       if (logfd >= 0) {
+               adjust_shared_perm(logfile->buf);
+               close(logfd);
+       }
+

        return 0;
 }
 
        return 0;
 }
 

-

 static int files_create_reflog(struct ref_store *ref_store,
                               const char *refname, int force_create,
                               struct strbuf *err)
 static int files_create_reflog(struct ref_store *ref_store,
                               const char *refname, int force_create,
                               struct strbuf *err)