Coder Social home page Coder Social logo

Comments (15)

btashton avatar btashton commented on May 30, 2024

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

from nuttx-apps.

xiaoxiang781216 avatar xiaoxiang781216 commented on May 30, 2024

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

from nuttx-apps.

protobits avatar protobits commented on May 30, 2024

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

from nuttx-apps.

protobits avatar protobits commented on May 30, 2024

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.

from nuttx-apps.

protobits avatar protobits commented on May 30, 2024

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.

from nuttx-apps.

btashton avatar btashton commented on May 30, 2024

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:

from nuttx-apps.

protobits avatar protobits commented on May 30, 2024

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

from nuttx-apps.

btashton avatar btashton commented on May 30, 2024

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.

from nuttx-apps.

xiaoxiang781216 avatar xiaoxiang781216 commented on May 30, 2024

We just hit the command line to large problem.

from nuttx-apps.

protobits avatar protobits commented on May 30, 2024

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.

from nuttx-apps.

protobits avatar protobits commented on May 30, 2024

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.

from nuttx-apps.

btashton avatar btashton commented on May 30, 2024

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?

from nuttx-apps.

btashton avatar btashton commented on May 30, 2024

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.

from nuttx-apps.

xiaoxiang781216 avatar xiaoxiang781216 commented on May 30, 2024

Here is the patch to workaround the long command line issue: #672

from nuttx-apps.

btashton avatar btashton commented on May 30, 2024

See apache/nuttx#3510

from nuttx-apps.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.