On 10.03.2023 14:38, Roger Pau Monné wrote: > 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,
What about the run-tests-% goal in the top level Makefile? > 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. Perhaps. I find $(LDFLAGS) odd to use anyway - by its name it ought to be passed to $(LD), not $(CC). Jan
