Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix use of designated initializers. #75

Merged
merged 2 commits into from Nov 16, 2023

Conversation

igorpeshansky
Copy link
Contributor

Make them compatible with earlier versions of GCC.

Fixes #74.

Make them compatible with earlier versions of GCC.
@lemsx1
Copy link

lemsx1 commented Nov 13, 2023

I can confirm this fixes the issue for me:

 /opt/ruby/3.1.4/lib64/ruby/gems/3.1.0/gems/unf_ext-0.0.9/ext/unf_ext # build-env-baapi.sh make
Using path '/opt/rh/devtoolset-7/root/usr/bin:/opt/ruby/3.1.4/bin:/opt/rh/devtoolset-7/root/usr/bin:/usr/bin:/bin:/usr/local/bin:/opt/ruby/3.1.4/bin:/bin:/usr/bin:/bb/bin:/opt/dell/srvadmin/bin:/opt/dell/srvadmin/sbin:/sbin:/usr/sbin:/usr/local/sbin:/bb/admin:/bb/admin/qc:/opt/utils/admin:/root/bin'
compiling unf.cc
linking shared-object unf_ext.so
/opt/ruby/3.1.4/lib64/ruby/gems/3.1.0/gems/unf_ext-0.0.9/ext/unf_ext # echo $?
0

lsb_release -r

Release: 7.9

using devtoolset-7

@huarmenta
Copy link

Hey @lemsx1 Can you please share with us what exactly should we do?

@lemsx1
Copy link

lemsx1 commented Nov 14, 2023

For this you can simply go to
the unpacked directory where gem failed to compile and copy/paste the patch (line 33 of the file) and hit: make && make install.
Ideally, you can clone the repo, apply this patch to your local copy, and do a new gem (rake build) to install that copy instead.

@huarmenta
Copy link

@lemsx1 Thanks! will try!

@loudgazelle
Copy link

@knu Will you please review this patch? It resolves #74 which was introduced by #72

AsefJafarli

This comment was marked as duplicate.

@ABHIJITH-EA
Copy link

ABHIJITH-EA commented Nov 14, 2023

I have tested the patch and wanted to bring to your attention a potential compatibility concern with Ruby versions lower than 2.7.0, despite the gemspec specifying a requirement of ">= 2.2". The error encountered during compilation in Ruby version 2.3.1 is as follows:

compiling unf.cc
cc1plus: warning: command line option ‘-Wdeclaration-after-statement’ is valid for C/ObjC but not for C++ [enabled by default]
cc1plus: warning: command line option ‘-Wimplicit-function-declaration’ is valid for C/ObjC but not for C++ [enabled by default]
unf.cc:45:3: error: ‘rb_data_type_struct::<anonymous struct>’ has no non-static data member named ‘dcompact’
   };
   ^
make: *** [unf.o] Error 1

The "dcompact" member, which I believe was introduced in Ruby version 2.7.0, is the source of this issue. This patch appears to not fully resolve the issue introduced by #72

@knu
Copy link
Owner

knu commented Nov 14, 2023

Hmm, and the irony here is that despite the extension being updated to support the C API of Ruby 2.7+, the main functionality this library provides is in Ruby 2.6+ as String#unicode_normalize. 🥺

@knu
Copy link
Owner

knu commented Nov 14, 2023

I've just released a new version of the domain_name gem that requires Ruby >=2.7 and no longer depends on unf and unf_ext. I think that will remove one of the biggest reasons people would ever need to get unf_ext installed.

@igorpeshansky
Copy link
Contributor Author

@ABHIJITH-EA Thanks for the heads-up. I just pushed a commit that should make this build with older Rubies as well. PTAL.

@Bo98
Copy link

Bo98 commented Nov 14, 2023

I think that will remove one of the biggest reasons people would ever need to get unf_ext installed.

Did the 0.2.0 betas on unf end up working? I noticed the changes tried there were the native parts being conditionally used based on the Ruby version.

@ABHIJITH-EA
Copy link

ABHIJITH-EA commented Nov 15, 2023

@igorpeshansky Thanks for the fix! I tested it.I can confirm it's all good now.

compiling unf.cc
cc1plus: warning: command line option ‘-Wdeclaration-after-statement’ is valid for C/ObjC but not for C++ [enabled by default]
cc1plus: warning: command line option ‘-Wimplicit-function-declaration’ is valid for C/ObjC but not for C++ [enabled by default]
linking shared-object unf_ext.so

@igorpeshansky
Copy link
Contributor Author

Thanks, @knu. Is this ready to merge now, or would you need me to squash the commits?

@knu
Copy link
Owner

knu commented Nov 16, 2023

No problem. I'll squash merge this.

@knu knu merged commit 285ab6d into knu:master Nov 16, 2023
16 checks passed
@igorpeshansky igorpeshansky deleted the fix-designated-initializers branch November 24, 2023 20:02
@tpowell-progress tpowell-progress mentioned this pull request Dec 4, 2023
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants