Before:
Checking for code snippet : yes
Checking for code snippet : yes
Checking for code snippet : yes
Checking for code snippet : yes
Checking for code snippet : no
Checking for code snippet : no
After:
Checking for HAVE_CMATH_ISFINITE : yes
Checking for HAVE_CMATH_ISINF : yes
Checking for HAVE_CMATH_ISNAN : yes
Checking for NEED_CMATH_ISFINITE_STD_NAMESPACE : yes
Checking for NEED_CMATH_ISINF_STD_NAMESPACE : no
Checking for NEED_CMATH_ISNAN_STD_NAMESPACE : no
The usual name for this header is config.h, but that's already used by
vehicles. Using uppercase could give the impression this is a
file to be modified, but it's not. Use lowercase instead.
Files that are not really part of the ROMFS in the folder might cause problems.
One problem that motivated this patch was caused because the make-based build
system copies the bootloader to the ROMFS in the source tree (mk/PX4/ROMFS)
instead of the build tree. That potentially could cause race condition between
the tasks created by 'px4_romfs_static_files' and 'px4_romfs_bootloader'.
Also, now we have only one task generator for static files.
That way we don't force other programs to be built on a directory of their
program group name. The directory name defaults to the program group.
We are separating those two concepts because of the upcoming support for
multiple groups for a program.
The find_realexec_path function was used for finding the toolchain path mostly
because of two reasons:
1) We couldn't really use CXX or CC variables because the user could set those
from the OS's environment and Waf wouldn't look for the executable file in
that case.
2) Our CI configuration sets up symlinks for ccache and find_realexec_path
works around that issue.
The bad side about using find_realexec_path() is that, besides working aroung
symlinks, it does the same thing that is done by Waf. This patch removes the
dependency for such a function by addressing each of the reasons above stated:
1) We create a local copy of os.environ and, if there's a variable with the
same name we are using, we remove it from the local copy.
2) As done before, we are looking for the cross ar program instead of gcc
program, since that is not used for ccache symlinks.
This is a better approach than checking command line options
--check-cxx-compiler and --check-c-compiler. Those values expect a list of
compilers to try instead of the compiler to use.
The benefits of this approach are:
- Allowing correct use of options --check-cxx-compiler and --check-c-compiler.
- Allowing user to pass CXX and CC environment variables, which is a common
way of selecting the compiler.
- Configuration is done *and committed* only for the specific compiler.
This is a better approach than checking command line options
--check-cxx-compiler and --check-c-compiler. Those values expect a list of
compilers to try instead of the compiler to use.
The benefits of this approach are:
- Allowing correct use of options --check-cxx-compiler and --check-c-compiler.
- Allowing user to pass CXX and CC environment variables, which is a common
way of selecting the compiler.
- Configuration is done *and committed* only for the specific compiler.
It makes more sense the toolchain Waf tool to be responsible of loading the
compilers. Furthermore, that allows toolchain tool to have control on doing
configuration before and after loading compiler tools.
- Use early return and reduce one indentation level.
- Set AR for both GNU compilers and clang just once and reduce redundancy.
- Reduce indentation level for clang-specific setup. There's no need to nest it
inside check if compilers are GNU or clang.
Copy the program library to the place where PX4Firmware CMake build will look
for and trigger the firmware build. There isn't really a real output defined
for px4_copy_lib because Waf would complain about multiple tasks having same
output when building multiple programs.
After the firmware build, copy it to the correct place (from program group and
name perspective) and add git hashes.
Since the place where the library is placed is shared by different target
programs, we need to synchronize the firmware build, that's why the use of
_firmware_semaphorish_tasks. That variable is set as a list because of the
upcoming upload task.
Recursively collect objects from dependency libraries and create a single
library. That way we just need to pass down one single library to PX4Firmware
build system.
In the upcoming build for PX4 boards, we will pass down the program a single
static library to PX4 Firmware's cmake build system. This patch is partially
providing a way to do that: the configuration for PX4 will define the
AP_PROGRAM_AS_STLIB environment variable.
The function ap_program() was the only one that was using it, so let's just
inline it. Besides, the name was misleading, since the (only) feature added was
for programs instead of general task generators.
Two things are fixed with this patch:
1. We sort dictionaries' keys if they aren't OrderedDict instances. Since
dict objects don't guarantee order, environment variables may have contents
in wrong order, causing unnecessary rebuilds.
2. We only use prepend_value() if there's already a value set for the key
and that value is list. Before this change, boards couldn't set
non-iterable values.
That will make platform specific naming be ignored. We use a string instead, to
let Waf tweak the target name correctly for us. The '#' prefix is to tell Waf
that the path is relative to bld.bldnode (instead of bld.path, which is the
default).
We need to remove CMakeCache.txt, otherwise cached variables would remain the
old value when they are removed from cmake_vars parameter.
We use `os.remove()` instead of `Node.delete()` because the latter removes the
node instance from its parent's children list. That makes the node be ignored
when storing persistent information after the build, thus the node signature
wouldn't be saved with that approach, which would make waf always think that
the task should be executed.
The default implementation takes into account the task's output and input
nodes. That isn't very well applicable for cmake tasks, so we define proper
uid() methods.
That allows options being declared where they're really used. Additionally, we
load ardupilotwaf after the other tools so that we can create our groups after
all non-ardupilot option groups are created. That makes our groups appear as
the last ones in the help message, which makes it easier to locate them.
The gtest header uses lots of undefined macros, showing lots of warnings
if we enable -Wundef. Ideally we could use a #pragma to ignore the
warning only from the correct header, but this currently doesn't work
with g++ - see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431
So for now we disable the warning completely when compiling gtest or any
test that uses its header.
Thanks Gustavo Sousa
Copy the missing warnings from AP_Common.h and reorder the warnings to
be more clear on intent. This will later let us remove the warnings from
the header.
By running waf with option -v, waf was complaining about the update_submodule
tasks having the same identifier. That happened because the default uid()
method depended on the class name, inputs and outputs. Since this type of task
doesn't have input nor output file, we need to override uid() so that it uses
the submodule path for the task identifier.
With this change, there's no need to verify if the submodule is initialized,
because the submodules in GIT_SUBMODULES are automatically initialized and
updated if necessary before the build tasks are performed.
One downside of this change is that Google Benchmark configuration is now done
only during build. However that is minor, since now there are easy ways to
separately build different targets and program groups, so that a fail in
benchmark build doesn't really affect the other targets.
That is a workaround so that one can use `waf bin tests`, for example. In our
context, the only restriction to the task generators creation is values defined
during configuration. Thus, ideally, the recursion and task generators creation
should be done only once for multiple build calls. We should do that in the
future.
Rationale:
1. That function creates a separate build context class instead of just
creating a wrapper for calling build (previous approach).
2. The check command isn't a build shortcut since there's no way of calling it
without using 'check' explicitly.
The following fixes where applied:
- Value for APM_BUILD_DIRECTORY must be prefixed with APM_BUILD_
- Renamed parameter name to sketchname, so we differentiate the real program
name from the legacy sketch name
- Use directory name instead of program name as argument for
_get_legacy_defines()
Bind functions used in wscripts to build context. Additionally, a new function
is created and also decorated with @conf, common_vehicle_libraries(), which
returns COMMON_VEHICLE_DEPENDENT_LIBRARIES. This patch is a preparation for
making wscripts use methods bound to the build context instead of using them
directly from ardupilotwaf.
The destination directory for binaries should be
<build_dir>/<board>/bin/ and not
<build_dir>/<board>/<where-wscript-file-is>/bin
The same reasoning can be applied for others: tools, examples, etc
should follow the same rule.
Before this patch, compiling for example ArduPlane for navio we would
have:
[339/339] Linking build/navio/ArduPlane/bin/ArduPlane
And now we have:
[339/339] Linking build/navio/bin/ArduPlane
Use platform-neutral way in python to join path components to improve
readability. Both will work when passing down to waf's Node object, even
on Windows.
waf's terminology might be a bit confusing regarding the word 'target'. As an
attribute for a task generator, it means the paths of the files supposed to be
built. As a command line option (--target), it means the list of names of the
task generators to be used in the build.
Before this commit, only vehicles programs had their task generators' target
parameter value different from the name parameter. Now, there's no distinction
between those two parameters for the case of programs.
That enable the easy creation of custom build commands with the purpose of
creating "shortcuts" for execution from command line.
For example, consider the following code fragment from a wscript:
```
copter = ardupilotwaf.build_shortcut(targets='ArduCopter')
```
With that, one can just issue `waf copter` instead of
`waf --target ArduCopter`.
The parameter target is made optional because more parameters might be added to
this function in the future.
Benchmarks now go to `build/<board>/benchmarks/` and tests to
`build/<board>/tests/`. That's done by using ardupilotwaf.program() and passing
blddestdir and program_name.
The only exception is for binaries that go in 'bin', like vehicles.
That way, partial builds follow the rule:
- If building a vehicle or some helper that goes into 'bin', just use the
binary name as the argument for --target. Example: `waf --target
ArduCopter`.
- For other binaries, the name of the directory they are placed in must be
used. Example: `waf --target tests/test_vectors`.
That param defines where the binary will be saved in the build directory,
namely `build/<board>/<blddestdir>`. The default is 'bin', for binaries that are
to be shipped for installation.
The configure function was reorganized for:
- removing variables that are not used
- making it more general, i.e., not relying in ardupilotmega.xml, so that this
tool can be used for generating other headers in the future
The option target for the waf task generator is meant for actual files that are
going to be build. Since our target files are dynamic, let's change the code to
use a new parameter output_dir instead.
By setting env.env during configure time makes changes be for the whole build
system scope and persistently, which may cause problems for other tasks and
tools.
Since the output list for mavgen is dynamic, in the sense that we don't have
the information of what files will be generated by the task, that is not
captured by default by waf and post_run can't save the task signature for those
files. Although that doesn't cause build errors, the build time increase
significantly for when tasks that use the generate files are included (for
example, vehicles builds).
This patch search for the headers that were created by the task and set the
task signature for them. Using ant_glob isn't a very good solution, since there
may be stray file in the local build directory, but let's use that for now
until we find a better approach.
There is an issue with gbenchmark and waf on Ubuntu (15.04 and 15.10, ). Waf doesn't link pthread to gbenchmark and linking failed :
````
[1652/1652] Linking build/linux/libraries/AP_Math/benchmarks/benchmark_matrix.linux
/home/khancyr/Workspace/APM/ardupilot/build/linux/gbenchmark/lib/libbenchmark.a(benchmark.cc.o): dans la fonction « benchmark::RunSpecifiedBenchmarks(benchmark::BenchmarkReporter*) »:
benchmark.cc:(.text+0x2e21): référence indéfinie vers « pthread_create »
/usr/bin/ld generated: référence indéfinie vers « pthread_create »
collect2: error: ld returned 1 exit status
Waf: Leaving directory `/home/khancyr/Workspace/APM/ardupilot/build/linux'
Build failed
-> task in 'benchmark_matrix.linux' failed (exit status 1):
{task 139784788162576: cxxprogram benchmark_matrix.cpp.1.o -> benchmark_matrix.linux}
['/usr/lib/ccache/g++', '-Wl,--gc-sections', 'libraries/AP_Math/benchmarks/benchmark_matrix.cpp.1.o', '-o', '/home/khancyr/Workspace/APM/ardupilot/build/linux/libraries/AP_Math/benchmarks/benchmark_matrix.linux', '-Wl,-Bstatic', '-L.', '-lap', '-Wl,-Bdynamic', '-L/home/khancyr/Workspace/APM/ardupilot/build/linux/gbenchmark/lib', '-lm', '-lpthread', '-lrt', '-lbenchmark']
`````
Adding 'pthread' to env.LIB_GBENCHMARK solve the issue
see https://github.com/diydrones/ardupilot/pull/3460 and https://github.com/diydrones/ardupilot/issues/3461
Some platforms (e.g. bebop) might need to create fully statically linked
binaries. This serves to force a program to be statically linked. It has only
been tested on GNU compilers, other compilers may have unexpected behavior.
The cmake checks for gbenchmark need to run some code. Calling
_configure_cmake() only during build can potentially fail build. That would
happen in some cross-compilations for example.
The TODOs removed with this patch were already fixed. Below is the explanation
for each.
- TODO: add support for unit tests.
- Supported already added.
- TODO: Check if we should simply use the signed 'waf' "binary" (after
verifying it) instead of generating it ourselves from the sources.
- We're using a submodule for waf.
- TODO: evaluate if we need shortcut commands for the common targets
(vehicles). currently using waf --targets=NAME the target name must contain
the board extension so make it less convenient, maybe hook to support
automatic filling this extension?
- There's no need of adding the extension anymore.
- TODO: Once HAL patches get in, need to filter out the HAL based on the
bld.env.BOARD.
- The board-specific HAL library folders is indicated in
bld.env.AP_LIBRARIES.
If there's no variant configuration, then cfg.variant will be '', which will
make bldnode be cfg.bldnode. Thus, this patch prepare gbenchmark build for
variant builds and doesn't break the current build.
We can pass a list of possible binaries to find_program. This gives us a
better output while configuring:
Checking for program 'ninja, ninja-build' : /usr/bin/ninja-build
instead of:
Checking for program 'ninja' : not found
Checking for program 'ninja-build' : /usr/bin/ninja-build
If the attribute name is passed, then it is the one used to process the option
--target[s] of waf build. The board name should be used only in configuration
time. The build targets should be board/platform agnostic.
Now, instead of using `waf --target ArduCopter.sitl`, we use `waf --target
ArduCopter` and the binary continues named as "ArduCopter.sitl".
Instead of a dictionary of dictionaries, have a dictionary of
ConfigSets. Using ConfigSet have two benefits: (1) allow easily copying
values from other, (2) have syntax for specifying the keys directly as
attributes.
With this change now it's easier to specify minlure without
repetition. New boards can override a value, append or prepend depending
on the need.
DEFINES attribute is treated as a dictionary instead of a list, so
that's easier to override values (at expense of ordering). When reading
the board environment, the code converts back to a list.
The board configuration is now stored in a separate file, there's also a
function to get the boards names.
It was implemented in such a way that gtest is required only if the user wants
to build and run tests. Initially we're considering all tests should be gtests.
We can change that assumption in the future if necessary.
We're currently using the tests standard error for reporting tests. We can add
TAP later to integrate with other tools.
Additionally, this patch simplifies the exclude patterns passed to
collect_dirs_to_recurse.
Francisco Ferreira
Andrew Tridgell
Lucas De Marchi
Ricardo de Almeida Gonzaga
Gustavo Jose de Sousa
Staroselskii Georgii
Daniel Frenzel
pkancir
Siddharth Bharat Purohit
khancyr
Caio Marcelo de Oliveira Filho