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 2021/04/06 07:59:14 UTC

[GitHub] [incubator-nuttx-apps] btashton opened a new issue #669: LTP cannot compile correctly due to VPATH limitation

btashton opened a new issue #669:
URL: https://github.com/apache/incubator-nuttx-apps/issues/669


   When compiling LTP there are missing symbols because the Make.dep file is being built incorrectly.
   
   For example see these entries that are all overriding each other:
   ```
   ❯ grep "12-1\.home" -A 2 ./Make.dep
   12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o: \
    ltp/testcases/open_posix_testsuite/conformance/interfaces/pthread_create/12-1.c \
    /usr/include/stdc-predef.h \
   --
   12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o: \
    ltp/testcases/open_posix_testsuite/conformance/interfaces/mq_open/12-1.c \
    /usr/include/stdc-predef.h /home/bashton/nuttx/wrk/nuttx/include/stdio.h \
   --
   12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o: \
    ltp/testcases/open_posix_testsuite/conformance/interfaces/timer_settime/speculative/12-1.c \
    /usr/include/stdc-predef.h /home/bashton/nuttx/wrk/nuttx/include/time.h \
   --
   12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o: \
    ltp/testcases/open_posix_testsuite/conformance/interfaces/sem_wait/12-1.c \
    /usr/include/stdc-predef.h /home/bashton/nuttx/wrk/nuttx/include/stdio.h \
   --
   12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o: \
    ltp/testcases/open_posix_testsuite/conformance/interfaces/mmap/12-1.c \
    /usr/include/stdc-predef.h /home/bashton/nuttx/wrk/nuttx/include/stdio.h \
   --
   12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o: \
    ltp/testcases/open_posix_testsuite/conformance/interfaces/mq_receive/12-1.c \
    /usr/include/stdc-predef.h /home/bashton/nuttx/wrk/nuttx/include/stdio.h \
   --
   12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o: \
    ltp/testcases/open_posix_testsuite/conformance/interfaces/sigqueue/12-1.c \
    /usr/include/stdc-predef.h \
   --
   12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o: \
    ltp/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/12-1.c \
    /usr/include/stdc-predef.h /home/bashton/nuttx/wrk/nuttx/include/stdio.h \
   ```
   
   
   Additionally I had to make these to be able to support the large amount of command line arguments:
   Changes to the OS:
   ```c
   diff --git a/tools/Config.mk b/tools/Config.mk
   index 6adbebf21a..6400121281 100644
   --- a/tools/Config.mk
   +++ b/tools/Config.mk
   @@ -328,9 +328,13 @@ endef
    #
    #   CONFIG_WINDOWS_NATIVE - Defined for a Windows native build
    
   +define NL
   +
   +
   +endef
   +
    define ARCHIVE_ADD
   -       @echo "AR (add): ${shell basename $(1)} $(2)"
   -       $(Q) $(AR) $1 $(2)
   +       $(Q) $(AR) $1 $(2) $(NL)
    endef
    
    # ARCHIVE - Same as above, but ensure the archive is
   @@ -467,12 +471,13 @@ define CLEAN
           $(Q) if exist *$(LIBEXT) (del /f /q *$(LIBEXT))
           $(Q) if exist *~ (del /f /q *~)
           $(Q) if exist (del /f /q  .*.swp)
   -       $(Q) if exist $(OBJS) (del /f /q $(OBJS))
   +       $(Q) $(foreach OBJ, $(OBJS), $(if exist $(OBJS) (del /f /q $(OBJ))))
           $(Q) if exist $(BIN) (del /f /q  $(BIN))
    endef
    else
    define CLEAN
   -       $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp $(OBJS) $(BIN)
   +       $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp $(BIN)
   +       $(Q) $(foreach OBJ, $(OBJS), $(rm -f $(OBJ)))
    endef
    endif
   ```
   
   And these changes to apps:
   ```
   diff --git a/Application.mk b/Application.mk
   index d1dba985..f606398d 100644
   --- a/Application.mk
   +++ b/Application.mk
   @@ -140,9 +140,9 @@ $(CXXOBJS): %$(SUFFIX)$(OBJEXT): %$(CXXEXT)
    
    archive:
    ifeq ($(CONFIG_CYGWIN_WINTOOL),y)
   -       $(call ARCHIVE_ADD, "${shell cygpath -w $(BIN)}", $(OBJS))
   +       $(foreach OBJ, $(OBJS), $(call ARCHIVE_ADD, "${shell cygpath -w $(BIN)}", $(OBJ)))
    else
   -       $(call ARCHIVE_ADD, $(BIN), $(OBJS))
   +       $(foreach OBJ, $(OBJS), $(call ARCHIVE_ADD, $(BIN) $(OBJ)))
    endif
    
    ifeq ($(BUILD_MODULE),y)
   ```
   
   The defconfig for the sim is here that I used:
   
   [defconfig.txt](https://github.com/apache/incubator-nuttx-apps/files/6263012/defconfig.txt)
   


-- 
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] btashton commented on issue #669: LTP cannot compile correctly due to VPATH limitation

Posted by GitBox <gi...@apache.org>.
btashton commented on issue #669:
URL: https://github.com/apache/incubator-nuttx-apps/issues/669#issuecomment-814188180


   > A potentially problematic line could be this one in Application.mk:
   > `.depend: Makefile $(wildcard $(foreach SRC, $(SRCS), $(addsuffix /$(SRC), $(subst :, ,$(VPATH))))) $(DEPCONFIG)`
   > 
   > I don't really understand how VPATH is supposed to work here, is it would result in all VPATH entries appearing with each source file.
   > 
   > Not quite sure how dependency generation should be dealt with when having external source files. In fact, I checked with nimBLE and the Make.dep is empty since it does not populate CSRCS, etc.
   
   The issue here is that we need to use the full path and not just the filename and then the base path of the application directory. If I had 
   ```
   myapp/src/1.c
   myapp/src/libs/1.c
   myapp/src/2.c
   ```
   I would have the same issue
   With two entries like
   ```
   1.full.app.dir:
   1.full.app.dir:
   2.full.app.dir:
   ```
   


-- 
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] btashton commented on issue #669: LTP cannot compile correctly due to duplicate object file names

Posted by GitBox <gi...@apache.org>.
btashton commented on issue #669:
URL: https://github.com/apache/incubator-nuttx-apps/issues/669#issuecomment-815016049


   > BTW, the "replace" argument is probably not needed anymore. With the change I made sometime ago about how libraries are built, they are always built from scratch on each build (even without make clean). So it should be safe to not do any replace.
   
   That would make this simpler. I can see if that resolves this problem. 


-- 
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] v01d commented on issue #669: LTP cannot compile correctly due to VPATH limitation

Posted by GitBox <gi...@apache.org>.
v01d commented on issue #669:
URL: https://github.com/apache/incubator-nuttx-apps/issues/669#issuecomment-814194672


   Yeah, I'm not sure how to handle this since VPATH is actually a concatenation of paths. Maybe always using the last entry would be correct but it would still require a particular layout of external code (it would change depending on if it uses recursive make calls or if it builds all files in subdirectories from the top one).


-- 
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] v01d commented on issue #669: LTP cannot compile correctly due to VPATH limitation

Posted by GitBox <gi...@apache.org>.
v01d commented on issue #669:
URL: https://github.com/apache/incubator-nuttx-apps/issues/669#issuecomment-814121369


   BTW, it seems I never applied the parallel dependency generation feature to apps (I thought I did). Maybe I can try to do that and deal with the VPATH issue while at it.


-- 
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] btashton commented on issue #669: LTP cannot compile correctly due to VPATH limitation

Posted by GitBox <gi...@apache.org>.
btashton commented on issue #669:
URL: https://github.com/apache/incubator-nuttx-apps/issues/669#issuecomment-813916247


   @xiaoxiang781216  This is very close to working, but I am a little unsure what to do with the VPATH issue.


-- 
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] btashton commented on issue #669: LTP cannot compile correctly due to duplicate object file names

Posted by GitBox <gi...@apache.org>.
btashton commented on issue #669:
URL: https://github.com/apache/incubator-nuttx-apps/issues/669#issuecomment-815014119


   > We just hit the command line to large problem.
   
   I resolved the command line too large issue with my changes I commented on the top. Do you want me to put my fixes in draft PRs?


-- 
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 issue #669: LTP cannot compile correctly due to VPATH limitation

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #669:
URL: https://github.com/apache/incubator-nuttx-apps/issues/669#issuecomment-813969045


   @ttnie and @anchao please take a look and fix the issue reported by @btashton ASAP.


-- 
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 issue #669: LTP cannot compile correctly due to duplicate object file names

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #669:
URL: https://github.com/apache/incubator-nuttx-apps/issues/669#issuecomment-814840634


   We just hit the command line to large problem.


-- 
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] v01d commented on issue #669: LTP cannot compile correctly due to VPATH limitation

Posted by GitBox <gi...@apache.org>.
v01d commented on issue #669:
URL: https://github.com/apache/incubator-nuttx-apps/issues/669#issuecomment-814115809


   @btashton is the collision in Make.dep happening for files with same name on different places? Or is it really just one file that appears multiple times on Make.dep?


-- 
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 issue #669: LTP cannot compile correctly due to duplicate object file names

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #669:
URL: https://github.com/apache/incubator-nuttx-apps/issues/669#issuecomment-815021554


   Here is the patch to workaround the long command line issue: https://github.com/apache/incubator-nuttx-apps/pull/672


-- 
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] btashton commented on issue #669: LTP cannot compile correctly due to duplicate object file names

Posted by GitBox <gi...@apache.org>.
btashton commented on issue #669:
URL: https://github.com/apache/incubator-nuttx-apps/issues/669#issuecomment-817200689


   See https://github.com/apache/incubator-nuttx/pull/3510


-- 
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] v01d commented on issue #669: LTP cannot compile correctly due to VPATH limitation

Posted by GitBox <gi...@apache.org>.
v01d commented on issue #669:
URL: https://github.com/apache/incubator-nuttx-apps/issues/669#issuecomment-814147702


   A potentially problematic line could be this one in Application.mk:
   `.depend: Makefile $(wildcard $(foreach SRC, $(SRCS), $(addsuffix /$(SRC), $(subst :, ,$(VPATH))))) $(DEPCONFIG)`
   
   I don't really understand how VPATH is supposed to work here, is it would result in all VPATH entries appearing with each source file.
   
   Not quite sure how dependency generation should be dealt with when having external source files. In fact, I checked with nimBLE and the Make.dep is empty since it does not populate CSRCS, etc.


-- 
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] btashton commented on issue #669: LTP cannot compile correctly due to VPATH limitation

Posted by GitBox <gi...@apache.org>.
btashton commented on issue #669:
URL: https://github.com/apache/incubator-nuttx-apps/issues/669#issuecomment-814689999


   I was able to make a small change to the mkdeps tool to allows the multiple entries, but the real issue here is that we are using replace in our calls to archive.
   Take this example where we have to objects with the same object name:
   ```
   apps/testing/ltp on  master [⇡$✘+?] took 25s 
   ❯ ar rcs  /home/bashton/nuttx/wrk/apps/libapps2.a ltp/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o 
   ❯ ar rcs  /home/bashton/nuttx/wrk/apps/libapps2.a ltp/testcases/open_posix_testsuite/conformance/interfaces/pthread_create/12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o  
   ❯ nm /home/bashton/nuttx/wrk/apps/libapps2.a
   
   12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o:
   00000000 t a_thread_func
   0000000a T ltp_interfaces_pthread_create_12_1_main
            U printf
            U pthread_create
            U strerror
   ```
   We only end up with a single entry in the archive for the two different object files with the same name.
   
   Removing the replace argument we no longer have this issue:
   ```
   apps/testing/ltp on  master [⇡$✘+?] took 3s 
   ❯ ar cs  /home/bashton/nuttx/wrk/apps/libapps.a ltp/testcases/open_posix_testsuite/conformance/interfaces/pthread_create/12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o  
   ❯ ar cs  /home/bashton/nuttx/wrk/apps/libapps.a ltp/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o 
   ❯ nm /home/bashton/nuttx/wrk/apps/libapps.a
   
   12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o:
   00000000 t a_thread_func
   0000000a T ltp_interfaces_pthread_create_12_1_main
            U printf
            U pthread_create
            U strerror
   
   12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o:
   00000000 t a_thread_func
   0000000a T ltp_interfaces_pthread_create_12_1_main
            U printf
            U pthread_create
            U strerror
   ```
   
   I think making that change would break a lot of other things.
   
   Another option would be to create a single object file that is then added to the libapp.a archive.
   
   ```
   apps/testing/ltp on  master [⇡$✘+?] took 17s 
   ❯ ld -m elf_i386 -r ltp/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o ltp/testcases/open_posix_testsuite/conformance/interfaces/pthread_create/12-1.home.bashton.nuttx.wrk.apps.testing.ltp.o  -o ltp.home.bashton.nuttx.wrk.apps.testing.ltp.o
   ❯ ar rcs libapps.a ltp.home.bashton.nuttx.wrk.apps.testing.ltp.o 
   ❯ nm libapps.a 
   
   ltp.home.bashton.nuttx.wrk.apps.testing.ltp.o:
   00000013 t a_thread_func
   00000518 t a_thread_func
   00000004 b barrier
            U __errno
            U exit
            U getpid
   00000000 b in_handler
   00000000 t justreturn_handler
   0000025a T ltp_interfaces_mq_timedsend_12_1_main
   00000522 T ltp_interfaces_pthread_create_12_1_main
            U mq_open
            U mq_timedsend
   00000000 d mq_timedsend_errno
            U mq_unlink
            U nanosleep
            U perror
            U printf
            U pthread_barrier_destroy
            U pthread_barrier_init
            U pthread_barrier_wait
            U pthread_create
            U pthread_exit
            U pthread_join
            U pthread_kill
            U sigaction
            U sigemptyset
            U sprintf
            U strerror
            U strlen
            U time
   ```
   
   @xiaoxiang781216  Do you have any thoughts on this?  I would be nice to be able to support this more generally without having this restriction of the symbols needing to be unique names across all apps.  We are just asking to be hit with an unexpected bug.


-- 
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] v01d commented on issue #669: LTP cannot compile correctly due to duplicate object file names

Posted by GitBox <gi...@apache.org>.
v01d commented on issue #669:
URL: https://github.com/apache/incubator-nuttx-apps/issues/669#issuecomment-814908906


   BTW, the "replace" argument is probably not needed anymore. With the change I made sometime ago about how libraries are built, they are always built from scratch on each build (even without make clean). So it should be safe to not do any replace.


-- 
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] v01d commented on issue #669: LTP cannot compile correctly due to duplicate object file names

Posted by GitBox <gi...@apache.org>.
v01d commented on issue #669:
URL: https://github.com/apache/incubator-nuttx-apps/issues/669#issuecomment-814907425


   > > A potentially problematic line could be this one in Application.mk:
   > > `.depend: Makefile $(wildcard $(foreach SRC, $(SRCS), $(addsuffix /$(SRC), $(subst :, ,$(VPATH))))) $(DEPCONFIG)`
   > > I don't really understand how VPATH is supposed to work here, is it would result in all VPATH entries appearing with each source file.
   > > Not quite sure how dependency generation should be dealt with when having external source files. In fact, I checked with nimBLE and the Make.dep is empty since it does not populate CSRCS, etc.
   > 
   > The issue here is that we need to use the full path and not just the filename and then the base path of the application directory. If I had
   > 
   > ```
   > myapp/src/1.c
   > myapp/src/libs/1.c
   > myapp/src/2.c
   > ```
   > 
   > I would have the same issue
   > With two entries like
   > 
   > ```
   > 1.full.app.dir:
   > 1.full.app.dir:
   > 2.full.app.dir:
   > ```
   
   I understand, but I was trying to come up with a fix that does this and at the same time does the right thing for NuttX's sources and couldn't think of a way. The line I highlighted uses VPATH in some way but I'm not entirely sure if it actually does anything useful. 


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