Re: [PATCH 2/2] order_only prereq shall not use parent per_target variables

[Paul Smith]

On Mon, 2014-09-15 at 13:03 +0200, Bernhard Reutner-Fischer wrote:
> Order-only prerequisites should not use the per_target variables of it's
> parent.
> 
> given:
> obj.o: X=-Dobj
> crt1.o:
> obj.o: | crt1.o
> 
> we do not want crt1.o to be built with X set.

I haven't applied this change yet: this is a change in the current
functionality of GNU make and it's not clear to me why target-specific
variables should work this way rather than the way they do now.

I can see arguments both ways.  Why shouldn't an order-only prerequisite
take target-specific values from its parent?


> Signed-off-by: Bernhard Reutner-Fischer <address@hidden>
> ---
>  tests/scripts/features/targetvars | 92 
> +++++++++++++++++++++++++++++++++++++++
>  variable.c                        | 20 ++++++++-
>  2 files changed, 110 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/scripts/features/targetvars 
> b/tests/scripts/features/targetvars
> index a9b8dbe..c1337e2 100644
> --- a/tests/scripts/features/targetvars
> +++ b/tests/scripts/features/targetvars
> @@ -270,4 +270,96 @@ a: ; @echo $(A)
>  # ',
>  #               '', "local\n");
>  
> +# TEST #21: per-target variable should not be inherited by order-only
> +#           prerequisite
> +$details = "\
> +Per-target variable should not be inherited by order-only prerequisite";
> +
> +$makefile = &get_tmpfile;
> +unlink $makefile if -f $makefile;
> +
> +open(MAKEFILE,"> $makefile");
> +
> +print MAKEFILE <<'EOF';
> +.SUFFIXES:
> +default: dir1/file1.o dir2/file1.o | dir3/file3.o
> +
> +CFLAGS := -O2
> +CFLAGS-dir1 := -Ddir1
> +CFLAGS-dir2 := -Ddir2
> +doit = @echo $(CC) -c a.c -o $@ $(CFLAGS) $(PER_DIR)
> +define add_per_dir
> +ifneq ($(strip $(2)),)
> +__add := $(2)
> +$$(__add): PER_DIR:=$$(CFLAGS-$(1))
> +endif
> +endef
> +$(eval $(call add_per_dir,dir1,dir1/file1.o))
> +$(eval $(call add_per_dir,dir2,dir2/file1.o))
> +%.o:
> +     $(doit)
> +     @#$(if $(PER_DIR),@test "$*" != "dir3/file3" || false 
> PER_DIR=$(PER_DIR) but should not be set)
> +ifndef OK
> +dir1/file1.o: | dir3/file3.o
> +endif
> +EOF
> +
> +close(MAKEFILE);
> +
> +# Variant 1
> +&run_make_with_options($makefile, "", &get_logfile);
> +$answer = "cc -c a.c -o dir3/file3.o -O2\ncc -c a.c -o dir1/file1.o -O2 
> -Ddir1\ncc -c a.c -o dir2/file1.o -O2 -Ddir2\n";
> +&compare_output($answer,&get_logfile(1));
> +
> +# Variant 2
> +&run_make_with_options($makefile, "OK=1", &get_logfile);
> +$answer = "cc -c a.c -o dir1/file1.o -O2 -Ddir1\ncc -c a.c -o dir2/file1.o 
> -O2 -Ddir2\ncc -c a.c -o dir3/file3.o -O2\n";
> +&compare_output($answer,&get_logfile(1));
> +
> +# TEST #22: per-target variable should not be inherited by order-only
> +#           prerequisite
> +
> +$makefile = &get_tmpfile;
> +unlink $makefile if -f $makefile;
> +open(MAKEFILE,"> $makefile");
> +
> +print MAKEFILE <<'EOF';
> +.SUFFIXES:
> +default: libdir1.so libdir2.so | dir3/file3.o
> +
> +CFLAGS := -O2
> +CFLAGS-dir1 := -Ddir1
> +CFLAGS-dir2 := -Ddir2
> +doit = @echo $(CC) -c a.c -o $@ $(CFLAGS) $(PER_DIR)
> +linkit = @echo $(CC) -o $@ dir3/file3.o $^ $(CFLAGS) $(PER_DIR)
> +define add_per_dir
> +ifneq ($(strip $(2)),)
> +__add := $(2)
> +$$(__add): PER_DIR:=$$(CFLAGS-$(1))
> +endif
> +endef
> +$(eval $(call add_per_dir,dir1,dir1/file1.o))
> +$(eval $(call add_per_dir,dir2,dir2/file1.o))
> +lib%.so: %/file1.o | dir3/file3.o
> +     $(linkit)
> +%.o:
> +     $(doit)
> +     @#$(if $(PER_DIR),@test "$*" != "dir3/file3" || false 
> PER_DIR=$(PER_DIR) but should not be set)
> +ifndef OK
> +dir1/file1.o: | dir3/file3.o
> +endif
> +EOF
> +
> +close(MAKEFILE);
> +
> +# Variant 3
> +&run_make_with_options($makefile, "", &get_logfile);
> +$answer = "cc -c a.c -o dir3/file3.o -O2\ncc -c a.c -o dir1/file1.o -O2 
> -Ddir1\ncc -o libdir1.so dir3/file3.o dir1/file1.o -O2\ncc -c a.c -o 
> dir2/file1.o -O2 -Ddir2\ncc -o libdir2.so dir3/file3.o dir2/file1.o -O2\n";
> +&compare_output($answer,&get_logfile(1));
> +
> +# Variant 4
> +&run_make_with_options($makefile, "OK=1", &get_logfile);
> +$answer = "cc -c a.c -o dir1/file1.o -O2 -Ddir1\ncc -c a.c -o dir3/file3.o 
> -O2\ncc -o libdir1.so dir3/file3.o dir1/file1.o -O2\ncc -c a.c -o 
> dir2/file1.o -O2 -Ddir2\ncc -o libdir2.so dir3/file3.o dir2/file1.o -O2\n";
> +&compare_output($answer,&get_logfile(1));
> +
>  1;
> diff --git a/variable.c b/variable.c
> index 3f57e7d..39dca10 100644
> --- a/variable.c
> +++ b/variable.c
> @@ -565,8 +565,24 @@ initialize_file_variables (struct file *file, int 
> reading)
>      l->next = &global_setlist;
>    else
>      {
> -      initialize_file_variables (file->parent, reading);
> -      l->next = file->parent->variables;
> +      struct dep *d;
> +      for (d = file->parent->deps; d; d = d->next)
> +     {
> +       if (d->file == file)
> +         /* Found the dep in our parent that points to ourself.  */
> +         break;
> +     }
> +      if (d && d->ignore_mtime)
> +     {
> +       /* We are an order_only prerequisite of our parent.
> +        * We are in no way interested in their per_target variables.  */
> +       l->next = &global_setlist;
> +     }
> +      else
> +     {
> +          initialize_file_variables (file->parent, reading);
> +          l->next = file->parent->variables;
> +     }
>      }
>    l->next_is_parent = 1;
>