libsubprocess: pass flags to remote subprocess

[chu11]

It appears that subprocess flags cannot be passed to remote sub processes.

Originally only the flags FLUX_SUBPROCESS_FLAGS_STDIO_FALLTHROUGH and FLUX_SUBPROCESS_FLAGS_SETPGRP were supported. The former flag has no purpose with remote subprocesses and it appears the latter is hard coded for the server.

    if (!(p = flux_local_exec_ex (flux_get_reactor (s->h),
                                  FLUX_SUBPROCESS_FLAGS_SETPGRP,
                                  cmd,
                                  &ops,
                                  NULL,
                                  s->llog,
                                  s->llog_data))) {
        errprintf (&error, "error launching process: %s", strerror (errno));
        errmsg = error.text;
        goto error;
    }

B/c of this, I'm guessing passing flags along didn't matter much, so it wasn't supported or was just outright missed.

With the addition of the FLUX_SUBPROCESS_FLAGS_FORK_EXEC flag (force fork() even if posix_spawn() is available), this is something we may want to pass along some day. As well as future flags.

This doesn't appear to be a problem at the moment, but its something to eventually support.

[chu11]

related to this

flux_future_t *subprocess_rexec (flux_t *h,                                                                                              
                                 const char *service_name,                                                                               
                                 uint32_t rank,                                                                                          
                                 flux_cmd_t *cmd,                                                                                        
                                 int flags);    

we should probably rename "flags" here, as these are rexec specific flags that are internal to the implementation, they aren't the subprocess flags from the API.

Unfortunately the rexec server uses flags generically

    if (!(f = flux_rpc_pack (h,                                                                                                          
                             topic,                                                                                                      
                             rank,                                                                                                       
                             FLUX_RPC_STREAMING,                                                                                         
                             "{s:O s:i}",                                                                                                
                             "cmd", ctx->cmd,                                                                                            
                             "flags", ctx->flags))                                                                                       
        || flux_future_aux_set (f,                                                                                                       
                                "flux::rexec",                                                                                           
                                ctx,                                                                                                     
                                (flux_free_f)rexec_ctx_destroy) < 0) {                                                                   
        rexec_ctx_destroy (ctx);                                                                                                         
        goto error;                                                                                                                      
    }                                                     

so to maintain backwards compatibility we wouldn't want to rename the key in the RPC if we pass along subprocess flags in some way.

[garlick]

For background the protocol was simplified in #5004 and that is when the flags were added as a replacement for three explicit booleans that told whether some of the callbacks had been registered. See also RFC 42. (The current flags are a little bit strange and might need an audit - I was trying not to change too much)

The point of switching to a flags integer from the booleans was to make it easier to add more possibly server dependent flags. So we could add that one you mentioned if there were a need. I don't think so at the moment though.

[chu11]

I experimented passing subprocess flags to remote processes and it is pretty annoying, especially having all calls to flux_rexec() now passing the SETPGRP flag.

Then it occurred to me, almost all calls to flux_local_exec() and flux_rexec() seem to set SETPGRP. The only test that fails when I simply make SETPGRP the default is the shell's nosetpgrp option.

Alternately, perhaps we could default to always setpgrp() (both local and remote) and change the flag to NO_SETPGRP. That way this one random subprocess flag isn't just assumed for remote subprocesses and not for local ones.

As an aside, the main reason this issue bothers me is that local processes default to not call setpgrp() and remote processes do default to call setpgrp(). So it just seems inconsistent and unexpected. It's probably a side effect of some random need for setpgrp() on remote subprocesses a long time ago, so it was just made a default.