-
Notifications
You must be signed in to change notification settings - Fork 622
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
Import regex from gnulib #3054
Import regex from gnulib #3054
Conversation
26ff0b9
to
06b2a5f
Compare
There are 3 types of errors remain. run units target on MSYS2 / testing (MINGW64)
build_and_test : fedora33_cross_aarch64configure finds pthread,
But ld cannot find the pthread functions.
Can I get config.log for this test? othersOthers are configurations which does not use Autoconf. This should be fixed easily. |
06b2a5f
to
03e0fc9
Compare
How about putting |
you should check |
ed61fa9
to
dc87545
Compare
Codecov Report
@@ Coverage Diff @@
## master #3054 +/- ##
==========================================
+ Coverage 87.35% 87.39% +0.03%
==========================================
Files 200 200
Lines 47797 47799 +2
==========================================
+ Hits 41755 41773 +18
+ Misses 6042 6026 -16
Continue to review full report at Codecov.
|
I will limit the input for the roundtrip harness. So we can ignore |
@masatake and @leleliu008, Thank you for your advices.
This works and I could find the place where test for pthread library is done.
The configure script test with FYI here is the comment in configure script.
fedora33_cross_aarch64 now works.
This was more difficult than I thought. To compile gnulib without Autoconf we have to generate config.h and some other header files. It is not practical to debug this via CI/CD environment.
This is the only error left. I missed segmentation faults yesterday. How can we debug this? |
See #3057. |
I understand that we cannot use a debugger interactively.... Failed on the following line;
I guess that one of arguments may be broken. Why gdb was not invoked by |
I'm very surprised at we don' have to modify ctags_vs2013.vcxproj and ctags_vs2013.vcxproj.filters. |
That is because I cheated.
|
Oh, I see. |
dc87545
to
d9c8aab
Compare
mk_mvc.mak
Outdated
# to be replaced by gnulib/regex.{ch} | ||
REGEX_HEADS = gnu_regex/regex.h | ||
REGEX_SRCS = gnu_regex/regex.c | ||
REGEX_OBJS = $(REGEX_SRCS:.c=.$(OBJEXT)) |
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.
Using a variable in replacement doesn't work on MSVC.
COMMON_DEFINES = | ||
DEFINES = -DWIN32 $(REGEX_DEFINES) -DHAVE_PACKCC $(COMMON_DEFINES) -DHAVE_REPOINFO_H -DREADTAGS_DSL | ||
INCLUDES = -I. -Imain -Ignu_regex -Ifnmatch -Iparsers -Ilibreadtags -Idsl | ||
OPT = /O2 /WX | ||
PACKCC = packcc.exe | ||
REGEX_OBJS = $(REGEX_SRCS:.c=.obj) |
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.
So, this line cannot be removed.
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.
Thank you.
It passed in
https://ci.appveyor.com/project/universalctags/ctags/builds/39626407/job/kklumhnaft1n0i7u?fullLog=true.
This is the only environment I can use.
I've fix it.
d9c8aab
to
da22346
Compare
@k-takata san,
For the configurations which does not use Autoconf, in other words which use I think we can use header files you generated on #3069 with
|
gnulib-generated-header.zip These are the generated files. |
Thanks. |
I misunderstood. Header files generated in the gnulib directory are architecture/OS independent. Now we are ready to migrate from old glibc based regex to the lastest gnulib based regex. |
a0bdef0
to
8124cf7
Compare
I've updated the commits of this PR.
As I commented on #3034, this PR just gives us the up-to-date implementation of GNU regex and better compatibility checks. |
I did and have the same error at the end of
The
|
I expected |
Something wrong in your branch management. I wrote:
Did you rebase your branch on the LATEST version of the "master" branch? As far as seeing https://github.com/hirooih/ctags/commits/import-regex-from-gnulib, you rebased your branch on a bit older commit. You may have to do:
I guess you didn't The latest master is a branch having a commit 0a3699d . This commit limits kernel to generate the core file. |
commit e707dbe7c6da9dd8300cb3d60141f144a7a5d5b1 (HEAD -> master, origin/master, origin/HEAD) Date: Wed Jul 14 23:55:30 2021 -0500 $ gnulib-tool --source-base=gnulib --no-vc-files --import regex gnulib-tool: *** minimum supported autoconf version is 2.64. Try adding AC_PREREQ([2.64]) to your configure.ac. gnulib-tool: *** Stop. edit AC_PREREQ version in configure.ac $ gnulib-tool --source-base=gnulib --no-vc-files --import regex
basically followed the output of gnulib-tool; ------------------------------- $ gnulib-tool --source-base=gnulib --no-vc-files --import regex Module list with included dependencies (indented): ... You may need to add #include directives for the following .h files. #include <regex.h> You may need to use the following Makefile variables when linking. Use them in <program>_LDADD when linking a program, or in <library>_a_LDFLAGS or <library>_la_LDFLAGS when linking a library. $(LIBTHREAD) $(LIB_HARD_LOCALE) $(LIB_MBRTOWC) $(LIB_SETLOCALE_NULL) $(LTLIBINTL) when linking with libtool, $(LIBINTL) otherwise Don't forget to - add "gnulib/Makefile" to AC_CONFIG_FILES in ./configure.ac, - mention "gnulib" in SUBDIRS in Makefile.am, - mention "-I m4" in ACLOCAL_AMFLAGS in Makefile.am, - mention "m4/gnulib-cache.m4" in EXTRA_DIST in Makefile.am, - replace AC_PROG_CC_C99 with AC_PROG_CC in ./configure.ac, - invoke gl_EARLY in ./configure.ac, right after AC_PROG_CC_C99, - invoke gl_INIT in ./configure.ac. $ ------------------------------- Makefile.am: - define variables, GNULIB_DIR, GNULIB_CPPFLAGS and GNULIB_LIBS. - add make rule for libgnu.a - for GNULIB_CPPFLAGS see https://www.gnu.org/software/automake/manual/automake.html#Program-Variables
import config.h for msys2-mingw64 as config_mingw.h and config_mvc.h.
- move defines (-Dxxx) into config_mingw.h - rename REGEX_* to GNULIB_*
- move defines (-Dxxx) into config_mvc.h - rename REGEX_* to GNULIB_*
This is not mandatory, but for readability.
1967181
to
cc51f65
Compare
I've pull the latest and pushed again.
I don't expect the fix, because conftest is an execution binary and a core file also removed the following line;
We will see the result. BTW can you reproduce the fail on your local machine? |
It passed!!! |
The core pattern doesn't match the name of the core file actually generated: The pattern should be 'core.*conftest'.
I will make a patch handling %E and send it to autoconf mailing list. |
@masatake san, Now I understand that @k-takata san, could you review
and
|
@hirooih, let's merge this. |
The test is passed on RHEL6. I wonder why. |
- describe the change made in universal-ctags#3034 - a regex engine may be imported from gnulib (see universal-ctags#3054) - describe how regex engine is chosen - describe the GNU extensions and discourages its use.
- describe the change made in universal-ctags#3034 - a regex engine may be imported from gnulib (see universal-ctags#3054) - describe how regex engine is chosen - describe the GNU extensions and discourages its use.
This had to be done in universal-ctags#3054 and universal-ctags#3101.
Currently ctags use a regex library of the system on which ctags is build.
As the result, there is no specification of the regex for
optlib
, and anoptlib
script cannot use full features of a regex library considering compatibility with other system using a regex library whose features are limited.ctags distribution includes regex library from glibc, but it is too old (from glibc-2.10.1 released in 2009).
See also #3034.
This PR imported the latest gnulib regex module.