Re: [PATCH v5 08/10] rebase -i: skip unnecessary picks using the rebase--helper

[Liam Beguin <liambeguin@gmail.com>]

Hi, 

On 14/06/17 09:08 AM, Johannes Schindelin wrote:
> diff --git a/sequencer.c b/sequencer.c
> index a697906d463..a0e020dab09 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2640,3 +2640,110 @@ int check_todo_list(void)
>  
>  	return res;
>  }
> +
> +/* skip picking commits whose parents are unchanged */
> +int skip_unnecessary_picks(void)
> +{
> +	const char *todo_file = rebase_path_todo();
> +	struct strbuf buf = STRBUF_INIT;
> +	struct todo_list todo_list = TODO_LIST_INIT;
> +	struct object_id onto_oid, *oid = &onto_oid, *parent_oid;
> +	int fd, i;
> +
> +	if (!read_oneliner(&buf, rebase_path_onto(), 0))
> +		return error(_("could not read 'onto'"));
> +	if (get_sha1(buf.buf, onto_oid.hash)) {

I missed this last time but we could also replace `get_sha1` with `get_oid`

> +		strbuf_release(&buf);
> +		return error(_("need a HEAD to fixup"));
> +	}
> +	strbuf_release(&buf);
> +
> +	fd = open(todo_file, O_RDONLY);
> +	if (fd < 0) {
> +		return error_errno(_("could not open '%s'"), todo_file);
> +	}
> +	if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
> +		close(fd);
> +		return error(_("could not read '%s'."), todo_file);
> +	}
> +	close(fd);
> +	if (parse_insn_buffer(todo_list.buf.buf, &todo_list) < 0) {
> +		todo_list_release(&todo_list);
> +		return -1;
> +	}
> +
> +	for (i = 0; i < todo_list.nr; i++) {
> +		struct todo_item *item = todo_list.items + i;
> +
> +		if (item->command >= TODO_NOOP)
> +			continue;
> +		if (item->command != TODO_PICK)
> +			break;
> +		if (parse_commit(item->commit)) {
> +			todo_list_release(&todo_list);
> +			return error(_("could not parse commit '%s'"),
> +				oid_to_hex(&item->commit->object.oid));
> +		}
> +		if (!item->commit->parents)
> +			break; /* root commit */
> +		if (item->commit->parents->next)
> +			break; /* merge commit */
> +		parent_oid = &item->commit->parents->item->object.oid;
> +		if (hashcmp(parent_oid->hash, oid->hash))
> +			break;
> +		oid = &item->commit->object.oid;
> +	}
> +	if (i > 0) {
> +		int offset = i < todo_list.nr ?
> +			todo_list.items[i].offset_in_buf : todo_list.buf.len;
> +		const char *done_path = rebase_path_done();
> +
> +		fd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666);
> +		if (fd < 0) {
> +			error_errno(_("could not open '%s' for writing"),
> +				    done_path);
> +			todo_list_release(&todo_list);
> +			return -1;
> +		}
> +		if (write_in_full(fd, todo_list.buf.buf, offset) < 0) {
> +			error_errno(_("could not write to '%s'"), done_path);
> +			todo_list_release(&todo_list);
> +			close(fd);
> +			return -1;
> +		}
> +		close(fd);
> +
> +		fd = open(rebase_path_todo(), O_WRONLY, 0666);
> +		if (fd < 0) {
> +			error_errno(_("could not open '%s' for writing"),
> +				    rebase_path_todo());
> +			todo_list_release(&todo_list);
> +			return -1;
> +		}
> +		if (write_in_full(fd, todo_list.buf.buf + offset,
> +				todo_list.buf.len - offset) < 0) {
> +			error_errno(_("could not write to '%s'"),
> +				    rebase_path_todo());
> +			close(fd);
> +			todo_list_release(&todo_list);
> +			return -1;
> +		}
> +		if (ftruncate(fd, todo_list.buf.len - offset) < 0) {
> +			error_errno(_("could not truncate '%s'"),
> +				    rebase_path_todo());
> +			todo_list_release(&todo_list);
> +			close(fd);
> +			return -1;
> +		}
> +		close(fd);
> +
> +		todo_list.current = i;
> +		if (is_fixup(peek_command(&todo_list, 0)))
> +			record_in_rewritten(oid, peek_command(&todo_list, 0));
> +	}
> +
> +	todo_list_release(&todo_list);
> +	printf("%s\n", oid_to_hex(oid));
> +
> +	return 0;
> +}
> diff --git a/sequencer.h b/sequencer.h
> index 878dd296f8c..04a57e09a1d 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -50,6 +50,7 @@ int sequencer_make_script(int keep_empty, FILE *out,
>  
>  int transform_todo_ids(int shorten_ids);
>  int check_todo_list(void);
> +int skip_unnecessary_picks(void);
>  
>  extern const char sign_off_header[];
>  
> 

 - Liam