[PATCH v2] tests/vpci: Use $(CC) instead of $(HOSTCC)

Michal Orzel posted 1 patch 4 months, 4 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20250604072128.16628-1-michal.orzel@amd.com
tools/tests/vpci/Makefile | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
[PATCH v2] tests/vpci: Use $(CC) instead of $(HOSTCC)
Posted by Michal Orzel 4 months, 4 weeks ago
When cross-compiling, HOSTCC can be different than CC. With the recent
`install` rule addition, this would put a binary of a wrong format in
the destdir (e.g. building tests on x86 host for Arm target).

Take the opportunity to adjust the `run` rule to only run the test if
HOSTCC is CC, else print a warning message.

Fixes: 96a587a05736 ("tools/tests: Add install target for vPCI")
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
Changes in v2:
 - change Fixes tag
 - add `run` rule adjustment from Roger
---
 tools/tests/vpci/Makefile | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/tests/vpci/Makefile b/tools/tests/vpci/Makefile
index 9450f7593a41..f2226a5543bc 100644
--- a/tools/tests/vpci/Makefile
+++ b/tools/tests/vpci/Makefile
@@ -8,10 +8,14 @@ all: $(TARGET)
 
 .PHONY: run
 run: $(TARGET)
+ifeq ($(CC),$(HOSTCC))
 	./$(TARGET)
+else
+	$(warning HOSTCC != CC, cannot run test)
+endif
 
 $(TARGET): vpci.c vpci.h list.h main.c emul.h
-	$(HOSTCC) $(CFLAGS_xeninclude) -g -o $@ vpci.c main.c
+	$(CC) $(CFLAGS_xeninclude) -g -o $@ vpci.c main.c
 
 .PHONY: clean
 clean:
-- 
2.25.1
Re: [PATCH v2] tests/vpci: Use $(CC) instead of $(HOSTCC)
Posted by Jan Beulich 4 months, 4 weeks ago
On 04.06.2025 09:21, Michal Orzel wrote:
> When cross-compiling, HOSTCC can be different than CC.

I'm sorry for being pedantic, but the two can also be different for other
reasons (and I'd like to avoid this becoming a bad precedent, then getting
copied elsewhere). Both may target the same architecture. Hence I'd like
to suggest ...

> With the recent
> `install` rule addition, this would put a binary of a wrong format in
> the destdir (e.g. building tests on x86 host for Arm target).
> 
> Take the opportunity to adjust the `run` rule to only run the test if
> HOSTCC is CC, else print a warning message.
> 
> Fixes: 96a587a05736 ("tools/tests: Add install target for vPCI")
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> ---
> Changes in v2:
>  - change Fixes tag
>  - add `run` rule adjustment from Roger
> ---
>  tools/tests/vpci/Makefile | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/tests/vpci/Makefile b/tools/tests/vpci/Makefile
> index 9450f7593a41..f2226a5543bc 100644
> --- a/tools/tests/vpci/Makefile
> +++ b/tools/tests/vpci/Makefile
> @@ -8,10 +8,14 @@ all: $(TARGET)
>  
>  .PHONY: run
>  run: $(TARGET)
> +ifeq ($(CC),$(HOSTCC))
>  	./$(TARGET)
> +else
> +	$(warning HOSTCC != CC, cannot run test)

... s/cannot/will not/ here. Alternatively, -dumpmachine output of both
could be compared, yet even that would leave us with false negatives
(e.g. x86_64-suse-linux vs x86_64-pc-linux-gnu as I can see for my
system compiler vs the ones I built myself).

Jan

> +endif
>  
>  $(TARGET): vpci.c vpci.h list.h main.c emul.h
> -	$(HOSTCC) $(CFLAGS_xeninclude) -g -o $@ vpci.c main.c
> +	$(CC) $(CFLAGS_xeninclude) -g -o $@ vpci.c main.c
>  
>  .PHONY: clean
>  clean:
Re: [PATCH v2] tests/vpci: Use $(CC) instead of $(HOSTCC)
Posted by Roger Pau Monné 4 months, 4 weeks ago
On Wed, Jun 04, 2025 at 10:07:46AM +0200, Jan Beulich wrote:
> On 04.06.2025 09:21, Michal Orzel wrote:
> > When cross-compiling, HOSTCC can be different than CC.
> 
> I'm sorry for being pedantic, but the two can also be different for other
> reasons (and I'd like to avoid this becoming a bad precedent, then getting
> copied elsewhere). Both may target the same architecture. Hence I'd like
> to suggest ...

Would you be fine with:

"Depending on the build environment, HOSTCC can be different than CC.
With ..."

And the s/cannot/will not/ adjustment below?

If so, I don't have any further objections:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

Re: [PATCH v2] tests/vpci: Use $(CC) instead of $(HOSTCC)
Posted by Jan Beulich 4 months, 4 weeks ago
On 04.06.2025 11:05, Roger Pau Monné wrote:
> On Wed, Jun 04, 2025 at 10:07:46AM +0200, Jan Beulich wrote:
>> On 04.06.2025 09:21, Michal Orzel wrote:
>>> When cross-compiling, HOSTCC can be different than CC.
>>
>> I'm sorry for being pedantic, but the two can also be different for other
>> reasons (and I'd like to avoid this becoming a bad precedent, then getting
>> copied elsewhere). Both may target the same architecture. Hence I'd like
>> to suggest ...
> 
> Would you be fine with:
> 
> "Depending on the build environment, HOSTCC can be different than CC.
> With ..."
> 
> And the s/cannot/will not/ adjustment below?

Yeah, that would be good enough for now.

Jan

> If so, I don't have any further objections:
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Thanks, Roger.


Re: [PATCH v2] tests/vpci: Use $(CC) instead of $(HOSTCC)
Posted by Orzel, Michal 4 months, 4 weeks ago

On 04/06/2025 10:07, Jan Beulich wrote:
> On 04.06.2025 09:21, Michal Orzel wrote:
>> When cross-compiling, HOSTCC can be different than CC.
> 
> I'm sorry for being pedantic, but the two can also be different for other
> reasons (and I'd like to avoid this becoming a bad precedent, then getting
> copied elsewhere). Both may target the same architecture. Hence I'd like
> to suggest ...
> 
>> With the recent
>> `install` rule addition, this would put a binary of a wrong format in
>> the destdir (e.g. building tests on x86 host for Arm target).
>>
>> Take the opportunity to adjust the `run` rule to only run the test if
>> HOSTCC is CC, else print a warning message.
>>
>> Fixes: 96a587a05736 ("tools/tests: Add install target for vPCI")
>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>> ---
>> Changes in v2:
>>  - change Fixes tag
>>  - add `run` rule adjustment from Roger
>> ---
>>  tools/tests/vpci/Makefile | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/tests/vpci/Makefile b/tools/tests/vpci/Makefile
>> index 9450f7593a41..f2226a5543bc 100644
>> --- a/tools/tests/vpci/Makefile
>> +++ b/tools/tests/vpci/Makefile
>> @@ -8,10 +8,14 @@ all: $(TARGET)
>>  
>>  .PHONY: run
>>  run: $(TARGET)
>> +ifeq ($(CC),$(HOSTCC))
>>  	./$(TARGET)
>> +else
>> +	$(warning HOSTCC != CC, cannot run test)
> 
> ... s/cannot/will not/ here. Alternatively, -dumpmachine output of both
If this is just s/cannot/will not/ I hope this can be done on commit.

~Michal

> could be compared, yet even that would leave us with false negatives
> (e.g. x86_64-suse-linux vs x86_64-pc-linux-gnu as I can see for my
> system compiler vs the ones I built myself).
> 
> Jan
> 
>> +endif
>>  
>>  $(TARGET): vpci.c vpci.h list.h main.c emul.h
>> -	$(HOSTCC) $(CFLAGS_xeninclude) -g -o $@ vpci.c main.c
>> +	$(CC) $(CFLAGS_xeninclude) -g -o $@ vpci.c main.c
>>  
>>  .PHONY: clean
>>  clean:
>