[LTP] [PATCH v4 1/3] lib: Add support option for .needs_cmds

[Wei Gao]

On Wed, Oct 22, 2025 at 05:23:16PM +0800, Li Wang wrote:
> Hi  Wei,
> 
> Petr Vorel <pvorel@suse.cz> wrote:
> 
> 
> >
> > > need every commits can pass compile phase then i have to combine all
> > > commits into a single big one, is that your request?
> >
> > No, that's other extreme :). There is something in between, right?
> > You did not get me correct, therefore in v4 you not only kept broken
> > functionality, but you also joined the part which could be separated.  At
> > least
> > "ioctl_loop01.c: Update to new .needs_cmds struct" from v3 could have been
> > added
> > as a separate commit (after the main change, not before). Or am I missing
> > something?
> >
> > Unfortunately "lib: Add support option for .needs_cmds" and "Update test
> > cases use
> > new needs_cmds" and "tst_run_shell.c: Add new function handle new
> > needs_cmds" needs to be in a single commit, but maybe you could add
> > functions
> > which implement it in a separate commits (e.g. preparation for a new
> > change) and
> > do the change (when it's actually used) in the last commit).  I'm not sure
> > if
> > it's worth of a separation, maybe not (hopefully we get a feedback from
> > others).
> > If yes:
> >
> 
> Yes.
> 
> 
> > 1) commit (lib preparation) would have: struct tst_cmd, bool
> > tst_cmd_present(const char *cmd)
> >
> > 2) commit (shell loader preparation) would have: enum cmd_ids, static
> > ujson_obj_attr cmd_attrs[], static ujson_obj cmd_obj, static struct tst_cmd
> > *parse_cmds(ujson_reader *reader, ujson_val *val).
> >
> > 3) commit (use new functionality) would have: from "lib: Add support
> > option for
> > .needs_cmds":
> >
> > -       const char *const *needs_cmds;
> > +       struct tst_cmd *needs_cmds;
> >
> > and use of tst_check_cmd()
> >
> > from "tst_run_shell.c: Add new function handle new needs_cmds"
> > -                       test.needs_cmds = parse_strarr(&reader, &val);
> > +                       test.needs_cmds = parse_cmds(&reader, &val);
> >
> > all code changes in "Update test cases use new needs_cmds"
> >
> > 4) "ioctl_loop01.c: Update to new .needs_cmds struct" from v3 can be
> > separate,
> > it just have to be after library function changed.
> >
> > 5) commit: modify some test to actually use some of new functionality.
> >
> 
> Obviously, this is a good suggestion.
> 
> Wei, if you take a look at the git-history of LTP feature change,
> most patchset organizations follow this principle.
> 
> We must keep a clean code and compile pass at the same time,
> I have roughly gone through your patches, and they are seems
> not so hard to rebase.
> 
> 
> 
> >
> > If we are ok to do too many separate commits, then:
> >
> > 1) commit: everything from this v4 in a single commit, but separate at
> > least
> > "ioctl_loop01.c: Update to new .needs_cmds struct" from v3.
> >
> > 2) "ioctl_loop01.c: Update to new .needs_cmds struct" from v3.
> >
> > 3) commit: modify some test to actually use some of new functionality.
> >
> 
> Yes, this is also acceptable to me.
Li, Thanks for your feedback, will plan next patch.
> 
> -- 
> Regards,
> Li Wang