fix bug #2056, tdebindings does not rely anymore on pkg-config #5

Merged
SlavekB merged 1 commits from fix/2056 into master 4 years ago
Ghost commented 4 years ago

no more pkg-config for Ruby binding, only rbconfig.rb is needed.

It builds here with Ruby-1.8.7 and Ruby-2.6.5

no more pkg-config for Ruby binding, only rbconfig.rb is needed. It builds here with Ruby-1.8.7 and Ruby-2.6.5
Ghost added the PR/rfc label 4 years ago
Owner

Test on Debian 7.x (Wheezy):

checking for ruby... ruby
-e:1:in `fetch': key not found: "rubyarchhdrdir" (KeyError)
        from -e:1:in `<main>'

                  archdir /usr/lib/ruby/1.9.1/x86_64-linux,
                  sitearchdir /usr/local/lib/site_ruby/1.9.1/x86_64-linux,
                  sitedir /usr/local/lib/site_ruby/1.9.1,
                  rubylibdir /usr/lib/ruby/1.9.1,
                  libdir /usr/lib,
                  includedir /usr/include/ruby-1.9.1,
                  librubyarg -lruby-1.9.1,
                  cflags -I/usr/include/ruby-1.9.1 -I

followed by FTBFS:

In file included from /usr/include/ruby-1.9.1/ruby.h:32:0,
                 from Qt.cpp:49,
                 from libqtrubyinternal_la.all_cpp.cpp:2:
/usr/include/ruby-1.9.1/ruby/ruby.h:24:25: fatal error: ruby/config.h: No such file or directory
compilation terminated.
Test on Debian 7.x (Wheezy): ``` checking for ruby... ruby -e:1:in `fetch': key not found: "rubyarchhdrdir" (KeyError) from -e:1:in `<main>' archdir /usr/lib/ruby/1.9.1/x86_64-linux, sitearchdir /usr/local/lib/site_ruby/1.9.1/x86_64-linux, sitedir /usr/local/lib/site_ruby/1.9.1, rubylibdir /usr/lib/ruby/1.9.1, libdir /usr/lib, includedir /usr/include/ruby-1.9.1, librubyarg -lruby-1.9.1, cflags -I/usr/include/ruby-1.9.1 -I ``` followed by FTBFS: ``` In file included from /usr/include/ruby-1.9.1/ruby.h:32:0, from Qt.cpp:49, from libqtrubyinternal_la.all_cpp.cpp:2: /usr/include/ruby-1.9.1/ruby/ruby.h:24:25: fatal error: ruby/config.h: No such file or directory compilation terminated. ```
Ghost added the PR/wip label 4 years ago
Ghost commented 4 years ago
Poster

Ruby-1.9.1 rbconfig.rb does not seem to have rubyarchhdrdir, thanks for the testing.

Ruby-1.9.1 rbconfig.rb does not seem to have **rubyarchhdrdir**, thanks for the testing.
SlavekB reviewed 4 years ago
SlavekB left a comment
Owner

The current version of the patch may cause FTBFS.

The current version of the patch may cause FTBFS.
if test "$RUBY_VERSION" -ge "191"; then
RUBY_INCLUDEDIR=`${RUBY_EXECUTABLE} -r rbconfig -e "puts RbConfig::CONFIG.fetch(%q(rubyhdrdir))"`
RUBY_ARCH=`${RUBY_EXECUTABLE} -r rbconfig -e "puts RbConfig::CONFIG.fetch(%q(arch))"`
RUBY_CFLAGS="-I${RUBY_INCLUDEDIR} -I${RUBY_INCLUDEDIR}/${RUBY_ARCH}"
Owner

The method used to compose arch include directory works well for Debian 7.x (Wheezy), where the path is like /usr/include/ruby-1.9.1/x86_64-linux, but fails for newer versions where the path is like /usr/include/x86_64-linux-gnu/ruby-2.5.0. We should use rubyarchhdrdir if defined.

The method used to compose arch include directory works well for Debian 7.x (Wheezy), where the path is like `/usr/include/ruby-1.9.1/x86_64-linux`, but fails for newer versions where the path is like `/usr/include/x86_64-linux-gnu/ruby-2.5.0`. We should use `rubyarchhdrdir` if defined.
Ghost commented 4 years ago
Poster

i don't see the need to push forwards in this direction with rbconfig.rb if we have to check regularly if the appropriate keys are there, it's no better than incrementing the ruby version with each new release. should i delete this?

i don't see the need to push forwards in this direction with rbconfig.rb if we have to check regularly if the appropriate keys are there, it's no better than incrementing the ruby version with each new release. should i delete this?
Owner

@cethyel I made a small modification to your patch – now rubyarchhdrdir is used, if defined. Similarly, a test for rubyhdrdir is performed, so there is no need test based on Ruby version number.

Please test on your systems. I did a test on Debian 7.x (Wheezy) and Debian 10.x (Buster).

@cethyel I made a small modification to your patch – now `rubyarchhdrdir` is used, if defined. Similarly, a test for `rubyhdrdir` is performed, so there is no need test based on Ruby version number. Please test on your systems. I did a test on Debian 7.x (Wheezy) and Debian 10.x (Buster).
Owner

i don’t see the need to push forwards in this direction with rbconfig.rb if we have to check regularly if the appropriate keys are there, it’s no better than incrementing the ruby version with each new release. should i delete this?

Your solution looks good – this will allow us to get rid of reliance on the Ruby version number. I think there is no reason to discard this PR.

> i don’t see the need to push forwards in this direction with rbconfig.rb if we have to check regularly if the appropriate keys are there, it’s no better than incrementing the ruby version with each new release. should i delete this? Your solution looks good – this will allow us to get rid of reliance on the Ruby version number. I think there is no reason to discard this PR.
Ghost commented 4 years ago
Poster

I'll test tomorrow, then you will choose to keep this or not, but I feel that Ruby's keys aren't consistent over time enough, a ruby version pushing that kind of changes over the last release and so on.

I'll test tomorrow, then you will choose to keep this or not, but I feel that Ruby's keys aren't consistent over time enough, a ruby version pushing that kind of changes over the last release and so on.
Collaborator

Patch tested and working on CentOS 6, 7 and 8.

Patch tested and working on CentOS 6, 7 and 8.
Ghost commented 4 years ago
Poster

@Francois Thanks for the testing.
@SlavekB we probably don't need RUBY_TEENY anymore.

@Francois Thanks for the testing. @SlavekB we probably don't need RUBY_TEENY anymore.
Ghost commented 4 years ago
Poster

@SlavekB Works for me, at least It builds fine here (ruby 2.6.5p114).
I've removed RUBY_TEENY btw.

t1.rb from qtruby/rubylib/tutorial/t1 does not work though, is this one of the (many) broken tutorials/examples? I fairly don't know!

@SlavekB Works for me, at least It builds fine here (ruby 2.6.5p114). I've removed RUBY_TEENY btw. t1.rb from qtruby/rubylib/tutorial/t1 does not work though, is this one of the (many) broken tutorials/examples? I fairly don't know!
Ghost removed the PR/wip label 4 years ago
Owner

To my knowledge, @MicheleC observed that many examples and tutorials in tdebindings did not work. However, it will be a different story than this pull-request. Nothing seems to prevent merge.

To my knowledge, @MicheleC observed that many examples and tutorials in tdebindings did not work. However, it will be a different story than this pull-request. Nothing seems to prevent merge.
Owner

One small note: @cethyel your Signed-off-by is now duplicated. This is probably not needed. Please, can you make an amend commit log and push again?

One small note: @cethyel your `Signed-off-by` is now duplicated. This is probably not needed. Please, can you make an amend commit log and push again?
SlavekB removed the PR/rfc label 4 years ago
Ghost closed this pull request 4 years ago
SlavekB closed this pull request 4 years ago
SlavekB deleted branch fix/2056 4 years ago
SlavekB added this to the R14.0.7 release milestone 4 years ago
Owner

@SlavekB: yes, tdebindings is mostly brokens and need work to fix it. There is an open PR, but it has been put on hold until R14.1.0 is released. It seems it needs some deep investigation 😧

@SlavekB: yes, tdebindings is mostly brokens and need work to fix it. There is an open PR, but it has been put on hold until R14.1.0 is released. It seems it needs some deep investigation :anguished:
The pull request has been merged as 1aaf11f05e.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
4 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: TDE/tdebindings#5
Loading…
There is no content yet.