{"thread":{"id":"8244","subject":"[PATCH] allow commands to be executed in submodules","startedAt":"2007-05-20T15:39:08Z","lastAt":"2007-05-23T20:21:39Z","messageCount":20,"participants":["Martin Waitz","Alex Riesen","Junio C Hamano","Shawn O. Pearce","Sven Verdoolaege"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"42717","messageId":"20070520153908.GF5412@admingilde.org","threadId":"8244","inReplyTo":null,"subject":"[PATCH] allow commands to be executed in submodules","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-05-20T15:39:08Z","receivedAt":"2007-05-20T15:39:08Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"Add an extra \"submodule\" field to struct child_process to be able to\neasily start commands which are to be executed in a submodule\nrepository.\n\nSigned-off-by: Martin Waitz <tali@admingilde.org>\n---\n\n run-command.c     |   13 ++++++\n run-command.h     |    1 +\n 2 files changed, 14 insertions(+), 0 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex eff523e..c2475e4 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -73,6 +73,19 @@ int start_command(struct child_process *cmd)\n \t\t\tclose(cmd->out);\n \t\t}\n \n+\t\tif (cmd->submodule) {\n+\t\t\tint err = chdir(cmd->submodule);\n+\t\t\tif (err) {\n+\t\t\t\tdie(\"cannot exec %s in %s.\",\n+\t\t\t\t\tcmd->argv[0], cmd->submodule);\n+\t\t\t}\n+\t\t\t/* don't inherit supermodule environment */\n+\t\t\tunsetenv(GIT_DIR_ENVIRONMENT);\n+\t\t\tunsetenv(DB_ENVIRONMENT);\n+\t\t\tunsetenv(INDEX_ENVIRONMENT);\n+\t\t\tunsetenv(GRAFT_ENVIRONMENT);\n+\t\t}\n+\n \t\tif (cmd->git_cmd) {\n \t\t\texecv_git_cmd(cmd->argv);\n \t\t} else {\ndiff --git a/run-command.h b/run-command.h\nindex 3680ef9..2940186 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -16,6 +16,7 @@ struct child_process {\n \tpid_t pid;\n \tint in;\n \tint out;\n+\tconst char *submodule;\n \tunsigned close_in:1;\n \tunsigned close_out:1;\n \tunsigned no_stdin:1;\n-- \n1.5.2.2.g081e\n\n\n-- \nMartin Waitz\n"},{"id":"42753","messageId":"20070520181433.GA19668@steel.home","threadId":"8244","inReplyTo":"20070520153908.GF5412@admingilde.org","subject":"Re: [PATCH] allow commands to be executed in submodules","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-05-20T18:14:33Z","receivedAt":"2007-05-20T18:14:33Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Martin Waitz, Sun, May 20, 2007 17:39:08 +0200:\n> Add an extra \"submodule\" field to struct child_process to be able to\n> easily start commands which are to be executed in a submodule\n> repository.\n\nHow about making it more generic by allowing to specify the directory\nto change to and environment for subprocess? You probably will be able\nto convert even some of existing code to your new run_command then\n(merge_recursive in builtin-revert.c, for example).\n\nSomething like this, perhaps:\n\ndiff --git a/run-command.c b/run-command.c\nindex eff523e..605aa1e 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -73,6 +73,13 @@ int start_command(struct child_process *cmd)\n \t\t\tclose(cmd->out);\n \t\t}\n \n+\t\tif (cmd->dir && chdir(cmd->dir))\n+\t\t\tdie(\"exec %s: cd to %s failed (%s)\", cmd->argv[0],\n+\t\t\t    cmd->dir, strerror(errno));\n+\t\tif (cmd->env) {\n+\t\t\tfor (; *cmd->env; cmd->env++)\n+\t\t\t\tputenv((char*)*cmd->env);\n+\t\t}\n \t\tif (cmd->git_cmd) {\n \t\t\texecv_git_cmd(cmd->argv);\n \t\t} else {\n@@ -133,13 +140,38 @@ int run_command(struct child_process *cmd)\n \treturn finish_command(cmd);\n }\n \n+static void prepare_run_command_v_opt(struct child_process *cmd,\n+\t\t\t\t      const char **argv,\n+\t\t\t\t      int opt)\n+{\n+\tmemset(cmd, 0, sizeof(*cmd));\n+\tcmd->argv = argv;\n+\tcmd->no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0;\n+\tcmd->git_cmd = opt & RUN_GIT_CMD ? 1 : 0;\n+\tcmd->stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;\n+}\n+\n int run_command_v_opt(const char **argv, int opt)\n {\n \tstruct child_process cmd;\n-\tmemset(&cmd, 0, sizeof(cmd));\n-\tcmd.argv = argv;\n-\tcmd.no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0;\n-\tcmd.git_cmd = opt & RUN_GIT_CMD ? 1 : 0;\n-\tcmd.stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;\n+\tprepare_run_command_v_opt(&cmd, argv, opt);\n \treturn run_command(&cmd);\n }\n+\n+int run_command_v_opt_cd(const char **argv, int opt, const char *dir)\n+{\n+\tstruct child_process cmd;\n+\tprepare_run_command_v_opt(&cmd, argv, opt);\n+\tcmd.dir = dir;\n+\treturn run_command(&cmd);\n+}\n+\n+int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const char *const *env)\n+{\n+\tstruct child_process cmd;\n+\tprepare_run_command_v_opt(&cmd, argv, opt);\n+\tcmd.dir = dir;\n+\tcmd.env = env;\n+\treturn run_command(&cmd);\n+}\n+\ndiff --git a/run-command.h b/run-command.h\nindex 3680ef9..af1e0bf 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -16,6 +16,8 @@ struct child_process {\n \tpid_t pid;\n \tint in;\n \tint out;\n+\tconst char *dir;\n+\tconst char *const *env;\n \tunsigned close_in:1;\n \tunsigned close_out:1;\n \tunsigned no_stdin:1;\n@@ -32,5 +34,7 @@ int run_command(struct child_process *);\n #define RUN_GIT_CMD\t     2\t/*If this is to be git sub-command */\n #define RUN_COMMAND_STDOUT_TO_STDERR 4\n int run_command_v_opt(const char **argv, int opt);\n+int run_command_v_opt_cd(const char **argv, int opt, const char *dir);\n+int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const char *const *env);\n \n #endif\n"},{"id":"42755","messageId":"7vhcq7mjxn.fsf@assigned-by-dhcp.cox.net","threadId":"8244","inReplyTo":"20070520181433.GA19668@steel.home","subject":"Re: [PATCH] allow commands to be executed in submodules","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-20T18:25:24Z","receivedAt":"2007-05-20T18:25:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen <raa.lkml@gmail.com> writes:\n\n> Martin Waitz, Sun, May 20, 2007 17:39:08 +0200:\n>> Add an extra \"submodule\" field to struct child_process to be able to\n>> easily start commands which are to be executed in a submodule\n>> repository.\n>\n> How about making it more generic by allowing to specify the directory\n> to change to and environment for subprocess? You probably will be able\n> to convert even some of existing code to your new run_command then\n> (merge_recursive in builtin-revert.c, for example).\n\nSounds useful and more generic.\n"},{"id":"42788","messageId":"20070520204801.GH5412@admingilde.org","threadId":"8244","inReplyTo":"7vhcq7mjxn.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] allow commands to be executed in submodules","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-05-20T20:48:02Z","receivedAt":"2007-05-20T20:48:02Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Sun, May 20, 2007 at 11:25:24AM -0700, Junio C Hamano wrote:\n> Sounds useful and more generic.\n\nI explicitly wanted to have a method to execute one command in\nthe environment of a submodule.  That way we can update it in\none place if we later add more environment variables which\ninfluence the repository.\n\nDo we really have so many places where we want to execute commands\nin a different directory or with different environment?  Is it worth\nkeeping run-command generic and having to introduce knowledge about\nhow to run submodule commands in multiple places?\n\nThat said I don't have any strong feeling about it, as long as one\nor the other patch is applied.\n\n-- \nMartin Waitz\n"},{"id":"42791","messageId":"20070520205933.GD25462@steel.home","threadId":"8244","inReplyTo":"20070520204801.GH5412@admingilde.org","subject":"Re: [PATCH] allow commands to be executed in submodules","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-05-20T20:59:33Z","receivedAt":"2007-05-20T20:59:33Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Martin Waitz, Sun, May 20, 2007 22:48:02 +0200:\n> Do we really have so many places where we want to execute commands\n> in a different directory or with different environment?  Is it worth\n> keeping run-command generic and having to introduce knowledge about\n> how to run submodule commands in multiple places?\n\nIs there multiple places? Is it hard to create a specific function out\nof a generic one? (which can be used from other places and your\nspecific can't and we would need the generic one anyway).\n\n\"Generic\" is not about \"multiple places\". Generic is about \"general\"\nas opposite to \"specific\". Gives you flexibility and wider application\nrange.\n"},{"id":"42793","messageId":"20070520210827.GI5412@admingilde.org","threadId":"8244","inReplyTo":"20070520205933.GD25462@steel.home","subject":"Re: [PATCH] allow commands to be executed in submodules","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-05-20T21:08:27Z","receivedAt":"2007-05-20T21:08:27Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Sun, May 20, 2007 at 10:59:33PM +0200, Alex Riesen wrote:\n> Is there multiple places? Is it hard to create a specific function out\n> of a generic one? (which can be used from other places and your\n> specific can't and we would need the generic one anyway).\n\nyou can add a specific new function for submodules, dropping the\nnice property of child_process that you only have to initialize a few\nfields and then can run the command.\n\n> \"Generic\" is not about \"multiple places\". Generic is about \"general\"\n> as opposite to \"specific\". Gives you flexibility and wider application\n> range.\n\nGeneric code and abstractions only make sense when they are _useful_.\nLets not overengineer it.  If we later see that we need more, then\nso be it.  KISS.\n\nBut now lets go on and don't discuss about such details.\nI'm happy if I can run commands in submodules and don't care about the\nactual code.\n\n-- \nMartin Waitz\n"},{"id":"42932","messageId":"20070521224828.GA10890@steel.home","threadId":"8244","inReplyTo":"20070521090339.GH942MdfPADPa@greensroom.kotnet.org","subject":"[PATCH] Add ability to specify environment extension to run_command","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-05-21T22:48:28Z","receivedAt":"2007-05-21T22:48:28Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"There is no way to specify and override for the environment: there is\nno visible user for it (yet, something in git-daemon could need it).\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nSven Verdoolaege, Mon, May 21, 2007 11:03:39 +0200:\n> Could you sign-off on this for me so I can use it my patch set?\n> \n\nSo here it is. On top of the previos patch regarding chdir before\nexec. Junio, if needed, I can resend that first patch about chdir.\n\n run-command.c |   17 ++++++++++++++++-\n run-command.h |    2 ++\n 2 files changed, 18 insertions(+), 1 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 043b570..605aa1e 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -76,6 +76,10 @@ int start_command(struct child_process *cmd)\n \t\tif (cmd->dir && chdir(cmd->dir))\n \t\t\tdie(\"exec %s: cd to %s failed (%s)\", cmd->argv[0],\n \t\t\t    cmd->dir, strerror(errno));\n+\t\tif (cmd->env) {\n+\t\t\tfor (; *cmd->env; cmd->env++)\n+\t\t\t\tputenv((char*)*cmd->env);\n+\t\t}\n \t\tif (cmd->git_cmd) {\n \t\t\texecv_git_cmd(cmd->argv);\n \t\t} else {\n@@ -137,7 +141,8 @@ int run_command(struct child_process *cmd)\n }\n \n static void prepare_run_command_v_opt(struct child_process *cmd,\n-\t\t\t\t      const char **argv, int opt)\n+\t\t\t\t      const char **argv,\n+\t\t\t\t      int opt)\n {\n \tmemset(cmd, 0, sizeof(*cmd));\n \tcmd->argv = argv;\n@@ -160,3 +165,13 @@ int run_command_v_opt_cd(const char **argv, int opt, const char *dir)\n \tcmd.dir = dir;\n \treturn run_command(&cmd);\n }\n+\n+int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const char *const *env)\n+{\n+\tstruct child_process cmd;\n+\tprepare_run_command_v_opt(&cmd, argv, opt);\n+\tcmd.dir = dir;\n+\tcmd.env = env;\n+\treturn run_command(&cmd);\n+}\n+\ndiff --git a/run-command.h b/run-command.h\nindex cbd7484..af1e0bf 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -17,6 +17,7 @@ struct child_process {\n \tint in;\n \tint out;\n \tconst char *dir;\n+\tconst char *const *env;\n \tunsigned close_in:1;\n \tunsigned close_out:1;\n \tunsigned no_stdin:1;\n@@ -34,5 +35,6 @@ int run_command(struct child_process *);\n #define RUN_COMMAND_STDOUT_TO_STDERR 4\n int run_command_v_opt(const char **argv, int opt);\n int run_command_v_opt_cd(const char **argv, int opt, const char *dir);\n+int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const char *const *env);\n \n #endif\n-- \n1.5.2.rc3.112.gc1e43\n"},{"id":"42933","messageId":"7v7ir1dbl9.fsf@assigned-by-dhcp.cox.net","threadId":"8244","inReplyTo":"20070521224828.GA10890@steel.home","subject":"Re: [PATCH] Add ability to specify environment extension to run_command","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-21T23:02:42Z","receivedAt":"2007-05-21T23:02:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen <raa.lkml@gmail.com> writes:\n\n> There is no way to specify and override for the environment: there is\n> no visible user for it (yet, something in git-daemon could need it).\n>\n> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>\n> ---\n>\n> Sven Verdoolaege, Mon, May 21, 2007 11:03:39 +0200:\n>> Could you sign-off on this for me so I can use it my patch set?\n>> \n>\n> So here it is. On top of the previos patch regarding chdir before\n> exec. Junio, if needed, I can resend that first patch about chdir.\n\nBoth of them in a row would be good, so yes, resend is\nappreciated.\n\n> @@ -76,6 +76,10 @@ int start_command(struct child_process *cmd)\n>  \t\tif (cmd->dir && chdir(cmd->dir))\n>  \t\t\tdie(\"exec %s: cd to %s failed (%s)\", cmd->argv[0],\n>  \t\t\t    cmd->dir, strerror(errno));\n> +\t\tif (cmd->env) {\n> +\t\t\tfor (; *cmd->env; cmd->env++)\n> +\t\t\t\tputenv((char*)*cmd->env);\n> +\t\t}\n>  \t\tif (cmd->git_cmd) {\n>  \t\t\texecv_git_cmd(cmd->argv);\n>  \t\t} else {\n\nI had a feeling that some callers needed to be able to unsetenv\nsome.  How would this patch help them, or are they outside of\nthe scope?\n"},{"id":"42946","messageId":"20070522060302.GH5412@admingilde.org","threadId":"8244","inReplyTo":"7v7ir1dbl9.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add ability to specify environment extension to run_command","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-05-22T06:03:02Z","receivedAt":"2007-05-22T06:03:02Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Mon, May 21, 2007 at 04:02:42PM -0700, Junio C Hamano wrote:\n> I had a feeling that some callers needed to be able to unsetenv\n> some.  How would this patch help them, or are they outside of\n> the scope?\n\nAt first I had the same objection but the putenv documentation\ntold me that at least in glibc you can unsetenv by providing\nthe variable name without a \"=\".\n\nBut perhaps we should check for other systems?\n\n-- \nMartin Waitz\n"},{"id":"42952","messageId":"7v646l9xkn.fsf@assigned-by-dhcp.cox.net","threadId":"8244","inReplyTo":"20070522060302.GH5412@admingilde.org","subject":"Re: [PATCH] Add ability to specify environment extension to run_command","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-22T06:33:44Z","receivedAt":"2007-05-22T06:33:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Waitz <tali@admingilde.org> writes:\n\n> On Mon, May 21, 2007 at 04:02:42PM -0700, Junio C Hamano wrote:\n>> I had a feeling that some callers needed to be able to unsetenv\n>> some.  How would this patch help them, or are they outside of\n>> the scope?\n>\n> At first I had the same objection but the putenv documentation\n> told me that at least in glibc you can unsetenv by providing\n> the variable name without a \"=\".\n\nI recall SysV putenv() does not remove \"ENVNAME\" without '=', and\nhttp://www.opengroup.org/onlinepubs/000095399/functions/putenv.html\nseems to say that as well.\n\n> But perhaps we should check for other systems?\n\nProbably.\n\nAt least we have setenv/unsetenv calls already (with emulation\nwhere they aren't available), we could do something like this:\n\n\tstruct child_process {\n        \t...\n                const struct {\n                        const char *name;\n                        const char *value; /* NULL to unsetenv */\n                } *env;\n\t\t...\n\t};\n\nand in start_command():\n\n\tif (cmd->env) {\n\t\tint i;\n                for (i = 0; cmd->env[i].name; i++) {\n\t\t\tif (cmd->env[i].value)\n\t\t\t\tsetenv(cmd->env[i].name, cmd->env[i].value, 1);\n\t\t\telse\n\t\t\t\tunsetenv(cmd->env[i].name);\n                }\n        }\n"},{"id":"42953","messageId":"20070522063821.GE11636@spearce.org","threadId":"8244","inReplyTo":"7v646l9xkn.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add ability to specify environment extension to run_command","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-05-22T06:38:22Z","receivedAt":"2007-05-22T06:38:22Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <junkio@cox.net> wrote:\n> Martin Waitz <tali@admingilde.org> writes:\n> \n> > On Mon, May 21, 2007 at 04:02:42PM -0700, Junio C Hamano wrote:\n> >> I had a feeling that some callers needed to be able to unsetenv\n> >> some.  How would this patch help them, or are they outside of\n> >> the scope?\n> >\n> > At first I had the same objection but the putenv documentation\n> > told me that at least in glibc you can unsetenv by providing\n> > the variable name without a \"=\".\n> \n> I recall SysV putenv() does not remove \"ENVNAME\" without '=', and\n> http://www.opengroup.org/onlinepubs/000095399/functions/putenv.html\n> seems to say that as well.\n\nAre we overbuilding this thing?\n\nI thought this thread all started because we wanted to run a\ncommand in a subproject, and did not want the parent's GIT_*\nenvironment variables to confuse the subproject process when\nit started.  That's a pretty simple concept: clear any GIT_*\nenvironment variable that can change behavior in the subproject.\nAnd almost everyone who is trying to use this API and alter the\nenv wants exactly that - a subproject command.\n\n-- \nShawn.\n"},{"id":"42956","messageId":"20070522065427.GS942MdfPADPa@greensroom.kotnet.org","threadId":"8244","inReplyTo":"20070522063821.GE11636@spearce.org","subject":"Re: [PATCH] Add ability to specify environment extension to run_command","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-05-22T06:54:27Z","receivedAt":"2007-05-22T06:54:27Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Tue, May 22, 2007 at 02:38:22AM -0400, Shawn O. Pearce wrote:\n> I thought this thread all started because we wanted to run a\n> command in a subproject, and did not want the parent's GIT_*\n> environment variables to confuse the subproject process when\n> it started.  That's a pretty simple concept: clear any GIT_*\n> environment variable that can change behavior in the subproject.\n> And almost everyone who is trying to use this API and alter the\n> env wants exactly that - a subproject command.\n\nThis would work for me, I suppose.\nRight now, I sometimes set GIT_DIR explicitly, but I could just chdir\n(or use --git-dir=).\n\nskimo\n"},{"id":"42993","messageId":"20070522214754.GD30871@steel.home","threadId":"8244","inReplyTo":"7v7ir1dbl9.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add ability to specify environment extension to run_command","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-05-22T21:47:54Z","receivedAt":"2007-05-22T21:47:54Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Tue, May 22, 2007 01:02:42 +0200:\n> >\n> > So here it is. On top of the previos patch regarding chdir before\n> > exec. Junio, if needed, I can resend that first patch about chdir.\n> \n> Both of them in a row would be good, so yes, resend is\n> appreciated.\n\nWill be resent.\n\n> > @@ -76,6 +76,10 @@ int start_command(struct child_process *cmd)\n> >  \t\tif (cmd->dir && chdir(cmd->dir))\n> >  \t\t\tdie(\"exec %s: cd to %s failed (%s)\", cmd->argv[0],\n> >  \t\t\t    cmd->dir, strerror(errno));\n> > +\t\tif (cmd->env) {\n> > +\t\t\tfor (; *cmd->env; cmd->env++)\n> > +\t\t\t\tputenv((char*)*cmd->env);\n> > +\t\t}\n> >  \t\tif (cmd->git_cmd) {\n> >  \t\t\texecv_git_cmd(cmd->argv);\n> >  \t\t} else {\n> \n> I had a feeling that some callers needed to be able to unsetenv\n> some.  How would this patch help them, or are they outside of\n> the scope?\n> \n\nOthers already discussed the issue. Just to be sure, I reimplemented\nthat comfortable putenv with unsetenv: if an environment entry ends\nwith a \"=\" it will be unset.\n"},{"id":"42994","messageId":"20070522214823.GE30871@steel.home","threadId":"8244","inReplyTo":"20070522214754.GD30871@steel.home","subject":"[PATCH] Add run_command_v_opt_cd: chdir into a directory before exec","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-05-22T21:48:23Z","receivedAt":"2007-05-22T21:48:23Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"It can make code simplier (no need to preserve cwd) and safer\n(no chance the cwd of the current process is accidentally forgotten).\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n run-command.c |   27 ++++++++++++++++++++++-----\n run-command.h |    2 ++\n 2 files changed, 24 insertions(+), 5 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex eff523e..043b570 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -73,6 +73,9 @@ int start_command(struct child_process *cmd)\n \t\t\tclose(cmd->out);\n \t\t}\n \n+\t\tif (cmd->dir && chdir(cmd->dir))\n+\t\t\tdie(\"exec %s: cd to %s failed (%s)\", cmd->argv[0],\n+\t\t\t    cmd->dir, strerror(errno));\n \t\tif (cmd->git_cmd) {\n \t\t\texecv_git_cmd(cmd->argv);\n \t\t} else {\n@@ -133,13 +136,27 @@ int run_command(struct child_process *cmd)\n \treturn finish_command(cmd);\n }\n \n+static void prepare_run_command_v_opt(struct child_process *cmd,\n+\t\t\t\t      const char **argv, int opt)\n+{\n+\tmemset(cmd, 0, sizeof(*cmd));\n+\tcmd->argv = argv;\n+\tcmd->no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0;\n+\tcmd->git_cmd = opt & RUN_GIT_CMD ? 1 : 0;\n+\tcmd->stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;\n+}\n+\n int run_command_v_opt(const char **argv, int opt)\n {\n \tstruct child_process cmd;\n-\tmemset(&cmd, 0, sizeof(cmd));\n-\tcmd.argv = argv;\n-\tcmd.no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0;\n-\tcmd.git_cmd = opt & RUN_GIT_CMD ? 1 : 0;\n-\tcmd.stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;\n+\tprepare_run_command_v_opt(&cmd, argv, opt);\n+\treturn run_command(&cmd);\n+}\n+\n+int run_command_v_opt_cd(const char **argv, int opt, const char *dir)\n+{\n+\tstruct child_process cmd;\n+\tprepare_run_command_v_opt(&cmd, argv, opt);\n+\tcmd.dir = dir;\n \treturn run_command(&cmd);\n }\ndiff --git a/run-command.h b/run-command.h\nindex 3680ef9..cbd7484 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -16,6 +16,7 @@ struct child_process {\n \tpid_t pid;\n \tint in;\n \tint out;\n+\tconst char *dir;\n \tunsigned close_in:1;\n \tunsigned close_out:1;\n \tunsigned no_stdin:1;\n@@ -32,5 +33,6 @@ int run_command(struct child_process *);\n #define RUN_GIT_CMD\t     2\t/*If this is to be git sub-command */\n #define RUN_COMMAND_STDOUT_TO_STDERR 4\n int run_command_v_opt(const char **argv, int opt);\n+int run_command_v_opt_cd(const char **argv, int opt, const char *dir);\n \n #endif\n-- \n1.5.2.51.g16099\n"},{"id":"42995","messageId":"20070522214847.GF30871@steel.home","threadId":"8244","inReplyTo":"20070522214823.GE30871@steel.home","subject":"[PATCH] Add ability to specify environment extension to run_command","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-05-22T21:48:47Z","receivedAt":"2007-05-22T21:48:47Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"There is no way to specify and override for the environment:\nthere'd be no user for it yet.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n run-command.c |   17 ++++++++++++++++-\n run-command.h |    2 ++\n 2 files changed, 18 insertions(+), 1 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 043b570..605aa1e 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -76,6 +76,10 @@ int start_command(struct child_process *cmd)\n \t\tif (cmd->dir && chdir(cmd->dir))\n \t\t\tdie(\"exec %s: cd to %s failed (%s)\", cmd->argv[0],\n \t\t\t    cmd->dir, strerror(errno));\n+\t\tif (cmd->env) {\n+\t\t\tfor (; *cmd->env; cmd->env++)\n+\t\t\t\tputenv((char*)*cmd->env);\n+\t\t}\n \t\tif (cmd->git_cmd) {\n \t\t\texecv_git_cmd(cmd->argv);\n \t\t} else {\n@@ -137,7 +141,8 @@ int run_command(struct child_process *cmd)\n }\n \n static void prepare_run_command_v_opt(struct child_process *cmd,\n-\t\t\t\t      const char **argv, int opt)\n+\t\t\t\t      const char **argv,\n+\t\t\t\t      int opt)\n {\n \tmemset(cmd, 0, sizeof(*cmd));\n \tcmd->argv = argv;\n@@ -160,3 +165,13 @@ int run_command_v_opt_cd(const char **argv, int opt, const char *dir)\n \tcmd.dir = dir;\n \treturn run_command(&cmd);\n }\n+\n+int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const char *const *env)\n+{\n+\tstruct child_process cmd;\n+\tprepare_run_command_v_opt(&cmd, argv, opt);\n+\tcmd.dir = dir;\n+\tcmd.env = env;\n+\treturn run_command(&cmd);\n+}\n+\ndiff --git a/run-command.h b/run-command.h\nindex cbd7484..af1e0bf 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -17,6 +17,7 @@ struct child_process {\n \tint in;\n \tint out;\n \tconst char *dir;\n+\tconst char *const *env;\n \tunsigned close_in:1;\n \tunsigned close_out:1;\n \tunsigned no_stdin:1;\n@@ -34,5 +35,6 @@ int run_command(struct child_process *);\n #define RUN_COMMAND_STDOUT_TO_STDERR 4\n int run_command_v_opt(const char **argv, int opt);\n int run_command_v_opt_cd(const char **argv, int opt, const char *dir);\n+int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const char *const *env);\n \n #endif\n-- \n1.5.2.51.g16099\n"},{"id":"42996","messageId":"20070522214921.GG30871@steel.home","threadId":"8244","inReplyTo":"20070522214847.GF30871@steel.home","subject":"[PATCH] Allow environment variables to be unset in the processes started by run_command","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-05-22T21:49:21Z","receivedAt":"2007-05-22T21:49:21Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Signed-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n run-command.c |   14 ++++++++++++--\n 1 files changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 605aa1e..3a5f737 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -77,8 +77,18 @@ int start_command(struct child_process *cmd)\n \t\t\tdie(\"exec %s: cd to %s failed (%s)\", cmd->argv[0],\n \t\t\t    cmd->dir, strerror(errno));\n \t\tif (cmd->env) {\n-\t\t\tfor (; *cmd->env; cmd->env++)\n-\t\t\t\tputenv((char*)*cmd->env);\n+\t\t\tchar *unsetbuf = NULL;\n+\t\t\tfor (; *cmd->env; cmd->env++) {\n+\t\t\t\tsize_t n = strlen(*cmd->env);\n+\t\t\t\tif (n && (*cmd->env)[n-1] == '=') {\n+\t\t\t\t\tunsetbuf = xrealloc(unsetbuf, n);\n+\t\t\t\t\tmemcpy(unsetbuf, *cmd->env, --n);\n+\t\t\t\t\tunsetbuf[n] = '\\0';\n+\t\t\t\t\tunsetenv(unsetbuf);\n+\t\t\t\t} else\n+\t\t\t\t\tputenv((char*)*cmd->env);\n+\t\t\t}\n+\t\t\tfree(unsetbuf);\n \t\t}\n \t\tif (cmd->git_cmd) {\n \t\t\texecv_git_cmd(cmd->argv);\n-- \n1.5.2.51.g16099\n"},{"id":"42997","messageId":"20070522215134.GH30871@steel.home","threadId":"8244","inReplyTo":"7v646l9xkn.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add ability to specify environment extension to run_command","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-05-22T21:51:34Z","receivedAt":"2007-05-22T21:51:34Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Tue, May 22, 2007 08:33:44 +0200:\n> \tstruct child_process {\n>         \t...\n>                 const struct {\n>                         const char *name;\n>                         const char *value; /* NULL to unsetenv */\n>                 } *env;\n> \t\t...\n> \t};\n\nI actually like how the environment is organized. And it is simple to\ndefine in the source. And there are well-known routines for\nenvironment-like array manipulation.\n"},{"id":"43002","messageId":"7v1wh88prw.fsf@assigned-by-dhcp.cox.net","threadId":"8244","inReplyTo":"20070522214754.GD30871@steel.home","subject":"Re: [PATCH] Add ability to specify environment extension to run_command","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-22T22:19:47Z","receivedAt":"2007-05-22T22:19:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen <raa.lkml@gmail.com> writes:\n\n> Others already discussed the issue. Just to be sure, I reimplemented\n> that comfortable putenv with unsetenv: if an environment entry ends\n> with a \"=\" it will be unset.\n\nAlthough combination of putenv and unsetenv gives a somewhat\nqueasy feeling for obvious reasons, I'll let it pass.  As we\nare coming up with an interface that uses only one string per\nenvironment element, that is probably a sensible thing to do,\nrather than trying to do the \"historically correct\" pairing of\nsetenv/unsetenv.\n\nHowever, I do not think \"VAR=\" to unset it is a good interface.\nHaving an environment variable whose value happens to be an\nempty string and not having the variable at all are two\ndifferent things.\n\nBecause you _scan_ the whole string in your patch to see if it\nends with = anyway, a trivial improvement would be to do:\n\n\tif (strchr(cmd->env, '='))\n                putenv(cmd->env);\n\telse\n        \tunsetenv(cmd->env);\n\nIf you do not mind such a special syntax (e.g. \"VAR=\"), I would\nsuggest doing that as a prefix (e.g. \"!VAR\") and do:\n\n\tif (cmd->env[0] != '!')\n        \tputenv(cmd->env);\n\telse\n\t\tunsetenv(cmd->env + 1);\n\nThe former look cleaner but less efficient; we are going to exec\nso I do not think micro-optimization would matter at all, so my\nsuggestion would be to do the strchr().\n"},{"id":"43006","messageId":"20070522231442.GM30871@steel.home","threadId":"8244","inReplyTo":"7v1wh88prw.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add ability to specify environment extension to run_command","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-05-22T23:14:42Z","receivedAt":"2007-05-22T23:14:42Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Wed, May 23, 2007 00:19:47 +0200:\n> > Others already discussed the issue. Just to be sure, I reimplemented\n> > that comfortable putenv with unsetenv: if an environment entry ends\n> > with a \"=\" it will be unset.\n> \n> Although combination of putenv and unsetenv gives a somewhat\n> queasy feeling for obvious reasons, I'll let it pass.  As we\n> are coming up with an interface that uses only one string per\n> environment element, that is probably a sensible thing to do,\n> rather than trying to do the \"historically correct\" pairing of\n> setenv/unsetenv.\n> \n> However, I do not think \"VAR=\" to unset it is a good interface.\n> Having an environment variable whose value happens to be an\n> empty string and not having the variable at all are two\n> different things.\n\nRight\n\n> Because you _scan_ the whole string in your patch to see if it\n> ends with = anyway, a trivial improvement would be to do:\n> \n> \tif (strchr(cmd->env, '='))\n>                 putenv(cmd->env);\n> \telse\n>         \tunsetenv(cmd->env);\n\nI like this one. The env field in struct child_process and run_command\nwill have to mention it in comments (in run-command.h), it's kind of\nspecial.\n\n> If you do not mind such a special syntax (e.g. \"VAR=\"), I would\n> suggest doing that as a prefix (e.g. \"!VAR\") and do:\n\nNah, !VAR is a _working_ environment variable name.\n\n    int main(int argc, char *argv[], char *envp[])\n    {\n\t    const char *argv1[] = {\"/usr/bin/perl\", \"-e\", \"print $ENV{'!VAR'}\", NULL};\n\t    const char *envp1[] = {\"!VAR=value\", NULL};\n\t    execve(*argv, (char**)argv1, (char**)envp1);\n\t    return 0;\n    }\n\n    $ gcc ... && ./a.out\n    value\n\nSomeone could want it. We surely could use \"=\", though :)\n"},{"id":"43047","messageId":"20070523202139.GC2554@steel.home","threadId":"8244","inReplyTo":"20070522231442.GM30871@steel.home","subject":"[PATCH] Allow environment variables to be unset in the processes started by run_command","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-05-23T20:21:39Z","receivedAt":"2007-05-23T20:21:39Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"To unset a variable, just specify its name, without \"=\". For example:\n\n    const char *env[] = {\"GIT_DIR=.git\", \"PWD\", NULL};\n    const char *argv[] = {\"git-ls-files\", \"-s\", NULL};\n    int err = run_command_v_opt_cd_env(argv, RUN_GIT_CMD, \".\", env);\n\nThe PWD will be unset before executing git-ls-files.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nAlex Riesen, Wed, May 23, 2007 01:14:42 +0200:\n> > Because you _scan_ the whole string in your patch to see if it\n> > ends with = anyway, a trivial improvement would be to do:\n> > \n> > \tif (strchr(cmd->env, '='))\n> >                 putenv(cmd->env);\n> > \telse\n> >         \tunsetenv(cmd->env);\n> \n> I like this one. The env field in struct child_process and run_command\n> will have to mention it in comments (in run-command.h), it's kind of\n> special.\n> \n\n run-command.c |    8 ++++++--\n run-command.h |    5 +++++\n 2 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 605aa1e..3b1899e 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -77,8 +77,12 @@ int start_command(struct child_process *cmd)\n \t\t\tdie(\"exec %s: cd to %s failed (%s)\", cmd->argv[0],\n \t\t\t    cmd->dir, strerror(errno));\n \t\tif (cmd->env) {\n-\t\t\tfor (; *cmd->env; cmd->env++)\n-\t\t\t\tputenv((char*)*cmd->env);\n+\t\t\tfor (; *cmd->env; cmd->env++) {\n+\t\t\t\tif (strchr(*cmd->env, '='))\n+\t\t\t\t\tputenv((char*)*cmd->env);\n+\t\t\t\telse\n+\t\t\t\t\tunsetenv(*cmd->env);\n+\t\t\t}\n \t\t}\n \t\tif (cmd->git_cmd) {\n \t\t\texecv_git_cmd(cmd->argv);\ndiff --git a/run-command.h b/run-command.h\nindex af1e0bf..7958eb1 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -35,6 +35,11 @@ int run_command(struct child_process *);\n #define RUN_COMMAND_STDOUT_TO_STDERR 4\n int run_command_v_opt(const char **argv, int opt);\n int run_command_v_opt_cd(const char **argv, int opt, const char *dir);\n+\n+/*\n+ * env (the environment) is to be formatted like environ: \"VAR=VALUE\".\n+ * To unset an environment variable use just \"VAR\".\n+ */\n int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const char *const *env);\n \n #endif\n-- \n1.5.2.67.gbd3c2\n"}]}