[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [UNIKRAFT PATCH v4 2/3] support/build: Add build functions for clang and gcc check

Hello Vlad,

Thanks for the work. Please find the comment inline.

Thanks & Regards


On 2/1/21 4:59 AM, Razvan Deaconescu wrote:
All good.


Vlad-Andrei Badoiu <vlad_andrei.badoiu@xxxxxx> writes:
We add have_gcc and have_clang. We modify gcc_version_ge to
use have_gcc.

Signed-off-by: Alice Suiu <alicesuiu17@xxxxxxxxx>
Signed-off-by: Vlad-Andrei Badoiu <vlad_andrei.badoiu@xxxxxx>
Reviewed-by: Razvan Deaconescu <razvan.deaconescu@xxxxxxxxx>
  support/build/Makefile.rules | 12 +++++++++++-
  1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/support/build/Makefile.rules b/support/build/Makefile.rules
index 2a8a294..c201bc4 100644
--- a/support/build/Makefile.rules
+++ b/support/build/Makefile.rules
@@ -29,10 +29,20 @@ lc = $(subst A,a,$(subst B,b,$(subst C,c,$(subst 
D,d,$(subst E,e,$(subst F,f,$(s
  # upper case
  uc = $(subst a,A,$(subst b,B,$(subst c,C,$(subst d,D,$(subst
e,E,$(subst f,F,$(subst g,G,$(subst h,H,$(subst i,I,$(subst
j,J,$(subst k,K,$(subst l,L,$(subst m,M,$(subst n,N,$(subst
o,O,$(subst p,P,$(subst q,Q,$(subst r,R,$(subst s,S,$(subst
t,T,$(subst u,U,$(subst v,V,$(subst w,W,$(subst x,X,$(subst
y,Y,$(subst z,Z,$(1)))))))))))))))))))))))))))
+# test if GCC is set as a compiler
+define have_gcc =
+$(shell if [ $(CC) = gcc ] ; then echo y ; else echo n ; fi)

shouldn't this check also for check for $(CONFIG_CROSSCOMPILE)gcc, wouldn't it return false when cross-compiling with gcc?

We can fix it within make using the this syntax instead of calling shell:

$(if $(filter %gcc,$(CC)),y,n)

+# test if CLANG is set as a compiler
+define have_clang =
+$(shell if [ $(CC) = clang ] ; then echo y ; else echo n ; fi)
  # test whether GCC version is greater than or equal to the minimum requirement
  # gcc_version_ge $gcc_major,$gcc_minor
  define gcc_version_ge =
-$(shell if [ $(CC_VER_MAJOR) -gt $(1) ] || ([ $(CC_VER_MAJOR) -eq $(1) ] && [ 
$(CC_VER_MINOR) -ge $(2) ]) ; then echo y ; fi)
+$(shell if [ $(call have_gcc) = y ] ; then if [ $(CC_VER_MAJOR) -gt $(1) ] || ([ 
$(CC_VER_MAJOR) -eq $(1) ] && [ $(CC_VER_MINOR) -ge $(2) ]) ; then echo y ; fi 
; else echo n ; fi)
You are changing the behavior of this definition. The definition is expected to return y or empty string. If you `echo n` here, the caller has to checked explicitly check if the value returned is y. Typically only not empty string check would be performed. So if you echo n it will result if clause evaluating true on both account.
# print error and stop build when GCC version doesn't meet the minimum requirement



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.