[CRIU] [PATCH 1/6] criu/pie/Makefile: rm old compel leftovers

[Kir Kolyshkin]

On 02/20/2017 02:04 AM, Dmitry Safonov wrote:
> 2017-02-20 6:13 GMT+03:00 Kir Kolyshkin <kir at openvz.org>:
>> Since we treat compel as an external tool, and since compel can now
>> provide ldflags, plugins and so on, there is no need to have this
>> extra code in Makefile.
>>
>> Note that because of the changes, parasite/restorer is no longer rebuilt
>> when there are changes in compel. To me, this fits well to a vision that
>> compel is an external thing. This means, unfortunately, that if you have
>> changed compel code, you need to run 'make clean' for a proper build.
> Hmm, not sure about this change.
> What's the point about building criu with external compel binary?

 From my POV, compel will ultimately be a separate tool/library,
maybe even hosted in a separate repository. With compel CLI tool,
it is now possible to compile criu/pie either with installed or uninstalled
compel (I think I even tested that some time ago). We are untying
compel from criu, and this includes these intra-dependencies.

>
>> This also makes it possible to use e.g. installed compel instance.
>>
>> Signed-off-by: Kir Kolyshkin <kir at openvz.org>
>> ---
>>   criu/pie/Makefile | 13 +++----------
>>   1 file changed, 3 insertions(+), 10 deletions(-)
>>
>> diff --git a/criu/pie/Makefile b/criu/pie/Makefile
>> index f6e9ff2..68a2f4b 100644
>> --- a/criu/pie/Makefile
>> +++ b/criu/pie/Makefile
>> @@ -1,8 +1,5 @@
>>   target         += parasite restorer
>>
>> -parasite-obj-e += ./compel/plugins/std.built-in.o
>> -restorer-obj-e += ./compel/plugins/std.built-in.o
>> -
>>   parasite-obj-y += parasite.o
>>   restorer-obj-y += restorer.o
>>   restorer-obj-y += ./$(ARCH_DIR)/restorer.o
>> @@ -33,7 +30,6 @@ ccflags-y     += -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0
>>
>>   ifneq ($(filter-out clean mrproper,$(MAKECMDGOALS)),)
>>           CFLAGS += $(shell $(COMPEL_BIN) cflags)
>> -       compel_std      := $(shell $(COMPEL_BIN) plugins)
>>   endif
>>
>>   ifeq ($(SRCARCH),arm)
>> @@ -43,19 +39,16 @@ endif
>>   asflags-y      += -D__ASSEMBLY__
>>
>>   BLOBS          += $(obj)/restorer-blob.h $(obj)/parasite-blob.h
>> -LDS            := $(SRC_DIR)/compel/arch/$(SRCARCH)/scripts/compel-pack.lds.S
>>
>>   .SECONDARY:
>>
>>   target-name = $(patsubst criu/pie/%-blob.h,%,$(1))
>>
>> -$(obj)/restorer.built-in.o: $(compel_std)
>> -$(obj)/parasite.built-in.o: $(compel_std)
>> -$(obj)/%.built-in.bin.o: $(obj)/%.built-in.o $(obj)/pie.lib.a $(LDS)
>> +$(obj)/%.built-in.bin.o: $(obj)/%.built-in.o $(obj)/pie.lib.a
> If you're building with external compel, this dependency does
> not make you rebuild builtins until you touch lds script.
> I do not see anything wrong with this.

1. With external compel, we don't really know where the linker script lives,
or is there a linker script at all -- we just ask compel to provide ldflags.

2. With any external tool, its dependencies are not added
(for example, you do not add a dependency upon /usr/bin/gcc etc).

3. I don't see anyone ever changing these linker scripts since they were 
added.


>
>>          $(call msg-gen, $@)
>> -       $(Q) $(LD) $(shell $(COMPEL_BIN) ldflags) -o $@ $^
>> +       $(Q) $(LD) $(shell $(COMPEL_BIN) ldflags) -o $@ $^ $(shell $(COMPEL_BIN) plugins)
>>
>> -$(obj)/%-blob.h: $(obj)/%.built-in.bin.o $(SRC_DIR)/compel/compel-host-bin
>> +$(obj)/%-blob.h: $(obj)/%.built-in.bin.o
> It may make sense, but then why not to change dependency
> `$(SRC_DIR)/compel/compel-host-bin' to `$(COMPEL_BIN)'?

I believe this dep was added to make sure compel binary 
(compel-host-bin) is built before
make executes this target. As we have this dependency in top level 
Makefile, it's not really
needed here.

>
>>          $(call msg-gen, $@)
>>          $(Q) $(COMPEL_BIN) hgen -f $< -o $@
>