| Message ID | 20210824105038.1257926-43-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: Build system improvements, now with out-of-tree build! | expand |
On 24.08.2021 12:50, Anthony PERARD wrote: > Rather than preparing the efi source file, we will copy them as needed > from the build location. > > Avoid the links as they seems fragile in out-of-tree builds. Also by > making a copy, we don't need to figure out the relative path or we > don't need to use absolute path. I agree that symlinks wouldn't be nice for the out-of-tree build case. Otoh please see https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=8c6740616cd244e5763e974cb737affbe71db385 albeit I'll admit the situation was a little different there because it's pre-built files which get populated into the build tree. > --- a/xen/arch/arm/efi/Makefile > +++ b/xen/arch/arm/efi/Makefile > @@ -1,4 +1,10 @@ > CFLAGS-y += -fshort-wchar > +CFLAGS-y += -I$(srctree)/common/efi Perhaps another opportunity for -iquote? > obj-y += boot.init.o pe.init.o ebmalloc.o runtime.o > obj-$(CONFIG_ACPI) += efi-dom0.init.o > + > +$(obj)/%.c: common/efi/%.c > + $(Q)cp -f $< $@ In case both trees are on the same file system, trying to hardlink first would seem desirable. When copying, I think you should also pass -p. > --- a/xen/arch/x86/efi/Makefile > +++ b/xen/arch/x86/efi/Makefile > @@ -1,4 +1,5 @@ > CFLAGS-y += -fshort-wchar > +CFLAGS-y += -I$(srctree)/common/efi > > quiet_cmd_objcopy_o_ihex = OBJCOPY $@ > cmd_objcopy_o_ihex = $(OBJCOPY) -I ihex -O binary $< $@ > @@ -19,3 +20,8 @@ obj-$(XEN_BUILD_EFI) := $(filter-out %.init.o,$(EFIOBJ-y)) > obj-bin-$(XEN_BUILD_EFI) := $(filter %.init.o,$(EFIOBJ-y)) > extra-$(XEN_BUILD_EFI) += buildid.o relocs-dummy.o > nocov-$(XEN_BUILD_EFI) += stub.o > + > +$(obj)/%.c: common/efi/%.c > + $(Q)cp -f $< $@ > + > +.PRECIOUS: $(obj)/%.c Seeing you repeat everything here, despite it not being all this much I wonder if there wouldn't better be a makefile fragment in common/efi/ which all interested architectures' arch/<arch>/efi/Makefile would then include. This could then also subsume -fshort-wchar. Jan
On Thu, Oct 14, 2021 at 10:51:44AM +0200, Jan Beulich wrote: > On 24.08.2021 12:50, Anthony PERARD wrote: > > --- a/xen/arch/arm/efi/Makefile > > +++ b/xen/arch/arm/efi/Makefile > > @@ -1,4 +1,10 @@ > > CFLAGS-y += -fshort-wchar > > +CFLAGS-y += -I$(srctree)/common/efi > > Perhaps another opportunity for -iquote? Yes. > > obj-y += boot.init.o pe.init.o ebmalloc.o runtime.o > > obj-$(CONFIG_ACPI) += efi-dom0.init.o > > + > > +$(obj)/%.c: common/efi/%.c > > + $(Q)cp -f $< $@ > > In case both trees are on the same file system, trying to hardlink first > would seem desirable. When copying, I think you should also pass -p. I don't know if doing an hardlink is a good thing to do, I'm not sure of the kind of issue this could bring. As for -p, I don't think it's a good idea to copy the mode, ownership, and timestamps of the source file, I'd rather have the timestamps that Make expect, e.i. "now". > > --- a/xen/arch/x86/efi/Makefile > > +++ b/xen/arch/x86/efi/Makefile > > @@ -1,4 +1,5 @@ > > CFLAGS-y += -fshort-wchar > > +CFLAGS-y += -I$(srctree)/common/efi > > > > quiet_cmd_objcopy_o_ihex = OBJCOPY $@ > > cmd_objcopy_o_ihex = $(OBJCOPY) -I ihex -O binary $< $@ > > @@ -19,3 +20,8 @@ obj-$(XEN_BUILD_EFI) := $(filter-out %.init.o,$(EFIOBJ-y)) > > obj-bin-$(XEN_BUILD_EFI) := $(filter %.init.o,$(EFIOBJ-y)) > > extra-$(XEN_BUILD_EFI) += buildid.o relocs-dummy.o > > nocov-$(XEN_BUILD_EFI) += stub.o > > + > > +$(obj)/%.c: common/efi/%.c > > + $(Q)cp -f $< $@ > > + > > +.PRECIOUS: $(obj)/%.c > > Seeing you repeat everything here, despite it not being all this much I > wonder if there wouldn't better be a makefile fragment in common/efi/ > which all interested architectures' arch/<arch>/efi/Makefile would then > include. This could then also subsume -fshort-wchar. That sounds good, I'll look into that. Thanks,
On 15.10.2021 18:29, Anthony PERARD wrote: > On Thu, Oct 14, 2021 at 10:51:44AM +0200, Jan Beulich wrote: >> On 24.08.2021 12:50, Anthony PERARD wrote: >>> --- a/xen/arch/arm/efi/Makefile >>> +++ b/xen/arch/arm/efi/Makefile >>> @@ -1,4 +1,10 @@ >>> CFLAGS-y += -fshort-wchar >>> +CFLAGS-y += -I$(srctree)/common/efi >> >> Perhaps another opportunity for -iquote? > > Yes. > >>> obj-y += boot.init.o pe.init.o ebmalloc.o runtime.o >>> obj-$(CONFIG_ACPI) += efi-dom0.init.o >>> + >>> +$(obj)/%.c: common/efi/%.c >>> + $(Q)cp -f $< $@ >> >> In case both trees are on the same file system, trying to hardlink first >> would seem desirable. When copying, I think you should also pass -p. > > I don't know if doing an hardlink is a good thing to do, I'm not sure of > the kind of issue this could bring. As for -p, I don't think it's a good > idea to copy the mode, ownership, and timestamps of the source file, I'd > rather have the timestamps that Make expect, e.i. "now". Why would "now" be correct (or expected) in any way? The cloned file is no different from the original. Nevertheless I agree that -p is not ideal; it's just that the more fine grained option to preserve just the timestamp is non-standard afaik. You could try that first and fall back to -p ... Otherwise, failing hard linking and using "cp -p", I'm afraid I'd prefer symlinking despite the arguments against it that you name in the description. Might be good to have someone else's view here as well. Jan
On Mon, Oct 18, 2021 at 10:48:26AM +0200, Jan Beulich wrote: > On 15.10.2021 18:29, Anthony PERARD wrote: > > On Thu, Oct 14, 2021 at 10:51:44AM +0200, Jan Beulich wrote: > >> On 24.08.2021 12:50, Anthony PERARD wrote: > >>> --- a/xen/arch/arm/efi/Makefile > >>> +++ b/xen/arch/arm/efi/Makefile > >>> @@ -1,4 +1,10 @@ > >>> CFLAGS-y += -fshort-wchar > >>> +CFLAGS-y += -I$(srctree)/common/efi > >> > >> Perhaps another opportunity for -iquote? > > > > Yes. > > > >>> obj-y += boot.init.o pe.init.o ebmalloc.o runtime.o > >>> obj-$(CONFIG_ACPI) += efi-dom0.init.o > >>> + > >>> +$(obj)/%.c: common/efi/%.c > >>> + $(Q)cp -f $< $@ > >> > >> In case both trees are on the same file system, trying to hardlink first > >> would seem desirable. When copying, I think you should also pass -p. > > > > I don't know if doing an hardlink is a good thing to do, I'm not sure of > > the kind of issue this could bring. As for -p, I don't think it's a good > > idea to copy the mode, ownership, and timestamps of the source file, I'd > > rather have the timestamps that Make expect, e.i. "now". > > Why would "now" be correct (or expected) in any way? The cloned file is no > different from the original. Nevertheless I agree that -p is not ideal; > it's just that the more fine grained option to preserve just the timestamp > is non-standard afaik. You could try that first and fall back to -p ... > Otherwise, failing hard linking and using "cp -p", I'm afraid I'd prefer > symlinking despite the arguments against it that you name in the > description. I guess I'm missing something, is there a reason to keep/copy the timestamps of the original files? > Might be good to have someone else's view here as well. Indeed.
On 21.10.2021 13:03, Anthony PERARD wrote: > On Mon, Oct 18, 2021 at 10:48:26AM +0200, Jan Beulich wrote: >> On 15.10.2021 18:29, Anthony PERARD wrote: >>> On Thu, Oct 14, 2021 at 10:51:44AM +0200, Jan Beulich wrote: >>>> On 24.08.2021 12:50, Anthony PERARD wrote: >>>>> --- a/xen/arch/arm/efi/Makefile >>>>> +++ b/xen/arch/arm/efi/Makefile >>>>> @@ -1,4 +1,10 @@ >>>>> CFLAGS-y += -fshort-wchar >>>>> +CFLAGS-y += -I$(srctree)/common/efi >>>> >>>> Perhaps another opportunity for -iquote? >>> >>> Yes. >>> >>>>> obj-y += boot.init.o pe.init.o ebmalloc.o runtime.o >>>>> obj-$(CONFIG_ACPI) += efi-dom0.init.o >>>>> + >>>>> +$(obj)/%.c: common/efi/%.c >>>>> + $(Q)cp -f $< $@ >>>> >>>> In case both trees are on the same file system, trying to hardlink first >>>> would seem desirable. When copying, I think you should also pass -p. >>> >>> I don't know if doing an hardlink is a good thing to do, I'm not sure of >>> the kind of issue this could bring. As for -p, I don't think it's a good >>> idea to copy the mode, ownership, and timestamps of the source file, I'd >>> rather have the timestamps that Make expect, e.i. "now". >> >> Why would "now" be correct (or expected) in any way? The cloned file is no >> different from the original. Nevertheless I agree that -p is not ideal; >> it's just that the more fine grained option to preserve just the timestamp >> is non-standard afaik. You could try that first and fall back to -p ... >> Otherwise, failing hard linking and using "cp -p", I'm afraid I'd prefer >> symlinking despite the arguments against it that you name in the >> description. > > I guess I'm missing something, is there a reason to keep/copy the > timestamps of the original files? Avoidance of confusion is my main aim here. I certainly would be puzzled to see what looks like a source file to have a time stamp much newer than expected. Jan
On Thu, Oct 21, 2021 at 01:24:27PM +0200, Jan Beulich wrote: > On 21.10.2021 13:03, Anthony PERARD wrote: > > On Mon, Oct 18, 2021 at 10:48:26AM +0200, Jan Beulich wrote: > >> On 15.10.2021 18:29, Anthony PERARD wrote: > >>> On Thu, Oct 14, 2021 at 10:51:44AM +0200, Jan Beulich wrote: > >>>> On 24.08.2021 12:50, Anthony PERARD wrote: > >>>>> obj-y += boot.init.o pe.init.o ebmalloc.o runtime.o > >>>>> obj-$(CONFIG_ACPI) += efi-dom0.init.o > >>>>> + > >>>>> +$(obj)/%.c: common/efi/%.c > >>>>> + $(Q)cp -f $< $@ > >>>> > >>>> In case both trees are on the same file system, trying to hardlink first > >>>> would seem desirable. When copying, I think you should also pass -p. > >>> > >>> I don't know if doing an hardlink is a good thing to do, I'm not sure of > >>> the kind of issue this could bring. As for -p, I don't think it's a good > >>> idea to copy the mode, ownership, and timestamps of the source file, I'd > >>> rather have the timestamps that Make expect, e.i. "now". > >> > >> Why would "now" be correct (or expected) in any way? The cloned file is no > >> different from the original. Nevertheless I agree that -p is not ideal; > >> it's just that the more fine grained option to preserve just the timestamp > >> is non-standard afaik. You could try that first and fall back to -p ... > >> Otherwise, failing hard linking and using "cp -p", I'm afraid I'd prefer > >> symlinking despite the arguments against it that you name in the > >> description. > > > > I guess I'm missing something, is there a reason to keep/copy the > > timestamps of the original files? > > Avoidance of confusion is my main aim here. I certainly would be puzzled > to see what looks like a source file to have a time stamp much newer than > expected. So, there isn't really anything to do with the timestamps :-). I guess we could keep using symbolic links, but force update the link at every build. I've tried that: $(obj)/%.c: $(abs_srctree)/common/efi/%.c FORCE $(Q)ln -nsf $< $@ and make seems happy. The link command run every time (due to adding FORCE), but the `CC` command isn't, so that seems good. The recipe that would run the `CC` command check if the prerequisite are newer than the target using $? so it doesn't matter if the rule that update the source file as run or not. Thanks,
On 21.10.2021 15:54, Anthony PERARD wrote: > On Thu, Oct 21, 2021 at 01:24:27PM +0200, Jan Beulich wrote: >> On 21.10.2021 13:03, Anthony PERARD wrote: >>> On Mon, Oct 18, 2021 at 10:48:26AM +0200, Jan Beulich wrote: >>>> On 15.10.2021 18:29, Anthony PERARD wrote: >>>>> On Thu, Oct 14, 2021 at 10:51:44AM +0200, Jan Beulich wrote: >>>>>> On 24.08.2021 12:50, Anthony PERARD wrote: >>>>>>> obj-y += boot.init.o pe.init.o ebmalloc.o runtime.o >>>>>>> obj-$(CONFIG_ACPI) += efi-dom0.init.o >>>>>>> + >>>>>>> +$(obj)/%.c: common/efi/%.c >>>>>>> + $(Q)cp -f $< $@ >>>>>> >>>>>> In case both trees are on the same file system, trying to hardlink first >>>>>> would seem desirable. When copying, I think you should also pass -p. >>>>> >>>>> I don't know if doing an hardlink is a good thing to do, I'm not sure of >>>>> the kind of issue this could bring. As for -p, I don't think it's a good >>>>> idea to copy the mode, ownership, and timestamps of the source file, I'd >>>>> rather have the timestamps that Make expect, e.i. "now". >>>> >>>> Why would "now" be correct (or expected) in any way? The cloned file is no >>>> different from the original. Nevertheless I agree that -p is not ideal; >>>> it's just that the more fine grained option to preserve just the timestamp >>>> is non-standard afaik. You could try that first and fall back to -p ... >>>> Otherwise, failing hard linking and using "cp -p", I'm afraid I'd prefer >>>> symlinking despite the arguments against it that you name in the >>>> description. >>> >>> I guess I'm missing something, is there a reason to keep/copy the >>> timestamps of the original files? >> >> Avoidance of confusion is my main aim here. I certainly would be puzzled >> to see what looks like a source file to have a time stamp much newer than >> expected. > > So, there isn't really anything to do with the timestamps :-). I guess > we could keep using symbolic links, but force update the link at every > build. > > I've tried that: > $(obj)/%.c: $(abs_srctree)/common/efi/%.c FORCE > $(Q)ln -nsf $< $@ > > and make seems happy. The link command run every time (due to adding > FORCE), but the `CC` command isn't, so that seems good. The recipe that > would run the `CC` command check if the prerequisite are newer than the > target using $? so it doesn't matter if the rule that update the source > file as run or not. Looks okay to me. One additional consideration, though: Linux puts a "source" link in the build tree. If we did so as well, then that could be the only absolute symlink that's needed. Links like the one you suggest could be relative ones into source/. But I guess this could as well be left as a future exercise, in case anyone cares to limit the number of absolute symlinks. Jan
diff --git a/xen/Makefile b/xen/Makefile index 950bee10ba38..4c1dd9ce2ea1 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -425,11 +425,6 @@ $(TARGET).gz: $(TARGET) $(TARGET): tools_fixdep FORCE $(MAKE) $(build)=tools $(MAKE) $(build)=. include/xen/compile.h - [ -e arch/$(TARGET_ARCH)/efi ] && for f in $$(cd common/efi; echo *.[ch]); \ - do test -r arch/$(TARGET_ARCH)/efi/$$f || \ - ln -nsf ../../../common/efi/$$f arch/$(TARGET_ARCH)/efi/; \ - done; \ - true $(MAKE) $(build)=include all $(MAKE) $(build)=arch/$(TARGET_ARCH) include $(MAKE) $(build)=. arch/$(TARGET_ARCH)/include/asm/asm-offsets.h diff --git a/xen/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile index 1b1ed06feddc..36e15ac280cd 100644 --- a/xen/arch/arm/efi/Makefile +++ b/xen/arch/arm/efi/Makefile @@ -1,4 +1,10 @@ CFLAGS-y += -fshort-wchar +CFLAGS-y += -I$(srctree)/common/efi obj-y += boot.init.o pe.init.o ebmalloc.o runtime.o obj-$(CONFIG_ACPI) += efi-dom0.init.o + +$(obj)/%.c: common/efi/%.c + $(Q)cp -f $< $@ + +.PRECIOUS: $(obj)/%.c diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile index ac815f02cb5e..da05935a9348 100644 --- a/xen/arch/x86/efi/Makefile +++ b/xen/arch/x86/efi/Makefile @@ -1,4 +1,5 @@ CFLAGS-y += -fshort-wchar +CFLAGS-y += -I$(srctree)/common/efi quiet_cmd_objcopy_o_ihex = OBJCOPY $@ cmd_objcopy_o_ihex = $(OBJCOPY) -I ihex -O binary $< $@ @@ -19,3 +20,8 @@ obj-$(XEN_BUILD_EFI) := $(filter-out %.init.o,$(EFIOBJ-y)) obj-bin-$(XEN_BUILD_EFI) := $(filter %.init.o,$(EFIOBJ-y)) extra-$(XEN_BUILD_EFI) += buildid.o relocs-dummy.o nocov-$(XEN_BUILD_EFI) += stub.o + +$(obj)/%.c: common/efi/%.c + $(Q)cp -f $< $@ + +.PRECIOUS: $(obj)/%.c
Rather than preparing the efi source file, we will copy them as needed from the build location. Avoid the links as they seems fragile in out-of-tree builds. Also by making a copy, we don't need to figure out the relative path or we don't need to use absolute path. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- xen/Makefile | 5 ----- xen/arch/arm/efi/Makefile | 6 ++++++ xen/arch/x86/efi/Makefile | 6 ++++++ 3 files changed, 12 insertions(+), 5 deletions(-)