HttpParser fails to build on recent ArchLinux and Julia Version 0.6.0 (2017-06-19 13:05 UTC)

[WilCrofter]

A fix which seems to have worked for me was to comment out the -Werror switch in ~/.julia/v0.6/HttpParser/deps/src/http-parser-2.7.1/Makefile line 57:

CFLAGS += -Wall -Wextra #-Werror

Otherwise, a warning which stems from a drop through in case 2: below (line 1817 ~/.julia/v0.6/HttpParser/deps/src/http-parser-2.7.1/http_parser.c is treated as an error and aborts the build. However, the drop through (missing break statement) in case 2: seems purposeful. I am guessing about that however.

        if (settings->on_headers_complete) {
          switch (settings->on_headers_complete(parser)) {
            case 0:
              break;

            case 2:
              parser->upgrade = 1;

            case 1:
              parser->flags |= F_SKIPBODY;
              break;

            default:
              SET_ERRNO(HPE_CB_headers_complete);
              RETURN(p - data); /* Error */
          }
        }

[aviks]

What gcc version are you on? This might need to be reported upstream https://github.com/nodejs/http-parser

[WilCrofter]

Apparently Arch and I are somewhat behind the curve, the current stable version of gcc being 7.2.0.

$ gcc --version
gcc (GCC) 7.1.1 20170630
$ pacman -Ss gcc
core/gcc 7.1.1-4 (base-devel) [installed]

I think you are right about reporting this upstream. Shall I do that and close the issue?

[WilCrofter]

Actually, the latest source at https://github.com/nodejs/http-parser builds without issue on the same Arch Linux box using gcc 7.1.1. (See abbreviated session below.) But the latest source differs significantly from that in my ~/.julia/v0.6/HttpParser/deps/src/http-parser-2.7.1, so it appears that Pkg.add("HttpParser") downloaded and attempted to build an earlier version.

Namely v2.7.1 which I see is specified in build.jl. Unsure how to proceed from here. Thinking.

Sorry to be so verbose, but version 2.7.1 does build with an older gcc on an ubuntu machine. (Ubuntu 4.8.4 vs latest stable release of 5.3.)

$ git clone [email protected]:nodejs/http-parser.git
Cloning into 'http-parser'...
$ cd http-parser/
$ make
cc  -I. -DHTTP_PARSER_STRICT=1  -Wall -Wextra -Werror -O0 -g  -c http_parser.c -o http_parser_g.o
...
test_g
...
requests okay
./test_fast
...
requests okay

[aviks]

Thank you for the investigation, this is very helpful. Especially the verbosity!

So the latest release of node-parser is 2.7.1, from about a year ago. They haven't made a new release since, but apparently fixed this issue in master. I'm not sure we want the julia package to download master by default. Can you make this change locally in your environment? And/or ask them when they propose to release a new version?

[aviks]

In particular, the commit that fixed this seems to be this one: nodejs/http-parser@0852bea

[WilCrofter]

Thanks, @aviks, that's a good plan. Let me educate myself about this gcc7 fallthrough issue a little more today. Then I'll open an issue at nodejs/http-parser as you suggest and link to it from here. We can close this issue or not as you think best.

I can easily make the appropriate changes in my local environment. Thanks.

[WilCrofter]

Issue requesting new release opened at nodejs/http_parser.

Shall we close here, or leave open until completely resolved?

[aviks]

Leave it open. We'll upgrade to a new version once released and tested.

[WilCrofter]

Just in case anyone else needs a quick fix, there are just two lines to change in .julia/v0.x/HttpParser/deps/src/http-parser-2.7.1/http_parser.c.

Line 1818 is blank. It should contain a /* FALLTHROUGH */ comment as follows.

            case 2:
              parser->upgrade = 1; # line 1817
              /* FALLTHROUGH */    # was blank
            case 1:
              parser->flags |= F_SKIPBODY;
              break;

Line 2378 contains a comment misspelled as /* FALLTROUGH */. The missing H should be added as follows.

      case s_req_server_with_at:
        found_at = 1;

      /* FALLTHROUGH */   # line 2378 --was misspelled
      case s_req_server:
        uf = UF_HOST;
        break;

Running make should then compile and successfully test the code.

[christopher-dG]

This is still present on Arch with gcc 7.2.0, but the above fix still works.

[WilCrofter]

@christopher-dG, I should have mentioned that if http-parser is first installed with the system package manager, e.g., sudo pacman -S http-parser for Arch, then julia> Pkg.add("HttpParser") will install without issue. I've also verified the process for Debian, sudo apt-get install http-parser. There may be problems with CentOS 7 and similar. Forums seem to indicate that http-parser was, at some point, missing from the nodejs dependencies. I don't know if this has been resolved.

So far, the nodejs project has not acted on my request for a new release.

[AShedko]

By the way I am still experiencing this very bug on ubuntu 17.10 on current master.
Any news on this?

[Azzaare]

@AShedko You can follow @WilCrofter instruction to modify .julia/v0.x/HttpParser/deps/src/http-parser-2.7.1/http_parser.c
You can then run make or even Pkg.build("HttpParser") in Julia REPL. It should fix it.

[WilCrofter]

You can also comment out the -Werror switch on line 49 of the Makefile. This is my preference (despite my original comment,) but opinions differ.

Http_parser is available as an npm package for all distributions, I think. I've ended up using the version available as an Arch Linux package. Once that is installed, the Julia package should build flawlessly.

[WilCrofter]

nodejs/http-parser, v2.8.0 has been released today and contains the fix.

[randyzwitch]

Can we tag a new version for METADATA? Checking out master resolved my error On Pop_OS! (Ubuntu 17.10), so it seems like it's worth bumping the version for others.

[Cevadra]

me 2 when building on Ubuntu 17.10
problem solved, thanks WilCrofter !~