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>

Michael Haggerty committed Jan 6, 2017 at 17:22 UTC 854bda6b4f7d6bc6087966bfa74a59c232eceac6
1 file changed +39 -20
refs/files-backend.c
+39 -20
@@ -2718,45 +2718,64 @@ static int commit_ref(struct ref_lock *lock)
2718 */
2719 static int log_ref_setup(const char *refname, struct strbuf *logfile, struct strbuf *err, int force_create)
2720 {
2721 - int logfd, oflags = O_APPEND | O_WRONLY;
2721 + int logfd;
2722
2723 strbuf_git_path(logfile, "logs/%s", refname);
2724 +
2725 if (force_create || should_autocreate_reflog(refname)) {
2726 if (safe_create_leading_directories(logfile->buf) < 0) {
2727 strbuf_addf(err, "unable to create directory for '%s': "
2728 "%s", logfile->buf, strerror(errno));
2729 return -1;
2730 }
2730 - oflags |= O_CREAT;
2731 - }
2732 -
2733 - logfd = open(logfile->buf, oflags, 0666);
2734 - if (logfd < 0) {
2735 - if (!(oflags & O_CREAT) && (errno == ENOENT || errno == EISDIR))
2736 - return 0;
2731 + logfd = open(logfile->buf, O_APPEND | O_WRONLY | O_CREAT, 0666);
2732 + if (logfd < 0) {
2733 + if (errno == EISDIR) {
2734 + /*
2735 + * The directory that is in the way might be
2736 + * empty. Try to remove it.
2737 + */
2738 + if (remove_empty_directories(logfile)) {
2739 + strbuf_addf(err, "there are still logs under "
2740 + "'%s'", logfile->buf);
2741 + return -1;
2742 + }
2743 + logfd = open(logfile->buf, O_APPEND | O_WRONLY | O_CREAT, 0666);
2744 + }
2745
2738 - if (errno == EISDIR) {
2739 - if (remove_empty_directories(logfile)) {
2740 - strbuf_addf(err, "there are still logs under "
2741 - "'%s'", logfile->buf);
2746 + if (logfd < 0) {
2747 + strbuf_addf(err, "unable to append to '%s': %s",
2748 + logfile->buf, strerror(errno));
2749 return -1;
2750 }
2744 - logfd = open(logfile->buf, oflags, 0666);
2751 }
2746 -
2752 + } else {
2753 + logfd = open(logfile->buf, O_APPEND | O_WRONLY, 0666);
2754 if (logfd < 0) {
2748 - strbuf_addf(err, "unable to append to '%s': %s",
2749 - logfile->buf, strerror(errno));
2750 - return -1;
2755 + if (errno == ENOENT || errno == EISDIR) {
2756 + /*
2757 + * The logfile doesn't already exist,
2758 + * but that is not an error; it only
2759 + * means that we won't write log
2760 + * entries to it.
2761 + */
2762 + ;
2763 + } else {
2764 + strbuf_addf(err, "unable to append to '%s': %s",
2765 + logfile->buf, strerror(errno));
2766 + return -1;
2767 + }
2768 }
2769 }
2770
2754 - adjust_shared_perm(logfile->buf);
2755 - close(logfd);
2771 + if (logfd >= 0) {
2772 + adjust_shared_perm(logfile->buf);
2773 + close(logfd);
2774 + }
2775 +
2776 return 0;
2777 }
2778
2759 -
2779 static int files_create_reflog(struct ref_store *ref_store,
2780 const char *refname, int force_create,
2781 struct strbuf *err)