[PATCH] Notify when finished

Mon Aug 26 14:19:21 UTC 2024

On Mon, 26 Aug 2024 14:00:29 +0000
Yao Zi <ziyao at disroot.org> wrote:

> On Mon, Aug 26, 2024 at 02:46:48PM +0200, Harkaitz Agirre wrote:
> > This patch introduces the "notify me when finished" feature in
> > Busybox Shell.
> > 
> > When executing long-running commands (such as compiling code or
> > performing backups), it can be useful to receive notifications when
> > these tasks finish.
> > 
> > This allows the user to step away from the terminal, perhaps to
> > grab a coffee.
> > 
> > The need for a notification is usually an afterthought, so it is not
> > practical to add a prefix to the commands in order to being
> > notified when finished. Instead, it is more convenient to have a
> > mechanism that automatically alerts the user when a command takes
> > longer than expected.
> > 
> > This change makes ash read two variables, TIMED_ALERT and
> > TIMED_ALERT_SECS.
> > 
> > If a command execution exceeds the time specified in
> > $TIMED_ALERT_SECS (by default 60 seconds) executes the command in
> > $TIMED_ALERT (if set).
> > 
> > The command in $TIMED_ALERT can consult
> > $TIMED_COMMAND,$TIMED_COMMAND_DURATION and $? to prepare the
> > message to send.
> > 
> > One example:
> > 
> > $ export TIMED_ALERT='notify-send "Command finished" "Command
> > $TIMED_COMMAND finished in $TIMED_COMMAND_DURATION seconds with
> > exit status $?"' $ export TIMED_ALERT_SECS=10
> > 
> > Services like ntfy.sh etc can be used to notify the phone.
> > ---
> >  shell/ash.c | 83
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file
> > changed, 82 insertions(+), 1 deletion(-)
> > 
> > diff --git a/shell/ash.c b/shell/ash.c
> > index bbd730770..a764f6131 100644
> > --- a/shell/ash.c
> > +++ b/shell/ash.c
> > @@ -119,6 +119,18 @@
> >  //config:	are checked for mtime changes, and "you have mail"
> >  //config:	message is printed if change is detected.
> >  //config:
> > +//config:config ASH_TIMED_ALERT
> > +//config:	bool "Enable notify when finished feature"
> > +//config:	default y
> > +//config:	depends on SHELL_ASH
> > +//config:	help
> > +//config:	Enable "notify when finished" function:
> > +//config:	if set, if $TIMED_ALERT is set and a command lasts
> > +//config:	longer that $TIMED_ALERT_SECS (by default 60) then
> > +//config:	the command in $TIMED_ALERT is executed. The
> > command +//config:	and it's duration can be fetched
> > consulting $TIMED_COMMAND +//config:	,
> > $TIMED_COMMAND_DURATION and $?. +//config:
> >  //config:config ASH_ECHO
> >  //config:	bool "echo builtin"
> >  //config:	default y
> > @@ -2134,6 +2146,10 @@ change_lc_ctype(const char *value)
> >  static void changemail(const char *var_value) FAST_FUNC;
> >  #endif
> >  static void changepath(const char *) FAST_FUNC;
> > +#if ENABLE_ASH_TIMED_ALERT
> > +static void change_timed_alert(const char *) FAST_FUNC;
> > +static void change_timed_alert_secs(const char *) FAST_FUNC;
> > +#endif
> >  #if ENABLE_ASH_RANDOM_SUPPORT
> >  static void change_random(const char *) FAST_FUNC;
> >  #endif
> > @@ -2155,6 +2171,10 @@ static const struct {
> >  #if ENABLE_ASH_MAIL
> >  	{ VSTRFIXED|VTEXTFIXED|VUNSET, "MAIL"      , changemail
> >   }, { VSTRFIXED|VTEXTFIXED|VUNSET, "MAILPATH"  , changemail      },
> > +#endif
> > +#if ENABLE_ASH_TIMED_ALERT
> > +	{ VSTRFIXED|VTEXTFIXED|VUNSET, "TIMED_ALERT"     ,
> > change_timed_alert     },
> > +	{ VSTRFIXED|VTEXTFIXED|VUNSET, "TIMED_ALERT_SECS",
> > change_timed_alert_secs}, #endif
> >  	{ VSTRFIXED|VTEXTFIXED       , bb_PATH_root_path,
> > changepath }, { VSTRFIXED|VTEXTFIXED       , "PS1=$ "    , NULL
> >        }, @@ -2214,7 +2234,7 @@ extern struct globals_var
> > *BB_GLOBAL_CONST ash_ptr_to_globals_var; # define vmail
> > varinit[1] # define vmpath   varinit[2]
> >  #endif
> > -#define VAR_OFFSET1 (ENABLE_ASH_MAIL*2)
> > +#define VAR_OFFSET1 (ENABLE_ASH_MAIL*2+ENABLE_ASH_TIMED_ALERT*2)
> >  #define vpath     varinit[VAR_OFFSET1 + 1]
> >  #define vps1      varinit[VAR_OFFSET1 + 2]
> >  #define vps2      varinit[VAR_OFFSET1 + 3]
> > @@ -9464,6 +9484,63 @@ evaltree(union node *n, int flags)
> >  	return exitstatus;
> >  }
> >  
> > +#if ENABLE_ASH_TIMED_ALERT
> > +
> > +static const char *timed_alert = NULL;
> > +static long        timed_alert_secs = 60;
> > +
> > +static void FAST_FUNC
> > +change_timed_alert(const char *val)
> > +{
> > +	timed_alert = val;
> > +}
> > +
> > +static void FAST_FUNC
> > +change_timed_alert_secs(const char *val)
> > +{
> > +	long l = atol(val);
> > +	if (l > 0)
> > +		timed_alert_secs = l;
> > +}  
> 
> It may be better to disable the alert if an invalid timeout duration
> is given, instead of keeping the original value.
> 
> > +
> > +/*
> > + * Evaluate a parse tree.  The value is left in the global variable
> > + * exitstatus.
> > + */
> > +static int
> > +evaltree_with_timed_alert(union node *n, int flags)
> > +{
> > +	time_t command_start_time, command_end_time,
> > command_duration;
> > +	int result;
> > +	  
> 
> There are some trailing spaces. Is it an editor issue? You should
> remove them.
> 
> > +	if (!iflag || !timed_alert || !timed_alert[0]) {
> > +		return evaltree(n, flags);
> > +	}
> > +	
> > +	command_start_time = time(NULL);
> > +	result = evaltree(n, flags);
> > +	command_end_time = time(NULL);
> > +	
> > +	command_duration = command_end_time - command_start_time;
> > +	
> > +	if (command_duration >= timed_alert_secs) {
> > +		if (g_parsefile->next_to_pgetc > g_parsefile->buf
> > && g_parsefile->next_to_pgetc[-1] == '\n') {
> > +			g_parsefile->next_to_pgetc[-1] = '\0';
> > +		} else {
> > +			*g_parsefile->next_to_pgetc = '\0';
> > +		}
> > +		setvar("TIMED_COMMAND", g_parsefile->buf, VEXPORT);
> > +		setvar("TIMED_COMMAND_DURATION",
> > utoa(command_duration), VEXPORT);
> > +		evalstring(timed_alert, 0);
> > +		unsetvar("TIMED_COMMAND");
> > +		unsetvar("TIMED_COMMAND_DURATION");
> > +	}
> > +	
> > +	return result;
> > +}
> > +
> > +#endif
> > +
> >  static int
> >  skiploop(void)
> >  {
> > @@ -13641,7 +13718,11 @@ cmdloop(int top)
> >  			/* job_warning can only be 2,1,0. Here
> > 2->1, 1/0->0 */ job_warning >>= 1;
> >  			numeof = 0;
> > +#if ENABLE_ASH_TIMED_ALERT
> > +			i = evaltree_with_timed_alert(n, 0);
> > +#else
> >  			i = evaltree(n, 0);
> > +#endif
> >  			if (n)
> >  				status = i;
> >  		}
> > -- 
> > 2.39.2
> >   
> 
> I doubt whether this function is valuable enough to be implemented in
> shell. Users could simply use a script to notify themselves when
> running a long script/command.
> 
> afaik, even bash doesn't have similar functions by default and I don't
> think busybox ash is the correct place to implement rich interactive
> functions. Things should be done in another way here.
> 
> Best regards,
> Yao Zi

Thank you for your suggestions and time,

I agree with the changes you propose to the code.

As I explained to Laurent the point here is that the need for
the notification is usually an afterthought, nearly always I
realize the need for the notification after I the command is
already running.

I already sent a patch to the Bash developers, I implemented this
for both bash and busybox long time ago.

Harkaitz 