Miklos Vajna <vmiklos@xxxxxxxxxxxxxx> writes:
> +static int read_tree_trivial(unsigned char *common, unsigned char *head,
> + unsigned char *one)
> +{
> + int i, nr_trees = 0;
> + struct tree *trees[MAX_UNPACK_TREES];
> + struct tree_desc t[MAX_UNPACK_TREES];
> + struct unpack_trees_options opts;
> +
> + memset(&opts, 0, sizeof(opts));
> + opts.head_idx = 2;
Do you still need this assignment here?
> +int cmd_merge(int argc, const char **argv, const char *prefix)
> +{
> + unsigned char result_tree[20];
> + struct strbuf buf;
> + const char *head_arg;
> + int flag, head_invalid = 0, i;
> + int best_cnt = -1, merge_was_ok = 0, automerge_was_ok = 0;
> + struct commit_list *common = NULL;
> + struct path_list_item *best_strategy = NULL, *wt_strategy = NULL;
> + struct commit_list **remotes = &remoteheads;
> +
> + setup_work_tree();
> + if (unmerged_cache())
> + die("You are in the middle of a conflicted merge.");
> +
> + /*
> + * Check if we are _not_ on a detached HEAD, i.e. if there is a
> + * current branch.
> + */
> + branch = resolve_ref("HEAD", head, 0, &flag);
> + if (branch && flag & REF_ISSYMREF) {
> + const char *ptr = skip_prefix(branch, "refs/heads/");
> + if (ptr)
> + branch = ptr;
> + } else
> + head_invalid = 1;
Wait a minute... Are you calling a detached HEAD as head_invalid here? I
am not too much worried about variable naming, but you later do ...
> + if (!have_message && is_old_style_invocation(argc, argv)) {
> ...
> + } else if (head_invalid) {
> + struct object *remote_head;
> + /*
> + * If the merged head is a valid one there is no reason
> + * to forbid "git merge" into a branch yet to be born.
> + * We do the same for "git pull".
> + */
> + if (argc != 1)
> + die("Can merge only exactly one commit into "
> + "empty head");
Which is about HEAD pointing at a branch that isn't born yet. They are
two very different concepts.
Either the above "else if (head_invalid)" is wrong, or more likely the
setting of head_invalid we saw earlier is wrong.
Probably what you meant was:
- "char *branch" points at either "HEAD" (when detached) or
the name of the branch (e.g. "master" when "refs/heads/master");
- "unsigned char head[]" stores the commit object name of the
current HEAD (or 0{40} if the current branch is unborn);
- set head_invalid to true only when the current branch is unborn.
So perhaps...
branch = resolve_ref("HEAD", head, 0, &flag);
if (branch && (flag & REF_ISSYMREF) && !prefixcmp(branch, "refs/heads/"))
branch += 11;
head_invalid = is_null_sha1(head);
And probably we can even drop (flag & REF_ISSYMREF) from the above safely.
Do we even care if the head is detached in this program? I doubt it.
> ...
> + strbuf_init(&msg, 0);
> + strbuf_addstr(&msg, "Fast forward");
> + if (have_message)
> + strbuf_addstr(&msg,
> + " (no commit created; -m option ignored)");
> + o = peel_to_type(sha1_to_hex(remoteheads->item->object.sha1),
> + 0, NULL, OBJ_COMMIT);
> + if (!o)
> + return 1;
> +
> + if (checkout_fast_forward(head, remoteheads->item->object.sha1))
> + return 1;
Not exiting with 0 status from around here upon error is an improvement,
but does the user see sensible error messages in addition to the exit
status?
Getting warmer ;-)
--
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
