[riot-notifications] [RIOT-OS/RIOT] pkg/wamr: add WAMR to provide WASM support in RIOT (#15329)

[Francisco]

@fjmolinas commented on this pull request.

Thanks for the contribution @kfessel, some comments on the `makefile` integration. Regarding
the PR description:

> Example will be provided

Would you say this PR is still WIP then?

> +	WAMR_BUILD_TARGET = X86_32
+else ifeq ($(findstring arm,$(CPU_ARCH)),arm)
+	WAMR_BUILD_TARGET = THUMB
+else ifeq ($(CPU_ARCH),mips32r2)
+	WAMR_BUILD_TARGET = MIPS
+else ifeq ($(CPU_ARCH),xtensa)
+	WAMR_BUILD_TARGET = XTENSA
+endif
+
+ifeq ($(QUIET), 0)
+	CMAKEMAKEFLAGS += VERBOSE=1

```suggestion
  WAMR_BUILD_TARGET = X86_32
else ifeq ($(findstring arm,$(CPU_ARCH)),arm)
  WAMR_BUILD_TARGET = THUMB
else ifeq ($(CPU_ARCH),mips32r2)
  WAMR_BUILD_TARGET = MIPS
else ifeq ($(CPU_ARCH),xtensa)
  WAMR_BUILD_TARGET = XTENSA
endif

ifeq ($(QUIET), 0)
  CMAKEMAKEFLAGS += VERBOSE=1
```

> +#WAMR_BUILD_TARGET is "X86_32" "AARCH64[sub]", "ARM[sub]",
+#                     "THUMB[sub]", "MIPS" or "XTENSA"
+#no msp430, no AVR support for now
+ifeq ($(CPU),native)
+#$(CPU) ist defined for every CPU
+#everyone build on x86_32
+	WAMR_BUILD_TARGET = X86_32
+else ifeq ($(findstring arm,$(CPU_ARCH)),arm)
+	WAMR_BUILD_TARGET = THUMB
+else ifeq ($(CPU_ARCH),mips32r2)
+	WAMR_BUILD_TARGET = MIPS
+else ifeq ($(CPU_ARCH),xtensa)
+	WAMR_BUILD_TARGET = XTENSA
+endif
+
+ifeq ($(QUIET), 0)

```suggestion
ifeq ($(QUIET),0)
```

> +include $(RIOTBASE)/pkg/pkg.mk
+
+#less Wall TODO: get things fixed upstream
+CFLAGS :=  $(filter-out -pedantic, $(CFLAGS))
+CFLAGS += -Wno-format
+CFLAGS += -Wno-strict-prototypes
+CFLAGS += -Wno-old-style-definition
+CFLAGS += -Wno-cast-function-type
+
+#Prepapre includes for cmake
+RIOT_INCLUDES = $(filter-out -%,$(subst -I,,$(INCLUDES)))
+
+#translate (CPU_ARCH) to Build Target
+#WAMR_BUILD_TARGET is "X86_32" "AARCH64[sub]", "ARM[sub]",
+#                     "THUMB[sub]", "MIPS" or "XTENSA"
+#no msp430, no AVR support for now

Should this be added to `FEATURES_BLACKLIST then?`

> @@ -0,0 +1,61 @@
+PKG_NAME=wamr
+PKG_URL=https://github.com/bytecodealliance/wasm-micro-runtime.git
+PKG_VERSION=main

Better use a commit hash here.

> @@ -0,0 +1,61 @@
+PKG_NAME=wamr
+PKG_URL=https://github.com/bytecodealliance/wasm-micro-runtime.git
+PKG_VERSION=main
+PKG_LICENSE=Apache-2.0
+
+include $(RIOTBASE)/pkg/pkg.mk
+
+#less Wall TODO: get things fixed upstream
+CFLAGS :=  $(filter-out -pedantic, $(CFLAGS))

Why is this needed I was able to build without? (by build I mean `USEPKG=wamr make -C examples/hello-world/`)

> +print_build_target:
+	@echo CPU_ARCH: $(CPU_ARCH)
+	@echo CPU: $(CPU)
+	@echo CFLAGS: $(CFLAGS)
+	@echo WAMR_BUILD_TARGET: $(WAMR_BUILD_TARGET)

This is not needed, you can use `make info-debug-variable-CPU_ARCH` for this.

> +# Copyright (C) 2019 Intel Corporation.  All rights reserved.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+cmake_minimum_required(VERSION 3.8.2)

Not very experiences in `cmake` to review this, maybe @cgundogan or @leandrolanzieri can take a look?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/15329#pullrequestreview-539158137
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20201126/0240c78d/attachment-0001.htm>