On Mon, Aug 07, 2023 at 04:42:54PM +0200, Marc Espie wrote:
> I think it could be occasionally useful to know which variables have
> been defined in make.
>
> Incidentally, this DOES exist in GNU make, so I've reused the same name
> for basically the same functionality.
>
> I haven't checked whether NetBSD/FreeBSD introduced something similar.
>
> This is a fairly straightforward patch, introduces .VARIABLES corresponding
> to the full list of global variables (from the command line and the Makefile)
> that have been defined.
>
> (^ says the guy who had to remember a few details from his own(!) var.c
> implementation from a few years ago)
>
> I just took var_get_value offline from the old macro, for readability,
> even though it's likely the compiler may still decide to inline it.
>
> For efficiency, that list is only computed as needed, since it is
> somewhat long.
>
> For debugging purposes, this can come in fairly handy, and I see at
> least another application in ports.
>
> Comments and nits welcome.
>
> Note that the list is completely unsorted. This could be sorted through
> since we already have the code for dumping purposes, but it would be even
> more expensive (the order will be "random", as per the hash)
I can't judge all the implications of this, but can speak to that I
compiled it and that I get a very long list of variables with:
$ make show=.VARIABLES
> Index: var.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/make/var.c,v
> retrieving revision 1.104
> diff -u -p -r1.104 var.c
> --- var.c 9 Jun 2022 13:13:14 -0000 1.104
> +++ var.c 7 Aug 2023 14:33:42 -0000
> @@ -104,6 +104,8 @@ static char varNoError[] = "";
> bool errorIsOkay;
> static bool checkEnvFirst; /* true if environment should be searched for
> * variables before the global context */
> + /* do we need to recompute varname_list */
> +static bool varname_list_changed = true;
I assume the comment /* do we need to recompute varname_list */ is not
meant to be contiguous with the 2 comment lines above (/* true if...
* variables before...). I suggest using whitespace (newline?) to make
that clearer.
I looked through the file and it seems that varname_list_changed is
never set to anything but true. Is there a bit missing, like down lower
in varname_list_retrieve()?
>
> void
> Var_setCheckEnvFirst(bool yes)
> @@ -228,9 +230,12 @@ typedef struct Var_ {
> */
> #define POISONS (POISON_NORMAL | POISON_EMPTY | POISON_NOT_DEFINED)
> /* Defined in var.h */
> +#define VAR_IS_NAMES 1024 /* Very expensive, only defined when needed */
> char name[1]; /* the variable's name */
> } Var;
>
> +/* for GNU make compatibility */
> +#define VARNAME_LIST ".VARIABLES"
>
> static struct ohash_info var_info = {
> offsetof(Var, name),
> @@ -245,10 +250,11 @@ static void fill_from_env(Var *);
> static Var *create_var(const char *, const char *);
> static void var_set_initial_value(Var *, const char *);
> static void var_set_value(Var *, const char *);
> -#define var_get_value(v) ((v)->flags & VAR_EXEC_LATER ? \
> - var_exec_cmd(v) : \
> - Buf_Retrieve(&((v)->val)))
> -static char *var_exec_cmd(Var *);
> +static char *var_get_value(Var *);
> +static void var_exec_cmd(Var *);
> +static void varname_list_retrieve(Var *);
> +
> +
> static void var_append_value(Var *, const char *);
> static void poison_check(Var *);
> static void var_set_append(const char *, const char *, const char *, int,
> bool);
> @@ -423,6 +429,7 @@ var_set_initial_value(Var *v, const char
> len = strlen(val);
> Buf_Init(&(v->val), len+1);
> Buf_AddChars(&(v->val), len, val);
> + varname_list_changed = true;
> }
>
> /* Normal version of var_set_value(), to be called after variable is fully
> @@ -440,6 +447,16 @@ var_set_value(Var *v, const char *val)
> }
> }
>
> +static char *
> +var_get_value(Var *v)
> +{
> + if (v->flags & VAR_IS_NAMES)
> + varname_list_retrieve(v);
> + else if (v->flags & VAR_EXEC_LATER)
> + var_exec_cmd(v);
> + return Buf_Retrieve(&(v->val));
> +}
> +
> /* Add to a variable, insert a separating space if the variable was already
> * defined.
> */
> @@ -628,6 +645,7 @@ Var_Deletei(const char *name, const char
>
> ohash_remove(&global_variables, slot);
> delete_var(v);
> + varname_list_changed = true;
> }
>
> /* Set or add a global variable, either to VAR_CMD or VAR_GLOBAL.
> @@ -687,7 +705,7 @@ Var_Appendi_with_ctxt(const char *name,
> var_set_append(name, ename, val, ctxt, true);
> }
>
> -static char *
> +static void
> var_exec_cmd(Var *v)
> {
> char *arg = Buf_Retrieve(&(v->val));
> @@ -699,7 +717,27 @@ var_exec_cmd(Var *v)
> var_set_value(v, res1);
> free(res1);
> v->flags &= ~VAR_EXEC_LATER;
> - return Buf_Retrieve(&(v->val));
> +}
> +
> +static void
> +varname_list_retrieve(Var *v)
> +{
> + unsigned int i;
> + void *e;
> + bool first = true;
> +
> + for (e = ohash_first(&global_variables, &i); e != NULL;
> + e = ohash_next(&global_variables, &i)) {
> + Var *v2 = e;
> + if (v2->flags & VAR_DUMMY)
> + continue;
> +
> + if (first)
> + var_set_value(v, v2->name);
> + else
> + var_append_value(v, v2->name);
> + first = false;
> + }
> }
>
> /* XXX different semantics for Var_Valuei() and Var_Definedi():
> @@ -1339,6 +1377,19 @@ set_magic_shell_variable()
> v->flags = VAR_IS_SHELL | VAR_SEEN_ENV;
> }
>
> +static void
> +set_magic_name_list_variable()
> +{
> + const char *name = VARNAME_LIST;
> + const char *ename = NULL;
> + uint32_t k;
> + Var *v;
> +
> + k = ohash_interval(name, &ename);
> + v = find_global_var_without_env(name, ename, k);
> + var_set_initial_value(v, "");
> + v->flags = VAR_IS_NAMES;
> +}
> /*
> * Var_Init
> * Initialize the module
> @@ -1348,11 +1399,10 @@ Var_Init(void)
> {
> ohash_init(&global_variables, 10, &var_info);
> set_magic_shell_variable();
> -
> + set_magic_name_list_variable();
>
> errorIsOkay = true;
> Var_setCheckEnvFirst(false);
> -
> VarModifiers_Init();
> Buf_Init(&subst_buffer, MAKE_BSIZE);
> }
>
