On Fri, Jun 19, 2015 at 4:26 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Paul Tan <pyokagan@xxxxxxxxx> writes:
>> @@ -0,0 +1,28 @@
>> +/*
>> + * Builtin "git am"
>> + *
>> + * Based on git-am.sh by Junio C Hamano.
>> + */
>> +#include "cache.h"
>> +#include "builtin.h"
>> +#include "exec_cmd.h"
>> +
>> +int cmd_am(int argc, const char **argv, const char *prefix)
>> +{
>> + /*
>> + * FIXME: Once all the features of git-am.sh have been re-implemented
>> + * in builtin/am.c, this preamble can be removed.
>> + */
>
> It's not broken, so "FIXME" is not quite appropriate (and that is
> why I sent you "NEEDSWORK").
OK.
> Also mention that the entry in the
> commands[] array needs "RUN_SETUP | NEED_WORK_TREE" added, I think.
OK.
>> + if (!getenv("_GIT_USE_BUILTIN_AM")) {
>> + const char *path = mkpath("%s/git-am", git_exec_path());
>> +
>> + if (sane_execvp(path, (char **)argv) < 0)
>> + die_errno("could not exec %s", path);
>> + } else {
>> + prefix = setup_git_directory();
>> + trace_repo_setup(prefix);
>> + setup_work_tree();
>> + }
>> +
>> + return 0;
>> +}
>> diff --git a/git.c b/git.c
>> index e7a7713..a671535 100644
>> --- a/git.c
>> +++ b/git.c
>> @@ -370,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
>>
>> static struct cmd_struct commands[] = {
>> { "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
>> + { "am", cmd_am, NO_SETUP },
>
> NO_SETUP is for things like init and clone that start without a
> repository and then work in the one that they create. I think
> imitating "archive" or "diff" is more appropriate.
Ah OK. I thought that handle_alias() in git.c would chdir() and set
GIT_DIR because it called setup_git_directory_gently(), and thus
requires NO_SETUP to restore the initial environment, but turns out it
chdir()s back to the original directory, and sets GIT_DIR
appropriately, so we're OK.
Thanks,
Paul
--
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