Re: [PATCH v3 5/6] common/config: Introduce _exit wrapper around exit command

[Dave Chinner <david@fromorbit.com>]

On Tue, Apr 08, 2025 at 10:13:54PM +0530, Nirjhar Roy (IBM) wrote:
> On 4/8/25 17:05, Dave Chinner wrote:
> > On Tue, Apr 08, 2025 at 05:37:21AM +0000, Nirjhar Roy (IBM) wrote:
> > > diff --git a/common/config b/common/config
> > > index 79bec87f..eb6af35a 100644
> > > --- a/common/config
> > > +++ b/common/config
> > > @@ -96,6 +96,14 @@ export LOCAL_CONFIGURE_OPTIONS=${LOCAL_CONFIGURE_OPTIONS:=--enable-readline=yes}
> > >   export RECREATE_TEST_DEV=${RECREATE_TEST_DEV:=false}
> > > +# This functions sets the exit code to status and then exits. Don't use
> > > +# exit directly, as it might not set the value of "status" correctly.
> > > +_exit()
> > > +{
> > > +	status="$1"
> > > +	exit "$status"
> > > +}
> > The only issue with putting this helper in common/config is that
> > calling _exit() requires sourcing common/config from the shell
> > context that calls it.
> > 
> > This means every test must source common/config and re-execute the
> > environment setup, even though we already have all the environment
> > set up because it was exported from check when it sourced
> > common/config.
> > 
> > We have the same problem with _fatal() - it is defined in
> > common/config instead of an independent common file. If we put such
> > functions in their own common file, they can be sourced
> > without dependencies on any other common file being included first.
> > 
> > e.g. we create common/exit and place all the functions that
> > terminate tests in it - _fatal, _notrun, _exit, etc and source that
> > file once per shell context before we source common/config,
> > common/rc, etc. This means we can source and call those termination
> > functions from any context without having to worry about
> > dependencies...
> 
> Yes, I agree to the above. Do you want this refactoring to be done as a part
> of this patch series in the further revisions, or can this be sent as a
> separate series?

Seperate series is fine. You're not making the dependency mess any
worse than it already is with this change...

-Dave.
-- 
Dave Chinner
david@fromorbit.com