Skip to content

Allow compilation on older gcc versions#3090

Merged
flavorjones merged 1 commit intosparklemotion:mainfrom
adfoster-r7:allow-compilation-on-older-gcc-versions
Jan 12, 2024
Merged

Allow compilation on older gcc versions#3090
flavorjones merged 1 commit intosparklemotion:mainfrom
adfoster-r7:allow-compilation-on-older-gcc-versions

Conversation

@adfoster-r7
Copy link
Copy Markdown
Contributor

@adfoster-r7 adfoster-r7 commented Jan 9, 2024

What problem is this PR intended to solve?

Compiling Nokogiri with older versions of GCC and centos causes the following error:

12:06:09 checking for gumbo_parse_with_options() in nokogiri_gumbo.h... yes
12:06:09 checking for xmlHasFeature()... yes
12:06:09 checking for xmlFirstElementChild()... yes
12:06:09 checking for xmlRelaxNGSetParserStructuredErrors()... yes
12:06:09 checking for xmlRelaxNGSetValidStructuredErrors()... yes
12:06:09 checking for xmlSchemaSetValidStructuredErrors()... yes
12:06:09 checking for xmlSchemaSetParserStructuredErrors()... yes
12:06:09 checking for rb_category_warning()... yes
12:06:09 checking for whether
12:06:09 -DNOKOGIRI_OTHER_LIBRARY_VERSIONS="\"libgumbo:1.0.0-nokogiri\"" is accepted as
12:06:09 CPPFLAGS... yes
12:06:09 creating Makefile
12:06:09 
12:06:09 current directory:
12:06:09 /opt/dev/embedded/lib/ruby/gems/3.0.0/gems/nokogiri-1.16.0/ext/nokogiri
12:06:09 make DESTDIR\= clean
12:06:09 
12:06:09 current directory:
12:06:09 /opt/dev/embedded/lib/ruby/gems/3.0.0/gems/nokogiri-1.16.0/ext/nokogiri
12:06:09 make DESTDIR\=
12:06:09 compiling gumbo.c
12:06:09 compiling html4_document.c
12:06:09 compiling html4_element_description.c
12:06:09 compiling html4_entity_lookup.c
12:06:09 compiling html4_sax_parser_context.c
12:06:09 compiling html4_sax_push_parser.c
12:06:09 compiling libxml2_backwards_compat.c
12:06:09 compiling nokogiri.c
12:06:09 compiling test_global_handlers.c
12:06:09 compiling xml_attr.c
12:06:09 compiling xml_attribute_decl.c
12:06:09 compiling xml_cdata.c
12:06:09 compiling xml_comment.c
12:06:09 compiling xml_document.c
12:06:09 xml_document.c: In function 'dealloc':
12:06:09 xml_document.c:77: error: #pragma GCC diagnostic not allowed inside functions
12:06:09 xml_document.c:78: error: #pragma GCC diagnostic not allowed inside functions
12:06:09 xml_document.c:90: warning: '__xmlDeregisterNodeDefaultValue' is deprecated
12:06:09 (declared at
12:06:09 /opt/dev/embedded/lib/ruby/gems/3.0.0/gems/nokogiri-1.16.0/ports/x86_64-linux/libxml2/2.12.3/include/libxml2/libxml/tree.h:684)
12:06:09 xml_document.c:93: error: #pragma GCC diagnostic not allowed inside functions
12:06:09 make: *** [xml_document.o] Error 1
12:06:09 
12:06:09 make failed, exit code 2
12:06:09 
12:06:09 Gem files will remain installed in
12:06:09 /opt/dev/embedded/lib/ruby/gems/3.0.0/gems/nokogiri-1.16.0 for
12:06:09 inspection.
12:06:09 Results logged to
12:06:09 /opt/dev/embedded/lib/ruby/gems/3.0.0/extensions/x86_64-linux/3.0.0/nokogiri-1.16.0/gem_make.out
12:06:09 
12:06:09 An error occurred while installing nokogiri (1.16.0), and Bundler cannot
12:06:09 continue.
12:06:09 Make sure that `gem install nokogiri -v '1.16.0' --source
12:06:09 'https://rubygems.org/'` succeeds before bundling.
12:06:09 
12:06:09 In Gemfile:
12:06:09   factory_bot_rails was resolved to 6.2.0, which depends on
12:06:09     railties was resolved to 7.0.8, which depends on
12:06:09       actionpack was resolved to 7.0.8, which depends on
12:06:09         actionview was resolved to 7.0.8, which depends on
12:06:09           rails-dom-testing was resolved to 2.2.0, which depends on
12:06:09             nokogiri

Have you included adequate test coverage?

No tests have been added; the solution implemented is similar to redis/redis#5394 - and has been verified as working locally in a fresh docker container

Does this change affect the behavior of either the C or the Java implementations?

Impacts the C behavior

@stevecheckoway
Copy link
Copy Markdown
Contributor

stevecheckoway commented Jan 9, 2024

This looks like the right approach to me. Running the CI now.

I'm not sure how to tell which version of GCC support which pragmas where though.

@adfoster-r7
Copy link
Copy Markdown
Contributor Author

adfoster-r7 commented Jan 9, 2024

I believe GCC versions prior to 4.6 are impacted - https://gcc.gnu.org/gcc-4.6/changes.html

New Languages and Language specific improvements

C family

  • ...
  • Support for selectively enabling and disabling warnings via #pragma GCC diagnostic has been added. For instance:
#pragma GCC diagnostic error "-Wuninitialized"
 foo(a);			/* error is given for this one */
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wuninitialized"
 foo(b);			/* no diagnostic for this one */
#pragma GCC diagnostic pop
 foo(c);			/* error is given for this one */
#pragma GCC diagnostic pop
 foo(d);			/* depends on command-line options */

I aligned with the approach taken by Redis for simplicity; Although I think that this could be tightened up a bit to check for minor version too

# https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html
/* Test for GCC > 3.2.0 */
#if __GNUC__ > 3 || \
    (__GNUC__ == 3 && (__GNUC_MINOR__ > 2 || \
                       (__GNUC_MINOR__ == 2 && \
                        __GNUC_PATCHLEVEL__ > 0))

I'm happy to go with whatever check you think is best - I just know the currently suggested fix works for centos

@flavorjones
Copy link
Copy Markdown
Member

flavorjones commented Jan 11, 2024

I'd love to be able to test this manually, but I couldn't easily find a docker image with gcc 4.6 or earlier. Do you know of one you can point me at? The centos:centos6 image will no longer yum update, and the gcc:4.6 image has old CA certs so I can't git clone.

I mean, I'm happy to merge this without testing it, too, if you're sure this solves your problem. Let me know?

@adfoster-r7
Copy link
Copy Markdown
Contributor Author

adfoster-r7 commented Jan 11, 2024 •
edited
Loading

@flavorjones These replication steps should hopefully work

  1. Git clone nokogiri on the host machine - or it can be git cloned inside the docker container
  2. In the same directory run the docker container. It's centos 6 with gcc (GCC) 4.4.7
# Run the container from the same directory as nokogiri - or don't do use volume mounts, and just git clone inside
docker run --rm  -i --tty -u jenkins --volume=$(pwd):$(pwd) -w $(pwd) rapid7/msf-centos6-x64-omnibus:2024_01 /bin/bash --login

# Install bundler
gem install bundler -v 2.3

# libyaml
sudo yum install -y libyaml-devel

# Bundle
bundle

# Compile nokogiri
bundle exec rake clean
bundle exec rake compile:nokogiri -- --use-system-libraries

Before

failure

$ bundle exec rake compile:nokogiri -- --use-system-libraries
....
/var/jenkins_home/.rvm/rubies/ruby-3.0.6/include/ruby-3.0.0/ruby/internal/arithmetic/int.h:155: warning: comparison is always true due to limited range of data type
../../../../ext/nokogiri/xml_document.c: In function ‘dealloc’:
../../../../ext/nokogiri/xml_document.c:77: error: #pragma GCC diagnostic not allowed inside functions
../../../../ext/nokogiri/xml_document.c:78: error: #pragma GCC diagnostic not allowed inside functions
../../../../ext/nokogiri/xml_document.c:93: error: #pragma GCC diagnostic not allowed inside functions
At top level:
cc1: warning: unrecognized command line option "-Wno-tautological-compare"
cc1: warning: unrecognized command line option "-Wno-self-assign"
cc1: warning: unrecognized command line option "-Wno-parentheses-equality"
cc1: warning: unrecognized command line option "-Wno-constant-logical-operand"
cc1: warning: unrecognized command line option "-Wno-cast-function-type"
gmake: *** [xml_document.o] Error 1
rake aborted!
Command failed with status (2): [/usr/bin/gmake]
/var/jenkins_home/.rvm/gems/ruby-3.0.6/gems/rake-compiler-1.2.5/lib/rake/extensiontask.rb:195:in `block (2 levels) in define_compile_tasks'
/var/jenkins_home/.rvm/gems/ruby-3.0.6/gems/rake-compiler-1.2.5/lib/rake/extensiontask.rb:194:in `block in define_compile_tasks'
/var/jenkins_home/.rvm/gems/ruby-3.0.6/gems/rake-13.1.0/exe/rake:27:in `<top (required)>'
/var/jenkins_home/.rvm/gems/ruby-3.0.6/bin/ruby_executable_hooks:22:in `eval'
/var/jenkins_home/.rvm/gems/ruby-3.0.6/bin/ruby_executable_hooks:22:in `<main>'
Tasks: TOP => compile:nokogiri => compile:nokogiri:x86_64-linux => copy:nokogiri:x86_64-linux:3.0.6 => tmp/x86_64-linux/nokogiri/3.0.6/nokogiri.so
(See full trace by running task with --trace)

After

success

$ bundle exec rake compile:nokogiri -- --use-system-libraries
cp tmp/x86_64-linux/nokogiri/3.0.6/nokogiri.so tmp/x86_64-linux/stage/lib/nokogiri/nokogiri.so
[jenkins@c4d13ce50246 nokogiri]$ echo $?
0
...

@flavorjones
Copy link
Copy Markdown
Member

flavorjones commented Jan 12, 2024

@adfoster-r7 Thank you for your help, and for submitting this. Looks good to me, will merge and get into the next patch release.

@flavorjones flavorjones merged commit 3be7ef1 into sparklemotion:main Jan 12, 2024
@flavorjones flavorjones added this to the v1.16.x patch releases milestone Jan 12, 2024
@adfoster-r7 adfoster-r7 mentioned this pull request Jan 23, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v1.16.x patch releases

Development

Successfully merging this pull request may close these issues.

3 participants

@adfoster-r7 @stevecheckoway @flavorjones