[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] order_only prereq shall not use parent per_target variab
From: |
Bernhard Reutner-Fischer |
Subject: |
Re: [PATCH 2/2] order_only prereq shall not use parent per_target variables |
Date: |
Tue, 16 Sep 2014 10:59:44 +0200 |
On 16 September 2014 06:21, Paul Smith <address@hidden> wrote:
> 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?
Because an order-only prerequisite is, well, order-only? :)
My reasoning is that per-target variables are to be used only for the target
they are defined for, not for random other targets, especially not for
order-only targets.
You would not want a per-target append variable to alter upper-scopes value
either (i.e. not -Udir1 -Udir2 in one cc invocation in the sample below).
How do you suggest would i build crt1.o above without X being in the scope
of crt1.o?
The only working workaround i can see is spawning a separate instance, but
that is something i really do not want to do:
$ /usr/bin/make -f GNUmakefile.workaround workaround=workaround
/usr/bin/make -f GNUmakefile.workaround crt1.o
make[1]: Entering directory '/scratch/src/bug-make'
cc -c a.c -o crt1.o
make[1]: Leaving directory '/scratch/src/bug-make'
cc -Udir1 -c a.c -o dir1/file1.o -Ddir1
cc -Udir2 -c a.c -o dir2/file1.o -Ddir2
$
Sample once again:
---8<---
.SUFFIXES:
MAKEFLAGS += -r
default: dir1/file1.o dir2/file1.o
# workaround is either 'workaround' or 'crt1.o'
workaround ?= crt1.o
workaround:
$(MAKE) -f $(word 1,$(MAKEFILE_LIST)) crt1.o
dir1/file1.o dir2/file1.o: | $(workaround)
CFLAGS-dir1 := -Ddir1
CFLAGS-dir2 := -Ddir2
doit = @echo $(CC) $(CFLAGS) -c a.c -o $@ $(PER_DIR)
define add_per_dir
ifneq ($(strip $(2)),)
__add := $(2)
$$(__add): PER_DIR:=$$(CFLAGS-$(1))
$$(__add): CFLAGS+=-U$(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),$(if $(findstring crt1,$*),false
PER_DIR=$(PER_DIR) but should not be set for $*))
---8<---
See what i mean?
thanks,
>
>
>> 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;
>>
>
>