| Message ID | ca9930ee-e6bf-3cf3-633c-02d5f21760f5@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | build: omit "source" symlink when building hypervisor in-tree | expand |
On Wed, Mar 15, 2023 at 03:56:21PM +0100, Jan Beulich wrote: > This symlink is getting in the way of using e.g. "find" on the xen/ > subtree, and it isn't really needed when not building out-of-tree: > the one use that there was can easily be avoided. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/common/efi/efi-common.mk > +++ b/xen/common/efi/efi-common.mk > @@ -5,11 +5,16 @@ CFLAGS-y += -fshort-wchar > CFLAGS-y += -iquote $(srctree)/common/efi > CFLAGS-y += -iquote $(srcdir) > > +source := > +ifneq ($(abs_objtree),$(abs_srctree)) Could you use "ifdef building_out_of_srctree" instead, or at least use the variable $(building_out_of_srctree)? At least that mean there's a single way in the tree to differentiate between both kind of build. > +source := source/ > +endif > + > # Part of the command line transforms $(obj) > # e.g.: It transforms "dir/foo/bar" into successively > # "dir foo bar", ".. .. ..", "../../.." > $(obj)/%.c: $(srctree)/common/efi/%.c FORCE > - $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, ,$(obj))))/source/common/efi/$(<F) $@ > + $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, ,$(obj))))/$(source)common/efi/$(<F) $@ Instead of $(source), I did proposed initially "$(if $(building_out_of_srctree),source/)" for here, or it that making the command line too long? https://lore.kernel.org/xen-devel/YebpHJk1JIArcdvW@perard/t/#u Having "source := $(if $(building_out_of_srctree),source/)" might be an ok alternative in place of the use if "ifneq/endif" which take 4 lines? Thanks,
On 15.03.2023 16:20, Anthony PERARD wrote: > On Wed, Mar 15, 2023 at 03:56:21PM +0100, Jan Beulich wrote: >> This symlink is getting in the way of using e.g. "find" on the xen/ >> subtree, and it isn't really needed when not building out-of-tree: >> the one use that there was can easily be avoided. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> --- a/xen/common/efi/efi-common.mk >> +++ b/xen/common/efi/efi-common.mk >> @@ -5,11 +5,16 @@ CFLAGS-y += -fshort-wchar >> CFLAGS-y += -iquote $(srctree)/common/efi >> CFLAGS-y += -iquote $(srcdir) >> >> +source := >> +ifneq ($(abs_objtree),$(abs_srctree)) > > Could you use "ifdef building_out_of_srctree" instead, or at least use > the variable $(building_out_of_srctree)? At least that mean there's a > single way in the tree to differentiate between both kind of build. I should have added a remark, I realize. I am actually aware of that variable and also the fact that it is getting exported, but I was seriously wondering why we do that: It's redundant information, and imo a variable of this name shouldn't really be exported. Furthermore I consider the conditional I'm presently using (matching the one controlling the definition of building_out_of_srctree) more descriptive: I had to go and convince myself that the variable really is set based on comparing the paths; I had suspected it might be some other conditional, not the least because of me not expecting that we'd carry (and even export) redundant information. So yes, if you insist I will switch. My preferred route would be to ditch building_out_of_srctree, though. >> +source := source/ >> +endif >> + >> # Part of the command line transforms $(obj) >> # e.g.: It transforms "dir/foo/bar" into successively >> # "dir foo bar", ".. .. ..", "../../.." >> $(obj)/%.c: $(srctree)/common/efi/%.c FORCE >> - $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, ,$(obj))))/source/common/efi/$(<F) $@ >> + $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, ,$(obj))))/$(source)common/efi/$(<F) $@ > > Instead of $(source), I did proposed initially > "$(if $(building_out_of_srctree),source/)" for here, or it that making > the command line too long? > https://lore.kernel.org/xen-devel/YebpHJk1JIArcdvW@perard/t/#u Oh, I'm sorry for driving you into adding that symlink, which is now getting in the way. But yes, putting it inline would imo make an already too long line yet worse. Otoh I could of course wrap the line, albeit some care may then be needed to not introduce whitespace in the wrong place. > Having "source := $(if $(building_out_of_srctree),source/)" might be an > ok alternative in place of the use if "ifneq/endif" which take 4 lines? As per above, if you're determined that building_out_of_srctree should stay around, then I could do so. Alternatively how about source := $(if $(patsubst $(abs_objtree),,$(abs_srctree)),,source/) or the same with $(subst ...) or yet shorter source := $(if $(abs_srctree:$(abs_objtree)=),,source/) if you're after cutting the number of lines? Jan
--- a/.gitignore +++ b/.gitignore @@ -295,7 +295,6 @@ xen/include/xen/*.new xen/include/xen/compile.h xen/include/xen/hypercall-defs.h xen/include/xen/lib/x86/cpuid-autogen.h -xen/source xen/test/livepatch/config.h xen/test/livepatch/expect_config.h xen/test/livepatch/*.livepatch --- a/xen/Makefile +++ b/xen/Makefile @@ -310,7 +310,6 @@ cmd_makefile = { \ } > Makefile outputmakefile: - $(Q)ln -fsn $(srctree) source ifdef building_out_of_srctree $(Q)if [ -f $(srctree)/.config -o \ -d $(srctree)/include/config -o \ @@ -321,6 +320,7 @@ ifdef building_out_of_srctree echo >&2 "***"; \ false; \ fi + $(Q)ln -fsn $(srctree) source $(call cmd,makefile) $(Q)test -e .gitignore || \ { echo "# this is build directory, ignore it"; echo "*"; } > .gitignore --- a/xen/common/efi/efi-common.mk +++ b/xen/common/efi/efi-common.mk @@ -5,11 +5,16 @@ CFLAGS-y += -fshort-wchar CFLAGS-y += -iquote $(srctree)/common/efi CFLAGS-y += -iquote $(srcdir) +source := +ifneq ($(abs_objtree),$(abs_srctree)) +source := source/ +endif + # Part of the command line transforms $(obj) # e.g.: It transforms "dir/foo/bar" into successively # "dir foo bar", ".. .. ..", "../../.." $(obj)/%.c: $(srctree)/common/efi/%.c FORCE - $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, ,$(obj))))/source/common/efi/$(<F) $@ + $(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, ,$(obj))))/$(source)common/efi/$(<F) $@ clean-files += $(patsubst %.o, %.c, $(EFIOBJ-y:.init.o=.o) $(EFIOBJ-)) clean-files += common-stub.c
This symlink is getting in the way of using e.g. "find" on the xen/ subtree, and it isn't really needed when not building out-of-tree: the one use that there was can easily be avoided. Signed-off-by: Jan Beulich <jbeulich@suse.com>