Re: [Buildroot] [PATCH v2 1/1] package: Add iotools package

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Fri, Oct 18, 2024 at 02:52:20PM +0300, Andy Shevchenko wrote:
> On Thu, Oct 17, 2024 at 10:45:23AM -0500, Aaron Sierra wrote:
> >  > On Mon, Oct 14, 2024 at 05:55:46PM +0000, Vincent Fazio wrote: 
> >  > > Aaron, Andy, 
> >  > > > From: buildroot buildroot-bounces@buildroot.org> On Behalf Of Aaron 
> >  > > > Sierra 
> >  > > > Sent: Monday, October 14, 2024 12:01 PM 
> >  > > > ---- On Mon, 14 Oct 2024 10:38:22 -0500 Andy Shevchenko  wrote --- 

...

> >  > > > 2. It would be nice if the package populated command symlinks in the target 
> >  > > > filesystem. That could be done in a number of way, like using an explicit list of 
> >  > > > commands maintained in the package Makefile (iotools.mk) or adding a host- 
> >  > > > build dependency and leveraging the --list-cmds output to get the list of 
> >  > > > embedded commands for linking. 
> >  > > 
> >  > > Caveat lector, I have not reviewed your patch or the sources in a while. 
> >  > > 
> >  > > WRT to this comment, I have this note from when we ported it: 
> >  > > 
> >  > > # we can't run make install because the symlink installation executes the binary 
> >  > > # which isn't guaranteed to work in cross-compile scenarios 
> >  > > define IOTOOLS_INSTALL_TARGET_CMDS 
> >  > >     $(INSTALL) -D -m 0755 $(@D)/iotools $(TARGET_DIR)/usr/sbin/ 
> >  > > 
> >  > >     for link in $( IOTOOLS_LINKS); do \ 
> >  > >         ln -sf iotools $(TARGET_DIR)/usr/sbin/$link; \ 
> >  > >     done 
> >  > > endef 
> >  > > 
> >  > > We then earlier in the mk file define the tools per platform, akin to: 
> >  >  
> >  > I deliberately won't populate those links. The naming is so generic 
> >  > that it's a real chance to get a collision with all pain of splitting it 
> >  > in the configuration what links we create and what not and what to do 
> >  > with those that may collide. 
> >  >  
> >  > TL;DR: no, I don't think it's a good idea. 
> > 
> > For the benefit of the uninitiated, iotools operates similarly to Busybox,
> > expressing behavior either by:
> >     1. taking the command name from argv[0] (i.e. symlinks named for internal commands)
> >        * This is the mechanism encouraged by the project's make install (i.e.
> >          iotools --make-links)
> >     2. taking the command name from argv[1] (i.e. iotools [[command] [arguments]])
> 
> There is a little nuance between two. When the Busybox is the *main(
> shell/coreutils/util-linux/etc provider it must populate the links, many of
> them (naming wise) are standardized by POSIX. In full opposition to the iotools
> with unpredictable consequences of the potential name clashing (esp. those that
> are called or/xor/and/etc.).

Worth to add that when Busybox is supplied *in the addition to* the main shell
it must NOT populate the links (for the very same reasoning: POSIX standardization).

> > I proposed populating the symlinks in the target filesystem to ensure the
> > subcommands are ready-to-use for people already familiar with the tool. It sounds
> > like you're advocating for #2 ("iotools_fallback") to be the primary mode of
> > operation for Buildroot users.
> 
> Yes, I highly suggest not to go this way.
> 
> > Can you point to documentation that Buildroot users can use to _know_ that the
> > second mode of operation is available and encouraged? I do not see any in the
> > help output or in the project README. Here is the current help output:
> > 
> > $ ./iotools --help
> > usage: ./iotools COMMAND
> >   COMMANDS:
> >     --make-links
> >     --clean-links
> >     --list-cmds
> >     -v --version
> 
> Is it full output? Because to me it shows two screens of the listed commands.
> I do not see an issue here.
> 
> > For the sake of usability, I think it's important to either improve utility and/or
> > project documentation or reconsider your stated position on symlink population.
> 
> I am against populating the links.
> 
> > Feel free to ask for help with either direction you choose.

-- 
With Best Regards,
Andy Shevchenko


_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot