Skip to content

include bool header for ruby 3.4#3

Open
hooleyhoop wants to merge 1 commit intoaudioBoom:masterfrom
hooleyhoop:ruby-3.4.2-fix
Open

include bool header for ruby 3.4#3
hooleyhoop wants to merge 1 commit intoaudioBoom:masterfrom
hooleyhoop:ruby-3.4.2-fix

Conversation

@hooleyhoop
Copy link

@hooleyhoop hooleyhoop commented Jun 16, 2025

https://bugs.ruby-lang.org/issues/21290

support 'CFLAGS=-std=gnu99' on ruby 3.4 and above (which removed #include <stdbool.h> from the ruby headers).

I think this is good practice, and if we are maintaining this gem we should probably add it.
It's also completely inadequate because so many gems are broken in this way and i think the real problem is that you have to build ruby with at least

CFLAGS=-std=gnu17

which.. i'm surprised.. given that i install an app just to build ruby for me.. but it doesnt seem to have an opinion on that

@jdelStrother jdelStrother requested a review from Copilot June 16, 2025 13:49

This comment was marked as off-topic.

@jdelStrother
Copy link
Member

I'm guessing you have gcc15? Would you add a comment about what this is fixing, and maybe a link to https://bugs.ruby-lang.org/issues/21290 assuming that's the same issue?

@jdelStrother
Copy link
Member

From RubyCrypto/ed25519#45, I wondered if the correct fix might be to use append_cflags rather than $CFLAGS << here -

$CFLAGS << " -std=c99 -fPIC -fms-extensions"

I'm not able to reproduce the problem with gcc 15.1.1 on the fedora 42 docker image with ruby 3.4.2 though, so not sure I follow the issue properly.

@hooleyhoop
Copy link
Author

as far as i understand it isnt anything to do with gcc15, or fedora.

Using bool without <stdbool.h> will compile only if:
a. we’re in a C99+ dialect, or
b. another header happens to pull it in as a side-effect.

Relying on b is fragile, so the explicit #include <stdbool.h> is a portable fix

@jdelStrother
Copy link
Member

Mm, ok - some more context here https://bugs.ruby-lang.org/issues/21340

Agree we ought to include stdbool where we're using bool in geoip2.c. I guess ideally the header file wouldn't include stdbool, but don't mind merging as-is if it's necessary to fix a bug in ruby.h.

@hooleyhoop
Copy link
Author

-- aside -- i was going to edit the above comment -- but i'm a bit murkier on this --

the $CFLAGS << " -std=c99" won't ensure c99, so it's not very good. append_cflags("-std=gnu99") would, but is having a spefic requirement the right thing?

@jdelStrother
Copy link
Member

the $CFLAGS << " -std=c99" won't ensure c99

Won't it? How come?

append_cflags("-std=gnu99") would

because of replacing c99 with gnu99, or the append_cflags, or are both needed?

is having a spefic requirement the right thing?

A compiler that supports c99 is necessary to build ruby, so I don't think it's a bad thing if you need c99 to compile the gem.

@hooleyhoop
Copy link
Author

hooleyhoop commented Jun 19, 2025

A compiler that supports c99 is necessary to build ruby

i dont think so, this whole thing started because my ruby was built with c89

and then 'mkmf' reuses those flags, so $CFLAGS << " -std=c99" gives you '-std=c89 -std=c99', which resolves to c89

@jdelStrother
Copy link
Member

and then 'mkmf' reuses those flags, so $CFLAGS << " -std=c99" gives you '-std=c89 -std=c99', which resolves to c89

hm. Presumably append_cflags("-std=c99") doesn't help either then? If I'm reading the mkmf source code right it looks like it just checks if a test file compiles with that extra flag, and then adds it to $CFLAGS if it succeeds. So presumably that still ends up with CFLAGS='-std=c89 -std=c99' ..?

@hooleyhoop
Copy link
Author

i believe it de-dupes - but that's only what i've read - (and if it's true) then in your build you would have had

'-std=gnu17 -std=c99'

resolved to c99, which i'm not sure is what you want

@jdelStrother
Copy link
Member

then in your build you would have had
'-std=gnu17 -std=c99'

I don't have any -std in my cflags. If I comment out the CFLAGS line entirely in extconf.rb, here's how it invokes clang:

clang -I. -I/nix/store/j3dk8jrvyyaqlviq0v3m6sbyrj3yraqi-ruby-3.4.4/include/ruby-3.4.0/arm64-darwin24.5.0 -I/nix/store/j3dk8jrvyyaqlviq0v3m6sbyrj3yraqi-ruby-3.4.4/include/ruby-3.4.0/ruby/backward -I/nix/store/j3dk8jrvyyaqlviq0v3m6sbyrj3yraqi-ruby-3.4.4/include/ruby-3.4.0 -I. -I/nix/store/hh9yilsxm2mz3q10z3p0ckqp154kx3fp-libmaxminddb-1.12.2/include
  -DHAVE_RB_SYM2STR -D_XOPEN_SOURCE -D_DARWIN_C_SOURCE -D_DARWIN_UNLIMITED_SELECT -D_REENTRANT   -fno-common -fdeclspec -O3 -fno-fast-math -ggdb3
  -Wall -Wextra -Wextra-tokens -Wdeprecated-declarations -Wdivision-by-zero -Wdiv-by-zero -Wimplicit-function-declaration -Wimplicit-int -Wpointer-arith -Wshorten-64-to-32 -Wwrite-strings -Wold-style-definition -Wmissing-noreturn -Wno-cast-function-type -Wno-constant-logical-operand -Wno-long-long -Wno-missing-field-initializers -Wno-overlength-strings -Wno-parentheses-equality -Wno-self-assign -Wno-tautological-compare -Wno-unused-parameter -Wno-unused-value -Wunused-variable -Wmisleading-indentation -Wundef
  -fno-common -pipe  -arch arm64 -o geoip2.o -c geoip2.c

i believe it de-dupes

I'm 99% sure it just appends, so long as that flag is valid. If I change extconf.rb to include:

$CFLAGS << "-std=gnu99"
append_cflags(%w[-std=gnu99 -std=c99 -fPIC -fms-extensions -somebullshitflags])

Then it invokes clang with:

clang -I. -I/nix/store/j3dk8jrvyyaqlviq0v3m6sbyrj3yraqi-ruby-3.4.4/include/ruby-3.4.0/arm64-darwin24.5.0 -I/nix/store/j3dk8jrvyyaqlviq0v3m6sbyrj3yraqi-ruby-3.4.4/include/ruby-3.4.0/ruby/backward -I/nix/store/j3dk8jrvyyaqlviq0v3m6sbyrj3yraqi-ruby-3.4.4/include/ruby-3.4.0 -I. -I/nix/store/hh9yilsxm2mz3q10z3p0ckqp154kx3fp-libmaxminddb-1.12.2/include
  -DHAVE_RB_SYM2STR -D_XOPEN_SOURCE -D_DARWIN_C_SOURCE -D_DARWIN_UNLIMITED_SELECT -D_REENTRANT   -fno-common  -fdeclspec  -O3 -fno-fast-math -ggdb3
  -Wall -Wextra -Wextra-tokens -Wdeprecated-declarations -Wdivision-by-zero -Wdiv-by-zero -Wimplicit-function-declaration -Wimplicit-int -Wpointer-arith -Wshorten-64-to-32 -Wwrite-strings -Wold-style-definition -Wmissing-noreturn -Wno-cast-function-type -Wno-constant-logical-operand -Wno-long-long -Wno-missing-field-initializers -Wno-overlength-strings -Wno-parentheses-equality -Wno-self-assign -Wno-tautological-compare -Wno-unused-parameter -Wno-unused-value -Wunused-variable -Wmisleading-indentation -Wundef
  -fno-common -pipe -std=gnu99 -std=gnu99 -std=c99 -fPIC -fms-extensions -arch arm64 -o geoip2.o -c geoip2.c
# here -            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Which successfully compiles. I think the last flag takes precedence, but not sure.


append_cflags sounds more sensible than blindly appending to $CFLAGS, but I can't reproduce the problems you're seeing so I'm reluctant to just change it for the sake of it.

@hooleyhoop
Copy link
Author

I don't have any -std in my cflags

sorry, yers that's correct. i meant Clang defaults to gnu17. I might well be wrong about the de-duping, append_cflags seems less fragile than <<

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants