Cygwin: repair NuttX build #8737
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MaEtUgR
force-pushed
the
cygwin-repair-nuttx
branch
from
fe9b301 to
24a1b73
Compare
all the NuttX specific WINTOOL define handling was skiped in cmake Make.defs generation
The Nuttx build is makfile based and needs its cygwin treatment
Additionally the ${PX4_SOURCE_DIR}/src/include which was added through cmake needs path conversion
The two root causes for the special handling are:
- ARM GCC for Windows doesn't support cygwin paths passed as an argument
so they need to be either relative or converted via cypath tool
- Symbolic links are totally different under Windows and because NuttX uses them extensively
it has special handling scripts that need to be fed with the correct defines
MaEtUgR
force-pushed
the
cygwin-repair-nuttx
branch
2 times, most recently
from
6bd962b to
336a1b1
Compare
|
@dagar Now it's completely relying on cmake for the paths and using only the existing makefile defines for the symbolic links (which have no else case). |
|
Let’s see what CI says! |
|
@LorenzMeier With the first commit it worked. I'm currently discussing with @dagar in slack about the TOPDIR. |
|
Looks like the second commit wasn't the best idea... |
dagar
reviewed
Jan 20, 2018
| @@ -33,8 +33,8 @@ | |||
| # | |||
| ############################################################################ | |||
|
|
|||
| include $(TOPDIR)/.config | |||
| include $(TOPDIR)/tools/Config.mk | |||
| include ${NUTTX_DIR}/.config | |||
There was a problem hiding this comment.
Do we need to preserve this to make NuttX happy?
There was a problem hiding this comment.
I can switch all back to TOPDIR, let me fix the second commit.
MaEtUgR
force-pushed
the
cygwin-repair-nuttx
branch
from
336a1b1 to
691b693
Compare
differentiate between WINTOOL define (MSYS & cygwin) and cygwin using uname command: - MSYS needs symbolic link handling for ARM GCC but no path conversion - Cygwin needs both
MaEtUgR
force-pushed
the
cygwin-repair-nuttx
branch
from
691b693 to
dab3233
Compare
|
@dagar I
|
dagar
approved these changes
Jan 21, 2018
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
after 1f63d85
all the NuttX specific WINTOOL define handling was skipped in cmake Make.defs generation (#8573)
The Nuttx build is makfile based and needs its cygwin treatment (https://github.com/PX4-NuttX/nuttx/blob/b18053574bf41712cef93e31bf178518f451a350/configs/stm32f4discovery/scripts/Make.defs#L42-L56)
Additionally the ${PX4_SOURCE_DIR}/src/include which was added through cmake needs path conversion
The two root causes for the special handling are:
so they need to be either relative or converted via cypath tool
it has special handling scripts that need to be fed with the correct defines
@dagar Now it's partly makefile
${shell ...}path conversion like NuttX did it before and partly cmakeCYGPATH(...)conversion because you added${PX4_SOURCE_DIR}/src/includevia cmake. What do you think?How was
${PX4_SOURCE_DIR}/src/includeincluded before? It always foundvisibility.hso it was included but not in theCFLAGS...I tested
make px4fmu-v4_defaultandmake px4fmu-v2_defaultand they run through fine with these changes.EDIT: For reference the pull requests that made this necessary/possible: #7873 #8573