Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake nuttx copy dependencies #5901

Merged
merged 1 commit into from
Nov 24, 2016
Merged

cmake nuttx copy dependencies #5901

merged 1 commit into from
Nov 24, 2016

Conversation

dagar
Copy link
Member

@dagar dagar commented Nov 23, 2016

@davids5 FYI this makes nuttx copy depend on all nuttx files.

I haven't made any progress on the intermittent build failures. Logging would definitely help. To me it looks like it must be a race condition within the actual nuttx build. In the PX4 nuttx export step remove > nuttx_build.log to get more output.

@dagar
Copy link
Member Author

dagar commented Nov 23, 2016

I caught one of the failures locally, but it's not that helpful.

src/firmware/nuttx/CMakeFiles/firmware_nuttx.dir/builtin_commands.c.obj:(.rodata.g_builtins+0x55c): undefined reference to `serdis_main'
src/firmware/nuttx/CMakeFiles/firmware_nuttx.dir/builtin_commands.c.obj:(.rodata.g_builtins+0x56c): undefined reference to `sercon_main'
collect2: error: ld returned 1 exit status

These are in apps/system/cdcacm/cdcacm_main.c, which was still successfully built here, however it didn't make it into nuttx/lib/libapps.a for some reason. There's no help in the non-verbose Nuttx build output.

Bad build

./nuttx/lib/libapps.a:builtin_list.o:         U serdis_main

Good build

./nuttx/lib/libapps.a:builtin_list.o:         U serdis_main
./nuttx/lib/libapps.a:cdcacm_main.o:00000001 T serdis_main

@davids5
Copy link
Member

davids5 commented Nov 23, 2016

That is it. What happens to build time if Nuttx is not built in ||?

@davids5
Copy link
Member

davids5 commented Nov 23, 2016

@dagar

The 3 things I do are to get more output:
make clean && VERBOSE=True NO_NINJA_BUILD=1 `make yada
and sometimes add V=1 in the nuttx build

But I fear that that will skew the build and we will not see the failure. (I only see the failure locally 1 out of 900+ builds)

Can we add V=1 and tee on the nuttx log file if VERBOSE=True or add NUTTX_VERBOSE=True?
We can even eliminate the nuttx log file by default unless the flags or a new one is set

@davids5
Copy link
Member

davids5 commented Nov 23, 2016

@dagar

This is nice! I feel safer and it does not seem to add any build Time

Can we avoid the Zip/Unzip if we patch the nuttx Makefile? We needed it it the past because of the build in src tree and not build directory.

@dagar
Copy link
Member Author

dagar commented Nov 23, 2016

  1. zip/unzip - I was actually going to ask you about this. If you modify mkexport (look at the bottom) we can skip the zip/unzip and just use the output folder directly. Also add -p to the cp's in mkexport to preserve timestamps, which might help ninja in some incremental rebuild circumstances.

  2. Later on I got the failure to reproduce with the verbose nuttx output logged. The failed build still compiles cdcacm_main and still shows adding it to libapps.a, however the symbols aren't present. Did we hit some subtle case where arm-none-eabi-ar fails (another simultaneous call to arm-none-eabi-ar?), but doesn't report an error? Instead of running ar for every single Nuttx app could we modify the build system to add them once in one step?

@davids5
Copy link
Member

davids5 commented Nov 23, 2016

@dagar -

I will give 1) a shot when I can
RE: 2) It sounds odd unless there is a tem location used by ar, but maybe we should create a test to see if we need the equivalent of a mutex on ar

@dagar
Copy link
Member Author

dagar commented Nov 23, 2016

tem location used by ar

What do you mean?
The nuttx build seems to spend a lot of time just calling ar, it'll likely be faster to add them in bulk.

Is this good to merge and we'll open a new issue to continue discussing the failures?

@davids5
Copy link
Member

davids5 commented Nov 24, 2016

@dagar s/tem/temp/g

Yep

@davids5 davids5 merged commit df6480c into PX4:nuttx_v3 Nov 24, 2016
@dagar dagar deleted the nuttx_v3 branch November 24, 2016 02:05
@davids5
Copy link
Member

davids5 commented Nov 24, 2016

@dagar - can you bang on #2?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants