| Message ID | 336aaf51-f163-8ee7-d8ee-297f6f3052fd@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tools/x86: adjust populating of tools/include/xen/ | expand |
Jan Beulich writes ("[PATCH 1/2][4.15?] tools/x86: don't rebuild cpuid-autogen.h every time"):
> The first thing the "xen-dir" rule does is delete the entire xen/
> subtree. Obviously this includes deleting xen/lib/x86/*autogen.h. As a
> result there's no original version for $(move-if-changed ...) to compare
> against, and hence the file and all its consumers would get rebuilt
> every time. Introduce a "prep-y" rule to move xen/lib/x86/ on the side,
> to then recover any *autogen.h from there prior to invoking the
> respective recursive $(MAKE) invocation.
Urgh. Thanks for working on this swamp.
However,
> +# Arrange for preserving of auto-generated headers (to avoid them getting
> +# rebuilt every time): Move the entire xen/lib/x86/ to a temporary place.
> +prep-$(CONFIG_X86):
> + rm -rf .xen-lib-x86
> + test ! -d xen/lib/x86 || mv xen/lib/x86 .xen-lib-x86
> +
> all-$(CONFIG_X86): xen-dir
> + $(if $(wildcard .xen-lib-x86/*autogen.h),mv .xen-lib-x86/*autogen.h xen/lib/x86/)
> + rm -rf .xen-lib-x86
> $(MAKE) -C xen/lib/x86 all XEN_ROOT=$(XEN_ROOT) PYTHON=$(PYTHON)
Isn't there some better way of doing this ? I am very wary of adding
additional on-disk Makefile-managed state to a Makefile which is
already going wrong. I haven't thought about this in enough detail to
identify a specific bug but I think convincing myself that it is
definitely correct is nontrivial.
Perhaps we could do the removal with a find rune instead, so we can
just skip the files we wanted to keep ?
Ian.
On 08.03.2021 10:59, Ian Jackson wrote: > Jan Beulich writes ("[PATCH 1/2][4.15?] tools/x86: don't rebuild cpuid-autogen.h every time"): >> The first thing the "xen-dir" rule does is delete the entire xen/ >> subtree. Obviously this includes deleting xen/lib/x86/*autogen.h. As a >> result there's no original version for $(move-if-changed ...) to compare >> against, and hence the file and all its consumers would get rebuilt >> every time. Introduce a "prep-y" rule to move xen/lib/x86/ on the side, >> to then recover any *autogen.h from there prior to invoking the >> respective recursive $(MAKE) invocation. > > Urgh. Thanks for working on this swamp. > > However, > >> +# Arrange for preserving of auto-generated headers (to avoid them getting >> +# rebuilt every time): Move the entire xen/lib/x86/ to a temporary place. >> +prep-$(CONFIG_X86): >> + rm -rf .xen-lib-x86 >> + test ! -d xen/lib/x86 || mv xen/lib/x86 .xen-lib-x86 >> + >> all-$(CONFIG_X86): xen-dir >> + $(if $(wildcard .xen-lib-x86/*autogen.h),mv .xen-lib-x86/*autogen.h xen/lib/x86/) >> + rm -rf .xen-lib-x86 >> $(MAKE) -C xen/lib/x86 all XEN_ROOT=$(XEN_ROOT) PYTHON=$(PYTHON) > > Isn't there some better way of doing this ? I am very wary of adding > additional on-disk Makefile-managed state to a Makefile which is > already going wrong. I haven't thought about this in enough detail to > identify a specific bug but I think convincing myself that it is > definitely correct is nontrivial. > > Perhaps we could do the removal with a find rune instead, so we can > just skip the files we wanted to keep ? Maybe, and I did consider the option, but it would have felt more fragile to me than this dedicated keep-just-the-few-files approach. The problems we've had with this symlinking don't make me confident in leaving around parts of this subtree; populating from scratch seems like the most robust model (short of the suggested but never carried out removal of the symlinking) to me. Jan
Jan Beulich writes ("Re: [PATCH 1/2][4.15?] tools/x86: don't rebuild cpuid-autogen.h every time"):
> On 08.03.2021 10:59, Ian Jackson wrote:
> > Jan Beulich writes ("[PATCH 1/2][4.15?] tools/x86: don't rebuild cpuid-autogen.h every time"):
> >> +# Arrange for preserving of auto-generated headers (to avoid them getting
> >> +# rebuilt every time): Move the entire xen/lib/x86/ to a temporary place.
> >> +prep-$(CONFIG_X86):
> >> + rm -rf .xen-lib-x86
> >> + test ! -d xen/lib/x86 || mv xen/lib/x86 .xen-lib-x86
> >> +
> >> all-$(CONFIG_X86): xen-dir
> >> + $(if $(wildcard .xen-lib-x86/*autogen.h),mv .xen-lib-x86/*autogen.h xen/lib/x86/)
> >> + rm -rf .xen-lib-x86
> >> $(MAKE) -C xen/lib/x86 all XEN_ROOT=$(XEN_ROOT) PYTHON=$(PYTHON)
> >
> > Isn't there some better way of doing this ? I am very wary of adding
> > additional on-disk Makefile-managed state to a Makefile which is
> > already going wrong. I haven't thought about this in enough detail to
> > identify a specific bug but I think convincing myself that it is
> > definitely correct is nontrivial.
> >
> > Perhaps we could do the removal with a find rune instead, so we can
> > just skip the files we wanted to keep ?
>
> Maybe, and I did consider the option, but it would have felt more
> fragile to me than this dedicated keep-just-the-few-files approach.
> The problems we've had with this symlinking don't make me confident
> in leaving around parts of this subtree; populating from scratch
> seems like the most robust model (short of the suggested but never
> carried out removal of the symlinking) to me.
I'm confused by your reply.
You aren't confident "leaving around parts of this subtree" but you
are happy to move it aside and put it back, which seems equivalent. I
don't understand why you think the latter would be more reliable.
It seems to me that a find rune which deletes individual files can be
at least as specific as your current wildcard and mv approach.
Thanks,
Ian.
On 08.03.2021 12:08, Ian Jackson wrote: > Jan Beulich writes ("Re: [PATCH 1/2][4.15?] tools/x86: don't rebuild cpuid-autogen.h every time"): >> On 08.03.2021 10:59, Ian Jackson wrote: >>> Jan Beulich writes ("[PATCH 1/2][4.15?] tools/x86: don't rebuild cpuid-autogen.h every time"): >>>> +# Arrange for preserving of auto-generated headers (to avoid them getting >>>> +# rebuilt every time): Move the entire xen/lib/x86/ to a temporary place. >>>> +prep-$(CONFIG_X86): >>>> + rm -rf .xen-lib-x86 >>>> + test ! -d xen/lib/x86 || mv xen/lib/x86 .xen-lib-x86 >>>> + >>>> all-$(CONFIG_X86): xen-dir >>>> + $(if $(wildcard .xen-lib-x86/*autogen.h),mv .xen-lib-x86/*autogen.h xen/lib/x86/) >>>> + rm -rf .xen-lib-x86 >>>> $(MAKE) -C xen/lib/x86 all XEN_ROOT=$(XEN_ROOT) PYTHON=$(PYTHON) >>> >>> Isn't there some better way of doing this ? I am very wary of adding >>> additional on-disk Makefile-managed state to a Makefile which is >>> already going wrong. I haven't thought about this in enough detail to >>> identify a specific bug but I think convincing myself that it is >>> definitely correct is nontrivial. >>> >>> Perhaps we could do the removal with a find rune instead, so we can >>> just skip the files we wanted to keep ? >> >> Maybe, and I did consider the option, but it would have felt more >> fragile to me than this dedicated keep-just-the-few-files approach. >> The problems we've had with this symlinking don't make me confident >> in leaving around parts of this subtree; populating from scratch >> seems like the most robust model (short of the suggested but never >> carried out removal of the symlinking) to me. > > I'm confused by your reply. > > You aren't confident "leaving around parts of this subtree" but you > are happy to move it aside and put it back, which seems equivalent. I > don't understand why you think the latter would be more reliable. I move a subdir aside and then move certain files out of it back. This doesn't leave much uncertainty as to whether other (symlinked) files / dirs may mistakenly also left around. > It seems to me that a find rune which deletes individual files can be > at least as specific as your current wildcard and mv approach. Possibly, but it may end up being more complex: We want to only retain files of specific names from a single dir. I don't think this is as straightforward to express in a find rune. Of course I'll be fine whichever way the bug gets fixed, but I'm afraid I don't feel convinced I want to put time into trying the alternative you suggest. If otoh you wanted to try out yours and it turned out equivalent or better, I wouldn't mind at all. Jan
Jan Beulich writes ("Re: [PATCH 1/2][4.15?] tools/x86: don't rebuild cpuid-autogen.h every time"):
> Possibly, but it may end up being more complex: We want to only
> retain files of specific names from a single dir. I don't think
> this is as straightforward to express in a find rune. Of course
> I'll be fine whichever way the bug gets fixed, but I'm afraid I
> don't feel convinced I want to put time into trying the alternative
> you suggest. If otoh you wanted to try out yours and it turned out
> equivalent or better, I wouldn't mind at all.
Untested, but I think something like this should DTRT
find xen ! -type d ! -path 'xen/lib/x86/*-autogen.h' -print0 | xargs -0r rm --
Since my background is GNU utilities, I checked the FreeBSD manpages
for find, xargs and rm. They support these options.
This will leave the entire directory structure but I think that is
fine. The xen-dir target uses mkdir -p and should there be any stale
directories (eg due to switching branches or whatever) they wouldn't
be a problem.
Ian.
On 08.03.2021 13:12, Ian Jackson wrote: > Jan Beulich writes ("Re: [PATCH 1/2][4.15?] tools/x86: don't rebuild cpuid-autogen.h every time"): >> Possibly, but it may end up being more complex: We want to only >> retain files of specific names from a single dir. I don't think >> this is as straightforward to express in a find rune. Of course >> I'll be fine whichever way the bug gets fixed, but I'm afraid I >> don't feel convinced I want to put time into trying the alternative >> you suggest. If otoh you wanted to try out yours and it turned out >> equivalent or better, I wouldn't mind at all. > > Untested, but I think something like this should DTRT > > find xen ! -type d ! -path 'xen/lib/x86/*-autogen.h' -print0 | xargs -0r rm -- > > Since my background is GNU utilities, I checked the FreeBSD manpages > for find, xargs and rm. They support these options. > > This will leave the entire directory structure but I think that is > fine. The xen-dir target uses mkdir -p and should there be any stale > directories (eg due to switching branches or whatever) they wouldn't > be a problem. Right. Thinking of it though - all we do is setting up symlinks plus produce this generated header. Couldn't we therefore have find simply arrange for all symlinks to be found and deleted? Jan
Jan Beulich writes ("Re: [PATCH 1/2][4.15?] tools/x86: don't rebuild cpuid-autogen.h every time"):
> On 08.03.2021 13:12, Ian Jackson wrote:
> > This will leave the entire directory structure but I think that is
> > fine. The xen-dir target uses mkdir -p and should there be any stale
> > directories (eg due to switching branches or whatever) they wouldn't
> > be a problem.
>
> Right. Thinking of it though - all we do is setting up symlinks
> plus produce this generated header. Couldn't we therefore have
> find simply arrange for all symlinks to be found and deleted?
I think that would be another alternative, if there is nothing else in
there but symlinks.
Ian.
--- a/.gitignore +++ b/.gitignore @@ -234,6 +234,7 @@ tools/hotplug/NetBSD/rc.d/xendriverdomain tools/include/acpi tools/include/_libxl*.h +tools/include/.xen*/* tools/include/_xentoolcore_list.h tools/include/xen/* tools/include/xen-xsm/* --- a/tools/include/Makefile +++ b/tools/include/Makefile @@ -10,15 +10,15 @@ include $(XEN_ROOT)/tools/Rules.mk # Relative to $(XEN_ROOT)/xen/xsm/flask FLASK_H_DEPEND := policy/initial_sids -.PHONY: all all-y build xen-dir +.PHONY: all all-y build prep-y xen-dir all build: all-y xen-foreign xen-dir xen-xsm/.dir -all-y: +all-y prep-y: .PHONY: xen-foreign xen-foreign: $(MAKE) -C xen-foreign -xen-dir: +xen-dir: prep-y @rm -rf xen acpi mkdir -p xen/libelf acpi ln -s $(XEN_ROOT)/xen/include/public/COPYING xen/ @@ -36,7 +36,15 @@ ifeq ($(CONFIG_X86),y) ln -s $(XEN_ROOT)/xen/include/xen/lib/x86/Makefile xen/lib/x86/ endif +# Arrange for preserving of auto-generated headers (to avoid them getting +# rebuilt every time): Move the entire xen/lib/x86/ to a temporary place. +prep-$(CONFIG_X86): + rm -rf .xen-lib-x86 + test ! -d xen/lib/x86 || mv xen/lib/x86 .xen-lib-x86 + all-$(CONFIG_X86): xen-dir + $(if $(wildcard .xen-lib-x86/*autogen.h),mv .xen-lib-x86/*autogen.h xen/lib/x86/) + rm -rf .xen-lib-x86 $(MAKE) -C xen/lib/x86 all XEN_ROOT=$(XEN_ROOT) PYTHON=$(PYTHON) # Not xen/xsm as that clashes with link to @@ -78,7 +86,7 @@ uninstall: .PHONY: clean clean: - rm -rf xen xen-xsm acpi + rm -rf xen xen-xsm .xen* acpi $(MAKE) -C xen-foreign clean .PHONY: dist
The first thing the "xen-dir" rule does is delete the entire xen/ subtree. Obviously this includes deleting xen/lib/x86/*autogen.h. As a result there's no original version for $(move-if-changed ...) to compare against, and hence the file and all its consumers would get rebuilt every time. Introduce a "prep-y" rule to move xen/lib/x86/ on the side, to then recover any *autogen.h from there prior to invoking the respective recursive $(MAKE) invocation. Fixes: eddf9559c977 ("libx86: generate cpuid-autogen.h in the libx86 include dir") Signed-off-by: Jan Beulich <jbeulich@suse.com>