Open patacongo opened 3 years ago
I took the time to do the git bisect to find the comment that caused this build failure. On each step of the bisection, I did the following:
make distclean
git checkout <revision>
tools/configure.sh -c stm32f4discovery:kostest
make staging/libc.a
git bisect <good|bad> <revision>
The final step of the bisection reported:
$ git bisect bad
d5b6ec450fde069a4e64569b0eb7e4fcb3b96e83 is the first bad commit
commit d5b6ec450fde069a4e64569b0eb7e4fcb3b96e83
Author: Matias N <matias@protobits.dev>
Date: Wed Nov 18 13:44:58 2020 -0300
Parallelize depend file generation
arch/arm/src/Makefile | 7 +++++--
arch/avr/src/Makefile | 7 +++++--
arch/hc/src/Makefile | 7 +++++--
arch/mips/src/Makefile | 7 +++++--
arch/misoc/src/Makefile | 7 +++++--
arch/or1k/src/Makefile | 7 +++++--
arch/renesas/src/Makefile | 6 +++++-
arch/risc-v/src/Makefile | 7 +++++--
arch/sim/src/Makefile | 7 +++++--
arch/x86/src/Makefile | 7 +++++--
arch/x86_64/src/Makefile | 7 +++++--
arch/xtensa/src/Makefile | 7 +++++--
audio/Makefile | 6 +++++-
binfmt/Makefile | 6 +++++-
boards/Makefile | 11 +++++------
crypto/Makefile | 6 +++++-
drivers/Makefile | 6 +++++-
fs/Makefile | 6 +++++-
graphics/Makefile | 6 +++++-
libs/libc/Makefile | 12 ++++++++++--
libs/libc/bin/Makefile | 2 +-
libs/libc/zoneinfo/Makefile | 6 +++++-
libs/libdsp/Makefile | 6 +++++-
libs/libnx/Makefile | 12 ++++++++++--
libs/libnx/kbin/Makefile | 2 +-
libs/libxx/Makefile | 6 +++++-
mm/Makefile | 12 ++++++++++--
net/Makefile | 6 +++++-
openamp/Makefile | 6 +++++-
pass1/Makefile | 6 +++++-
sched/Makefile | 6 +++++-
syscall/Makefile | 7 +++++--
tools/Config.mk | 17 +++++++++++++++++
video/Makefile | 6 +++++-
wireless/Makefile | 6 +++++-
35 files changed, 194 insertions(+), 54 deletions(-)
I will take a look and try to infer the failure, as I'm really not familiar with CYGWIN. From the looks of it the include path is missing the delimiter.
Now that we have the minimal builds for macOS I can look at bringing back the Cygwin CI, previously it was super slow and I never got to the bottom of that.
That would be super useful since it would also allow me to validate the cmake changes
I'm taking a break for the US holiday weekend l, but I'll push this up the priority after along with the nxflat changes from last week that seem to be good and should be added to CI.
@patacongo did you continue the bisection to the end? If I look at the diff of that commit I only see changes to the depend stage, which should not affect the build itself. Or is that during depend target?
From the looks of it the include path is missing the delimiter.
No, the delimiters were there but disappeared. For quoted values like \, one of the backslashes will disappear from the Bash command line each time it is processed. You can see the good values a little clearer with V=1:
make[2]: Leaving directory '/cygdrive/d/Spuda/Documents/projects/nuttx/master/nuttx_fork/libs/libc'
make makekdepfile CFLAGS="-fno-builtin -funwind-tables -Wall -Wstrict-prototypes -Wshadow -Wundef -Os -fno-strict-aliasing -fomit-frame-pointer -fno-strength-reduce -mcpu=cortex-m4 -mthumb -mfloat-abi=soft -isystem "D:\Spuda\Documents\projects\nuttx\master\nuttx_fork\include" -D__NuttX__ -pipe -I "D:\Spuda\Documents\projects\nuttx\master\nuttx_fork\libs\libc" -D__KERNEL__"
make[2]: Entering directory '/cygdrive/d/Spuda/Documents/projects/nuttx/master/nuttx_fork/libs/libc'
/cygdrive/d/Spuda/Documents/projects/nuttx/master/nuttx_fork/tools/mkwindeps.sh --obj-suffix .o --dep-path assert --dep-path ctype --dep-path dirent --dep-path endian --dep-path errno --dep-path fixedmath --dep-path grp --dep-path inttypes --dep-path libgen --dep-path machine/arm/armv7-m --dep-path machine/arm --dep-path misc --dep-path net --dep-path pthread --dep-path pwd --dep-path queue --dep-path sched --dep-path semaphore --dep-path signal --dep-path spawn --dep-path stdio --dep-path stdlib --dep-path string --dep-path symtab --dep-path syslog --dep-path time --dep-path tls --dep-path uio --dep-path unistd --dep-path uuid "arm-none-eabi-gcc" -- -fno-builtin -funwind-tables -Wall -Wstrict-prototypes -Wshadow -Wundef -Os -fno-strict-aliasing -fomit-frame-pointer -fno-strength-reduce -mcpu=cortex-m4 -mthumb -mfloat-abi=soft -isystem D:SpudaDocumentsprojectsnuttxmasternuttx_forkinclude -D__NuttX__ -pipe -I D:SpudaDocumentsprojectsnuttxmasternuttx_forklibslibc -D__KERNEL__ -- assert/lib_assert.c > lib_assert.ddc
.\assert/lib_assert.c:25:10: fatal error: nuttx/arch.h: No such file or directory
25 | #include <nuttx/arch.h>
| ^~~~~~~~~~~~~~
compilation terminated.
ERROR: arm-none-eabi-gcc failed: 1
command: arm-none-eabi-gcc -M '-fno-builtin' '-funwind-tables' '-Wall' '-Wstrict-prototypes' '-Wshadow' '-Wundef' '-Os' '-fno-strict-aliasing' '-fomit-frame-pointer' '-fno-strength-reduce' '-mcpu=cortex-m4' '-mthumb' '-mfloat-abi=soft' '-isystem' 'D:SpudaDocumentsprojectsnuttxmasternuttx_forkinclude' '-D__NuttX__' '-pipe' '-I' 'D:SpudaDocumentsprojectsnuttxmasternuttx_forklibslibc' '-D__KERNEL__' .\\assert/lib_assert.c
make[2]: *** [/cygdrive/d/Spuda/Documents/projects/nuttx/master/nuttx_fork/tools/Config.mk:209: lib_assert.ddc] Error 1
make[2]: Leaving directory '/cygdrive/d/Spuda/Documents/projects/nuttx/master/nuttx_fork/libs/libc'
@patacongo did you continue the bisection to the end? If I look at the diff of that commit I only see changes to the depend stage, which should not affect the build itself. Or is that during depend target?
Yes:
$ git bisect good
Bisecting: 1 revision left to test after this (roughly 1 step)
[9ceb61d3a9ce3ca730d1e735349f7d5415d1b97f] risc-v 64-bit: Fix SCN/PRI.PTR definitions
$ git bisect bad
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[d5b6ec450fde069a4e64569b0eb7e4fcb3b96e83] Parallelize depend file generation
$ git bisect bad
d5b6ec450fde069a4e64569b0eb7e4fcb3b96e83 is the first bad commit
commit d5b6ec450fde069a4e64569b0eb7e4fcb3b96e83
...
I'm guessing it is due to CFLAGS not being inside quotes here: https://github.com/apache/incubator-nuttx/blob/eb133ad527ebea7c190c1d0b3eb60f97e4d9349d/tools/Config.mk#L185 Could you add the quotes and see if that fixes it?
Now that we have the minimal builds for macOS I can look at bringing back the Cygwin CI, previously it was super slow and I never got to the bottom of that.
Cygwin is slower than Linux for two reasons: First, the real time virus checking that you get with Windows. When I am am doing lots of builds, I normally turn of realtime virus checking during the build. The speeds thing up by maybe 40%
The other reason, which we can't do anything about, is due to the layer of simulated POSIX interfaces. In particular, fork(). All of the GCC build tools do lots of forks() but Windows does not support fork() and my understanding is that the processing to emulate fork() behavior is slow. You can see this if you make a tight loop in a Bash script and run some do-almost-nothing process as fast as you can. You will find that this is limited to something like 7 forks per second.
As a workaround, is there a way to limit parallelization to 1 core, effectively turning off parallelization?
Could you add the quotes and see if that fixes it?
Yes, but that will have to be tomorrow morning.
Could you add the quotes and see if that fixes it?
Yes, but that will have to be tomorrow morning.
That, of course, will fail if the CFLAGS from the Make.defs file already contains quotes.
Could you add the quotes and see if that fixes it?
Yes, but that will have to be tomorrow morning.
Unfortunately, that does not fix the problem. Here is the change I made:
diff --git a/tools/Config.mk b/tools/Config.mk
index 6adbebf21a..23a1acc1c2 100644
--- a/tools/Config.mk
+++ b/tools/Config.mk
@@ -182,19 +182,19 @@ endif
OBJPATH ?= .
%.dds: %.S
- $(Q) $(MKDEP) --obj-path $(OBJPATH) --obj-suffix $(OBJEXT) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+ $(Q) $(MKDEP) --obj-path $(OBJPATH) --obj-suffix $(OBJEXT) $(DEPPATH) "$(CC)" -- "$(CFLAGS)" -- $< > $@
%.ddc: %.c
- $(Q) $(MKDEP) --obj-path $(OBJPATH) --obj-suffix $(OBJEXT) $(DEPPATH) "$(CC)" -- $(CFLAGS) -- $< > $@
+ $(Q) $(MKDEP) --obj-path $(OBJPATH) --obj-suffix $(OBJEXT) $(DEPPATH) "$(CC)" -- "$(CFLAGS)" -- $< > $@
%.ddp: %.cpp
- $(Q) $(MKDEP) --obj-path $(OBJPATH) --obj-suffix $(OBJEXT) $(DEPPATH) "$(CXX)" -- $(CXXFLAGS) -- $< > $@
+ $(Q) $(MKDEP) --obj-path $(OBJPATH) --obj-suffix $(OBJEXT) $(DEPPATH) "$(CXX)" -- "$(CXXFLAGS)" -- $< > $@
%.ddx: %.cxx
- $(Q) $(MKDEP) --obj-path $(OBJPATH) --obj-suffix $(OBJEXT) $(DEPPATH) "$(CXX)" -- $(CXXFLAGS) -- $< > $@
+ $(Q) $(MKDEP) --obj-path $(OBJPATH) --obj-suffix $(OBJEXT) $(DEPPATH) "$(CXX)" -- "$(CXXFLAGS)" -- $< > $@
%.ddh: %.c
- $(Q) $(MKDEP) --obj-path $(OBJPATH) --obj-suffix $(OBJEXT) $(DEPPATH) "$(CC)" -- $(HOSTCFLAGS) -- $< > $@
+ $(Q) $(MKDEP) --obj-path $(OBJPATH) --obj-suffix $(OBJEXT) $(DEPPATH) "$(CC)" -- "$(HOSTCFLAGS)" -- $< > $@
# INCDIR - Convert a list of directory paths to a list of compiler include
# directories
@@ -258,7 +258,7 @@ endef
define COMPILE
@echo "CC: $1"
- $(Q) $(CC) -c $(CFLAGS) $($(strip $1)_CFLAGS) $1 -o $2
+ $(Q) $(CC) -c "$(CFLAGS)" $($(strip $1)_CFLAGS) $1 -o $2
endef
# COMPILEXX - Default macro to compile one C++ file
@@ -276,7 +276,7 @@ endef
define COMPILEXX
@echo "CXX: $1"
- $(Q) $(CXX) -c $(CXXFLAGS) $($(strip $1)_CXXFLAGS) $1 -o $2
+ $(Q) $(CXX) -c "$(CXXFLAGS)" $($(strip $1)_CXXFLAGS) $1 -o $2
endef
# ASSEMBLE - Default macro to assemble one assembly language file
And here is the result:
$ make staging/libc.a
...
make[1]: Entering directory '/cygdrive/d/Spuda/Documents/projects/nuttx/master/nuttx_fork/libs/libc'
make[2]: Entering directory '/cygdrive/d/Spuda/Documents/projects/nuttx/master/nuttx_fork/libs/libc'
.\dirent/lib_readdirr.c:25:10: fatal error: nuttx/config.h: No such file or directory
25 | #include <nuttx/config.h>
| ^~~~~~~~~~~~~~~~
compilation terminated.
ERROR: arm-none-eabi-gcc failed: 1
command: arm-none-eabi-gcc -MT bin\\lib_readdirr.o -M '-fno-builtin' '-funwind-tables' '-Wall' '-Wstrict-prototypes' '-Wshadow' '-Wundef' '-Os' '-fno-strict-aliasing' '-fomit-frame-pointer' '-fno-strength-reduce' '-mcpu=cortex-m4' '-mthumb' '-mfloat-abi=soft' '-isystem' 'D:SpudaDocumentsprojectsnuttxmasternuttx_forkinclude' '-D__NuttX__' '-pipe' '-I' 'D:SpudaDocumentsprojectsnuttxmasternuttx_forklibslibc' .\\dirent/lib_readdirr.c
make[2]: *** [/cygdrive/d/Spuda/Documents/projects/nuttx/master/nuttx_fork/tools/Config.mk:188: lib_readdirr.ddc] Error 1
make[2]: Leaving directory '/cygdrive/d/Spuda/Documents/projects/nuttx/master/nuttx_fork/libs/libc'
make[1]: *** [Makefile:169: .depend] Error 2
make[1]: Leaving directory '/cygdrive/d/Spuda/Documents/projects/nuttx/master/nuttx_fork/libs/libc'
make: *** [tools/Makefile.unix:468: pass1dep] Error 2
I think you can see the failure pretty clearly with make V=1 (see above):
The dependency file for libkc.a is create with the following in libs/libc/Makefile:
.depend: Makefile $(SRCS) $(TOPDIR)$(DELIM).config
$(Q) $(MAKE) makedepfile OBJPATH="bin"
ifneq ($(CONFIG_BUILD_FLAT),y)
$(Q) $(MAKE) makekdepfile CFLAGS="$(CFLAGS) $(KDEFINE)" OBJPATH="kbin"
endif
...
Where:
makekdepfile: $(CSRCS:.c=.ddc) $(ASRCS:.S=.dds)
$(call CATFILE, kbin/Make.dep, $^)
$(call DELFILE, $^)
We can see this with make V=1. Here the CFLAGS are fine. BUT since this is passed on the Bash command line, the backslashes will be eliminated. Thanks Bash:
make makekdepfile CFLAGS="-fno-builtin -funwind-tables -Wall -Wstrict-prototypes -Wshadow -Wundef -Os -fno-strict-aliasing -fomit-frame-pointer -fno-strength-reduce -mcpu=cortex-m4 -mthumb -mfloat-abi=soft -isystem "D:\Spuda\Documents\projects\nuttx\master\nuttx_fork\include" -D__NuttX__ -pipe -I "D:\Spuda\Documents\projects\nuttx\master\nuttx_fork\libs\libc" -D__KERNEL__"
So when mkwindesp.sh sees the paths, the backslashes have been eliminated:
make[2]: Entering directory '/cygdrive/d/Spuda/Documents/projects/nuttx/master/nuttx_fork/libs/libc'
/cygdrive/d/Spuda/Documents/projects/nuttx/master/nuttx_fork/tools/mkwindeps.sh --obj-suffix .o --dep-path assert --dep-path ctype --dep-path dirent --dep-path endian --dep-path errno --dep-path fixedmath --dep-path grp --dep-path inttypes --dep-path libgen --dep-path machine/arm/armv7-m --dep-path machine/arm --dep-path misc --dep-path net --dep-path pthread --dep-path pwd --dep-path queue --dep-path sched --dep-path semaphore --dep-path signal --dep-path spawn --dep-path stdio --dep-path stdlib --dep-path string --dep-path symtab --dep-path syslog --dep-path time --dep-path tls --dep-path uio --dep-path unistd --dep-path uuid "arm-none-eabi-gcc" -- -fno-builtin -funwind-tables -Wall -Wstrict-prototypes -Wshadow -Wundef -Os -fno-strict-aliasing -fomit-frame-pointer -fno-strength-reduce -mcpu=cortex-m4 -mthumb -mfloat-abi=soft -isystem D:SpudaDocumentsprojectsnuttxmasternuttx_forkinclude -D__NuttX__ -pipe -I D:SpudaDocumentsprojectsnuttxmasternuttx_forklibslibc -D__KERNEL__ -- assert/lib_assert.c > lib_assert.ddc
And the error follows:
.\assert/lib_assert.c:25:10: fatal error: nuttx/arch.h: No such file or directory
25 | #include <nuttx/arch.h>
| ^~~~~~~~~~~~~~
compilation terminated.
make makekdepfile CFLAGS="-fno-builtin -funwind-tables -Wall -Wstrict-prototypes -Wshadow -Wundef -Os -fno-strict-aliasing -fomit-frame-pointer -fno-strength-reduce -mcpu=cortex-m4 -mthumb -mfloat-abi=soft -isystem "D:\Spuda\Documents\projects\nuttx\master\nuttx_fork\include" -DNuttX -pipe -I "D:\Spuda\Documents\projects\nuttx\master\nuttx_fork\libs\libc" -DKERNEL"
Is this how the executed command actually looks like? If so, the problem are the quotes: note that the quote starts after the =
and closes right before the path. I remember prior issues like this which broke cygwin, regarding nested quotes. I think the solution would be to escape the inner quotes, but I'm not sure where the ones surrounding the include path are added.
Could you add the quotes and see if that fixes it?
Yes, but that will have to be tomorrow morning.
That, of course, will fail if the CFLAGS from the Make.defs file already contains quotes.
Right, I see you already thought about it. So indeed the backslashes need to be escaped. How is this usually handled? I'm not quite sure why this is breaking here and not in other places (ie: other places where make is recursively called passing flags)
I'm not quite sure why this is breaking here and not in other places (ie: other places where make is recursively called passing flags)
If is breaking on the very first invocation of 'make makekdepfile' That is the culprit because the CFLAGS are passed on the command line here in libs/libc/Makefile. Then the very first file that is processes fails because the backslashes in CFLAGS have been lost:
171 $(Q) $(MAKE) makekdepfile CFLAGS="$(CFLAGS) $(KDEFINE)" OBJPATH="kbin"
That is also where the outermost quotes are picked up as you asked earlier. So this is source of the problem on both accounts.
It is Bash command line processing that mucks with the backslashes. Quoting of various forms sometimes works but is not robust.
This issue only occurs when you are using a POSIX environment with a Windows native toolchain. In my environment, I use the ARM embedded toolchain for Windows. In this case, we have to do special path handling based on CONFIG_CYGWIN_WINTOOL=y. If we used a GCC toolchain such as the NuttX buildroot built under Cygwin, then there would be no problem (because there would be no Windows paths).
We could really simplify the build system is we removed support for Windows native toolchains under Cygwin. But people really like their Windows tools. In the past, Windows was the predominant NuttX build environment. Sourceforge used to keep the statistics and 60% of the downloads were from Windows machines, 30% from Linux, and the remaining 10% from macOS and "Other". But I suspect that that has changed; that fact that this build failure has been in the last two releases says that no one out there is using the Cygwin + Windows native toolchain.
Here are the download stats: https://sourceforge.net/projects/nuttx/files/stats/os?dates=2007-02-17+to+2021-05-29
63% of the downloads are to Windows machines. People are still downloading old releases from Sourceforge! 9 downloads this week! Linux is less than 25% and macOS is a thin sliver in the pie chart. Hackers see the world differently than it really is. It is not all Linux and GCC and ARM and GIT.
I'm surprised by those stats, since as you mention we rarely get issue reports for cygwin. Also, I wonder if those downloads are from actual users since they would be downloading quite an old release by now. I agree that it is important to get a proper idea of which are the leading build environments and focus our maintenance on those.
I'm surprised by those stats, since as you mention we rarely get issue reports for cygwin. Also, I wonder if those downloads are from actual users since they would be downloading quite an old release by now.
I am sure that there are a percentage of people in all platforms that are downloading just out of curiosity without actually being users. But I have no reason to believe that that percentages would be different for Windows vs. Linux downloaders.
I agree that it is important to get a proper idea of which are the leading build environments and focus our maintenance on those.
Maybe we could get some kind of semi-professional poll to determine who NuttX users really are and what is really important to them. My experience is the engineers usually don't really know their customers.
I am sure that there are a percentage of people in all platforms that are downloading just out of curiosity without actually being users. But I have no reason to believe that that percentages would be different for Windows vs. Linux downloaders.
Yes, and also some Windows users may use Linux VMs to build.
Maybe we could get some kind of semi-professional poll to determine who NuttX users really are and what is really important to them. My experience is the engineers usually don't really know their customers.
Yes, that is a good idea. We could send it on the mailing list and via other social networks to get good reach.
A temporary work-around is to use the NuttX buildroot tools built under Cygwin. That builds and runs just fine (although I could not build GDB or the NxFLAT tools from the buildroot package). Most people prefer to use industry standard Windows-based tools, however, no DIY tools.
Quoted from the Bash manual:
Enclosing characters in single quotes (') preserves the literal value of each character within the quotes. A single quote may not occur between single quotes, even when preceded by a backslash.
Enclosing characters in double quotes (") preserves the literal value of all characters within the quotes, with the exception of $,
, \, and, when history expansion is enabled, !. The characters $ and
retain their special meaning within double quotes (see Shell Expansions). The backslash retains its special meaning only when followed by one of the following characters: $, `, ", \, or newline. Within double quotes, backslashes that are followed by one of these characters are removed. Backslashes preceding characters without a special meaning are left unmodified. A double quote may be quoted within double quotes by preceding it with a backslash. If enabled, history expansion will be performed unless an ! appearing in double quotes is escaped using a backslash. The backslash preceding the ! is not removed.
So this error is expected in Cygwin w/ native ARM tools due to "\" chars inside double quotes. A simple fix would be as follows.
diff --git a/tools/incdir.c b/tools/incdir.c
index d2564e6502..1b363de8d1 100644
--- a/tools/incdir.c
+++ b/tools/incdir.c
@@ -438,11 +438,11 @@ int main(int argc, char **argv, char **envp)
if (response == NULL)
{
- ret = my_asprintf(&segment, "%s \"%s\"", cmdarg, incpath);
+ ret = my_asprintf(&segment, "%s '%s'", cmdarg, incpath);
}
else
{
- ret = my_asprintf(&segment, " %s \"%s\"", cmdarg, incpath);
+ ret = my_asprintf(&segment, " %s '%s'", cmdarg, incpath);
}
}
Failure occurred on first compilation of stm32f4discovery:kostest configuration. The problem probably does not depend on configuration. Most likely all Cygwin builds that use a Windows native toolchain are broken. But perhaps this is limited to Cygwin 2 pass builds with a Windows native toolchain only?