You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/03/15 03:25:08 UTC
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #1075: add Lua interpreter
xiaoxiang781216 commented on a change in pull request #1075:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1075#discussion_r826533496
##########
File path: interpreters/lua/Kconfig
##########
@@ -0,0 +1,90 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig INTERPRETERS_LUA
+ bool "Lua interpreter support"
+ default n
+ select ARCH_SETJMP_H
+ select LIBC_FLOATINGPOINT
+ select LIBC_LOCALE
+ select LIBM
Review comment:
remove LIBM, so the user can select libm from either nuttx or toolchain.
##########
File path: interpreters/lua/nuttx_linit.c
##########
@@ -0,0 +1,67 @@
+/****************************************************************************
+ * apps/interpreters/lua/nuttx_linit.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stddef.h>
+
+#include <lua.h>
+#include <lualib.h>
+#include <lauxlib.h>
+
+#include <nuttx/config.h>
+
+#include "luamod_proto.h"
+
+/****************************************************************************
+ * Private Data
+ *****************************************************************************/
+
+static const luaL_Reg loadedlibs[] = {
+#ifdef CONFIG_INTERPRETER_LUA_CORE_LIBS
+ {"_G", luaopen_base},
Review comment:
why skip other builtin library?
##########
File path: interpreters/lua/Kconfig
##########
@@ -0,0 +1,90 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig INTERPRETERS_LUA
+ bool "Lua interpreter support"
+ default n
+ select ARCH_SETJMP_H
+ select LIBC_FLOATINGPOINT
+ select LIBC_LOCALE
+ select LIBM
+ select SYSTEM_SYSTEM
+ ---help---
+ Embed Lua language interpreter.
+ Select the Lua version with the INTERPRETER_LUA_VERSION config.
+ It's suggested to enable the SYSTEM_READLINE config.
+
+if INTERPRETERS_LUA
+
+config INTERPRETER_LUA_VERSION
+ string "Lua interpreter version"
+ default "5.4.0"
+ ---help---
+ Lua release version to fetch and build.
+ Versions 5.2.0 and up are supported.
+
+config INTERPRETER_LUA_CORE_LIBS
+ bool "Load core Lua modules"
+ default y
+ ---help---
+ Load core Lua modules like "string" and "table".
+
+menuconfig INTERPRETER_LUA_SET_PATH
+ bool "Override default search path for Lua modules"
+ default n
+ ---help---
+ Set a custom package.path search path for Lua modules.
+
+if INTERPRETER_LUA_SET_PATH
+
+config INTERPRETER_LUA_PATH
Review comment:
why not use the empty string to indicate no path instead of INTERPRETER_LUA_SET_PATH?
##########
File path: interpreters/lua/Make.defs
##########
@@ -0,0 +1,31 @@
+############################################################################
+# apps/interpreters/lua/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifneq ($(CONFIG_INTERPRETERS_LUA),)
+CONFIGURED_APPS += $(APPDIR)/interpreters/lua
+
+# Enable <lua.h> and <lauxlib.h> includes.
+
+CFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" \
+ $(APPDIR)/interpreters/lua/lua-$(CONFIG_INTERPRETER_LUA_VERSION)/src}
+CXXFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" \
Review comment:
it's very easy to introduce the conflict if we expose the whole code base to the global search path.
##########
File path: interpreters/lua/Kconfig
##########
@@ -0,0 +1,90 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig INTERPRETERS_LUA
+ bool "Lua interpreter support"
+ default n
+ select ARCH_SETJMP_H
+ select LIBC_FLOATINGPOINT
+ select LIBC_LOCALE
+ select LIBM
+ select SYSTEM_SYSTEM
+ ---help---
+ Embed Lua language interpreter.
+ Select the Lua version with the INTERPRETER_LUA_VERSION config.
+ It's suggested to enable the SYSTEM_READLINE config.
+
+if INTERPRETERS_LUA
+
+config INTERPRETER_LUA_VERSION
+ string "Lua interpreter version"
+ default "5.4.0"
+ ---help---
+ Lua release version to fetch and build.
+ Versions 5.2.0 and up are supported.
+
+config INTERPRETER_LUA_CORE_LIBS
+ bool "Load core Lua modules"
+ default y
+ ---help---
+ Load core Lua modules like "string" and "table".
+
+menuconfig INTERPRETER_LUA_SET_PATH
+ bool "Override default search path for Lua modules"
+ default n
+ ---help---
+ Set a custom package.path search path for Lua modules.
+
+if INTERPRETER_LUA_SET_PATH
+
+config INTERPRETER_LUA_PATH
+ string "Lua interpreter search path for Lua modules"
+ default "/mnt/?.lua;/mnt/?/init.lua;./?.lua;./?/init.lua"
+ ---help---
+ Default package.path search path for Lua modules.
+
+endif # INTERPRETER_LUA_SET_PATH
+
+menuconfig INTERPRETER_LUA_SET_CPATH
Review comment:
the same question like INTERPRETER_LUA_SET_PATH
##########
File path: interpreters/lua/Kconfig
##########
@@ -0,0 +1,90 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig INTERPRETERS_LUA
+ bool "Lua interpreter support"
+ default n
+ select ARCH_SETJMP_H
+ select LIBC_FLOATINGPOINT
+ select LIBC_LOCALE
+ select LIBM
+ select SYSTEM_SYSTEM
Review comment:
why need system
##########
File path: interpreters/lua/LuaModule.mk
##########
@@ -0,0 +1,37 @@
+############################################################################
+# apps/interpreters/lua/LuaModule.mk
Review comment:
LuaModue.mk to Module.mk, the folder name(lua) is enough
##########
File path: interpreters/lua/Kconfig
##########
@@ -0,0 +1,90 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig INTERPRETERS_LUA
+ bool "Lua interpreter support"
+ default n
+ select ARCH_SETJMP_H
+ select LIBC_FLOATINGPOINT
Review comment:
can we let user select LIBC_FLOATINGPOINT?
##########
File path: interpreters/lua/Makefile
##########
@@ -0,0 +1,133 @@
+############################################################################
+# apps/interpreters/lua/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Lua built-in application info
+
+PROGNAME = lua
+PRIORITY = $(CONFIG_INTERPRETER_LUA_PRIORITY)
+STACKSIZE = $(CONFIG_INTERPRETER_LUA_STACKSIZE)
+MODULE = $(CONFIG_INTERPRETERS_LUA)
+
+# Lua library
+
+LUA_VERSION = $(patsubst "%",%,$(strip $(CONFIG_INTERPRETER_LUA_VERSION)))
+LUA_TARBALL = lua-$(LUA_VERSION).tar.gz
+LUA_UNPACK = lua-$(LUA_VERSION)
+LUA_URL_BASE = http://www.lua.org/ftp
+LUA_URL = $(LUA_URL_BASE)/$(LUA_TARBALL)
+LUA_SRC = $(LUA_UNPACK)/src
+
+MAINSRC = $(LUA_SRC)/lua.c
+CORELIB_SRCS = $(filter-out $(LUA_SRC)/lauxlib.c,$(wildcard $(LUA_SRC)/*lib.c))
Review comment:
why exclude lauxlib.c?
##########
File path: interpreters/lua/Makefile
##########
@@ -0,0 +1,133 @@
+############################################################################
+# apps/interpreters/lua/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Lua built-in application info
+
+PROGNAME = lua
+PRIORITY = $(CONFIG_INTERPRETER_LUA_PRIORITY)
+STACKSIZE = $(CONFIG_INTERPRETER_LUA_STACKSIZE)
+MODULE = $(CONFIG_INTERPRETERS_LUA)
+
+# Lua library
+
+LUA_VERSION = $(patsubst "%",%,$(strip $(CONFIG_INTERPRETER_LUA_VERSION)))
+LUA_TARBALL = lua-$(LUA_VERSION).tar.gz
+LUA_UNPACK = lua-$(LUA_VERSION)
+LUA_URL_BASE = http://www.lua.org/ftp
+LUA_URL = $(LUA_URL_BASE)/$(LUA_TARBALL)
+LUA_SRC = $(LUA_UNPACK)/src
+
+MAINSRC = $(LUA_SRC)/lua.c
+CORELIB_SRCS = $(filter-out $(LUA_SRC)/lauxlib.c,$(wildcard $(LUA_SRC)/*lib.c))
+EXCLUDE_SRCS = $(MAINSRC) $(CORELIB_SRCS) $(LUA_SRC)/luac.c $(LUA_SRC)/linit.c
+CSRCS = $(filter-out $(EXCLUDE_SRCS),$(wildcard $(LUA_SRC)/*.c))
+CSRCS += nuttx_linit.c
+
+CFLAGS += -DLUA_MAXINPUT=$(CONFIG_INTERPRETER_LUA_IOBUFSIZE)
+CFLAGS += -DLUA_PROGNAME=\"$(PROGNAME)\"
+
+ifeq ($(CONFIG_INTERPRETER_LUA_32BITS),y)
+CFLAGS += -DLUA_32BITS
Review comment:
why not let source code check _FILE_OFFSET_BITS instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org