{"thread":{"id":"37698","subject":"Some PRETTY FORMATS format:<string> options don't work with git-archive export-subst $Format:$","startedAt":"2014-10-09T15:56:19Z","lastAt":"2014-10-09T19:12:59Z","messageCount":4,"participants":["Derek Moore","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"250428","messageId":"CAMsgyKb_-Xm0DH1-xVZa0XHs59nZsOxGwwLWKx-YiTmsJtrFHQ@mail.gmail.com","threadId":"37698","inReplyTo":null,"subject":"Some PRETTY FORMATS format:<string> options don't work with git-archive export-subst $Format:$","fromName":"Derek Moore","fromEmail":"derek.p.moore@gmail.com","sentAt":"2014-10-09T15:56:19Z","receivedAt":"2014-10-09T15:56:19Z","isPatch":false,"sender":{"key":"derek.p.moore@gmail.com","avatar":"https://gravatar.com/avatar/4bf86633cdd04eb5d07180791de5ae0ece9f3d04b34f6e13fdb81d563fc62c23?d=mp&s=160"},"body":"I first noticed this using the system git provided in Fedora 21, so I\ncloned the official git repo, built from source, and it would appear\nthere are even more options that don't work with export-subst in the\nlatest code.\n\nI tested this under commit 63a45136a329bab550425c3142db6071434d935e\n(HEAD, origin/master, origin/HEAD, master).\n\nTake, for example, the following script:\n\nFILE=Foo.pm\nDIR=$(mktemp -d /tmp/git.XXXXXX)\ncd $DIR\ngit init\necho '*.pm export-subst ident' >> .gitattributes\necho '#   Last Author    : $Author: $Format:%an$ $' > $FILE\necho '#   Last Date      : $Date: $Format:%ai (%ad)% $' >> $FILE\necho '#   Last Version   : $Revision: $Format:%d$ $' >> $FILE\necho '#   Last Version v2: $Revision: $Format:%D$ $' >> $FILE\necho '#   Last Commit    : $Commit: $Format:%H$ $' >> $FILE\necho '#   This File      : $Id$' >> $FILE\necho 'our $VERSION = (qw($Format:%N$))[1];' >> $FILE\ngit add .\ngit commit -a -m \"Initial commit.\"\ngit notes add -f -m '$Release: 0048 $'\ngit tag -f releases/R0048\ngit archive HEAD $FILE | tar xf - --to-stdout\nrm $FILE\ngit checkout -- $FILE\ncat $FILE\ngit log --format='%N' -1\n\n\nYou can tell I'm attempting to recreate CVS keywords (bwahahaa!) for a\nproject that is insisting they need them.\n\nI'd be happy to write a bunch of unit tests for export-subst and all\nPRETTY FORMATS format:<string> options, if that would be desirable. I\nsee the t/ directory, and the t/test-lib.sh stuff looks simple enough\n(TAP in bash, hmm).\n\nThanks,\n\nDerek\n"},{"id":"250430","messageId":"CAMsgyKYKAUXamcGOtUi3Bq-u4B9JaG2sk-Gttf+A1U2mwhghrw@mail.gmail.com","threadId":"37698","inReplyTo":"CAMsgyKb_-Xm0DH1-xVZa0XHs59nZsOxGwwLWKx-YiTmsJtrFHQ@mail.gmail.com","subject":"Re: Some PRETTY FORMATS format:<string> options don't work with git-archive export-subst $Format:$","fromName":"Derek Moore","fromEmail":"derek.p.moore@gmail.com","sentAt":"2014-10-09T17:42:39Z","receivedAt":"2014-10-09T17:42:39Z","isPatch":false,"sender":{"key":"derek.p.moore@gmail.com","avatar":"https://gravatar.com/avatar/4bf86633cdd04eb5d07180791de5ae0ece9f3d04b34f6e13fdb81d563fc62c23?d=mp&s=160"},"body":"On Thu, Oct 9, 2014 at 10:56 AM, Derek Moore <derek.p.moore@gmail.com> wrote:\n> I first noticed this using the system git provided in Fedora 21, so I\n> cloned the official git repo, built from source, and it would appear\n> there are even more options that don't work with export-subst in the\n> latest code.\n\nI'm a dumb ass. After installing my newly compiled git to /usr/local,\nI neglected to spawn a new shell so the new binary could get picked up\nby my $PATH. Despite /usr/local/bin preceding /usr/bin, /usr/bin/git\nwas cached by my shell, and 'which git' was lying to me, therefore my\ntesting of new format:<string> options was invalid.\n\nI must retract the statement \"and it would appear there are even more\noptions that don't work with export-subst in the latest code.\"\n\nAs far as I've tested it would seem only %N doesn't resolve inside of\n$Format:$, until I maybe do unit tests for this to identify any\nothers.\n"},{"id":"250439","messageId":"20141009190722.GA415@peff.net","threadId":"37698","inReplyTo":"CAMsgyKYKAUXamcGOtUi3Bq-u4B9JaG2sk-Gttf+A1U2mwhghrw@mail.gmail.com","subject":"Re: Some PRETTY FORMATS format:<string> options don't work with git-archive export-subst $Format:$","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-10-09T19:07:22Z","receivedAt":"2014-10-09T19:07:22Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 09, 2014 at 12:42:39PM -0500, Derek Moore wrote:\n\n> As far as I've tested it would seem only %N doesn't resolve inside of\n> $Format:$, until I maybe do unit tests for this to identify any\n> others.\n\nYes, %N is somewhat special in that the calling code needs to initialize\nthe notes tree itself. We can't just do it lazily when we see the first\n%N because _which_ notes we show depends on other options (e.g., for\nlog, if you've used --notes-ref, --show-notes=..., etc).\n\nSo in theory you need something like 5b16360 (pretty: Initialize notes\nif %N is used, 2010-04-13), but adapted for git-archive. The trick,\nthough, is that we do not even see the format string until we are\nlooking at a particular file with a $Format$ marker. So you'd have to\nlazily initialize notes there (and if you want to support picking\nspecific notes refs, you'd have to teach git-archive new options to do\nso[1]).\n\nHere's a quick-and-dirty patch that makes the snippet you posted earlier\ndo what I think you expected. I haven't tested it beyond that, and am\nnot planning to push it forward myself, but please feel free to use it\nas a basis for building a solution.\n\n---\ndiff --git a/archive.c b/archive.c\nindex 952a659..3af781e 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -5,6 +5,7 @@\n #include \"archive.h\"\n #include \"parse-options.h\"\n #include \"unpack-trees.h\"\n+#include \"notes.h\"\n \n static char const * const archive_usage[] = {\n \tN_(\"git archive [options] <tree-ish> [<path>...]\"),\n@@ -38,6 +39,8 @@ static void format_subst(const struct commit *commit,\n \tif (src == buf->buf)\n \t\tto_free = strbuf_detach(buf, NULL);\n \tfor (;;) {\n+\t\tstruct userformat_want ufw = {0};\n+\t\tstruct strbuf notes = STRBUF_INIT;\n \t\tconst char *b, *c;\n \n \t\tb = memmem(src, len, \"$Format:\", 8);\n@@ -50,10 +53,31 @@ static void format_subst(const struct commit *commit,\n \t\tstrbuf_reset(&fmt);\n \t\tstrbuf_add(&fmt, b + 8, c - b - 8);\n \n+\t\tuserformat_find_requirements(fmt.buf, &ufw);\n+\t\tif (ufw.notes) {\n+\t\t\tstatic int initialized;\n+\t\t\tif (!initialized) {\n+\t\t\t\tinit_display_notes(NULL);\n+\t\t\t\tinitialized = 1;\n+\t\t\t}\n+\t\t\tformat_display_notes(commit->object.sha1, &notes,\n+\t\t\t\t\t     get_log_output_encoding(), 1);\n+\t\t\t/*\n+\t\t\t * trim trailing newlines from note content, which is\n+\t\t\t * probably more appropriate for $Format$; should\n+\t\t\t * this actually remove internal newlines, too?\n+\t\t\t */\n+\t\t\tstrbuf_rtrim(&notes);\n+\t\t}\n+\t\tctx.notes_message = notes.buf;\n+\n \t\tstrbuf_add(buf, src, b - src);\n \t\tformat_commit_message(commit, fmt.buf, buf, &ctx);\n \t\tlen -= c + 1 - src;\n \t\tsrc  = c + 1;\n+\n+\t\tctx.notes_message = NULL;\n+\t\tstrbuf_release(&notes);\n \t}\n \tstrbuf_add(buf, src, len);\n \tstrbuf_release(&fmt);\n\n-Peff\n\n[1] I think you could get pretty far using `git -c core.notesRef=foo` to\n    give ad-hoc config, as the notes code should use that as the\n    ultimate default. But I didn't try it.\n"},{"id":"250440","messageId":"CAMsgyKaYMbHDWQrskEOizcwVZS330Rpt+hLoOaST=_0Yk9PMVA@mail.gmail.com","threadId":"37698","inReplyTo":"20141009190722.GA415@peff.net","subject":"Re: Some PRETTY FORMATS format:<string> options don't work with git-archive export-subst $Format:$","fromName":"Derek Moore","fromEmail":"derek.p.moore@gmail.com","sentAt":"2014-10-09T19:12:59Z","receivedAt":"2014-10-09T19:12:59Z","isPatch":false,"sender":{"key":"derek.p.moore@gmail.com","avatar":"https://gravatar.com/avatar/4bf86633cdd04eb5d07180791de5ae0ece9f3d04b34f6e13fdb81d563fc62c23?d=mp&s=160"},"body":"Thanks, Jeff,\n\nI may look into cleaning up your patch to propose a proper solution to\nthis list, as time allows.\n\nIncidentally, I did stumble across:\n\nhttp://git.kernel.org/cgit/git/git.git/commit/?id=5b16360330822527eac1fa84131d185ff784c9fb\n\nwhich also references you, but I forgot to mention it.\n\n\nOn Thu, Oct 9, 2014 at 2:07 PM, Jeff King <peff@peff.net> wrote:\n> On Thu, Oct 09, 2014 at 12:42:39PM -0500, Derek Moore wrote:\n>\n>> As far as I've tested it would seem only %N doesn't resolve inside of\n>> $Format:$, until I maybe do unit tests for this to identify any\n>> others.\n>\n> Yes, %N is somewhat special in that the calling code needs to initialize\n> the notes tree itself. We can't just do it lazily when we see the first\n> %N because _which_ notes we show depends on other options (e.g., for\n> log, if you've used --notes-ref, --show-notes=..., etc).\n>\n> So in theory you need something like 5b16360 (pretty: Initialize notes\n> if %N is used, 2010-04-13), but adapted for git-archive. The trick,\n> though, is that we do not even see the format string until we are\n> looking at a particular file with a $Format$ marker. So you'd have to\n> lazily initialize notes there (and if you want to support picking\n> specific notes refs, you'd have to teach git-archive new options to do\n> so[1]).\n>\n> Here's a quick-and-dirty patch that makes the snippet you posted earlier\n> do what I think you expected. I haven't tested it beyond that, and am\n> not planning to push it forward myself, but please feel free to use it\n> as a basis for building a solution.\n>\n> ---\n> diff --git a/archive.c b/archive.c\n> index 952a659..3af781e 100644\n> --- a/archive.c\n> +++ b/archive.c\n> @@ -5,6 +5,7 @@\n>  #include \"archive.h\"\n>  #include \"parse-options.h\"\n>  #include \"unpack-trees.h\"\n> +#include \"notes.h\"\n>\n>  static char const * const archive_usage[] = {\n>         N_(\"git archive [options] <tree-ish> [<path>...]\"),\n> @@ -38,6 +39,8 @@ static void format_subst(const struct commit *commit,\n>         if (src == buf->buf)\n>                 to_free = strbuf_detach(buf, NULL);\n>         for (;;) {\n> +               struct userformat_want ufw = {0};\n> +               struct strbuf notes = STRBUF_INIT;\n>                 const char *b, *c;\n>\n>                 b = memmem(src, len, \"$Format:\", 8);\n> @@ -50,10 +53,31 @@ static void format_subst(const struct commit *commit,\n>                 strbuf_reset(&fmt);\n>                 strbuf_add(&fmt, b + 8, c - b - 8);\n>\n> +               userformat_find_requirements(fmt.buf, &ufw);\n> +               if (ufw.notes) {\n> +                       static int initialized;\n> +                       if (!initialized) {\n> +                               init_display_notes(NULL);\n> +                               initialized = 1;\n> +                       }\n> +                       format_display_notes(commit->object.sha1, &notes,\n> +                                            get_log_output_encoding(), 1);\n> +                       /*\n> +                        * trim trailing newlines from note content, which is\n> +                        * probably more appropriate for $Format$; should\n> +                        * this actually remove internal newlines, too?\n> +                        */\n> +                       strbuf_rtrim(&notes);\n> +               }\n> +               ctx.notes_message = notes.buf;\n> +\n>                 strbuf_add(buf, src, b - src);\n>                 format_commit_message(commit, fmt.buf, buf, &ctx);\n>                 len -= c + 1 - src;\n>                 src  = c + 1;\n> +\n> +               ctx.notes_message = NULL;\n> +               strbuf_release(&notes);\n>         }\n>         strbuf_add(buf, src, len);\n>         strbuf_release(&fmt);\n>\n> -Peff\n>\n> [1] I think you could get pretty far using `git -c core.notesRef=foo` to\n>     give ad-hoc config, as the notes code should use that as the\n>     ultimate default. But I didn't try it.\n"}]}