On Fri, Mar 10, 2023 at 12:06:29PM +0100, Jan Beulich wrote: > On 09.03.2023 17:58, Roger Pau Monne wrote: > > Introduce an install target, like it's used by other tests. This > > allows running the test on the installed systems, which is easier than > > running it during the build phase when dealing with automated testing. > > Strictly speaking the vpci test doesn't require to be run on a Xen > > host currently, but that allows easier integration with logic that > > runs the rest of the tests. > > I accept that as a possible way of looking at things, but personally I > remain unconvinced of this model. To me what is installed should be of > value to users. If there was a properly separated directory where all > (and only) tests were put, I might agree with installing. (Nevertheless > this isn't an objection, merely a remark.) > > > While there also adjust the makefile to use $(RM), and rename the > > resulting binary to use a dash instead of an underscore (again to > > match the rest of the tests). > > > > Since the resulting test binary is now part of the distribution CC > > must be used instead of HOSTCC. > > This breaks the run: goal, doesn't it? If the new mode is wanted, I > think the two kinds of binaries (and rules) need separating (maybe a > way can be found to avoid duplicating the rules, which would seem > desirable).
The run rule is not hooked up in any of the upper level makefile logic, so I think it's usage (like in other tests that also use CC and have such rule) is left to callers that know that HOSTCC == CC. > > --- a/tools/tests/vpci/Makefile > > +++ b/tools/tests/vpci/Makefile > > @@ -1,7 +1,7 @@ > > XEN_ROOT=$(CURDIR)/../../.. > > include $(XEN_ROOT)/tools/Rules.mk > > > > -TARGET := test_vpci > > +TARGET := test-vpci > > > > .PHONY: all > > all: $(TARGET) > > @@ -11,17 +11,23 @@ run: $(TARGET) > > ./$(TARGET) > > > > $(TARGET): vpci.c vpci.h list.h main.c emul.h > > - $(HOSTCC) -g -o $@ vpci.c main.c > > + $(CC) -o $@ vpci.c main.c > > You're losing -g and you're also not covering for it by adding $(CFLAGS) > (there should have been use of $(HOSTCFLAGS) already before, I suppose). Wasn't sure whether I should add CFLAGS and LDFLAGS here, I guess LDFLAGS is really not needed because the test is not linked against any library, but could be added just in case. Thanks, Roger.
