# [PATCH] compat/regex: fix compilation on Windows 10 messages from 2017-05-11 to 2017-05-15. Participants: Johannes Schindelin, Ævar Arnfjörð Bjarmason, Junio C Hamano, Ramsay Jones. Thread: https://gitlist.dev/t/45932 ## Johannes Schindelin, 2017-05-11 13:50 Subject: [PATCH] compat/regex: fix compilation on Windows Message-ID: <6a6782863a74432094a1d7fba7f2477991ef2d16.1494510612.git.johannes.schindelin@gmx.de> URL: https://gitlist.dev/e/6a6782863a74432094a1d7fba7f2477991ef2d16.1494510612.git.johannes.schindelin%40gmx.de ``` The real issue here is that GNU awk's regex implementation assumes a bit too much about the relative sizes of pointers and long integers. What they really want is to use intptr_t. This patch recapitulates what 56a1a3ab449 (Silence GCC's "cast of pointer to integer of a different size" warning, 2015-10-26) did to our previous copy of GNU awk's regex engine. Signed-off-by: Johannes Schindelin --- Published-As: https://github.com/dscho/git/releases/tag/compat-regex-fixes-v1 Fetch-It-Via: git fetch https://github.com/dscho/git compat-regex-fixes-v1 .../0003-Use-intptr_t-instead-of-long.patch | 22 ++++++++++++++++++++++ compat/regex/regcomp.c | 4 ++-- 2 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 compat/regex/patches/0003-Use-intptr_t-instead-of-long.patch diff --git a/compat/regex/patches/0003-Use-intptr_t-instead-of-long.patch b/compat/regex/patches/0003-Use-intptr_t-instead-of-long.patch new file mode 100644 index 00000000000..246ff256fb8 --- /dev/null +++ b/compat/regex/patches/0003-Use-intptr_t-instead-of-long.patch @@ -0,0 +1,22 @@ +diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c +index 5e9ea26cd46..e6469167a80 100644 +--- a/compat/regex/regcomp.c ++++ b/compat/regex/regcomp.c +@@ -2641,7 +2641,7 @@ parse_dup_op (bin_tree_t *elem, re_string_t *regexp, re_dfa_t *dfa, + old_tree = NULL; + + if (elem->token.type == SUBEXP) +- postorder (elem, mark_opt_subexp, (void *) (long) elem->token.opr.idx); ++ postorder (elem, mark_opt_subexp, (void *) (intptr_t) elem->token.opr.idx); + + tree = create_tree (dfa, elem, NULL, (end == -1 ? OP_DUP_ASTERISK : OP_ALT)); + if (BE (tree == NULL, 0)) +@@ -3868,7 +3868,7 @@ create_token_tree (re_dfa_t *dfa, bin_tree_t *left, bin_tree_t *right, + static reg_errcode_t + mark_opt_subexp (void *extra, bin_tree_t *node) + { +- int idx = (int) (long) extra; ++ int idx = (int) (intptr_t) extra; + if (node->token.type == SUBEXP && node->token.opr.idx == idx) + node->token.opt_subexp = 1; + diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c index 5e9ea26cd46..e6469167a80 100644 --- a/compat/regex/regcomp.c +++ b/compat/regex/regcomp.c @@ -2641,7 +2641,7 @@ parse_dup_op (bin_tree_t *elem, re_string_t *regexp, re_dfa_t *dfa, old_tree = NULL; if (elem->token.type == SUBEXP) - postorder (elem, mark_opt_subexp, (void *) (long) elem->token.opr.idx); + postorder (elem, mark_opt_subexp, (void *) (intptr_t) elem->token.opr.idx); tree = create_tree (dfa, elem, NULL, (end == -1 ? OP_DUP_ASTERISK : OP_ALT)); if (BE (tree == NULL, 0)) @@ -3868,7 +3868,7 @@ create_token_tree (re_dfa_t *dfa, bin_tree_t *left, bin_tree_t *right, static reg_errcode_t mark_opt_subexp (void *extra, bin_tree_t *node) { - int idx = (int) (long) extra; + int idx = (int) (intptr_t) extra; if (node->token.type == SUBEXP && node->token.opr.idx == idx) node->token.opt_subexp = 1; base-commit: 4e23cefb4da69a2d884c2d5a303825f40008ca42 -- 2.12.2.windows.2.800.gede8f145e06 ``` ## Ævar Arnfjörð Bjarmason, 2017-05-11 14:30 Subject: Re: [PATCH] compat/regex: fix compilation on Windows Message-ID: URL: https://gitlist.dev/e/CACBZZX7i4Sz2w1obCVEKKoq%2Ba%2BSmzkJ6AYDvaVQgkGRGDU%2Brng%40mail.gmail.com In-Reply-To: <6a6782863a74432094a1d7fba7f2477991ef2d16.1494510612.git.johannes.schindelin@gmx.de> ``` On Thu, May 11, 2017 at 3:50 PM, Johannes Schindelin wrote: > The real issue here is that GNU awk's regex implementation assumes a bit > too much about the relative sizes of pointers and long integers. What they > really want is to use intptr_t. Thanks, looks good! > This patch recapitulates what 56a1a3ab449 (Silence GCC's "cast of pointer > to integer of a different size" warning, 2015-10-26) did to our previous > copy of GNU awk's regex engine. > > Signed-off-by: Johannes Schindelin > --- > Published-As: https://github.com/dscho/git/releases/tag/compat-regex-fixes-v1 > Fetch-It-Via: git fetch https://github.com/dscho/git compat-regex-fixes-v1 > > .../0003-Use-intptr_t-instead-of-long.patch | 22 ++++++++++++++++++++++ > compat/regex/regcomp.c | 4 ++-- > 2 files changed, 24 insertions(+), 2 deletions(-) > create mode 100644 compat/regex/patches/0003-Use-intptr_t-instead-of-long.patch > > diff --git a/compat/regex/patches/0003-Use-intptr_t-instead-of-long.patch b/compat/regex/patches/0003-Use-intptr_t-instead-of-long.patch > new file mode 100644 > index 00000000000..246ff256fb8 > --- /dev/null > +++ b/compat/regex/patches/0003-Use-intptr_t-instead-of-long.patch > @@ -0,0 +1,22 @@ > +diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c > +index 5e9ea26cd46..e6469167a80 100644 > +--- a/compat/regex/regcomp.c > ++++ b/compat/regex/regcomp.c > +@@ -2641,7 +2641,7 @@ parse_dup_op (bin_tree_t *elem, re_string_t *regexp, re_dfa_t *dfa, > + old_tree = NULL; > + > + if (elem->token.type == SUBEXP) > +- postorder (elem, mark_opt_subexp, (void *) (long) elem->token.opr.idx); > ++ postorder (elem, mark_opt_subexp, (void *) (intptr_t) elem->token.opr.idx); > + > + tree = create_tree (dfa, elem, NULL, (end == -1 ? OP_DUP_ASTERISK : OP_ALT)); > + if (BE (tree == NULL, 0)) > +@@ -3868,7 +3868,7 @@ create_token_tree (re_dfa_t *dfa, bin_tree_t *left, bin_tree_t *right, > + static reg_errcode_t > + mark_opt_subexp (void *extra, bin_tree_t *node) > + { > +- int idx = (int) (long) extra; > ++ int idx = (int) (intptr_t) extra; > + if (node->token.type == SUBEXP && node->token.opr.idx == idx) > + node->token.opt_subexp = 1; > + > diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c > index 5e9ea26cd46..e6469167a80 100644 > --- a/compat/regex/regcomp.c > +++ b/compat/regex/regcomp.c > @@ -2641,7 +2641,7 @@ parse_dup_op (bin_tree_t *elem, re_string_t *regexp, re_dfa_t *dfa, > old_tree = NULL; > > if (elem->token.type == SUBEXP) > - postorder (elem, mark_opt_subexp, (void *) (long) elem->token.opr.idx); > + postorder (elem, mark_opt_subexp, (void *) (intptr_t) elem->token.opr.idx); > > tree = create_tree (dfa, elem, NULL, (end == -1 ? OP_DUP_ASTERISK : OP_ALT)); > if (BE (tree == NULL, 0)) > @@ -3868,7 +3868,7 @@ create_token_tree (re_dfa_t *dfa, bin_tree_t *left, bin_tree_t *right, > static reg_errcode_t > mark_opt_subexp (void *extra, bin_tree_t *node) > { > - int idx = (int) (long) extra; > + int idx = (int) (intptr_t) extra; > if (node->token.type == SUBEXP && node->token.opr.idx == idx) > node->token.opt_subexp = 1; > > > base-commit: 4e23cefb4da69a2d884c2d5a303825f40008ca42 > -- > 2.12.2.windows.2.800.gede8f145e06 ``` ## Junio C Hamano, 2017-05-12 01:18 Subject: Re: [PATCH] compat/regex: fix compilation on Windows Message-ID: URL: https://gitlist.dev/e/xmqqd1be98kk.fsf%40gitster.mtv.corp.google.com In-Reply-To: <6a6782863a74432094a1d7fba7f2477991ef2d16.1494510612.git.johannes.schindelin@gmx.de> ``` Johannes Schindelin writes: > The real issue here is that GNU awk's regex implementation assumes a bit > too much about the relative sizes of pointers and long integers. What they > really want is to use intptr_t. Good. I got annoyed enough to do the same myself before opening my mailbox. One thing that is curious about your version is that it still has "#include " behind HAVE_STDINT_H; when I tried to compile an equivalent of your change last night, the compilation failed because intptr_t wasn't available without exposing Where is Windows build getting its intptr_t, I wonder. ``` ## Johannes Schindelin, 2017-05-12 10:25 Subject: Re: [PATCH] compat/regex: fix compilation on Windows Message-ID: URL: https://gitlist.dev/e/alpine.DEB.2.21.1.1705121219150.146734%40virtualbox In-Reply-To: ``` Hi Junio, On Fri, 12 May 2017, Junio C Hamano wrote: > Johannes Schindelin writes: > > > The real issue here is that GNU awk's regex implementation assumes a > > bit too much about the relative sizes of pointers and long integers. > > What they really want is to use intptr_t. > > Good. I got annoyed enough to do the same myself before opening my > mailbox. One thing that is curious about your version is that it still > has "#include " behind HAVE_STDINT_H; when I tried to compile > an equivalent of your change last night, the compilation failed because > intptr_t wasn't available without exposing > > Where is Windows build getting its intptr_t, I wonder. I'd place a bet on this part of compat/mingw.h for GCC builds: #ifdef __MINGW64_VERSION_MAJOR #include #include typedef _sigset_t sigset_t; #endif and on this part of git-compat-util.h for MSVC builds: #ifndef NO_INTTYPES_H #include #else #include #endif For the record, it seems that our current version of compat/regex/regex.c has this: /* On some systems, limits.h sets RE_DUP_MAX to a lower value than GNU regex allows. Include it before , which correctly #undefs RE_DUP_MAX and sets it to the right value. */ #include #include while the one in `pu` lacks the last line. That may be the reason why things compiled neatly before, and stopped working for you now. So yeah, I think you are right in that the HAVE_STDINT_H guard needs to be removed either before my patch, or squashed into it. Ciao, Dscho ``` ## Ramsay Jones, 2017-05-12 21:02 Subject: Re: [PATCH] compat/regex: fix compilation on Windows Message-ID: <413493c2-24ee-e085-f4cc-371b677d2d02@ramsayjones.plus.com> URL: https://gitlist.dev/e/413493c2-24ee-e085-f4cc-371b677d2d02%40ramsayjones.plus.com In-Reply-To: ``` On 12/05/17 11:25, Johannes Schindelin wrote: > Hi Junio, > > On Fri, 12 May 2017, Junio C Hamano wrote: > >> Johannes Schindelin writes: >> >>> The real issue here is that GNU awk's regex implementation assumes a >>> bit too much about the relative sizes of pointers and long integers. >>> What they really want is to use intptr_t. >> >> Good. I got annoyed enough to do the same myself before opening my >> mailbox. One thing that is curious about your version is that it still >> has "#include " behind HAVE_STDINT_H; when I tried to compile >> an equivalent of your change last night, the compilation failed because >> intptr_t wasn't available without exposing >> >> Where is Windows build getting its intptr_t, I wonder. > > I'd place a bet on this part of compat/mingw.h for GCC builds: > > #ifdef __MINGW64_VERSION_MAJOR > #include > #include > typedef _sigset_t sigset_t; > #endif > > and on this part of git-compat-util.h for MSVC builds: > > #ifndef NO_INTTYPES_H > #include > #else > #include > #endif > > For the record, it seems that our current version of compat/regex/regex.c > has this: > > /* On some systems, limits.h sets RE_DUP_MAX to a lower value than > GNU regex allows. Include it before , which correctly > #undefs RE_DUP_MAX and sets it to the right value. */ > #include > #include > > while the one in `pu` lacks the last line. That may be the reason why > things compiled neatly before, and stopped working for you now. Yep, see commit bd8f005583. ;-P So, sparse is, once again, complaining about the SIZE_MAX macro redefinition. (Along with two other warnings, one of which is a _very_ long standing warning and one of which is new - I have yet to investigate). ATB, Ramsay Jones ``` ## Junio C Hamano, 2017-05-12 21:26 Subject: Re: [PATCH] compat/regex: fix compilation on Windows Message-ID: URL: https://gitlist.dev/e/xmqqy3u14vim.fsf%40gitster.mtv.corp.google.com In-Reply-To: ``` Johannes Schindelin writes: > I'd place a bet on this part of compat/mingw.h for GCC builds: > > #ifdef __MINGW64_VERSION_MAJOR > #include > #include > typedef _sigset_t sigset_t; > #endif > > and on this part of git-compat-util.h for MSVC builds: > > #ifndef NO_INTTYPES_H > #include > #else > #include > #endif Yeah, but I had an impression that git-compat-util.h was not in use to build compat/regex/regex.o (regex.c #include's regcomp.c which is a rather unusual arragement), and that was why I am wondering. > For the record, it seems that our current version of compat/regex/regex.c > has this: > > /* On some systems, limits.h sets RE_DUP_MAX to a lower value than > GNU regex allows. Include it before , which correctly > #undefs RE_DUP_MAX and sets it to the right value. */ > #include > #include > > while the one in `pu` lacks the last line. That may be the reason why > things compiled neatly before, and stopped working for you now. Yes, that explains how the old one worked and the new one didn't without dropping "#ifdef HAVE_STDINT_H". Thanks. ``` ## Ævar Arnfjörð Bjarmason, 2017-05-12 22:27 Subject: Re: [PATCH] compat/regex: fix compilation on Windows Message-ID: URL: https://gitlist.dev/e/CACBZZX4UUwzRQmyH8joYaqHnuVTjVtGBHp%2BiZKcnAnwoM_ZJhg%40mail.gmail.com In-Reply-To: <6a6782863a74432094a1d7fba7f2477991ef2d16.1494510612.git.johannes.schindelin@gmx.de> ``` On Thu, May 11, 2017 at 3:50 PM, Johannes Schindelin wrote: > The real issue here is that GNU awk's regex implementation assumes a bit > too much about the relative sizes of pointers and long integers. What they > really want is to use intptr_t. > > This patch recapitulates what 56a1a3ab449 (Silence GCC's "cast of pointer > to integer of a different size" warning, 2015-10-26) did to our previous > copy of GNU awk's regex engine. > > Signed-off-by: Johannes Schindelin > --- > Published-As: https://github.com/dscho/git/releases/tag/compat-regex-fixes-v1 > Fetch-It-Via: git fetch https://github.com/dscho/git compat-regex-fixes-v1 > > .../0003-Use-intptr_t-instead-of-long.patch | 22 ++++++++++++++++++++++ > compat/regex/regcomp.c | 4 ++-- > 2 files changed, 24 insertions(+), 2 deletions(-) > create mode 100644 compat/regex/patches/0003-Use-intptr_t-instead-of-long.patch > > diff --git a/compat/regex/patches/0003-Use-intptr_t-instead-of-long.patch b/compat/regex/patches/0003-Use-intptr_t-instead-of-long.patch > new file mode 100644 > index 00000000000..246ff256fb8 > --- /dev/null > +++ b/compat/regex/patches/0003-Use-intptr_t-instead-of-long.patch > @@ -0,0 +1,22 @@ > +diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c > +index 5e9ea26cd46..e6469167a80 100644 > +--- a/compat/regex/regcomp.c > ++++ b/compat/regex/regcomp.c > +@@ -2641,7 +2641,7 @@ parse_dup_op (bin_tree_t *elem, re_string_t *regexp, re_dfa_t *dfa, > + old_tree = NULL; > + > + if (elem->token.type == SUBEXP) > +- postorder (elem, mark_opt_subexp, (void *) (long) elem->token.opr.idx); > ++ postorder (elem, mark_opt_subexp, (void *) (intptr_t) elem->token.opr.idx); > + > + tree = create_tree (dfa, elem, NULL, (end == -1 ? OP_DUP_ASTERISK : OP_ALT)); > + if (BE (tree == NULL, 0)) > +@@ -3868,7 +3868,7 @@ create_token_tree (re_dfa_t *dfa, bin_tree_t *left, bin_tree_t *right, > + static reg_errcode_t > + mark_opt_subexp (void *extra, bin_tree_t *node) > + { > +- int idx = (int) (long) extra; > ++ int idx = (int) (intptr_t) extra; > + if (node->token.type == SUBEXP && node->token.opr.idx == idx) > + node->token.opt_subexp = 1; > + > diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c > index 5e9ea26cd46..e6469167a80 100644 > --- a/compat/regex/regcomp.c > +++ b/compat/regex/regcomp.c > @@ -2641,7 +2641,7 @@ parse_dup_op (bin_tree_t *elem, re_string_t *regexp, re_dfa_t *dfa, > old_tree = NULL; > > if (elem->token.type == SUBEXP) > - postorder (elem, mark_opt_subexp, (void *) (long) elem->token.opr.idx); > + postorder (elem, mark_opt_subexp, (void *) (intptr_t) elem->token.opr.idx); > > tree = create_tree (dfa, elem, NULL, (end == -1 ? OP_DUP_ASTERISK : OP_ALT)); > if (BE (tree == NULL, 0)) > @@ -3868,7 +3868,7 @@ create_token_tree (re_dfa_t *dfa, bin_tree_t *left, bin_tree_t *right, > static reg_errcode_t > mark_opt_subexp (void *extra, bin_tree_t *node) > { > - int idx = (int) (long) extra; > + int idx = (int) (intptr_t) extra; > if (node->token.type == SUBEXP && node->token.opr.idx == idx) > node->token.opt_subexp = 1; > > > base-commit: 4e23cefb4da69a2d884c2d5a303825f40008ca42 > -- > 2.12.2.windows.2.800.gede8f145e06 Let's drop this current gawk import series. After talking to the gawk author it turns out it's better to use the version from gnulib, this includes the equivalent of your patch. The following one-liner works for me on linux to import that library, on the master branch: $ git reset --hard; rm compat/regex/*.[ch]; rm -rfv /tmp/git.rx; test -e /tmp/git-gnulib || git clone https://git.savannah.gnu.org/git/gnulib.git /tmp/git-gnulib; mkdir /tmp/git.rx; touch /tmp/git.rx/configure.ac; /tmp/git-gnulib/gnulib-tool --lgpl --add-import --dir=/tmp/git.rx regex; cp /tmp/git.rx/lib/{intprops.h,reg*} compat/regex/; perl -0666 -pi.bak -e 's[compat/regex/regex.o: EXTRA_CPPFLAGS = \K[^\n]+\n[^\n]+][]s' Makefile; echo '#define _GNU_SOURCE' >compat/regex/config.h I.e. remove the existing engine, import new one from gnulib, then wipe the extra -D flags that exist now, and define _GNU_SOURCE. ``` ## Johannes Schindelin, 2017-05-13 18:30 Subject: Re: [PATCH] compat/regex: fix compilation on Windows Message-ID: URL: https://gitlist.dev/e/alpine.DEB.2.21.1.1705131251260.146734%40virtualbox In-Reply-To: ``` Hi Ævar, I originally replied in a very verbose manner, going step by step through the "one-liner", but decided to rephrase everything. So here goes. On Sat, 13 May 2017, Ævar Arnfjörð Bjarmason wrote: > Let's drop this current gawk import series. Well, the reason why you imported the current gawk regex is that we (you?) decided originally that it'd be easier to go with that one rather than with the gnulib one (which they friendly-forked). If you switch to gnulib, I would like to see (in the commit message) the original reason we picked gawk (which forked gnulib's regex code, after all), and why that reason no longer applies. I also would strongly prefer a *non* one-liner, in a commit message that adds the relevant source code from gawk or gnulib *verbatim*, followed by patches that adjust the code to Git's needs. That way, a future update can repeat the commands outlined in the first commit message and then cherry-pick the subsequent patches. And remember that you do not need to clone the entire repository. You can 1) fetch into the current one, and 2) use a shallow fetch. Example: git fetch --depth 1 http://git.savannah.gnu.org/r/gawk.git \ gawk-4.1.4 The next commands could be something like git read-tree --prefix=compat/regex/ FETCH_HEAD: git clean -dfx -- compat/regex/ Oh, and please do not use `master`. If Git is any indication, a tagged release will most often be a bit more robust than any in-between commit. Thanks, Dscho ``` ## Ævar Arnfjörð Bjarmason, 2017-05-13 19:31 Subject: Re: [PATCH] compat/regex: fix compilation on Windows Message-ID: URL: https://gitlist.dev/e/CACBZZX6Qx%2Boy83GjTOahUyHM4%3DgESzX%2BQNMLrY2DHgdYMo3cNw%40mail.gmail.com In-Reply-To: ``` On Sat, May 13, 2017 at 8:30 PM, Johannes Schindelin wrote: > Hi Ævar, > > I originally replied in a very verbose manner, going step by step through > the "one-liner", but decided to rephrase everything. > > So here goes. > > On Sat, 13 May 2017, Ævar Arnfjörð Bjarmason wrote: > >> Let's drop this current gawk import series. > > Well, the reason why you imported the current gawk regex is that we (you?) > decided originally that it'd be easier to go with that one rather than > with the gnulib one (which they friendly-forked). If you switch to gnulib, > I would like to see (in the commit message) the original reason we picked > gawk (which forked gnulib's regex code, after all), and why that reason no > longer applies. It was just easier to extract the code from gawk than gnulib at the time. The thread starting at <20100715220059.GA3312@burratino> has some details. > I also would strongly prefer a *non* one-liner, in a commit message that > adds the relevant source code from gawk or gnulib *verbatim*, followed by > patches that adjust the code to Git's needs. That way, a future update can > repeat the commands outlined in the first commit message and then > cherry-pick the subsequent patches. > > And remember that you do not need to clone the entire repository. You can > 1) fetch into the current one, and 2) use a shallow fetch. Example: > > git fetch --depth 1 http://git.savannah.gnu.org/r/gawk.git \ > gawk-4.1.4 > > The next commands could be something like > > git read-tree --prefix=compat/regex/ FETCH_HEAD: > git clean -dfx -- compat/regex/ Sure, if I submit this again I'll update whatever one liner is in there to import it via git. > Oh, and please do not use `master`. If Git is any indication, a tagged > release will most often be a bit more robust than any in-between commit. The reason to use the master branch of gawk is that since the last release only one trivial change has been made to it, but the files were all renamed as well, so it was easier for that one-liner in the README to use master than the latest release. If we move to gnulib we'll also use the master branch, because that's what you're supposed to do according to their docs, they don't make releases. ``` ## Junio C Hamano, 2017-05-15 01:39 Subject: Re: [PATCH] compat/regex: fix compilation on Windows Message-ID: URL: https://gitlist.dev/e/xmqqo9uuyk3u.fsf%40gitster.mtv.corp.google.com In-Reply-To: ``` Ævar Arnfjörð Bjarmason writes: > Let's drop this current gawk import series. > > After talking to the gawk author it turns out it's better to use the > version from gnulib, this includes the equivalent of your patch. OK. That may make things simpler ;-) Thanks. ```