Hi Phillip, On Thu, 19 Jul 2018, Phillip Wood wrote: > On 18/07/18 18:17, Junio C Hamano wrote: > > Phillip Wood <phillip.wood@xxxxxxxxxxxx> writes: > > > >>> (I think we had code to do so in "git am" > >>> that was rewritten in C first). > >> > >> The code in builtin/am.c doesn't try to write valid posix shell (if > >> one assumes it is the only consumer of the author script then it > >> doesn't need to) which results in simpler code, but external scripts > >> cannot safely eval it anymore. > > > > Are you sure about that? If so we probably should see if we can fix> the writer, and better yet, if we can share code with the writer > > discussed here, as presumably we are fixing it in this thread. > > > > But I do not see how builtin/am.c::write_author_script() would > > produce something that would not eval correctly. sq_quote_buf() was > > introduced specifically to write correct string for shell's > > consumption. > > You're right, I'm not sure how I missed the calls to sq_quote_buf() > yesterday, sharing the am code with the sequencer would clean things up > nicely. No, actually Phillip was right. The `author-script` file written by `git-am` was always an implementation detail, and as there was no (intended) way to call shell scripts while running `git-am`, the only shell script to intentionally use `author-script` was `git-am` itself. Ever since `git-am` is a builtin, the `author-script` file format could be changed, because it is an implementation detail, no more nor less, and I think it *should* be changed, too. We're spending useless cycles on quoting and dequoting, when writing a NUL-separated list of var=value pairs would be totally sufficient to our ends. Ciao, Dscho
