search

ffi / ffi-compiler

Apache License 2.0
32 stars 10 forks source link

1.2.0 seems to break some packages with arg splitting #23

Closed thesamesam closed 1 year ago

thesamesam commented 1 year ago

We had a report in Gentoo at https://bugs.gentoo.org/906892 of http-parser-1.2.3 failing to build with ffi-compiler-1.2.0 like so:

mkdir -p x86_64-linux/http-parser
x86_64-pc-linux-gnu-gcc -O2 -pipe -march\=native -fno-diagnostics-color -Wall\ -Wextra\ -O3 -fPIC -o x86_64-linux/http-parser/http_parser.o -c ./http-parser/http_parser.c
x86_64-pc-linux-gnu-gcc: error: unrecognized command-line option -Wall -Wextra -O3
rake aborted!
Command failed with status (1): [x86_64-pc-linux-gnu-gcc -O2 -pipe -march\=...]

@matoro points out that the args from https://github.com/cotag/http-parser/blob/master/ext/Rakefile#L4 (the package itself) seem to not be split by ffi-compiler.

See also https://github.com/ffi/ffi-compiler/pull/22.

matoro commented 1 year ago

Note that in this case the environment flags (-O2 -pipe -march\=native -fno-diagnostics-color) ARE split correctly, it is just the flags set by the package itself that are not.

davispuh commented 1 year ago

Yeah ehhh, turns out 5e173ae1de068bea5890c4c2438c5b1e1cdad702 changed API and thus each flag should be a separate item rather putting everything in same string. ie. t.cflags << "-Wall" << "-Wextra" << "-O3" and that would work fine. But there's probably a lot of applications that do this so I restored API compatibility in #24

Also this library has 0 tests so :sweat_smile:

davispuh commented 1 year ago

Just merged #24 and tagged 1.2.1 https://github.com/ffi/ffi-compiler/releases/tag/1.2.1