Support GCC 4.8, using Boost.Regex #43

Luthaf · 2016-03-16T20:18:20Z

I got one failure on my machine:

$ ./run_tests
========================================
UsAgE: prog [options]

OpTiOnS: --path=<files>  Path to files
                [dEfAuLt: /root]


::::::::::::::::::::
prog
--------------------
{ "--path": null }

 ** JSON does not match expected: {u'--path': u'/root'}
1 failures

Also, I checked that the code still compiles normally using clang.
I can try to add GCC 4.8 to travis, but this will make it fail.

Fixes #42.

Luthaf · 2016-03-16T20:24:21Z

docopt.cpp

 #include <iostream>
 #include <cassert>
 #include <cstddef>

+// Workaround GCC 4.8 not having std::regex
+#if (__GNUC__ == 4) && (__GNUC_MINOR__ == 8)


I am not sure what is the best thing to do here. We can either make the use of Boost.Regex opt-in (with some flag), or default like this.

We also can add some preprocessor warning to the user:

#if (__GNUC__ == 4) && (__GNUC_MINOR__ == 8) #ifdef DOCTOPT_USE_BOOST_REGEX #include <boost/regex.hpp> [...] #else #error "GCC 4.8 can not build docopt alone. You can opt-in using Boost.Regex by defining DOCTOPT_USE_BOOST_REGEX" #endif #else #include <regex> #endif

Perhaps we just have "#if DOCTOPT_USE_BOOST_REGEX" without the version check? If people really want to use GCC 4.8 then it would be on them to fix up the flag rather than us trying to guard it.

Luthaf · 2016-05-16T14:38:37Z

Updated to use DOCTOPT_USE_BOOST_REGEX, and added the corresponding option to CMakeLists.txt

jaredgrubb · 2016-05-22T05:18:39Z

CMakeLists.txt

+		target_link_libraries(docopt_s ${Boost_LIBRARIES})
+	endif()
+endif()
+


Do you think this really should be implicit? Or should we just assume that people on 4.8 will pass in the "use boost" option?

Yeah, I mean, if that's the only way this'll work, then try as hard as possible to make it work.

I forgot to update this part. The condition in the if should be if(USE_BOOST_REGEX), and maybe add an error message with a suggestion to define this variable on GCC 4.8.

jaredgrubb · 2016-05-22T05:19:31Z

Just a question and very minor typo... thanks for working on this!

jaredgrubb · 2016-05-22T05:21:49Z

Also, can you rebase this on top of latest? Looks like there's a merge conflict.

The only caveat is that the user should make sure to link boost libraries as needed.

Luthaf · 2016-05-25T13:08:05Z

Updated, with comments addressed. I still get the same test failure on my machine, i.e.:

$ ./run_tests
========================================
UsAgE: prog [options]

OpTiOnS: --path=<files>  Path to files
                [dEfAuLt: /root]


::::::::::::::::::::
prog
--------------------
{ "--path": null }

 ** JSON does not match expected: {u'--path': u'/root'}
1 failures

Should I add GCC 4.8 to Travis too ?

jaredgrubb · 2016-05-28T00:21:04Z

This looks great ... if you could add it to Travis, that would be awesome. That would atleast make sure no one in the future ever breaks it. Are you willing to do that? If so, I'll merge this right away. (I'll merge it even if you say you wont do that part, but then there's nothing to stop it from breaking later)

Luthaf · 2016-05-28T09:26:46Z

Ok, I'll add it to Travis.

What should I do for the failing tests? Is there a way to prevent this exact test from running?

Luthaf · 2016-07-03T13:21:37Z

Ok, so now after the updates and 134760a, the failure is a legit one: https://travis-ci.org/docopt/docopt.cpp/jobs/141999566.

Does anyone with doctopt and regex knowledge can help to fix this?

rhd · 2016-07-07T17:23:34Z

Is this just a case sensitivity issue?

http://stackoverflow.com/questions/15619665/ignore-case-with-boostregexp

static const boost::regex bad_words("...your regex...", boost::regex::icase);

Luthaf · 2016-07-07T18:57:25Z

It does not seems to be. If I update this line to read boost::regex{"\\[default: (.*)\\]", boost::regex::icase})), I still have the failure.

rhd · 2016-07-07T21:09:03Z

@Luthaf I wanted to check out this code and poke around a bit, but I've failed so far.

first problem

cmake couldn't find boost_regex with that capitol 'R'. I tested this on redhad 6.8 with g++4.8.1.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index ecad376..3a2b751 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -55,7 +55,7 @@ endif()

 if(USE_BOOST_REGEX)
        add_definitions("-DDOCTOPT_USE_BOOST_REGEX")
-    find_package(Boost REQUIRED COMPONENTS Regex)
+    find_package(Boost REQUIRED COMPONENTS regex)
     include_directories(${Boost_INCLUDE_DIRS})
     target_link_libraries(docopt ${Boost_LIBRARIES})
        if(WITH_STATIC)

second problem

✔ ~/work/docopt.cpp/build [master|✚ 1…4495] 
14:03 $ make
[ 14%] Built target docopt_o
[ 28%] Linking CXX shared library libdocopt.so
/usr/bin/ld: /usr/local/boost_1_60_0/lib/libboost_regex.a(instances.o): relocation R_X86_64_32S against `.rodata' can not be used when making a shared object; recompile with -fPIC
/usr/local/boost_1_60_0/lib/libboost_regex.a: could not read symbols: Bad value
collect2: error: ld returned 1 exit status
make[2]: *** [libdocopt.so] Error 1
make[1]: *** [CMakeFiles/docopt.dir/all] Error 2
make: *** [all] Error 2

'sh g++-5 --version || true' will fail as sh try to open a file named g++-5

Luthaf · 2016-07-08T19:03:10Z

@rhd I updated the PR, both issues should be fixed now. Thank you for your help.

If the option definition spans over multiple lines, then parsing of the [default: x] was not working on Windows - tested on MS Visual Studio 2015. This is due to the differences in regex's multiline property implementation. On some platforms - Linux libstd++ the multiline property is off, on some platforms - msvc2015 the multiline property is on. It cannot be changed programmatically AFAIK. I was not able to come up with a regex expression which would work on Linux and on Windows as well. So I tried to be smart and replace '\n' by '\f' in the string beforehand. The regex was easy and worked on Linux and Windows just fine - up until I tried a longer option definition. Then the msvc2015 regex failed with 'stack' exception. Hence this change. It splits the option definition section by a simple regex which does not use the problematic '$'. modified: docopt.cpp modified: docopt_util.h

Luthaf · 2016-07-12T12:34:19Z

The issue was not with case sensitivity, but with multiline matching. By cherry-picking fdc4826 from #23, it seems to work.

@jaredgrubb is it good to merge for you?

jaredgrubb · 2016-07-24T15:10:06Z

docopt.cpp

 	};

 	std::vector<Option> defaults;
+	for (auto s : parse_section("options:", doc)) {
+		s.erase(s.begin(), s.begin() + s.find(':') + 1); // get rid of "options:"



You got rid of a static_cast<std::ptrdiff_t> here .. I think we need to put that back in order to quiet some warnings.

jaredgrubb · 2016-07-24T15:16:24Z

I'm going to merge this as-is and then put back the static_cast .. thanks for you work on this!

Luthaf mentioned this pull request Mar 16, 2016

Building with gcc 4.8 #42

Closed

Luthaf reviewed Mar 16, 2016
View reviewed changes

jaredgrubb reviewed May 22, 2016
View reviewed changes

jaredgrubb added the almost-there label May 22, 2016

Support GCC 4.8, using Boost.Regex

cee3d10

The only caveat is that the user should make sure to link boost libraries as needed.

mfrischknecht mentioned this pull request Jun 7, 2016

Regex expressions cause stack overflows in MSVC 2015 #49

Closed

Luthaf added 5 commits July 8, 2016 21:02

Allow to use Boost.Regex with cmake

8e45a51

Enable gcc-48 testing with travis

bf1d121

Make run_tests.py exit with an error for travis

43ff91d

Fix version check on travis

d36e13c

'sh g++-5 --version || true' will fail as sh try to open a file named g++-5

Clarify the boost version requirement

6f24219

Fix cmake installation on OS X in Travis

8f926b6

jaredgrubb reviewed Jul 24, 2016
View reviewed changes

jaredgrubb merged commit 52e3064 into docopt:master Jul 24, 2016

jaredgrubb removed the almost-there label Jul 24, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support GCC 4.8, using Boost.Regex #43

Support GCC 4.8, using Boost.Regex #43

Luthaf commented Mar 16, 2016

Luthaf Mar 16, 2016

jaredgrubb May 15, 2016

Luthaf commented May 16, 2016

jaredgrubb May 22, 2016

nabijaczleweli May 22, 2016

Luthaf May 22, 2016 •

edited

Loading

jaredgrubb commented May 22, 2016

jaredgrubb commented May 22, 2016

Luthaf commented May 25, 2016

jaredgrubb commented May 28, 2016

Luthaf commented May 28, 2016

Luthaf commented Jul 3, 2016

rhd commented Jul 7, 2016

Luthaf commented Jul 7, 2016

rhd commented Jul 7, 2016

Luthaf commented Jul 8, 2016

Luthaf commented Jul 12, 2016

jaredgrubb Jul 24, 2016 •

edited

Loading

jaredgrubb commented Jul 24, 2016

Support GCC 4.8, using Boost.Regex #43

Support GCC 4.8, using Boost.Regex #43

Conversation

Luthaf commented Mar 16, 2016

Luthaf Mar 16, 2016

Choose a reason for hiding this comment

jaredgrubb May 15, 2016

Choose a reason for hiding this comment

Luthaf commented May 16, 2016

jaredgrubb May 22, 2016

Choose a reason for hiding this comment

nabijaczleweli May 22, 2016

Choose a reason for hiding this comment

Luthaf May 22, 2016 • edited Loading

Choose a reason for hiding this comment

jaredgrubb commented May 22, 2016

jaredgrubb commented May 22, 2016

Luthaf commented May 25, 2016

jaredgrubb commented May 28, 2016

Luthaf commented May 28, 2016

Luthaf commented Jul 3, 2016

rhd commented Jul 7, 2016

Luthaf commented Jul 7, 2016

rhd commented Jul 7, 2016

first problem

second problem

Luthaf commented Jul 8, 2016

Luthaf commented Jul 12, 2016

jaredgrubb Jul 24, 2016 • edited Loading

Choose a reason for hiding this comment

jaredgrubb commented Jul 24, 2016

Luthaf May 22, 2016 •

edited

Loading

jaredgrubb Jul 24, 2016 •

edited

Loading