Re: [PATCH 0/8] sequencer refactoring

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mar 25, 2023, 11:08 UTC
Message-ID: <f72b3820-124c-3e2c-30e2-ca3f46b74dc0@dunelm.org.uk>
In-Reply-To: <20230323162235.995574-1-oswald.buddenhagen@gmx.de>

Hi Oswald

On 23/03/2023 16:22, Oswald Buddenhagen wrote:

> This is a preparatory series for the separately posted 'rebase --rewind' patch,
> but I think it has value in itself.

I had a hard time applying these patches. In the end I had success with checking out next, applying https://lore.kernel.org/git/20230323162234.995514-1-oswald.buddenhagen@gmx.de and then applying this series. It is very helpful to detail the base commit in the cover letter. A series like this should normally be based on master see Documentation/SubmittingPatches.

Having applied the patches I'm unable to compile them with DEVELOPER=1 (see Documentation/CodingGuidelines)

In file included from log-tree.c:20:
sequencer.h:7:6: error: ISO C forbids forward references to ‘enum’ types 
[-Werror=pedantic]
     7 | enum rebase_action;
       |      ^~~~~~~~~~~~~
sequencer.h:140:34: error: ISO C forbids forward references to ‘enum’ 
types [-Werror=pedantic]
   140 |                             enum rebase_action action);
       |                                  ^~~~~~~~~~~~~
sequencer.h:196:26: error: ISO C forbids forward references to ‘enum’ 
types [-Werror=pedantic]
   196 |                     enum rebase_action action);
       |                          ^~~~~~~~~~~~~

In file included from ./cache.h:12,
                  from ./builtin.h:6,
                  from builtin/rebase.c:8:
builtin/rebase.c: In function ‘cmd_rebase’:
builtin/rebase.c:1246:95: error: left-hand operand of comma expression 
has no effect [-Werror=unused-value]
  1246 | 
(BUILD_ASSERT_OR_ZERO(ARRAY_SIZE(action_names) == ACTION_LAST),
       | 
                               ^
./trace2.h:158:69: note: in definition of macro ‘trace2_cmd_mode’
   158 | #define trace2_cmd_mode(sv) trace2_cmd_mode_fl(__FILE__, 
__LINE__, (sv))
       | 
     ^~

sequencer.c: In function ‘todo_list_rearrange_squash’:
sequencer.c:6346:23: error: operation on ‘items’ may be undefined 
[-Werror=sequence-point]
  6346 |                 items = ALLOC_ARRAY(items, todo_list->nr);

Best Wishes

Phillip

Show 21 quoted lines

> 
> Oswald Buddenhagen (8):
>    rebase: simplify code related to imply_merge()
>    rebase: move parse_opt_keep_empty() down
>    sequencer: pass around rebase action explicitly
>    sequencer: create enum for edit_todo_list() return value
>    rebase: preserve interactive todo file on checkout failure
>    sequencer: simplify allocation of result array in
>      todo_list_rearrange_squash()
>    sequencer: pass `onto` to complete_action() as object-id
>    rebase: improve resumption from incorrect initial todo list
> 
>   builtin/rebase.c              |  63 +++++++--------
>   builtin/revert.c              |   3 +-
>   rebase-interactive.c          |  36 ++++-----
>   rebase-interactive.h          |  27 ++++++-
>   sequencer.c                   | 139 +++++++++++++++++++---------------
>   sequencer.h                   |  15 ++--
>   t/t3404-rebase-interactive.sh |  34 ++++++++-
>   7 files changed, 196 insertions(+), 121 deletions(-)
>