[m-rev.] for review: Ignore target-specific make variables when computing VPATH.

[Zoltan Somogyi]

On 2023-10-16 14:13 +11:00 AEDT, "Peter Wang" <novalazy at gmail.com> wrote:
>     Make EXTRA_INIT_DIRS_FOR_VPATH refer to EXTRA_INIT_DIRS_NO_TARGET.

I would add "instead of ...".

> @@ -47,20 +47,29 @@ endif
>  # function is problematic; hence the `$(nullstring)' hack.
>  # As VPATH uses colon as the separator we also need to convert all the
>  # directories into cygwin unix format if they are in windows format.
> +# XXX VPATH also allows space separators since 1992.
>  # XXX Note that directory names with spaces in them (e.g. under Windows)
>  # will cause problems for the VPATH settings below, as well as every
>  # occurrence of $(patsubst ...), since GNU Make does not respect quotes.
> -UNIX_MERCURY_EXTRA_INT_DIRS = $(foreach dir, $(MERCURY_EXTRA_INT_DIRS),\
> +#
> +# VPATH is evaluated before any targets are considered. At that time, the
> +# automatic variables $@ and $* will be undefined, so the calculation of VPATH
> +# cannot take into account any target-specific variables, e.g. GRADEFLAGS-prog.
> +# Changing the grade with target-specific variables is therefore discouraged,
> +# as the VPATH will not reflect the grade that a particular target is being
> +# built in.

That is a valid concern, but the fact that changing the grade on a module-specific
basis will result in attempts to link together object files compiled in non-matching
and therefore inconsistent grades, is an even greater concern, so I would mention
that concern as well.

> +EXTRA_INT_DIRS_FOR_VPATH = $(foreach dir, $(EXTRA_INT_DIRS),\
>  				$(shell @CYGPATHU@ "$(dir)"))
>  
> -UNIX_MERC_INT_DIR = $(shell @CYGPATHU@ "$(MERC_INT_DIR)")
> -UNIX_MERCURY_EXTRA_INIT_DIRS = $(foreach dir, $(MERCURY_EXTRA_INIT_DIRS),\
> +MERC_INT_DIR_FOR_VPATH = $(shell @CYGPATHU@ "$(MERC_INT_DIR)")
> +EXTRA_INIT_DIRS_FOR_VPATH = $(foreach dir, $(EXTRA_INIT_DIRS_NO_TARGET),\
>  				$(shell @CYGPATHU@ "$(dir)"))
>  
>  nullstring =
>  MMAKE_VPATH	= $(subst $(nullstring) ,:,$(strip \
> -			$(UNIX_MERCURY_EXTRA_INT_DIRS) $(UNIX_MERC_INT_DIR)\
> -			$(UNIX_MERCURY_EXTRA_INIT_DIRS)))
> +			$(EXTRA_INT_DIRS_FOR_VPATH) \
> +			$(MERC_INT_DIR_FOR_VPATH)\
> +			$(EXTRA_INIT_DIRS_FOR_VPATH)))
>  VPATH		= $(MMAKE_VPATH) # do not remove the `:' from this comment!!!
>  #			 the above comment works around a misfeature of
>  #			 autoconf which causes it to delete assignments to
> @@ -70,6 +79,8 @@ GPATH		= $(VPATH)
>  DEFAULT_GRADE	= $(MERCURY_DEFAULT_GRADE)
>  GRADE		= $(DEFAULT_GRADE)
>  GRADESTRING	= $(shell $(MCOGS) $(ALL_GRADEFLAGS) $(ALL_MCFLAGS))
> +GRADESTRING_NO_TARGET = $(shell $(MCOGS) $(ALL_GRADEFLAGS_NO_TARGET) \
> +			  $(ALL_MCFLAGS_NO_TARGET))

I don't know whether my mail agent is screwing up indentation, but in cases like this,
I like to line up the identical parts of definitions, so people can see more easily
which parts of the right hand side are identical between the two variables
and which aren't.

There are more instances of that below.

The rest seems fine.

Zoltan.