git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] sequencer.c: terminate the last line of author-script properly

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Jul 26, 2018, 12:39 UTC
Message-ID
<nycvar.QRO.7.76.6.1807261434550.71@tvgsbejvaqbjf.bet>
In-Reply-To
<9ecdf160-bc8b-b91d-a70e-ca6fb9d2a756@talktalk.net>
Hi Phillip,
On Thu, 19 Jul 2018, Phillip Wood wrote:
Show 22 quoted lines
> On 18/07/18 18:17, Junio C Hamano wrote:
> > Phillip Wood <phillip.wood@talktalk.net> 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

Previous: Phillip WoodNext: Junio C Hamano
Message 14 of 27 in “sequencer.c: terminate the last line of author-script properly”
  1. sequencer.c: terminate the last line of author-script properlyAkinori MUSHA, Jul 12, 2018
  2. Junio C HamanoJul 12, 2018
  3. Phillip WoodJul 18, 2018
  4. Phillip WoodJul 18, 2018
  5. sequencer: fix quoting in write_author_scriptPhillip Wood, Jul 18, 2018
  6. Junio C HamanoJul 24, 2018
  7. Johannes SchindelinJul 26, 2018
  8. Phillip WoodJul 27, 2018
  9. Johannes SchindelinJul 27, 2018
  10. Phillip WoodJul 30, 2018
  11. Junio C HamanoJul 18, 2018
  12. Junio C HamanoJul 18, 2018
  13. Phillip WoodJul 19, 2018
  14. Johannes SchindelinJul 26, 2018
  15. Junio C HamanoJul 26, 2018
  16. Junio C HamanoJul 12, 2018
  17. Eric SunshineJul 12, 2018
  18. Junio C HamanoJul 12, 2018
  19. Junio C HamanoJul 17, 2018
  20. Akinori MUSHAJul 18, 2018
  21. Johannes SchindelinJul 26, 2018
  22. Junio C HamanoJul 26, 2018
  23. Johannes SchindelinJul 27, 2018
  24. Junio C HamanoJul 12, 2018
  25. Phillip WoodJul 18, 2018
  26. Phillip WoodJul 18, 2018
  27. Phillip WoodJul 18, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.