-
Notifications
You must be signed in to change notification settings - Fork 620
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
build-sys: fix cross-compilation failure caused by building/executing packcc #2747
Conversation
Just "issues 2731 solution" is not a good commit log header. I need a well-descriptive commit log to understand your change. You turned off checking the existence of strnlen. This causes building failures on Windows. |
1、use
|
@@ -28,6 +28,7 @@ uname -mrsv 2>/dev/null | |||
AM_INIT_AUTOMAKE([foreign subdir-objects tar-ustar]) | |||
AM_SILENT_RULES([yes]) | |||
AC_CONFIG_HEADERS([config.h]) | |||
AC_CONFIG_MACRO_DIR([m4]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here we used ax_prog_cc_for_build.m4
AC_ARG_VAR(CC_FOR_BUILD,[build system C compiler]) | ||
AC_ARG_VAR(CFLAGS_FOR_BUILD,[CFLAGS for build system C compiler]) | ||
AC_ARG_VAR(CPPFLAGS_FOR_BUILD,[CPPFLAGS for build system C compiler]) | ||
AC_ARG_VAR(LDFLAGS_FOR_BUILD,[LDFLAGS for build system C compiler]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let user config those variables when cross-compilation.
# https://www.gnu.org/software/autoconf-archive/ax_prog_cc_for_build.html | ||
AX_PROG_CC_FOR_BUILD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inset a ready-to-use macro to support cross-compilation.
@@ -1,3 +1,5 @@ | |||
ACLOCAL_AMFLAGS = -I m4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aclocal -I m4
Note that if you use aclocal from Automake to generate aclocal.m4, you must also set ACLOCAL_AMFLAGS = -I dir in your top-level Makefile.am. Due to a limitation in the Autoconf implementation of autoreconf, these include directives currently must be set on a single line in Makefile.am, without any backslash-newlines.
I think that --- a/Makefile.am
+++ b/Makefile.am
@@ -147,14 +147,14 @@ endif
packcc_verbose = $(packcc_verbose_@AM_V@)
packcc_verbose_ = $(packcc_verbose_@AM_DEFAULT_V@)
packcc_verbose_0 = @echo PACKCC " $@";
-PACKCC = $(top_builddir)/packcc$(EXEEXT)
+PACKCC = $(top_builddir)/packcc$(BUILD_EXEEXT)
SUFFIXES += .peg
.peg.c:
$(packcc_verbose)$(PACKCC) -i \"general.h\" -o $(top_builddir)/peg/$(*F) "$<"
.peg.h:
$(packcc_verbose)$(PACKCC) -i \"general.h\" -o $(top_builddir)/peg/$(*F) "$<"
# You cannot use $(PACKCC) as a target name here.
-$(PEG_SRCS) $(PEG_HEADS): packcc$(EXEEXT) Makefile
+$(PEG_SRCS) $(PEG_HEADS): packcc$(BUILD_EXEEXT) Makefile
dist_libctags_a_SOURCES = $(ALL_LIB_HEADS) $(ALL_LIB_SRCS)
ctags_CPPFLAGS = $(libctags_a_CPPFLAGS) |
Codecov Report
@@ Coverage Diff @@
## master #2747 +/- ##
==========================================
+ Coverage 86.99% 87.04% +0.04%
==========================================
Files 190 191 +1
Lines 40450 40757 +307
==========================================
+ Hits 35190 35477 +287
- Misses 5260 5280 +20
Continue to review full report at Codecov.
|
PACKCC = $(top_builddir)/packcc$(BUILD_EXEEXT) | ||
|
||
$(PACKCC): | ||
$(CC_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(CPPFLAGS_FOR_BUILD) $(EXTRA_CPPFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) -o $@ $(top_srcdir)/misc/packcc/packcc.c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
build packcc don't follow The Uniform Naming Scheme. because automake auto append $(EXEEXT)
to *_PROGRAMS
when generating the Makefile, but here we want to use $(BUILD_EXEEXT)
.
@leleliu008, thank you for putting comments in your change.
|
@masatake OK, I will rewrite my commit messages. |
I tried this change and I confirmed the change works expectedly. Pull a container image for fedora33 Run the container for the image Prepare a repo file for a toolchain (in the container)
Install packages_nameb Get ctags source code Build Verify varlink implies packcc works well during building. |
valgrind reports many errors. I wonder why. I guess the errors are nothing to do with the changes for cross-compilation. |
It couldn't be reproduced locally. |
The commit is somewhat strange. It includes an unnecessary merge commit and some duplicated commits. This should be fixed by:
And the commit messages don't include the |
@k-takata, thank you. @leleliu008. In addition, I would like to do cherry-pick There are two extra TODO items I would like to solve before merging this pull request:
|
… packcc there exists a ready-to-use macro called `AX_PROG_CXX_FOR_BUILD` which is provided by `ax_prog_cc_for_build.m4` and is widely used for supporting cross-compilation. this macro defined `CC_FOR_BUILD`, `CFLAGS_FOR_BUILD`, `CPPFLAGS_FOR_BUILD`, `LDFLAGS_FOR_BUILD`, `BUILD_EXEEXT` variables. user can set `CC_FOR_BUILD`、`CFLAGS_FOR_BUILD`、`CPPFLAGS_FOR_BUILD`、`LDFLAGS_FOR_BUILD` when cross-compiling. when native-compiling, `FOO_FOR_BUILD` is same as `FOO`. here is a example show you how to use these variables: configure \ --host=armv7a-linux-androideabi \ --prefix=`pwd`/out \ --enable-static \ --disable-seccomp \ CC=/usr/local/opt/android-sdk/ndk-bundle/toolchains/llvm/prebuilt/darwin-x86_64/bin/armv7a-linux-androideabi21-clang \ CFLAGS='-v' \ CPP='/usr/local/opt/android-sdk/ndk-bundle/toolchains/llvm/prebuilt/darwin-x86_64/bin/armv7a-linux-androideabi21-clang -E' \ CPPFLAGS='-I/Users/leleliu008/.ndk-pkg/pkg/jansson/armeabi-v7a/include -I/Users/leleliu008/.ndk-pkg/pkg/libyaml/armeabi-v7a/include -I/Users/leleliu008/.ndk-pkg/pkg/libxml2/armeabi-v7a/include -I/Users/leleliu008/.ndk-pkg/pkg/libiconv/armeabi-v7a/include --sysroot /usr/local/opt/android-sdk/ndk-bundle/toolchains/llvm/prebuilt/darwin-x86_64/sysroot -Qunused-arguments -Dftello=ftell -Dfseeko=fseek' \ LDFLAGS='-L/Users/leleliu008/.ndk-pkg/pkg/jansson/armeabi-v7a/lib -L/Users/leleliu008/.ndk-pkg/pkg/libyaml/armeabi-v7a/lib -L/Users/leleliu008/.ndk-pkg/pkg/libxml2/armeabi-v7a/lib -L/Users/leleliu008/.ndk-pkg/pkg/libiconv/armeabi-v7a/lib --sysroot /usr/local/opt/android-sdk/ndk-bundle/toolchains/llvm/prebuilt/darwin-x86_64/sysroot' \ AR=/usr/local/opt/android-sdk/ndk-bundle/toolchains/llvm/prebuilt/darwin-x86_64/bin/arm-linux-androideabi-ar \ RANLIB=/usr/local/opt/android-sdk/ndk-bundle/toolchains/llvm/prebuilt/darwin-x86_64/bin/arm-linux-androideabi-ranlib \ CC_FOR_BUILD=/usr/bin/cc \ CFLAGS_FOR_BUILD='-v' \ PKG_CONFIG_PATH=/Users/leleliu008/.ndk-pkg/pkg/libiconv/armeabi-v7a/lib/pkgconfig:/Users/leleliu008/.ndk-pkg/pkg/libxml2/armeabi-v7a/lib/pkgconfig:/Users/leleliu008/.ndk-pkg/pkg/libyaml/armeabi-v7a/lib/pkgconfig:/Users/leleliu008/.ndk-pkg/pkg/jansson/armeabi-v7a/lib/pkgconfig \ PKG_CONFIG_LIBDIR=/Users/leleliu008/.ndk-pkg/pkg ax_prog_cc_for_build.m4 has been downloaded from `http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=blob_plain;f=m4/ax_prog_cc_for_build.m4` and placed in m4 directory. to use aclocal from Automake to generate aclocal.m4, we must also set `ACLOCAL_AMFLAGS = -I m4` in top-level Makefile.am. build packcc don't follow `The Uniform Naming Scheme`. because automake auto append `$(EXEEXT)` to `*_PROGRAMS` when generating the Makefile, but here we want to use `$(BUILD_EXEEXT)`. as we don't use `The Uniform Naming Scheme`, we must clean `packcc` and `packcc.exe` by myself. References: *https://www.gnu.org/software/autoconf-archive/ax_prog_cc_for_build.html *https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Input.html *https://www.gnu.org/software/automake/manual/html_node/Uniform.html
@masatake I have been squashed all of my commits into one. Do you wish me to run |
I found the bug report is about -O2 option. Temporarily, I use '-O0' for running tests under valgrind. |
--from offical valgrind document |
@leleliu008, thank you! Using -O1 looks good for us. |
I wrote a draft version of the document based on your commit log.
|
@leleliu008, I have questions. I got these questions because your pull request is very related to #2719. |
@masatake I'm not working on |
I see. Thank you. |
@leleliu008, thank you. |
https://www.valgrind.org/docs/manual/quick-start.html Compile your program with -g to include debugging information so that Memcheck's error messages include exact line numbers. Using -O0 is also a good idea, if you can tolerate the slowdown. With -O1 line numbers in error messages can be inaccurate, although generally speaking running Memcheck on code compiled at -O1 works fairly well, and the speed improvement compared to running -O0 is quite significant. Use of -O2 and above is not recommended as Memcheck occasionally reports uninitialised-value errors which don't really exist. Suggested by @leleliu008 at universal-ctags#2747
https://www.valgrind.org/docs/manual/quick-start.html Compile your program with -g to include debugging information so that Memcheck's error messages include exact line numbers. Using -O0 is also a good idea, if you can tolerate the slowdown. With -O1 line numbers in error messages can be inaccurate, although generally speaking running Memcheck on code compiled at -O1 works fairly well, and the speed improvement compared to running -O0 is quite significant. Use of -O2 and above is not recommended as Memcheck occasionally reports uninitialised-value errors which don't really exist. Suggested by @leleliu008 at universal-ctags#2747
https://www.valgrind.org/docs/manual/quick-start.html Compile your program with -g to include debugging information so that Memcheck's error messages include exact line numbers. Using -O0 is also a good idea, if you can tolerate the slowdown. With -O1 line numbers in error messages can be inaccurate, although generally speaking running Memcheck on code compiled at -O1 works fairly well, and the speed improvement compared to running -O0 is quite significant. Use of -O2 and above is not recommended as Memcheck occasionally reports uninitialised-value errors which don't really exist. Suggested by @leleliu008 at universal-ctags#2747
Newer versions fail on aarch64-linux: ``` checking for aarch64-unknown-linux-gnu-gcc... aarch64-unknown-linux-gnu-gcc checking whether the compiler supports GNU C... yes checking whether aarch64-unknown-linux-gnu-gcc accepts -g... yes checking for aarch64-unknown-linux-gnu-gcc option to enable C11 features... (cached) none needed checking whether aarch64-unknown-linux-gnu-gcc understands -c and -o together... yes checking dependency style of aarch64-unknown-linux-gnu-gcc... none checking whether the C compiler works... no configure: error: in `/build/source': configure: error: C compiler cannot create executables See `config.log' for more details ``` The following PR is likely culprit: * universal-ctags/ctags#2747
#2731