Carlos Rica <jasampler@gmail.com> writes:

quoted text
> +static int unmerged_files(void)
> +{
> +	char b;
> +	ssize_t len;
> +	struct child_process cmd;
> +	const char *argv_ls_files[] = {"ls-files", "--unmerged", NULL};
> +
> +	memset(&cmd, 0, sizeof(cmd));
> +	cmd.argv = argv_ls_files;
> +	cmd.git_cmd = 1;
> +	cmd.out = -1;
> +
> +	if (start_command(&cmd))
> +		die("Could not run sub-command: git ls-files");
> +
> +	len = xread(cmd.out, &b, 1);
> +	if (len < 0)
> +		die("Could not read output from git ls-files: %s",
> +						strerror(errno));
> +	finish_command(&cmd);
> +
> +	return len;
> +}

Not that git-reset is so performance sensitive, but you could do
this from built-in without exec, by just reading the index and
checking if you have a higher-stage entry yourself.

quoted text
> +static int update_index_refresh(void)
> +{
> +	const char *argv_update_index[] = {"update-index", "--refresh", NULL};
> +	return run_command_v_opt(argv_update_index, RUN_GIT_CMD);
> +}

Instead of making a call to this one after read_from_tree()
returns, immediately before writing the index out at the end of
read_from_tree(), you have the index in core.  I think you can
call read-cache.c::refresh_index() at that point and then do the
write_cache(), no?

quoted text
> +static int read_from_tree(const char *prefix, const char **argv,
> +		unsigned char *tree_sha1)
> +{
> +        struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
> +	int index_fd;
> +	struct diff_options opt;

s/^ {8}/\t/;

quoted text
> +
> +	memset(&opt, 0, sizeof(opt));
> +	diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);
> +	opt.output_format = DIFF_FORMAT_CALLBACK;
> +	opt.format_callback = update_index_from_diff;
> +
> +	index_fd = hold_locked_index(lock, 1);
> +	read_cache();
> +	if (do_diff_cache(tree_sha1, &opt))
> +		return 1;
> +	diffcore_std(&opt);
> +	diff_flush(&opt);
> +	return write_cache(index_fd, active_cache, active_nr) ||
> +		close(index_fd) ||
> +		commit_locked_index(lock);
> +}

This part is interesting for a couple of reasons.

quoted text
> +	if (i < argc && argv[i][0] != '-')
> +		rev = argv[i++];
> +
> +	if (get_sha1(rev, sha1))
> +		die("Failed to resolve '%s' as a valid ref.", rev);
> +
> +	commit = lookup_commit_reference(sha1);
> +	if (!commit)
> +		die("Could not parse object '%s'.", rev);
> +	hashcpy(sha1, commit->object.sha1);

 * When we can have both rev and path params, and the user calls
   us without '--' disambiguation, we make sure the rev
   parameter (i.e. the first non-option) cannot be interpreted
   as path and the path parameters (i.e. the rest) cannot be
   interpreted as rev, but in git-reset traditionally we did not
   do so.  Maybe we'd want to, but that would be a very isolated
   change and does not have to be a part of this round.  Just
   something to keep in mind...

 * You hint in a comment in the later part of the code that "git
   reset <tree-ish> -- paths" is to selectively read-tree into
   the index.  I do not thinkn we advertize git-reset as such,
   but we do not have to forbid non commit here.  Not that I
   think allowing arbitrary tree object is not so useful in
   practice.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html