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

Re: [PATCH 4/6] rebase -i: fix --committer-date-is-author-date

From
Elijah Newren <newren@gmail.com>
Date
Apr 7, 2020, 15:05 UTC
Message-ID
<CABPp-BFu0mgNnxcuZZ_kBJ3WWXxCNSZAFdmq9zKT4H9i47sNsA@mail.gmail.com>
In-Reply-To
<20200407141125.30872-5-phillip.wood123@gmail.com>
On Tue, Apr 7, 2020 at 7:11 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
Show 77 quoted lines
>
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Johanathan Nieder reported that `rebase --committer-date-is-author-date`
> failed with the error "invalid date format: @@2592000 +0000" when the
> backend was switched to use the sequencer [1]. This is because when we
> read the date from the author script file is already prefixed by '@' so
> we do not need to add one. This did not show up in our tests
> because once match_object_header_date() fails to match because of the
> extra '@' parse_date_basic() strips the prefix and a date with more than
> 8 digits is parsed as a number of seconds by match_digit().
>
> While fixing this I also noticed that we were not setting
> GIT_COMMITTER_DATE in the environment when forking `git merge`. This was
> untested. The tests we did have used commits where the author date was
> the same as the current value $GIT_COMMITTER_DATE in the environment so
> they did not test that we were actually using the author date when
> creating the commit.
>
> As we have already read GIT_AUTHOR_DATE into cmd.env_array we now use
> that rather than re-reading the author-script file. I've moved the code
> that handles opts->ignore date so that all the code setting the child
> environment is together and changed it so we no longer set
> GIT_COMMITTER_DATE twice when --ignore-date is combined with
> --committer-date-is-author-date.
>
> [1] https://lore.kernel.org/git/<20200110231436.GA24315@google.com>
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  sequencer.c                             | 68 ++++++++++---------------
>  t/t3433-rebase-options-compatibility.sh | 29 +++++++++--
>  2 files changed, 52 insertions(+), 45 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 763ccbbc45..56f6e49289 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -880,17 +880,6 @@ static char *get_author(const char *message)
>         return NULL;
>  }
>
> -/* Returns a "date" string that needs to be free()'d by the caller */
> -static char *read_author_date_or_null(void)
> -{
> -       char *date;
> -
> -       if (read_author_script(rebase_path_author_script(),
> -                              NULL, NULL, &date, 0))
> -               return NULL;
> -       return date;
> -}
> -
>  /* Construct a free()able author string with current time as the author date */
>  static char *ignore_author_date(const char *author)
>  {
> @@ -909,16 +898,20 @@ static char *ignore_author_date(const char *author)
>         return strbuf_detach(&new_author, NULL);
>  }
>
> -static void push_dates(struct child_process *child, int change_committer_date)
> +static const char *author_date_from_env_array(const struct argv_array *env)
>  {
> -       time_t now = time(NULL);
> -       struct strbuf date = STRBUF_INIT;
> +       int i;
> +       const char *date;
>
> -       strbuf_addf(&date, "@%"PRIuMAX, (uintmax_t)now);
> -       argv_array_pushf(&child->env_array, "GIT_AUTHOR_DATE=%s", date.buf);
> -       if (change_committer_date)
> -               argv_array_pushf(&child->env_array, "GIT_COMMITTER_DATE=%s", date.buf);
> -       strbuf_release(&date);
> +       for (i = 0; i < env->argc; i++)
> +               if (skip_prefix(env->argv[i],
> +                               "GIT_AUTHOR_DATE=", &date))
> +                       return date;

Is there any risk that GIT_AUTHOR_DATE=<somedate> will appear twice in @child->env_array ? If there is, should we read through the whole list to make sure we take the last one?

Show 142 quoted lines
> +       /*
> +        * If GIT_AUTHOR_DATE is missing we should have already errored out when
> +        * reading the script
> +        */
> +       BUG("GIT_AUTHOR_DATE missing from author script");
>  }
>
>  static const char staged_changes_advice[] =
> @@ -980,31 +973,19 @@ static int run_git_commit(struct repository *r,
>
>         cmd.git_cmd = 1;
>
> -       if (opts->committer_date_is_author_date) {
> -               int res = -1;
> -               struct strbuf datebuf = STRBUF_INIT;
> -               char *date = read_author_date_or_null();
> -
> -               if (!date)
> -                       return -1;
> -
> -               strbuf_addf(&datebuf, "@%s", date);
> -               res = setenv("GIT_COMMITTER_DATE",
> -                            opts->ignore_date ? "" : datebuf.buf, 1);
> -
> -               strbuf_release(&datebuf);
> -               free(date);
> -
> -               if (res)
> -                       return -1;
> -       }
> -
>         if (is_rebase_i(opts) && read_env_script(&cmd.env_array)) {
>                 const char *gpg_opt = gpg_sign_opt_quoted(opts);
>
>                 return error(_(staged_changes_advice),
>                              gpg_opt, gpg_opt);
>         }
> +       if (opts->committer_date_is_author_date)
> +               argv_array_pushf(&cmd.env_array, "GIT_COMMITTER_DATE=%s",
> +                                opts->ignore_date ?
> +                                "" :
> +                                author_date_from_env_array(&cmd.env_array));
> +       if (opts->ignore_date)
> +               argv_array_push(&cmd.env_array, "GIT_AUTHOR_DATE=");
>
>         argv_array_push(&cmd.args, "commit");
>
> @@ -1014,8 +995,6 @@ static int run_git_commit(struct repository *r,
>                 argv_array_push(&cmd.args, "--amend");
>         if (opts->gpg_sign)
>                 argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign);
> -       if (opts->ignore_date)
> -               push_dates(&cmd, opts->committer_date_is_author_date);
>         if (defmsg)
>                 argv_array_pushl(&cmd.args, "-F", defmsg, NULL);
>         else if (!(flags & EDIT_MSG))
> @@ -3615,6 +3594,13 @@ static int do_merge(struct repository *r,
>                         ret = error(_(staged_changes_advice), gpg_opt, gpg_opt);
>                         goto leave_merge;
>                 }
> +               if (opts->committer_date_is_author_date)
> +                       argv_array_pushf(&cmd.env_array, "GIT_COMMITTER_DATE=%s",
> +                                        opts->ignore_date ?
> +                                        "" :
> +                                        author_date_from_env_array(&cmd.env_array));
> +               if (opts->ignore_date)
> +                       argv_array_push(&cmd.env_array, "GIT_AUTHOR_DATE=");
>
>                 cmd.git_cmd = 1;
>                 argv_array_push(&cmd.args, "merge");
> @@ -3635,8 +3621,6 @@ static int do_merge(struct repository *r,
>                 argv_array_push(&cmd.args, git_path_merge_msg(r));
>                 if (opts->gpg_sign)
>                         argv_array_push(&cmd.args, opts->gpg_sign);
> -               if (opts->ignore_date)
> -                       push_dates(&cmd, opts->committer_date_is_author_date);
>
>                 /* Add the tips to be merged */
>                 for (j = to_merge; j; j = j->next)
> diff --git a/t/t3433-rebase-options-compatibility.sh b/t/t3433-rebase-options-compatibility.sh
> index 132f577fc9..8247d01442 100755
> --- a/t/t3433-rebase-options-compatibility.sh
> +++ b/t/t3433-rebase-options-compatibility.sh
> @@ -71,15 +71,15 @@ test_expect_success '--ignore-whitespace works with interactive backend' '
>  '
>
>  test_expect_success '--committer-date-is-author-date works with am backend' '
> -       git commit --amend &&
> +       GIT_AUTHOR_DATE="@1234 +0300" git commit --amend --reset-author &&
>         git rebase --committer-date-is-author-date HEAD^ &&
>         git log -1 --pretty="format:%ai" >authortime &&
>         git log -1 --pretty="format:%ci" >committertime &&
>         test_cmp authortime committertime
>  '
>
>  test_expect_success '--committer-date-is-author-date works with interactive backend' '
> -       git commit --amend &&
> +       GIT_AUTHOR_DATE="@1234 +0300" git commit --amend --reset-author &&
>         git rebase -i --committer-date-is-author-date HEAD^ &&
>         git log -1 --pretty="format:%ai" >authortime &&
>         git log -1 --pretty="format:%ci" >committertime &&
> @@ -88,13 +88,36 @@ test_expect_success '--committer-date-is-author-date works with interactive back
>
>  test_expect_success '--committer-date-is-author-date works with rebase -r' '
>         git checkout side &&
> -       git merge --no-ff commit3 &&
> +       GIT_AUTHOR_DATE="@1234 +0300" git merge --no-ff commit3 &&
>         git rebase -r --root --committer-date-is-author-date &&
>         git log --pretty="format:%ai" >authortime &&
>         git log --pretty="format:%ci" >committertime &&
>         test_cmp authortime committertime
>  '
>
> +test_expect_success '--committer-date-is-author-date works when forking merge' '
> +       git checkout side &&
> +       GIT_AUTHOR_DATE="@1234 +0300" git merge --no-ff commit3 &&
> +       git rebase -r --root --strategy=resolve --committer-date-is-author-date &&
> +       git log --pretty="format:%ai" >authortime &&
> +       git log --pretty="format:%ci" >committertime &&
> +       test_cmp authortime committertime
> +
> +'
> +
> +test_expect_success '--committer-date-is-author-date works when committing conflict resolution' '
> +       git checkout commit2 &&
> +       GIT_AUTHOR_DATE="@1980 +0000" git commit --amend --only --reset-author &&
> +       git log -1 --format=%at HEAD >expect &&
> +       test_must_fail git rebase -i --committer-date-is-author-date \
> +               --onto HEAD^^ HEAD^ &&
> +       echo resolved > foo &&
> +       git add foo &&
> +       git rebase --continue &&
> +       git log -1 --format=%ct HEAD >actual &&
> +       test_cmp expect actual
> +'
> +
>  # Checking for +0000 in author time is enough since default
>  # timezone is UTC, but the timezone used while committing
>  # sets to +0530.
> --
> 2.26.0
>
Previous: Phillip WoodNext: Phillip Wood
Message 14 of 130 in “fixup ra/rebase-i-more-options”
  1. 0/6 fixup ra/rebase-i-more-optionsPhillip Wood, Apr 7, 2020
  2. 2/6 t3433: remove loops from testsPhillip Wood, Apr 7, 2020
  3. Elijah NewrenApr 7, 2020
  4. 1/6 Revert "Revert "Merge branch 'ra/rebase-i-more-options'""Phillip Wood, Apr 7, 2020
  5. Elijah NewrenApr 7, 2020
  6. Phillip WoodApr 7, 2020
  7. Junio C HamanoApr 7, 2020
  8. Junio C HamanoApr 7, 2020
  9. Johannes SchindelinApr 12, 2020
  10. Phillip WoodApr 13, 2020
  11. Junio C HamanoApr 13, 2020
  12. 3/6 t3433: only compare commit datesPhillip Wood, Apr 7, 2020
  13. 4/6 rebase -i: fix --committer-date-is-author-datePhillip Wood, Apr 7, 2020
  14. Elijah NewrenApr 7, 2020
  15. Phillip WoodApr 7, 2020
  16. 5/6 Revert "sequencer: allow callers of read_author_script() to ignore fields"Phillip Wood, Apr 7, 2020
  17. Elijah NewrenApr 7, 2020
  18. 6/6 t3433: improve coveragePhillip Wood, Apr 7, 2020
  19. Elijah NewrenApr 7, 2020
  20. Phillip WoodApr 7, 2020
  21. Elijah NewrenApr 7, 2020
  22. Phillip WoodApr 7, 2020
  23. Junio C HamanoApr 7, 2020
  24. 0/5 cleanup ra/rebase-i-more-optionsPhillip Wood, Apr 29, 2020
  25. 1/5 rebase -i: add --ignore-whitespace flagPhillip Wood, Apr 29, 2020
  26. Elijah NewrenMay 13, 2020
  27. Phillip WoodMay 14, 2020
  28. 3/5 sequencer: rename amend_author to author_to_freePhillip Wood, Apr 29, 2020
  29. 2/5 rebase -i: support --committer-date-is-author-datePhillip Wood, Apr 29, 2020
  30. Alban GruinMay 10, 2020
  31. Elijah NewrenMay 13, 2020
  32. 5/5 rebase: add --reset-author-datePhillip Wood, Apr 29, 2020
  33. 4/5 rebase -i: support --ignore-datePhillip Wood, Apr 29, 2020
  34. Alban GruinMay 10, 2020
  35. Phillip WoodMay 12, 2020
  36. Junio C HamanoMay 13, 2020
  37. Elijah NewrenMay 13, 2020
  38. Junio C HamanoApr 29, 2020
  39. Elijah NewrenMay 13, 2020
  40. 0/5 cleanup ra/rebase-i-more-optionsPhillip Wood, May 21, 2020
  41. 1/5 rebase -i: add --ignore-whitespace flagPhillip Wood, May 21, 2020
  42. 2/5 rebase -i: support --committer-date-is-author-datePhillip Wood, May 21, 2020
  43. 5/5 rebase: add --reset-author-datePhillip Wood, May 21, 2020
  44. 4/5 rebase -i: support --ignore-datePhillip Wood, May 21, 2020
  45. Đoàn Trần Công DanhMay 23, 2020
  46. Phillip WoodMay 23, 2020
  47. Đoàn Trần Công DanhMay 23, 2020
  48. Phillip WoodMay 23, 2020
  49. Đoàn Trần Công DanhMay 23, 2020
  50. Phillip WoodMay 27, 2020
  51. Junio C HamanoMay 24, 2020
  52. 3/5 sequencer: rename amend_author to author_to_freePhillip Wood, May 21, 2020
  53. Elijah NewrenMay 22, 2020
  54. Phillip WoodMay 23, 2020
  55. Johannes SchindelinMay 23, 2020
  56. 0/5 cleanup ra/rebase-i-more-optionsPhillip Wood, May 27, 2020
  57. 1/5 rebase -i: add --ignore-whitespace flagPhillip Wood, May 27, 2020
  58. Johannes SchindelinMay 29, 2020
  59. Kerry, RichardJun 1, 2020
  60. Phillip WoodJun 1, 2020
  61. Johannes SchindelinJun 1, 2020
  62. 2/5 rebase -i: support --committer-date-is-author-datePhillip Wood, May 27, 2020
  63. Johannes SchindelinMay 29, 2020
  64. Phillip WoodJun 1, 2020
  65. 3/5 sequencer: rename amend_author to author_to_freePhillip Wood, May 27, 2020
  66. 4/5 rebase -i: support --ignore-datePhillip Wood, May 27, 2020
  67. 5/5 rebase: add --reset-author-datePhillip Wood, May 27, 2020
  68. 6/5 fixup! rebase: add --reset-author-datePhillip Wood, May 27, 2020
  69. Đoàn Trần Công DanhMay 28, 2020
  70. Johannes SchindelinMay 29, 2020
  71. Phillip WoodJun 1, 2020
  72. Junio C HamanoMay 27, 2020
  73. 0/5 cleanup ra/rebase-i-more-optionsPhillip Wood, Jun 26, 2020
  74. 1/5 rebase -i: add --ignore-whitespace flagPhillip Wood, Jun 26, 2020
  75. Đoàn Trần Công DanhJun 26, 2020
  76. Phillip WoodJun 26, 2020
  77. Junio C HamanoJun 26, 2020
  78. Đoàn Trần Công DanhJun 29, 2020
  79. Phillip WoodJul 13, 2020
  80. Junio C HamanoJun 26, 2020
  81. 2/5 rebase -i: support --committer-date-is-author-datePhillip Wood, Jun 26, 2020
  82. 3/5 sequencer: rename amend_author to author_to_freePhillip Wood, Jun 26, 2020
  83. 4/5 rebase -i: support --ignore-datePhillip Wood, Jun 26, 2020
  84. Đoàn Trần Công DanhJun 26, 2020
  85. Phillip WoodJun 26, 2020
  86. Junio C HamanoJun 26, 2020
  87. 5/5 rebase: add --reset-author-datePhillip Wood, Jun 26, 2020
  88. Junio C HamanoJun 26, 2020
  89. Phillip WoodJun 26, 2020
  90. Junio C HamanoJun 26, 2020
  91. 0/5 fixup ra/rebase-i-more-optionsPhillip Wood, Jul 13, 2020
  92. 2/5 rebase -i: support --committer-date-is-author-datePhillip Wood, Jul 13, 2020
  93. Đoàn Trần Công DanhJul 15, 2020
  94. Phillip WoodJul 16, 2020
  95. Đoàn Trần Công DanhJul 16, 2020
  96. Phillip WoodJul 16, 2020
  97. Phillip WoodJul 16, 2020
  98. Đoàn Trần Công DanhJul 17, 2020
  99. 1/5 rebase -i: add --ignore-whitespace flagPhillip Wood, Jul 13, 2020
  100. 3/5 sequencer: rename amend_author to author_to_freePhillip Wood, Jul 13, 2020
  101. 4/5 rebase -i: support --ignore-datePhillip Wood, Jul 13, 2020
  102. 5/5 rebase: add --reset-author-datePhillip Wood, Jul 13, 2020
  103. Junio C HamanoJul 13, 2020
  104. Phillip WoodJul 15, 2020
  105. 0/5 cleanup ra/rebase-i-more-optionsPhillip Wood, Jul 16, 2020
  106. 1/5 rebase -i: add --ignore-whitespace flagPhillip Wood, Jul 16, 2020
  107. 2/5 rebase -i: support --committer-date-is-author-datePhillip Wood, Jul 16, 2020
  108. Johannes SchindelinAug 13, 2020
  109. 3/5 sequencer: rename amend_author to author_to_freePhillip Wood, Jul 16, 2020
  110. 5/5 rebase: add --reset-author-datePhillip Wood, Jul 16, 2020
  111. 4/5 rebase -i: support --ignore-datePhillip Wood, Jul 16, 2020
  112. Johannes SchindelinAug 13, 2020
  113. Junio C HamanoJul 16, 2020
  114. Johannes SchindelinAug 13, 2020
  115. Junio C HamanoAug 13, 2020
  116. Phillip WoodAug 13, 2020
  117. 0/5 cleanup ra/rebase-i-more-optionsPhillip Wood, Aug 17, 2020
  118. 2/5 am: stop exporting GIT_COMMITTER_DATEPhillip Wood, Aug 17, 2020
  119. Junio C HamanoAug 17, 2020
  120. Phillip WoodAug 19, 2020
  121. Junio C HamanoAug 19, 2020
  122. Phillip WoodAug 20, 2020
  123. 4/5 rebase -i: support --ignore-datePhillip Wood, Aug 17, 2020
  124. Phillip WoodAug 19, 2020
  125. Junio C HamanoAug 19, 2020
  126. Phillip WoodAug 20, 2020
  127. 5/5 rebase: add --reset-author-datePhillip Wood, Aug 17, 2020
  128. 3/5 rebase -i: support --committer-date-is-author-datePhillip Wood, Aug 17, 2020
  129. Junio C HamanoAug 26, 2020
  130. 1/5 rebase -i: add --ignore-whitespace flagPhillip Wood, Aug 17, 2020

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.