Merge branch 'js/fopen-harder' into maint
authorJunio C Hamano <gitster@pobox.com>
Fri, 5 Feb 2016 22:54:11 +0000 (14:54 -0800)
committerJunio C Hamano <gitster@pobox.com>
Fri, 5 Feb 2016 22:54:11 +0000 (14:54 -0800)
Some codepaths used fopen(3) when opening a fixed path in $GIT_DIR
(e.g. COMMIT_EDITMSG) that is meant to be left after the command is
done. This however did not work well if the repository is set to
be shared with core.sharedRepository and the umask of the previous
user is tighter. They have been made to work better by calling
unlink(2) and retrying after fopen(3) fails with EPERM.

* js/fopen-harder:
Handle more file writes correctly in shared repos
commit: allow editing the commit message even in shared repos

1  2 
builtin/commit.c
builtin/fast-export.c
builtin/fetch.c
git-compat-util.h
wrapper.c
diff --combined builtin/commit.c
index d054f849606b248a82f7b29103308feab36ca9e9,ce11a17f8ac51f4f9045db419ea2c253524cf078..89bf6ad38abbbee068ca29bad2ce2a4b728fa388
@@@ -300,7 -300,7 +300,7 @@@ static void create_base_index(const str
        opts.dst_index = &the_index;
  
        opts.fn = oneway_merge;
 -      tree = parse_tree_indirect(current_head->object.sha1);
 +      tree = parse_tree_indirect(current_head->object.oid.hash);
        if (!tree)
                die(_("failed to unpack HEAD tree object"));
        parse_tree(tree);
@@@ -761,7 -761,7 +761,7 @@@ static int prepare_to_commit(const cha
                hook_arg2 = "";
        }
  
-       s->fp = fopen(git_path(commit_editmsg), "w");
+       s->fp = fopen_for_writing(git_path(commit_editmsg));
        if (s->fp == NULL)
                die_errno(_("could not open '%s'"), git_path(commit_editmsg));
  
@@@ -1769,7 -1769,7 +1769,7 @@@ int cmd_commit(int argc, const char **a
        if (!transaction ||
            ref_transaction_update(transaction, "HEAD", sha1,
                                   current_head
 -                                 ? current_head->object.sha1 : null_sha1,
 +                                 ? current_head->object.oid.hash : null_sha1,
                                   0, sb.buf, &err) ||
            ref_transaction_commit(transaction, &err)) {
                rollback_index_files();
                cfg = init_copy_notes_for_rewrite("amend");
                if (cfg) {
                        /* we are amending, so current_head is not NULL */
 -                      copy_note_for_rewrite(cfg, current_head->object.sha1, sha1);
 +                      copy_note_for_rewrite(cfg, current_head->object.oid.hash, sha1);
                        finish_copy_notes_for_rewrite(cfg, "Notes added by 'git commit --amend'");
                }
 -              run_rewrite_hook(current_head->object.sha1, sha1);
 +              run_rewrite_hook(current_head->object.oid.hash, sha1);
        }
        if (!quiet)
                print_summary(prefix, sha1, !current_head);
diff --combined builtin/fast-export.c
index d9ac5d8410b30e02fbb8edb06a1991655c5be8f4,aacf3b460bdce53aeb96a81ebb1167f001e6f1ec..2471297f7101964c61c055b10e24a7aa4b65326a
@@@ -544,13 -544,13 +544,13 @@@ static void handle_commit(struct commi
        author = strstr(commit_buffer, "\nauthor ");
        if (!author)
                die ("Could not find author in commit %s",
 -                   sha1_to_hex(commit->object.sha1));
 +                   oid_to_hex(&commit->object.oid));
        author++;
        author_end = strchrnul(author, '\n');
        committer = strstr(author_end, "\ncommitter ");
        if (!committer)
                die ("Could not find committer in commit %s",
 -                   sha1_to_hex(commit->object.sha1));
 +                   oid_to_hex(&commit->object.oid));
        committer++;
        committer_end = strchrnul(committer, '\n');
        message = strstr(committer_end, "\n\n");
            get_object_mark(&commit->parents->item->object) != 0 &&
            !full_tree) {
                parse_commit_or_die(commit->parents->item);
 -              diff_tree_sha1(commit->parents->item->tree->object.sha1,
 -                             commit->tree->object.sha1, "", &rev->diffopt);
 +              diff_tree_sha1(commit->parents->item->tree->object.oid.hash,
 +                             commit->tree->object.oid.hash, "", &rev->diffopt);
        }
        else
 -              diff_root_tree_sha1(commit->tree->object.sha1,
 +              diff_root_tree_sha1(commit->tree->object.oid.hash,
                                    "", &rev->diffopt);
  
        /* Export the referenced blobs, and remember the marks. */
@@@ -661,13 -661,13 +661,13 @@@ static void handle_tag(const char *name
        }
        if (tagged->type == OBJ_TREE) {
                warning("Omitting tag %s,\nsince tags of trees (or tags of tags of trees, etc.) are not supported.",
 -                      sha1_to_hex(tag->object.sha1));
 +                      oid_to_hex(&tag->object.oid));
                return;
        }
  
 -      buf = read_sha1_file(tag->object.sha1, &type, &size);
 +      buf = read_sha1_file(tag->object.oid.hash, &type, &size);
        if (!buf)
 -              die ("Could not read tag %s", sha1_to_hex(tag->object.sha1));
 +              die ("Could not read tag %s", oid_to_hex(&tag->object.oid));
        message = memmem(buf, size, "\n\n", 2);
        if (message) {
                message += 2;
                        case ABORT:
                                die ("Encountered signed tag %s; use "
                                     "--signed-tags=<mode> to handle it.",
 -                                   sha1_to_hex(tag->object.sha1));
 +                                   oid_to_hex(&tag->object.oid));
                        case WARN:
                                warning ("Exporting signed tag %s",
 -                                       sha1_to_hex(tag->object.sha1));
 +                                       oid_to_hex(&tag->object.oid));
                                /* fallthru */
                        case VERBATIM:
                                break;
                        case WARN_STRIP:
                                warning ("Stripping signature from tag %s",
 -                                       sha1_to_hex(tag->object.sha1));
 +                                       oid_to_hex(&tag->object.oid));
                                /* fallthru */
                        case STRIP:
                                message_size = signature + 1 - message;
                case ABORT:
                        die ("Tag %s tags unexported object; use "
                             "--tag-of-filtered-object=<mode> to handle it.",
 -                           sha1_to_hex(tag->object.sha1));
 +                           oid_to_hex(&tag->object.oid));
                case DROP:
                        /* Ignore this tag altogether */
                        return;
                case REWRITE:
                        if (tagged->type != OBJ_COMMIT) {
                                die ("Tag %s tags unexported %s!",
 -                                   sha1_to_hex(tag->object.sha1),
 +                                   oid_to_hex(&tag->object.oid),
                                     typename(tagged->type));
                        }
                        p = (struct commit *)tagged;
                                        break;
                                if (!p->parents)
                                        die ("Can't find replacement commit for tag %s\n",
 -                                           sha1_to_hex(tag->object.sha1));
 +                                           oid_to_hex(&tag->object.oid));
                                p = p->parents->item;
                        }
                        tagged_mark = get_object_mark(&p->object);
@@@ -777,7 -777,7 +777,7 @@@ static struct commit *get_commit(struc
  
                /* handle nested tags */
                while (tag && tag->object.type == OBJ_TAG) {
 -                      parse_object(tag->object.sha1);
 +                      parse_object(tag->object.oid.hash);
                        string_list_append(&extra_refs, full_name)->util = tag;
                        tag = (struct tag *)tag->tagged;
                }
@@@ -828,7 -828,7 +828,7 @@@ static void get_tags_and_duplicates(str
                case OBJ_COMMIT:
                        break;
                case OBJ_BLOB:
 -                      export_blob(commit->object.sha1);
 +                      export_blob(commit->object.oid.hash);
                        continue;
                default: /* OBJ_TAG (nested tags) is already handled */
                        warning("Tag points to object of unexpected type %s, skipping.",
@@@ -880,7 -880,7 +880,7 @@@ static void export_marks(char *file
        FILE *f;
        int e = 0;
  
-       f = fopen(file, "w");
+       f = fopen_for_writing(file);
        if (!f)
                die_errno("Unable to open marks file %s for writing.", file);
  
                if (deco->base && deco->base->type == 1) {
                        mark = ptr_to_mark(deco->decoration);
                        if (fprintf(f, ":%"PRIu32" %s\n", mark,
 -                              sha1_to_hex(deco->base->sha1)) < 0) {
 +                              oid_to_hex(&deco->base->oid)) < 0) {
                            e = 1;
                            break;
                        }
diff --combined builtin/fetch.c
index c85f3471d4b53ce7db7662b5531ddebe5513b7b0,6475a0b65e16e6b41417adcd0b643239c9840f98..9e24bb485f4ba438c8e3cd84856b13f4ab7fa927
@@@ -196,7 -196,7 +196,7 @@@ static int will_fetch(struct ref **head
  {
        struct ref *rm = *head;
        while (rm) {
 -              if (!hashcmp(rm->old_sha1, sha1))
 +              if (!hashcmp(rm->old_oid.hash, sha1))
                        return 1;
                rm = rm->next;
        }
@@@ -224,8 -224,8 +224,8 @@@ static void find_non_local_tags(struct 
                 * as one to ignore by setting util to NULL.
                 */
                if (ends_with(ref->name, "^{}")) {
 -                      if (item && !has_sha1_file(ref->old_sha1) &&
 -                          !will_fetch(head, ref->old_sha1) &&
 +                      if (item && !has_object_file(&ref->old_oid) &&
 +                          !will_fetch(head, ref->old_oid.hash) &&
                            !has_sha1_file(item->util) &&
                            !will_fetch(head, item->util))
                                item->util = NULL;
                        continue;
  
                item = string_list_insert(&remote_refs, ref->name);
 -              item->util = (void *)ref->old_sha1;
 +              item->util = (void *)&ref->old_oid;
        }
        string_list_clear(&existing_refs, 1);
  
                {
                        struct ref *rm = alloc_ref(item->string);
                        rm->peer_ref = alloc_ref(item->string);
 -                      hashcpy(rm->old_sha1, item->util);
 +                      oidcpy(&rm->old_oid, item->util);
                        **tail = rm;
                        *tail = &rm->next;
                }
@@@ -419,8 -419,8 +419,8 @@@ static int s_update_ref(const char *act
        transaction = ref_transaction_begin(&err);
        if (!transaction ||
            ref_transaction_update(transaction, ref->name,
 -                                 ref->new_sha1,
 -                                 check_old ? ref->old_sha1 : NULL,
 +                                 ref->new_oid.hash,
 +                                 check_old ? ref->old_oid.hash : NULL,
                                   0, msg, &err))
                goto fail;
  
@@@ -453,11 -453,11 +453,11 @@@ static int update_local_ref(struct ref 
        struct branch *current_branch = branch_get(NULL);
        const char *pretty_ref = prettify_refname(ref->name);
  
 -      type = sha1_object_info(ref->new_sha1, NULL);
 +      type = sha1_object_info(ref->new_oid.hash, NULL);
        if (type < 0)
 -              die(_("object %s not found"), sha1_to_hex(ref->new_sha1));
 +              die(_("object %s not found"), oid_to_hex(&ref->new_oid));
  
 -      if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
 +      if (!oidcmp(&ref->old_oid, &ref->new_oid)) {
                if (verbosity > 0)
                        strbuf_addf(display, "= %-*s %-*s -> %s",
                                    TRANSPORT_SUMMARY(_("[up to date]")),
        if (current_branch &&
            !strcmp(ref->name, current_branch->name) &&
            !(update_head_ok || is_bare_repository()) &&
 -          !is_null_sha1(ref->old_sha1)) {
 +          !is_null_oid(&ref->old_oid)) {
                /*
                 * If this is the head, and it's not okay to update
                 * the head, and the old value of the head isn't empty...
                return 1;
        }
  
 -      if (!is_null_sha1(ref->old_sha1) &&
 +      if (!is_null_oid(&ref->old_oid) &&
            starts_with(ref->name, "refs/tags/")) {
                int r;
                r = s_update_ref("updating tag", ref, 0);
                return r;
        }
  
 -      current = lookup_commit_reference_gently(ref->old_sha1, 1);
 -      updated = lookup_commit_reference_gently(ref->new_sha1, 1);
 +      current = lookup_commit_reference_gently(ref->old_oid.hash, 1);
 +      updated = lookup_commit_reference_gently(ref->new_oid.hash, 1);
        if (!current || !updated) {
                const char *msg;
                const char *what;
  
                if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
                    (recurse_submodules != RECURSE_SUBMODULES_ON))
 -                      check_for_new_submodule_commits(ref->new_sha1);
 +                      check_for_new_submodule_commits(ref->new_oid.hash);
                r = s_update_ref(msg, ref, 0);
                strbuf_addf(display, "%c %-*s %-*s -> %s%s",
                            r ? '!' : '*',
        }
  
        if (in_merge_bases(current, updated)) {
 -              char quickref[83];
 +              struct strbuf quickref = STRBUF_INIT;
                int r;
 -              strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
 -              strcat(quickref, "..");
 -              strcat(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
 +              strbuf_add_unique_abbrev(&quickref, current->object.oid.hash, DEFAULT_ABBREV);
 +              strbuf_addstr(&quickref, "..");
 +              strbuf_add_unique_abbrev(&quickref, ref->new_oid.hash, DEFAULT_ABBREV);
                if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
                    (recurse_submodules != RECURSE_SUBMODULES_ON))
 -                      check_for_new_submodule_commits(ref->new_sha1);
 +                      check_for_new_submodule_commits(ref->new_oid.hash);
                r = s_update_ref("fast-forward", ref, 1);
                strbuf_addf(display, "%c %-*s %-*s -> %s%s",
                            r ? '!' : ' ',
 -                          TRANSPORT_SUMMARY_WIDTH, quickref,
 +                          TRANSPORT_SUMMARY_WIDTH, quickref.buf,
                            REFCOL_WIDTH, remote, pretty_ref,
                            r ? _("  (unable to update local ref)") : "");
 +              strbuf_release(&quickref);
                return r;
        } else if (force || ref->force) {
 -              char quickref[84];
 +              struct strbuf quickref = STRBUF_INIT;
                int r;
 -              strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
 -              strcat(quickref, "...");
 -              strcat(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
 +              strbuf_add_unique_abbrev(&quickref, current->object.oid.hash, DEFAULT_ABBREV);
 +              strbuf_addstr(&quickref, "...");
 +              strbuf_add_unique_abbrev(&quickref, ref->new_oid.hash, DEFAULT_ABBREV);
                if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
                    (recurse_submodules != RECURSE_SUBMODULES_ON))
 -                      check_for_new_submodule_commits(ref->new_sha1);
 +                      check_for_new_submodule_commits(ref->new_oid.hash);
                r = s_update_ref("forced-update", ref, 1);
                strbuf_addf(display, "%c %-*s %-*s -> %s  (%s)",
                            r ? '!' : '+',
 -                          TRANSPORT_SUMMARY_WIDTH, quickref,
 +                          TRANSPORT_SUMMARY_WIDTH, quickref.buf,
                            REFCOL_WIDTH, remote, pretty_ref,
                            r ? _("unable to update local ref") : _("forced update"));
 +              strbuf_release(&quickref);
                return r;
        } else {
                strbuf_addf(display, "! %-*s %-*s -> %s  %s",
@@@ -580,7 -578,7 +580,7 @@@ static int iterate_ref_map(void *cb_dat
        if (!ref)
                return -1; /* end of the list */
        *rm = ref->next;
 -      hashcpy(sha1, ref->old_sha1);
 +      hashcpy(sha1, ref->old_oid.hash);
        return 0;
  }
  
@@@ -631,7 -629,7 +631,7 @@@ static int store_updated_refs(const cha
                                continue;
                        }
  
 -                      commit = lookup_commit_reference_gently(rm->old_sha1, 1);
 +                      commit = lookup_commit_reference_gently(rm->old_oid.hash, 1);
                        if (!commit)
                                rm->fetch_head_status = FETCH_HEAD_NOT_FOR_MERGE;
  
                                continue;
  
                        if (rm->peer_ref) {
 -                              ref = xcalloc(1, sizeof(*ref) + strlen(rm->peer_ref->name) + 1);
 -                              strcpy(ref->name, rm->peer_ref->name);
 -                              hashcpy(ref->old_sha1, rm->peer_ref->old_sha1);
 -                              hashcpy(ref->new_sha1, rm->old_sha1);
 +                              ref = alloc_ref(rm->peer_ref->name);
 +                              oidcpy(&ref->old_oid, &rm->peer_ref->old_oid);
 +                              oidcpy(&ref->new_oid, &rm->old_oid);
                                ref->force = rm->peer_ref->force;
                        }
  
                                /* fall-through */
                        case FETCH_HEAD_MERGE:
                                fprintf(fp, "%s\t%s\t%s",
 -                                      sha1_to_hex(rm->old_sha1),
 +                                      oid_to_hex(&rm->old_oid),
                                        merge_status_marker,
                                        note.buf);
                                for (i = 0; i < url_len; ++i)
@@@ -837,7 -836,7 +837,7 @@@ static void check_not_current_branch(st
  static int truncate_fetch_head(void)
  {
        const char *filename = git_path_fetch_head();
-       FILE *fp = fopen(filename, "w");
+       FILE *fp = fopen_for_writing(filename);
  
        if (!fp)
                return error(_("cannot open %s: %s\n"), filename, strerror(errno));
@@@ -928,7 -927,7 +928,7 @@@ static int do_fetch(struct transport *t
                                                   rm->peer_ref->name);
                        if (peer_item) {
                                struct object_id *old_oid = peer_item->util;
 -                              hashcpy(rm->peer_ref->old_sha1, old_oid->hash);
 +                              oidcpy(&rm->peer_ref->old_oid, old_oid);
                        }
                }
        }
@@@ -1157,8 -1156,11 +1157,8 @@@ int cmd_fetch(int argc, const char **ar
                        die(_("--depth and --unshallow cannot be used together"));
                else if (!is_repository_shallow())
                        die(_("--unshallow on a complete repository does not make sense"));
 -              else {
 -                      static char inf_depth[12];
 -                      sprintf(inf_depth, "%d", INFINITE_DEPTH);
 -                      depth = inf_depth;
 -              }
 +              else
 +                      depth = xstrfmt("%d", INFINITE_DEPTH);
        }
  
        /* no need to be strict, transport_set_option() will validate it again */
diff --combined git-compat-util.h
index 2da0a75a380da198bd65fe3d9b0c95ef98a0b4aa,d98f3af5e7627e2608a2c83ac4d260e2ee12a348..e8f286722864e13e8d5e1ad1e7380c586e505320
@@@ -229,7 -229,7 +229,7 @@@ typedef unsigned long uintptr_t
  #else
  #define precompose_str(in,i_nfd2nfc)
  #define precompose_argv(c,v)
 -#define probe_utf8_pathname_composition(a,b)
 +#define probe_utf8_pathname_composition()
  #endif
  
  #ifdef MKDIR_WO_TRAILING_SLASH
@@@ -733,6 -733,7 +733,7 @@@ extern int xmkstemp_mode(char *template
  extern int odb_mkstemp(char *template, size_t limit, const char *pattern);
  extern int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1);
  extern char *xgetcwd(void);
+ extern FILE *fopen_for_writing(const char *path);
  
  #define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), (alloc) * sizeof(*(x)))
  
@@@ -748,9 -749,6 +749,9 @@@ static inline size_t xsize_t(off_t len
        return (size_t)len;
  }
  
 +__attribute__((format (printf, 3, 4)))
 +extern int xsnprintf(char *dst, size_t max, const char *fmt, ...);
 +
  /* in ctype.c, for kwset users */
  extern const unsigned char tolower_trans_tbl[256];
  
@@@ -821,9 -819,6 +822,9 @@@ static inline int strtoul_ui(char cons
        char *p;
  
        errno = 0;
 +      /* negative values would be accepted by strtoul */
 +      if (strchr(s, '-'))
 +              return -1;
        ul = strtoul(s, &p, base);
        if (errno || *p || p == s || (unsigned int) ul != ul)
                return -1;
diff --combined wrapper.c
index c95e2906b850749cbeb109050359b671979969f2,371a7f0b63719f0a81ee64abb17b4ca6dbf9ba16..52001789ded0372becd3f55dda4cd1226c4b6dae
+++ b/wrapper.c
@@@ -375,6 -375,19 +375,19 @@@ FILE *xfdopen(int fd, const char *mode
        return stream;
  }
  
+ FILE *fopen_for_writing(const char *path)
+ {
+       FILE *ret = fopen(path, "w");
+       if (!ret && errno == EPERM) {
+               if (!unlink(path))
+                       ret = fopen(path, "w");
+               else
+                       errno = EPERM;
+       }
+       return ret;
+ }
  int xmkstemp(char *template)
  {
        int fd;
@@@ -609,22 -622,6 +622,22 @@@ char *xgetcwd(void
        return strbuf_detach(&sb, NULL);
  }
  
 +int xsnprintf(char *dst, size_t max, const char *fmt, ...)
 +{
 +      va_list ap;
 +      int len;
 +
 +      va_start(ap, fmt);
 +      len = vsnprintf(dst, max, fmt, ap);
 +      va_end(ap);
 +
 +      if (len < 0)
 +              die("BUG: your snprintf is broken");
 +      if (len >= max)
 +              die("BUG: attempt to snprintf into too-small buffer");
 +      return len;
 +}
 +
  static int write_file_v(const char *path, int fatal,
                        const char *fmt, va_list params)
  {