[jamming] Command built-in questions (was Re: Executing shell command, save results to variable)

[David Turner]

Helo Diane,

> Only if you promise never to put two n's in my name again :)
> 
ok, I promise :-) And I'm sorry

> Here you go... the usual disclaimers regarding not being warranted,
> expressed or implied, as to usability, correctness, and all that jazz
> apply
> ... IOW, use at your own risk :)
> 

ok, I've tested it yesterday night on my build. It seems that "popen" is
an issue in Win32 with at least Watcom C++ and Digital Mars C++ which don't
provide it in their C runtime; Also Borland C++ does compile the Jam executable
which later insists on dynamically linking to BC3250MT.DLL :-( I could link
statically to the Borland C library, but something tells me this is going to
bloat the executable in interesting ways...

I think we're going to need a proper Win32 implementation in the sources.

There is another more important issue that I'd like to address with the people
on this list. Diane's current patch decomposes the command output in the following
way:

- if the output contains a single '\n' character, each element of the result list
  corresponds to a single line of the output

- otherwise, i.e. if the output is made of a single line, the result is a list
  of words taken from the output

Some could say that the "Command" built-in introduces some amount to randomness
to Jamfiles, and I'd like to control this as much as possible. This is why I would
propose that:

- the default behaviour is to always return the output as a list of words

- the builtin accepts parameters as its second argument, specifying modifiers
  for example:

    # retrieve each line of the output as a single element of the result
    X = [ Command  some command : lines ] ;

    # equivalent to default behaviour
    Y = [ Command other command : words ] ;

    # Diane's default behaviour
    Z = [ Command again another command : mixed ] ;

the "lines", "words", and "mixed" modifiers given there are simple examples, but you
get the idea. So my question would be:

- is there some serious use to the "mixed" and "lines" cases ? we really don't want
  to turn Jamfiles into parsing programs, don't we.

- which default behaviour would you prefer ? (I'm for "words", but YMMV)


Voila, any comments welcomed. Hopefully the next release should be soon...

- David



> One thing I was reminded of, when I went to look the change back up, is
> that
> I put a hard-coded
> 
> #define HAVE_POPEN 1
> 
> in jam.h. You may want to consider doing something a bit more legitimate
> for
> determining whether HAVE_POPEN should be defined or not :)
> 
> Also, if you have any suggestions for improvements (or find anything
> buggy),
> I hope you'll share them back.
> 
> The diff is for builtins.c (see my earlier posting for actually using
> Command).
> 
> Have fun,
> Diane
> ========================================================================
> 
> @@ -55,6 +55,7 @@
>  LIST *builtin_flags( PARSE *parse, LOL *args, int *jmp );
>  LIST *builtin_glob( PARSE *parse, LOL *args, int *jmp );
>  LIST *builtin_match( PARSE *parse, LOL *args, int *jmp );
> +LIST *builtin_command( PARSE *parse, LOL *args, int *jmp );
> 
>  int glob( const char *s, const char *c );
> 
> @@ -111,6 +112,10 @@
>      bindrule( "Temporary" )->procedure =
>      bindrule( "TEMPORARY" )->procedure =
>         parse_make( builtin_flags, P0, P0, P0, C0, C0, T_FLAG_TEMP );
> +
> +       bindrule( "Command" )->procedure =
> +       bindrule( "COMMAND" )->procedure =
> +       parse_make( builtin_command, P0, P0, P0, C0, C0, 0);
>  }
> 
>  /*
> @@ -316,3 +321,105 @@
> 
>         return result;
>  }
> +
> +
> +#ifdef HAVE_POPEN
> +#if defined(_MSC_VER) || defined(__BORLANDC__)
> +    #define popen _popen
> +    #define pclose _pclose
> +#endif
> +
> +typedef struct {
> +       int     len;
> +       char *str;
> +} string;
> +
> +void string_new(string *s) {
> +       s->len = 0;
> +       s->str = NULL;
> +}
> +
> +void string_append(string *s, char *buf) {
> +       /* we don't care too much for efficiency */
> +       int l2 = s->len + strlen(buf);
> +       char *n = (char*)malloc(l2 + 1);
> +       /* or error recovery */
> +       if (s->len) strcpy(n, s->str);
> +       strcpy(n+s->len, buf);
> +       free(s->str);
> +       s->str = n;
> +       s->len = l2;
> +}
> +
> +void string_free(string *s) {
> +       if (s->str != NULL)
> +           free(s->str);
> +       s->str = NULL;
> +       s->len = 0;
> +}
> +
> +LIST *builtin_command( PARSE *parse, LOL *args, int *jmp )
> +{
> +       LIST* arg = lol_get( args, 0 );
> +       LIST* result = L0;
> +       string s;
> +       int ret;
> +       char buffer[1024];
> +       FILE *p = NULL;
> +
> +       string_new( &s );
> +
> +       fflush(NULL);
> +
> +       p = popen(arg->string, "r");
> +       if ( p == NULL )
> +           return L0;
> +
> +       while ( (ret = fread(buffer, sizeof(char), sizeof(buffer)-1, p))
> > 0
> )
> +       {
> +           buffer[ret] = 0;
> +           string_append( &s, buffer );
> +       }
> +
> +       pclose(p);
> +
> +       // Return separate list element for each NL-separated string
> +       // in the output of the command. Or, if the ouput has no NL
> +       // but has space-separated words, make each word a list element
> +       // (ie., echo -n output). Otherwise, if it's just one word (NL or
> +       // no), return that.
> +
> +       if (s.str && s.len > 0)
> +       {
> +           int i;
> +           char token = '\n';
> +           const char *sp = s.str;
> +
> +           if (strchr(s.str, '\n') == NULL)
> +               token = ' ';
> +           for (i=0 ; i < s.len ; i++)
> +           {
> +               if ( s.str[i] == token )
> +               {
> +                   s.str[i] = '\0';
> +                   result = list_new( result, sp, 0 );
> +                   sp = (const char*)&s.str[i+1];
> +               }
> +           }
> +           if (*sp != '\0')
> +               result = list_new( result, sp, 0 );
> +       }
> +
> +       string_free(&s);
> +       return result;
> +}
> +
> +#else
> +
> +LIST *builtin_command( PARSE *parse, LOL *args, int *jmp )
> +{
> +    return L0;
> +}
> +
> +#endif
> +