Fix cmake build errors related to ac_cfg.h #1707
Merged
Conversation
This file contains hidden or 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
Directly use the AVRDUDE_FULL_VERSION macro instead of first defining an initialized variable which is never changed and then using that variable. This means one less layer of indirection both for the computer and for the programmer to go through when trying to understand the code.
When there is no `src/ac_cfg.h` (e.g. a fresh `git clone` or after a working dir cleanup like `git -f -d -x`), running `cmake` will complain about not being able to find `ac_cfg.h`. This is probably because cmake looks for source files in the `CMAKE_CURRENT_SOURCE_DIR`, but when cmake creates `ac_cfg.h` it does so ´CMAKE_CURRENT_BINARY_DIR`, which is often different. Anyway, this makes the location of files generated by `configure_file` explicit as `CMAKE_CURRENT_BINARY_DIR`, and uses that same location in the source file list for the `libavrdude` target (instead of just `ac_cfg.h` with the implicit `CMAKE_CURRENT_SOURCE_DIR`) and adds the same source file to the `avrdude` target to allow cmake to determine the dependencies properly.
As it can happen that there is a leftover `src/ac_cfg.h` when running an out of tree cmake build (you might have run `cmake .` or `./src/bootstrap`), the out of tree cmake build must look for `ac_cfg.h` in its builddir first (e.g. `build_linux/src/`). Otherwise the cmake build picks up and uses whatever data happens to be in `src/ac_cfg.h`. Both the MSVC and GCC C processors are documented to look in the location of the `#include` directive for the included file first for double quoted includes, so the old `#include "ac_cfg.h"` was exactly the wrong thing to do. clang probably does the same, and ISO C specifies the sequence of places to look for include files as implementation defined. So this changes all occurrences of `#include "ac_cfg.h"` to `#include <ac_cfg.h>` which follows the sequence of `-I` or `/I` directives as cmake builds add via `include_directories` or `target_include_directories`. Fixes: avrdudes#1706
Contributor
Author
|
Note that this PR does not add checks to Letting the C preprocessor look in the out of tree build directory first means such a check is not needed any more. |
Contributor
Author
|
The issue of the rogue |
Collaborator
|
Looks like a good change to me. Tested under Windows with different build (autotools build under MSYS2 mingw64, CMAKE mingw64 build and CMAKE VS2019 build, no build issues found, the version info of the binaries generated is also correct. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This fixes a few errors related to how cmake builds deal with
ac_cfg.h. This has been discussed in #1681 (comment) and following comments, and then in issue #1706.While testing, I ran
cmake .a few times which generates a cmake buildsystem inside the source tree. Some of those files still had to be added to.gitignore.Using the identifier
versioninsidesrc/main.cto refer toAVRDUDE_FULL_VERSIONis about as confusing as usingVERSIONinstead ofAVRDUDE_FULL_VERSIONwas. So I removedchar *versionand just let the functions referenceAVRDUDE_FULL_VERSIONdirectly. I trust compilers these days to have all those references refer to the same bit of constant memory, and not allocate several copies ofAVRDUDE_FULL_VERSION.Fix cmake not finding the generated
ac_cfg.hwhen the (often unused and unnecessary)src/ac_cfg.hdoes NOT exist. See commit message.And most importantly, fix the C preprocessor mistakenly looking in
src/beforebuild_$ostype/src/when looking forac_cfg.h: Change from#include "ac_cfg.h"to#include <ac_cfg.h>in all source files.