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 2020/08/12 10:24:07 UTC

[GitHub] [incubator-nuttx-apps] spiriou opened a new pull request #362: system: add libuv port for NuttX

spiriou opened a new pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362


   Summary
   From http://docs.libuv.org/en/v1.x/
   "libuv is a multi-platform support library with a focus on asynchronous I/O. It was primarily developed for use by Node.js, but it’s also used by Luvit, Julia, pyuv, and others."
   
   Initial port to NuttX from libuv v1.38.1.
   
   Impact
   None, disabled by default (CONFIG_LIBUV)
   
   Testing
   Select CONFIG_LIBUV and CONFIG_LIBUV_UNIT_TESTS. Then run "libuvtest" in nsh.
   Only few tests enabled for now:
   - async
   - loop_backend_timeout
   - loop_update_time


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#discussion_r470063821



##########
File path: system/libuv/Makefile
##########
@@ -0,0 +1,52 @@
+############################################################################
+# system/libuv/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
+
+LIBUV_PATCHS ?= $(sort $(wildcard 000*.patch))
+WGET ?= wget
+UNPACK ?= unzip -q
+
+$(CONFIG_LIBUV_TARBALL):
+	@echo "Downloading: $(CONFIG_LIBUV_TARBALL)"
+	$(Q) $(WGET) $(CONFIG_LIBUV_URL)/$(CONFIG_LIBUV_TARBALL) \
+		-O $(CONFIG_LIBUV_TARBALL)
+
+$(LIBUV_UNPACKDIR): $(CONFIG_LIBUV_TARBALL)
+	@echo "Unpacking: $(CONFIG_LIBUV_TARBALL) -> $(LIBUV_UNPACKDIR)"
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(Q) $(UNPACK) "$(CONFIG_LIBUV_TARBALL)"
+	$(Q) cat $(LIBUV_PATCHS) | patch -s -N -d $(LIBUV_UNPACKDIR) -p1
+	$(Q) touch $(LIBUV_UNPACKDIR)
+
+context: $(LIBUV_UNPACKDIR)
+
+# FIXME make clean does not work without this target
+.depend:
+	$(Q) touch $@
+
+depend: .depend
+
+distclean:
+	$(call DELFILE, .depend)
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(call DELFILE, $(CONFIG_LIBUV_TARBALL))
+
+include $(APPDIR)/Directory.mk

Review comment:
       You can do that with an Application.mk makefile as well.
   The reason I'm saying this is because you are not using any of the prepared targets from Directory.mk and you are recreating targets that already exist in Application.mk.
   For instance Application.mk's file already creates the .depend file in its depend target and it cleans it during its distclean.
   




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] spiriou commented on a change in pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
spiriou commented on a change in pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#discussion_r471018332



##########
File path: system/libuv/libuv/Makefile
##########
@@ -0,0 +1,107 @@
+############################################################################
+# system/libuv/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
+
+LIBUV_PATCHS ?= $(sort $(wildcard $(LIBUV_DIR)/000*.patch))
+WGET ?= wget
+UNPACK ?= unzip -q
+
+$(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL):
+	@echo "Downloading: $(CONFIG_LIBUV_TARBALL)"
+	$(Q) $(WGET) $(CONFIG_LIBUV_URL)/$(CONFIG_LIBUV_TARBALL) \
+		-O $(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)
+
+$(LIBUV_UNPACKDIR): $(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)
+	@echo "Unpacking: $(CONFIG_LIBUV_TARBALL) -> $(LIBUV_UNPACKDIR)"
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(Q) unzip -q -d "$(LIBUV_DIR)" "$(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)"
+	$(Q) cat $(LIBUV_PATCHS) | \
+		patch -s -N -d $(LIBUV_UNPACKDIR) -p1
+	$(Q) touch $(LIBUV_UNPACKDIR)
+
+# Build libuv library
+
+CFLAGS += -I$(LIBUV_UNPACKDIR)/src
+CFLAGS += -I$(LIBUV_UNPACKDIR)/src/unix
+CFLAGS += -D__NUTTX__
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/core.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/poll.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/loop.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/thread.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/no-proctitle.c
+ifeq ($(CONFIG_LIBUV_LOOP_WATCHERS),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/loop-watcher.c
+endif
+
+# FIXME signal does not work yet
+ifeq ($(CONFIG_LIBUV_SIGNAL),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/signal.c
+endif
+
+# FIXME process does not work yet
+ifeq ($(CONFIG_LIBUV_PROCESS),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/process.c
+endif
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx.c
+
+ifeq ($(CONFIG_LIBUV_STREAM),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_stream.c
+endif
+
+ifeq ($(CONFIG_LIBUV_TCP),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_tcp.c
+endif
+
+ifeq ($(CONFIG_LIBUV_WQ),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_threadpool.c
+endif
+
+ifeq ($(CONFIG_LIBUV_ASYNC),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/async.c
+endif
+
+ifeq ($(CONFIG_LIBUV_TIMER),y)
+ifeq ($(CONFIG_LIBUV_TIMER_NUTTX),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_timer.c
+else
+CSRCS += $(LIBUV_UNPACKDIR)/src/timer.c
+endif
+endif
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/uv-common.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/strscpy.c
+
+ifeq ($(CONFIG_LIBUV_NET),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/inet.c
+endif
+
+context:: $(LIBUV_UNPACKDIR)
+
+clean::
+	$(call DELFILE, $(OBJS))

Review comment:
       Yes, all the objects are cleaned with this trick, even the ones not in libuv/ or tests/ folders like "./libuv-1.38.1/src/unix/nuttx_stream.path.to.nuttx.incubator-nuttx-apps.system.libuv.libuv.o".
   OBJS is defined based on $(CSRCS:.c=$(SUFFIX)$(OBJEXT)) so it works fine.
   How do you feel about this ?
   What would be the correct solution to ensure all the objects are cleaned without extending the clean:: target ?
   




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] spiriou commented on a change in pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
spiriou commented on a change in pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#discussion_r470278960



##########
File path: system/libuv/Makefile
##########
@@ -0,0 +1,52 @@
+############################################################################
+# system/libuv/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
+
+LIBUV_PATCHS ?= $(sort $(wildcard 000*.patch))
+WGET ?= wget
+UNPACK ?= unzip -q
+
+$(CONFIG_LIBUV_TARBALL):
+	@echo "Downloading: $(CONFIG_LIBUV_TARBALL)"
+	$(Q) $(WGET) $(CONFIG_LIBUV_URL)/$(CONFIG_LIBUV_TARBALL) \
+		-O $(CONFIG_LIBUV_TARBALL)
+
+$(LIBUV_UNPACKDIR): $(CONFIG_LIBUV_TARBALL)
+	@echo "Unpacking: $(CONFIG_LIBUV_TARBALL) -> $(LIBUV_UNPACKDIR)"
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(Q) $(UNPACK) "$(CONFIG_LIBUV_TARBALL)"
+	$(Q) cat $(LIBUV_PATCHS) | patch -s -N -d $(LIBUV_UNPACKDIR) -p1
+	$(Q) touch $(LIBUV_UNPACKDIR)
+
+context: $(LIBUV_UNPACKDIR)
+
+# FIXME make clean does not work without this target
+.depend:
+	$(Q) touch $@
+
+depend: .depend
+
+distclean:
+	$(call DELFILE, .depend)
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(call DELFILE, $(CONFIG_LIBUV_TARBALL))
+
+include $(APPDIR)/Directory.mk

Review comment:
       The Makefiles should be fine now, I managed to remove the .depend hack.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#discussion_r471039972



##########
File path: system/libuv/libuv/Makefile
##########
@@ -0,0 +1,107 @@
+############################################################################
+# system/libuv/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
+
+LIBUV_PATCHS ?= $(sort $(wildcard $(LIBUV_DIR)/000*.patch))
+WGET ?= wget
+UNPACK ?= unzip -q
+
+$(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL):
+	@echo "Downloading: $(CONFIG_LIBUV_TARBALL)"
+	$(Q) $(WGET) $(CONFIG_LIBUV_URL)/$(CONFIG_LIBUV_TARBALL) \
+		-O $(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)
+
+$(LIBUV_UNPACKDIR): $(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)
+	@echo "Unpacking: $(CONFIG_LIBUV_TARBALL) -> $(LIBUV_UNPACKDIR)"
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(Q) unzip -q -d "$(LIBUV_DIR)" "$(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)"
+	$(Q) cat $(LIBUV_PATCHS) | \
+		patch -s -N -d $(LIBUV_UNPACKDIR) -p1
+	$(Q) touch $(LIBUV_UNPACKDIR)
+
+# Build libuv library
+
+CFLAGS += -I$(LIBUV_UNPACKDIR)/src
+CFLAGS += -I$(LIBUV_UNPACKDIR)/src/unix
+CFLAGS += -D__NUTTX__
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/core.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/poll.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/loop.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/thread.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/no-proctitle.c
+ifeq ($(CONFIG_LIBUV_LOOP_WATCHERS),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/loop-watcher.c
+endif
+
+# FIXME signal does not work yet
+ifeq ($(CONFIG_LIBUV_SIGNAL),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/signal.c
+endif
+
+# FIXME process does not work yet
+ifeq ($(CONFIG_LIBUV_PROCESS),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/process.c
+endif
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx.c
+
+ifeq ($(CONFIG_LIBUV_STREAM),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_stream.c
+endif
+
+ifeq ($(CONFIG_LIBUV_TCP),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_tcp.c
+endif
+
+ifeq ($(CONFIG_LIBUV_WQ),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_threadpool.c
+endif
+
+ifeq ($(CONFIG_LIBUV_ASYNC),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/async.c
+endif
+
+ifeq ($(CONFIG_LIBUV_TIMER),y)
+ifeq ($(CONFIG_LIBUV_TIMER_NUTTX),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_timer.c
+else
+CSRCS += $(LIBUV_UNPACKDIR)/src/timer.c
+endif
+endif
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/uv-common.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/strscpy.c
+
+ifeq ($(CONFIG_LIBUV_NET),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/inet.c
+endif
+
+context:: $(LIBUV_UNPACKDIR)
+
+clean::
+	$(call DELFILE, $(OBJS))

Review comment:
       No. You are right.
   The clean target called from Application only removes the files at the current level, it doesn't clean subdirectories, we need to extend it to clean any other file generated. The way you extended it to clean all OBJS does go through all the subdirectories.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#discussion_r471143621



##########
File path: system/libuv/libuv/Makefile
##########
@@ -0,0 +1,107 @@
+############################################################################
+# system/libuv/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
+
+LIBUV_PATCHS ?= $(sort $(wildcard $(LIBUV_DIR)/000*.patch))
+WGET ?= wget
+UNPACK ?= unzip -q
+
+$(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL):
+	@echo "Downloading: $(CONFIG_LIBUV_TARBALL)"
+	$(Q) $(WGET) $(CONFIG_LIBUV_URL)/$(CONFIG_LIBUV_TARBALL) \
+		-O $(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)
+
+$(LIBUV_UNPACKDIR): $(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)
+	@echo "Unpacking: $(CONFIG_LIBUV_TARBALL) -> $(LIBUV_UNPACKDIR)"
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(Q) unzip -q -d "$(LIBUV_DIR)" "$(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)"
+	$(Q) cat $(LIBUV_PATCHS) | \
+		patch -s -N -d $(LIBUV_UNPACKDIR) -p1
+	$(Q) touch $(LIBUV_UNPACKDIR)
+
+# Build libuv library
+
+CFLAGS += -I$(LIBUV_UNPACKDIR)/src
+CFLAGS += -I$(LIBUV_UNPACKDIR)/src/unix
+CFLAGS += -D__NUTTX__
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/core.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/poll.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/loop.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/thread.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/no-proctitle.c
+ifeq ($(CONFIG_LIBUV_LOOP_WATCHERS),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/loop-watcher.c
+endif
+
+# FIXME signal does not work yet
+ifeq ($(CONFIG_LIBUV_SIGNAL),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/signal.c
+endif
+
+# FIXME process does not work yet
+ifeq ($(CONFIG_LIBUV_PROCESS),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/process.c
+endif
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx.c
+
+ifeq ($(CONFIG_LIBUV_STREAM),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_stream.c
+endif
+
+ifeq ($(CONFIG_LIBUV_TCP),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_tcp.c
+endif
+
+ifeq ($(CONFIG_LIBUV_WQ),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_threadpool.c
+endif
+
+ifeq ($(CONFIG_LIBUV_ASYNC),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/async.c
+endif
+
+ifeq ($(CONFIG_LIBUV_TIMER),y)
+ifeq ($(CONFIG_LIBUV_TIMER_NUTTX),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_timer.c
+else
+CSRCS += $(LIBUV_UNPACKDIR)/src/timer.c
+endif
+endif
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/uv-common.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/strscpy.c
+
+ifeq ($(CONFIG_LIBUV_NET),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/inet.c
+endif
+
+context:: $(LIBUV_UNPACKDIR)
+
+clean::
+	$(call DELFILE, $(OBJS))

Review comment:
       The clean target as called from the general Application.mk does the regular CLEAN.  The application specific Makefile extends that to clean any other files that can not be reached by CLEAN.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#issuecomment-672930127


   > We need to be careful with this. We have to handle third party code very differently: We cannot use the Apache license header and we have to document the use of third party software in the LICENSE file. See also https://www.apache.org/legal/resolved.html
   > 
   > Most people opt correctly to NOT explicitly include third party software in the repository, but rather to download it on demand and build time and patch it as needed.
   
   @papatience libuv is downloaded from the official location, the code in here is the porting layer written by @spiriou , so I think we don't need to mention license specially.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] spiriou commented on a change in pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
spiriou commented on a change in pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#discussion_r469447955



##########
File path: system/libuv/Makefile
##########
@@ -0,0 +1,52 @@
+############################################################################
+# system/libuv/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
+
+LIBUV_PATCHS ?= $(sort $(wildcard 000*.patch))
+WGET ?= wget
+UNPACK ?= unzip -q
+
+$(CONFIG_LIBUV_TARBALL):
+	@echo "Downloading: $(CONFIG_LIBUV_TARBALL)"
+	$(Q) $(WGET) $(CONFIG_LIBUV_URL)/$(CONFIG_LIBUV_TARBALL) \
+		-O $(CONFIG_LIBUV_TARBALL)
+
+$(LIBUV_UNPACKDIR): $(CONFIG_LIBUV_TARBALL)
+	@echo "Unpacking: $(CONFIG_LIBUV_TARBALL) -> $(LIBUV_UNPACKDIR)"
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(Q) $(UNPACK) "$(CONFIG_LIBUV_TARBALL)"
+	$(Q) cat $(LIBUV_PATCHS) | patch -s -N -d $(LIBUV_UNPACKDIR) -p1
+	$(Q) touch $(LIBUV_UNPACKDIR)
+
+context: $(LIBUV_UNPACKDIR)
+
+# FIXME make clean does not work without this target
+.depend:
+	$(Q) touch $@
+
+depend: .depend
+
+distclean:
+	$(call DELFILE, .depend)
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(call DELFILE, $(CONFIG_LIBUV_TARBALL))
+
+include $(APPDIR)/Directory.mk

Review comment:
       I'm not sure about this. I want to build the test executable (system/libuv/tests/Makefile) which uses "Application.mk", and also the libuv library (system/libuv/libuv/Makefile) that I declared as another "Application.mk". Is this the right approach ? How can I build both the test tool and the library in the same Makefile ?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#discussion_r470292903



##########
File path: system/libuv/Makefile
##########
@@ -0,0 +1,52 @@
+############################################################################
+# system/libuv/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
+
+LIBUV_PATCHS ?= $(sort $(wildcard 000*.patch))
+WGET ?= wget
+UNPACK ?= unzip -q
+
+$(CONFIG_LIBUV_TARBALL):
+	@echo "Downloading: $(CONFIG_LIBUV_TARBALL)"
+	$(Q) $(WGET) $(CONFIG_LIBUV_URL)/$(CONFIG_LIBUV_TARBALL) \
+		-O $(CONFIG_LIBUV_TARBALL)
+
+$(LIBUV_UNPACKDIR): $(CONFIG_LIBUV_TARBALL)
+	@echo "Unpacking: $(CONFIG_LIBUV_TARBALL) -> $(LIBUV_UNPACKDIR)"
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(Q) $(UNPACK) "$(CONFIG_LIBUV_TARBALL)"
+	$(Q) cat $(LIBUV_PATCHS) | patch -s -N -d $(LIBUV_UNPACKDIR) -p1
+	$(Q) touch $(LIBUV_UNPACKDIR)
+
+context: $(LIBUV_UNPACKDIR)
+
+# FIXME make clean does not work without this target
+.depend:
+	$(Q) touch $@
+
+depend: .depend
+
+distclean:
+	$(call DELFILE, .depend)
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(call DELFILE, $(CONFIG_LIBUV_TARBALL))
+
+include $(APPDIR)/Directory.mk

Review comment:
       So you went with the Directory.mk?  It's all right if it fits your need, however you need to add a .gitignore file and ignore the autogenerated Kconfig (this is what's causing the CI to fail).




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] spiriou commented on a change in pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
spiriou commented on a change in pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#discussion_r470959125



##########
File path: system/libuv/libuv/Makefile
##########
@@ -0,0 +1,107 @@
+############################################################################
+# system/libuv/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
+
+LIBUV_PATCHS ?= $(sort $(wildcard $(LIBUV_DIR)/000*.patch))
+WGET ?= wget
+UNPACK ?= unzip -q
+
+$(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL):
+	@echo "Downloading: $(CONFIG_LIBUV_TARBALL)"
+	$(Q) $(WGET) $(CONFIG_LIBUV_URL)/$(CONFIG_LIBUV_TARBALL) \
+		-O $(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)
+
+$(LIBUV_UNPACKDIR): $(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)
+	@echo "Unpacking: $(CONFIG_LIBUV_TARBALL) -> $(LIBUV_UNPACKDIR)"
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(Q) unzip -q -d "$(LIBUV_DIR)" "$(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)"
+	$(Q) cat $(LIBUV_PATCHS) | \
+		patch -s -N -d $(LIBUV_UNPACKDIR) -p1
+	$(Q) touch $(LIBUV_UNPACKDIR)
+
+# Build libuv library
+
+CFLAGS += -I$(LIBUV_UNPACKDIR)/src
+CFLAGS += -I$(LIBUV_UNPACKDIR)/src/unix
+CFLAGS += -D__NUTTX__
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/core.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/poll.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/loop.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/thread.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/no-proctitle.c
+ifeq ($(CONFIG_LIBUV_LOOP_WATCHERS),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/loop-watcher.c
+endif
+
+# FIXME signal does not work yet
+ifeq ($(CONFIG_LIBUV_SIGNAL),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/signal.c
+endif
+
+# FIXME process does not work yet
+ifeq ($(CONFIG_LIBUV_PROCESS),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/process.c
+endif
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx.c
+
+ifeq ($(CONFIG_LIBUV_STREAM),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_stream.c
+endif
+
+ifeq ($(CONFIG_LIBUV_TCP),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_tcp.c
+endif
+
+ifeq ($(CONFIG_LIBUV_WQ),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_threadpool.c
+endif
+
+ifeq ($(CONFIG_LIBUV_ASYNC),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/async.c
+endif
+
+ifeq ($(CONFIG_LIBUV_TIMER),y)
+ifeq ($(CONFIG_LIBUV_TIMER_NUTTX),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_timer.c
+else
+CSRCS += $(LIBUV_UNPACKDIR)/src/timer.c
+endif
+endif
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/uv-common.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/strscpy.c
+
+ifeq ($(CONFIG_LIBUV_NET),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/inet.c
+endif
+
+context:: $(LIBUV_UNPACKDIR)
+
+clean::
+	$(call DELFILE, $(OBJS))

Review comment:
       my understanding is CLEAN only removes "$(Q) rm -f *$(OBJEXT) *$(LIBEXT)", so "*.o" and "*.a".
   It can be tricky to use VPATH here due to potential file name collisions between libuv/{libuv,tests} and libuv/libuv-1.38.1/.
   So I added the files in CSRCS variable with absolute path. Hence the CLEAN target does not work anymore because the ".o" files are located in another folder, "libuv-1.38.1/src/[unix]".
   
   If it's an issue, I can switch to VPATH and ensure there is no filename collision.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#discussion_r469407557



##########
File path: system/libuv/Makefile
##########
@@ -0,0 +1,52 @@
+############################################################################
+# system/libuv/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
+
+LIBUV_PATCHS ?= $(sort $(wildcard 000*.patch))
+WGET ?= wget
+UNPACK ?= unzip -q
+
+$(CONFIG_LIBUV_TARBALL):
+	@echo "Downloading: $(CONFIG_LIBUV_TARBALL)"
+	$(Q) $(WGET) $(CONFIG_LIBUV_URL)/$(CONFIG_LIBUV_TARBALL) \
+		-O $(CONFIG_LIBUV_TARBALL)
+
+$(LIBUV_UNPACKDIR): $(CONFIG_LIBUV_TARBALL)
+	@echo "Unpacking: $(CONFIG_LIBUV_TARBALL) -> $(LIBUV_UNPACKDIR)"
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(Q) $(UNPACK) "$(CONFIG_LIBUV_TARBALL)"
+	$(Q) cat $(LIBUV_PATCHS) | patch -s -N -d $(LIBUV_UNPACKDIR) -p1
+	$(Q) touch $(LIBUV_UNPACKDIR)
+
+context: $(LIBUV_UNPACKDIR)
+
+# FIXME make clean does not work without this target
+.depend:
+	$(Q) touch $@
+
+depend: .depend
+
+distclean:
+	$(call DELFILE, .depend)
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(call DELFILE, $(CONFIG_LIBUV_TARBALL))
+
+include $(APPDIR)/Directory.mk

Review comment:
       Is this really a "Directory" type Makefile?  It looks like an Application type.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] acassis merged pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
acassis merged pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#discussion_r471042903



##########
File path: system/libuv/libuv/Makefile
##########
@@ -0,0 +1,107 @@
+############################################################################
+# system/libuv/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
+
+LIBUV_PATCHS ?= $(sort $(wildcard $(LIBUV_DIR)/000*.patch))
+WGET ?= wget
+UNPACK ?= unzip -q
+
+$(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL):
+	@echo "Downloading: $(CONFIG_LIBUV_TARBALL)"
+	$(Q) $(WGET) $(CONFIG_LIBUV_URL)/$(CONFIG_LIBUV_TARBALL) \
+		-O $(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)
+
+$(LIBUV_UNPACKDIR): $(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)
+	@echo "Unpacking: $(CONFIG_LIBUV_TARBALL) -> $(LIBUV_UNPACKDIR)"
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(Q) unzip -q -d "$(LIBUV_DIR)" "$(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)"
+	$(Q) cat $(LIBUV_PATCHS) | \
+		patch -s -N -d $(LIBUV_UNPACKDIR) -p1
+	$(Q) touch $(LIBUV_UNPACKDIR)
+
+# Build libuv library
+
+CFLAGS += -I$(LIBUV_UNPACKDIR)/src
+CFLAGS += -I$(LIBUV_UNPACKDIR)/src/unix
+CFLAGS += -D__NUTTX__
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/core.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/poll.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/loop.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/thread.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/no-proctitle.c
+ifeq ($(CONFIG_LIBUV_LOOP_WATCHERS),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/loop-watcher.c
+endif
+
+# FIXME signal does not work yet
+ifeq ($(CONFIG_LIBUV_SIGNAL),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/signal.c
+endif
+
+# FIXME process does not work yet
+ifeq ($(CONFIG_LIBUV_PROCESS),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/process.c
+endif
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx.c
+
+ifeq ($(CONFIG_LIBUV_STREAM),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_stream.c
+endif
+
+ifeq ($(CONFIG_LIBUV_TCP),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_tcp.c
+endif
+
+ifeq ($(CONFIG_LIBUV_WQ),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_threadpool.c
+endif
+
+ifeq ($(CONFIG_LIBUV_ASYNC),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/async.c
+endif
+
+ifeq ($(CONFIG_LIBUV_TIMER),y)
+ifeq ($(CONFIG_LIBUV_TIMER_NUTTX),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_timer.c
+else
+CSRCS += $(LIBUV_UNPACKDIR)/src/timer.c
+endif
+endif
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/uv-common.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/strscpy.c
+
+ifeq ($(CONFIG_LIBUV_NET),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/inet.c
+endif
+
+context:: $(LIBUV_UNPACKDIR)
+
+clean::
+	$(call DELFILE, $(OBJS))

Review comment:
       The CLEAN operation of tools/Config.mk is also over-ridden by architecture-specific operations that generate other kinds of files.  See for example, tools/zds/Config.mk.  If you simply remove the $(OBJS) in the sub-directory, the clean will be incomplete in some cases.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#discussion_r470292903



##########
File path: system/libuv/Makefile
##########
@@ -0,0 +1,52 @@
+############################################################################
+# system/libuv/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
+
+LIBUV_PATCHS ?= $(sort $(wildcard 000*.patch))
+WGET ?= wget
+UNPACK ?= unzip -q
+
+$(CONFIG_LIBUV_TARBALL):
+	@echo "Downloading: $(CONFIG_LIBUV_TARBALL)"
+	$(Q) $(WGET) $(CONFIG_LIBUV_URL)/$(CONFIG_LIBUV_TARBALL) \
+		-O $(CONFIG_LIBUV_TARBALL)
+
+$(LIBUV_UNPACKDIR): $(CONFIG_LIBUV_TARBALL)
+	@echo "Unpacking: $(CONFIG_LIBUV_TARBALL) -> $(LIBUV_UNPACKDIR)"
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(Q) $(UNPACK) "$(CONFIG_LIBUV_TARBALL)"
+	$(Q) cat $(LIBUV_PATCHS) | patch -s -N -d $(LIBUV_UNPACKDIR) -p1
+	$(Q) touch $(LIBUV_UNPACKDIR)
+
+context: $(LIBUV_UNPACKDIR)
+
+# FIXME make clean does not work without this target
+.depend:
+	$(Q) touch $@
+
+depend: .depend
+
+distclean:
+	$(call DELFILE, .depend)
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(call DELFILE, $(CONFIG_LIBUV_TARBALL))
+
+include $(APPDIR)/Directory.mk

Review comment:
       So you went with the Directory.mk?  It's all right if it fits your need, however you need to add a .gitignore file and ignore the autogenerated Kconfig.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#discussion_r470951188



##########
File path: system/libuv/libuv/Makefile
##########
@@ -0,0 +1,107 @@
+############################################################################
+# system/libuv/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
+
+LIBUV_PATCHS ?= $(sort $(wildcard $(LIBUV_DIR)/000*.patch))
+WGET ?= wget
+UNPACK ?= unzip -q
+
+$(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL):
+	@echo "Downloading: $(CONFIG_LIBUV_TARBALL)"
+	$(Q) $(WGET) $(CONFIG_LIBUV_URL)/$(CONFIG_LIBUV_TARBALL) \
+		-O $(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)
+
+$(LIBUV_UNPACKDIR): $(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)
+	@echo "Unpacking: $(CONFIG_LIBUV_TARBALL) -> $(LIBUV_UNPACKDIR)"
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(Q) unzip -q -d "$(LIBUV_DIR)" "$(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)"
+	$(Q) cat $(LIBUV_PATCHS) | \
+		patch -s -N -d $(LIBUV_UNPACKDIR) -p1
+	$(Q) touch $(LIBUV_UNPACKDIR)
+
+# Build libuv library
+
+CFLAGS += -I$(LIBUV_UNPACKDIR)/src
+CFLAGS += -I$(LIBUV_UNPACKDIR)/src/unix
+CFLAGS += -D__NUTTX__
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/core.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/poll.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/loop.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/thread.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/no-proctitle.c
+ifeq ($(CONFIG_LIBUV_LOOP_WATCHERS),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/loop-watcher.c
+endif
+
+# FIXME signal does not work yet
+ifeq ($(CONFIG_LIBUV_SIGNAL),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/signal.c
+endif
+
+# FIXME process does not work yet
+ifeq ($(CONFIG_LIBUV_PROCESS),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/process.c
+endif
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx.c
+
+ifeq ($(CONFIG_LIBUV_STREAM),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_stream.c
+endif
+
+ifeq ($(CONFIG_LIBUV_TCP),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_tcp.c
+endif
+
+ifeq ($(CONFIG_LIBUV_WQ),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_threadpool.c
+endif
+
+ifeq ($(CONFIG_LIBUV_ASYNC),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/async.c
+endif
+
+ifeq ($(CONFIG_LIBUV_TIMER),y)
+ifeq ($(CONFIG_LIBUV_TIMER_NUTTX),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_timer.c
+else
+CSRCS += $(LIBUV_UNPACKDIR)/src/timer.c
+endif
+endif
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/uv-common.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/strscpy.c
+
+ifeq ($(CONFIG_LIBUV_NET),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/inet.c
+endif
+
+context:: $(LIBUV_UNPACKDIR)
+
+clean::
+	$(call DELFILE, $(OBJS))

Review comment:
       Is this necessary?  Application.mk already deletes object files.  If the object files are generated else where then you need to extend clean::.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#discussion_r471042049



##########
File path: system/libuv/libuv/Makefile
##########
@@ -0,0 +1,107 @@
+############################################################################
+# system/libuv/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
+
+LIBUV_PATCHS ?= $(sort $(wildcard $(LIBUV_DIR)/000*.patch))
+WGET ?= wget
+UNPACK ?= unzip -q
+
+$(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL):
+	@echo "Downloading: $(CONFIG_LIBUV_TARBALL)"
+	$(Q) $(WGET) $(CONFIG_LIBUV_URL)/$(CONFIG_LIBUV_TARBALL) \
+		-O $(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)
+
+$(LIBUV_UNPACKDIR): $(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)
+	@echo "Unpacking: $(CONFIG_LIBUV_TARBALL) -> $(LIBUV_UNPACKDIR)"
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(Q) unzip -q -d "$(LIBUV_DIR)" "$(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)"
+	$(Q) cat $(LIBUV_PATCHS) | \
+		patch -s -N -d $(LIBUV_UNPACKDIR) -p1
+	$(Q) touch $(LIBUV_UNPACKDIR)
+
+# Build libuv library
+
+CFLAGS += -I$(LIBUV_UNPACKDIR)/src
+CFLAGS += -I$(LIBUV_UNPACKDIR)/src/unix
+CFLAGS += -D__NUTTX__
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/core.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/poll.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/loop.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/thread.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/no-proctitle.c
+ifeq ($(CONFIG_LIBUV_LOOP_WATCHERS),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/loop-watcher.c
+endif
+
+# FIXME signal does not work yet
+ifeq ($(CONFIG_LIBUV_SIGNAL),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/signal.c
+endif
+
+# FIXME process does not work yet
+ifeq ($(CONFIG_LIBUV_PROCESS),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/process.c
+endif
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx.c
+
+ifeq ($(CONFIG_LIBUV_STREAM),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_stream.c
+endif
+
+ifeq ($(CONFIG_LIBUV_TCP),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_tcp.c
+endif
+
+ifeq ($(CONFIG_LIBUV_WQ),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_threadpool.c
+endif
+
+ifeq ($(CONFIG_LIBUV_ASYNC),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/async.c
+endif
+
+ifeq ($(CONFIG_LIBUV_TIMER),y)
+ifeq ($(CONFIG_LIBUV_TIMER_NUTTX),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_timer.c
+else
+CSRCS += $(LIBUV_UNPACKDIR)/src/timer.c
+endif
+endif
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/uv-common.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/strscpy.c
+
+ifeq ($(CONFIG_LIBUV_NET),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/inet.c
+endif
+
+context:: $(LIBUV_UNPACKDIR)
+
+clean::
+	$(call DELFILE, $(OBJS))

Review comment:
       The CLEAN operation does more than just remove object files.  See definitions in tools/Config.mk




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] patacongo commented on pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#issuecomment-672893063


   We need to be careful with this.  We have to handle third party code very differently:  We cannot use the Apache license header and we have to document the use of third party software in the LICENSE file.  See also https://www.apache.org/legal/resolved.html
   
   Most people opt correctly to NOT explicitly include third party software in the repository, but rather to download it on demand and build time and patch it as needed.
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#discussion_r470991157



##########
File path: system/libuv/libuv/Makefile
##########
@@ -0,0 +1,107 @@
+############################################################################
+# system/libuv/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
+
+LIBUV_PATCHS ?= $(sort $(wildcard $(LIBUV_DIR)/000*.patch))
+WGET ?= wget
+UNPACK ?= unzip -q
+
+$(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL):
+	@echo "Downloading: $(CONFIG_LIBUV_TARBALL)"
+	$(Q) $(WGET) $(CONFIG_LIBUV_URL)/$(CONFIG_LIBUV_TARBALL) \
+		-O $(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)
+
+$(LIBUV_UNPACKDIR): $(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)
+	@echo "Unpacking: $(CONFIG_LIBUV_TARBALL) -> $(LIBUV_UNPACKDIR)"
+	$(call DELDIR, $(LIBUV_UNPACKDIR))
+	$(Q) unzip -q -d "$(LIBUV_DIR)" "$(LIBUV_DIR)/$(CONFIG_LIBUV_TARBALL)"
+	$(Q) cat $(LIBUV_PATCHS) | \
+		patch -s -N -d $(LIBUV_UNPACKDIR) -p1
+	$(Q) touch $(LIBUV_UNPACKDIR)
+
+# Build libuv library
+
+CFLAGS += -I$(LIBUV_UNPACKDIR)/src
+CFLAGS += -I$(LIBUV_UNPACKDIR)/src/unix
+CFLAGS += -D__NUTTX__
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/core.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/poll.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/loop.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/thread.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/no-proctitle.c
+ifeq ($(CONFIG_LIBUV_LOOP_WATCHERS),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/loop-watcher.c
+endif
+
+# FIXME signal does not work yet
+ifeq ($(CONFIG_LIBUV_SIGNAL),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/signal.c
+endif
+
+# FIXME process does not work yet
+ifeq ($(CONFIG_LIBUV_PROCESS),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/process.c
+endif
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx.c
+
+ifeq ($(CONFIG_LIBUV_STREAM),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_stream.c
+endif
+
+ifeq ($(CONFIG_LIBUV_TCP),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_tcp.c
+endif
+
+ifeq ($(CONFIG_LIBUV_WQ),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_threadpool.c
+endif
+
+ifeq ($(CONFIG_LIBUV_ASYNC),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/async.c
+endif
+
+ifeq ($(CONFIG_LIBUV_TIMER),y)
+ifeq ($(CONFIG_LIBUV_TIMER_NUTTX),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/unix/nuttx_timer.c
+else
+CSRCS += $(LIBUV_UNPACKDIR)/src/timer.c
+endif
+endif
+
+CSRCS += $(LIBUV_UNPACKDIR)/src/uv-common.c
+CSRCS += $(LIBUV_UNPACKDIR)/src/strscpy.c
+
+ifeq ($(CONFIG_LIBUV_NET),y)
+CSRCS += $(LIBUV_UNPACKDIR)/src/inet.c
+endif
+
+context:: $(LIBUV_UNPACKDIR)
+
+clean::
+	$(call DELFILE, $(OBJS))

Review comment:
       It's not an issue it's exactly how it should work.  But are your $(LIBUV_UNPACKDIR)/src/*.o files being cleaned? As I see it, it cleans only at the libuv/libuv level.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] spiriou commented on pull request #362: system: add libuv port for NuttX

Posted by GitBox <gi...@apache.org>.
spiriou commented on pull request #362:
URL: https://github.com/apache/incubator-nuttx-apps/pull/362#issuecomment-672980555


   @patacongo @xiaoxiang781216 indeed most of the code+unit-tests is downloaded from https://github.com/libuv/libuv/. I added in the patch the apache license (with the libuv license below) to protect the modifications to support NuttX on those files.
   
   There are still few exceptions. Those files are issued from a copy/paste, and modified so they can work on NuttX. The modifications are too heavy to be managed with #ifdef #endif. What I did is reuse the libuv license, with apache license on top of it. Those files can be moved in the patch instead.
   system/libuv/libuv/nuttx_async.c
   system/libuv/libuv/nuttx_stream.c
   system/libuv/libuv/nuttx_threadpool.c
   system/libuv/libuv/nuttx.c
   system/libuv/libuv/nuttx_tcp.c
   
   system/libuv/libuv/nuttx_timer.c: that one is done from scratch so no license issue. It just implements the libuv API for that feature.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org