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

tools/incdir.sh: Correct error in select of include path #895

Merged
merged 1 commit into from
Apr 27, 2020

Conversation

patacongo
Copy link
Contributor

@patacongo patacongo commented Apr 27, 2020

Summary

Commit 3b9efc9 introduced an error in the generation of include file paths. The logic that determined if cygpath should be called to create a Windows native path for the case of Cygwin using a native toolchain was incorrect.

Impact

This corrects this warning noted in Issue #886:

./tools/configure.sh -c sim:nsh
$ make
<CUT>
make[1]: Entering directory '/home/btashton/apache/apps'
make[2]: Entering directory '/home/btashton/apache/apps/builtin'
./exec_builtin.c:54:10: fatal error: builtin/builtin.h: No such file or directory
   54 | #include "builtin/builtin.h"
      |          ^~~~~~~~~~~~~~~~~~~
compilation terminated.
ERROR: cc failed: 1
       command: cc -MT ./exec_builtin.home.btashton.apache.apps.builtin.o  -M -Wall -Wstrict-prototypes -Wshadow -Wundef -g -fno-builtin -fno-common -I. -isystem /home/btashton/apache/nuttx/include -D__KERNEL__ -pipe -I C:\cygwin64\home\btashton\apache\apps\include ./exec_builtin.c
make[2]: *** [/home/btashton/apache/apps/Application.mk:224: .depend] Error 1
make[2]: Leaving directory '/home/btashton/apache/apps/builtin'
make[1]: *** [Makefile:67: /home/btashton/apache/apps/builtin_depend] Error 2

In this case a Cygwin POSIX toolchain is being used but the path in the CFLAGS to apps/include is incorrectly a Windows native path. This error is corrected by this change to tools/incdir.sh

Testing

Verified by building the sim:nsh configuration on a Cygwin development platform.

Commit 3b9efc9 introduced an error in the generation of include file paths.  The logic that determined if cygpath should be called to create a Windows native path for the case of Cygwin using a native toolchain was incorrect.  This corrects this warning:

 ./tools/configure.sh -c sim:nsh
$ make
<CUT>
make[1]: Entering directory '/home/btashton/apache/apps'
make[2]: Entering directory '/home/btashton/apache/apps/builtin'
./exec_builtin.c:54:10: fatal error: builtin/builtin.h: No such file or directory
   54 | #include "builtin/builtin.h"
      |          ^~~~~~~~~~~~~~~~~~~
compilation terminated.
ERROR: cc failed: 1
       command: cc -MT ./exec_builtin.home.btashton.apache.apps.builtin.o  -M -Wall -Wstrict-prototypes -Wshadow -Wundef -g -fno-builtin -fno-common -I. -isystem /home/btashton/apache/nuttx/include -D__KERNEL__ -pipe -I C:\cygwin64\home\btashton\apache\apps\include ./exec_builtin.c
make[2]: *** [/home/btashton/apache/apps/Application.mk:224: .depend] Error 1
make[2]: Leaving directory '/home/btashton/apache/apps/builtin'
make[1]: *** [Makefile:67: /home/btashton/apache/apps/builtin_depend] Error 2

In this case a Cygwin POSIX toolchain is being used by the path in the CFLAGS to apps/include is incorrectly a Windows native path.  This error is corrected by this change to tools/incdir.sh
@patacongo patacongo linked an issue Apr 27, 2020 that may be closed by this pull request
@Ouss4
Copy link
Member

Ouss4 commented Apr 27, 2020

@btashton we should backport this and then proceed with RC1, right?

@Ouss4 Ouss4 merged commit e11c1e7 into apache:master Apr 27, 2020
@btashton
Copy link
Contributor

@btashton we should backport this and then proceed with RC1, right?

Yes, this should be backported. I'll cut the release tonight.

@patacongo patacongo deleted the incdir branch April 27, 2020 17:31
@btashton btashton added this to To-Add in Release Notes - 9.1 Jun 4, 2020
@btashton btashton moved this from To-Add to Not Applicable in Release Notes - 9.1 Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Release Notes - 9.1
  
Not Applicable
Development

Successfully merging this pull request may close these issues.

Cygwin sim:nsh failed on apps/builtin
4 participants