#108 Remove the patch applied to pass the test/fiddle/test_import.rb on PPC.
Merged 2 years ago by jaruga. Opened 2 years ago by jaruga.
rpms/ jaruga/ruby wip/ppc64le-power9-fiddle  into  rawhide

@@ -1,30 +0,0 @@ 

- From 346e147ba6480839b87046e9a9efab0bf6ed3660 Mon Sep 17 00:00:00 2001

- From: =?UTF-8?q?V=C3=ADt=20Ondruch?= <vondruch@redhat.com>

- Date: Wed, 10 Aug 2016 17:35:48 +0200

- Subject: [PATCH] Rely on ldd to detect glibc.

- 

- This is just workaround, since we know we are quite sure this will be successful

- on Red Hat platforms.

- 

- This workaround rhbz#1361037

- ---

-  test/fiddle/helper.rb | 3 +++

-  1 file changed, 3 insertions(+)

- 

- diff --git a/test/fiddle/helper.rb b/test/fiddle/helper.rb

- index 1da3d93..65148a1 100644

- --- a/test/fiddle/helper.rb

- +++ b/test/fiddle/helper.rb

- @@ -139,6 +139,9 @@

-    libc_so = libm_so = "/usr/lib/libSystem.B.dylib"

-  end

-  

- +# Just ignore the heuristic, because it is not reliable on all platforms.

- +libc_so = libm_so = nil

- +

-  if !libc_so || !libm_so

-    ruby = EnvUtil.rubybin

-    # When the ruby binary is 32-bit and the host is 64-bit,

- -- 

- 2.9.2

- 

file modified
+4 -6
@@ -22,7 +22,7 @@ 

  %endif

  

  

- %global release 156

+ %global release 157

  %{!?release_string:%define release_string %{?development_release:0.}%{release}%{?development_release:.%{development_release}}%{?dist}}

  

  # The RubyGems library has to stay out of Ruby directory tree, since the
@@ -138,10 +138,6 @@ 

  # https://lists.fedoraproject.org/archives/list/ruby-sig@lists.fedoraproject.org/message/LH6L6YJOYQT4Y5ZNOO4SLIPTUWZ5V45Q/

  # For now, load the ABRT hook via this simple patch:

  Patch6: ruby-2.7.0-Initialize-ABRT-hook.patch

- # Workaround "an invalid stdio handle" error on PPC, due to recently introduced

- # hardening features of glibc (rhbz#1361037).

- # https://bugs.ruby-lang.org/issues/12666

- Patch9: ruby-2.3.1-Rely-on-ldd-to-detect-glibc.patch

  # Fix DWARF5 support.

  # https://bugzilla.redhat.com/show_bug.cgi?id=1920533

  # https://bugs.ruby-lang.org/issues/17585
@@ -674,7 +670,6 @@ 

  %patch4 -p1

  %patch5 -p1

  %patch6 -p1

- %patch9 -p1

  %patch15 -p1

  %patch16 -p1

  %patch17 -p1
@@ -1461,6 +1456,9 @@ 

  

  

  %changelog

+ * Tue Jan 11 2022 Jun Aruga <jaruga@redhat.com> - 3.0.3-157

+ - Remove the patch applied to pass the test/fiddle/test_import.rb on PPC.

+ 

  * Mon Jan 10 2022 Miro Hrončok <mhroncok@redhat.com> - 3.0.3-156

  - Rebuilt for https://fedoraproject.org/wiki/Changes/LIBFFI34

  

The test test/fiddle/test_import.rb including the regular expression logic to detect the library path in test/fiddle/helper.rb doesn't work with the following error on a Power 9 environment where the path is /lib64/glibc-hwcaps/power9/libc-2.28.so.

/builddir/build/BUILD/ruby-2.6.9/.ext/common/fiddle/import.rb:299:in `import_function': cannot find the function: strcpy() (Fiddle::DLError)
    from /builddir/build/BUILD/ruby-2.6.9/.ext/common/fiddle/import.rb:172:in `extern'
    from /builddir/build/BUILD/ruby-2.6.9/test/fiddle/test_import.rb:17:in `<module:LIBC>'
    from /builddir/build/BUILD/ruby-2.6.9/test/fiddle/test_import.rb:10:in `<module:Fiddle>'
    from /builddir/build/BUILD/ruby-2.6.9/test/fiddle/test_import.rb:9:in `<top (required)>'
    from /builddir/build/BUILD/ruby-2.6.9/test/lib/test/unit.rb:958:in `require'
    from /builddir/build/BUILD/ruby-2.6.9/test/lib/test/unit.rb:958:in `block in non_options'
    from /builddir/build/BUILD/ruby-2.6.9/test/lib/test/unit.rb:952:in `each'
    from /builddir/build/BUILD/ruby-2.6.9/test/lib/test/unit.rb:952:in `non_options'
    from /builddir/build/BUILD/ruby-2.6.9/test/lib/test/unit.rb:64:in `process_args'
    from /builddir/build/BUILD/ruby-2.6.9/test/lib/test/unit.rb:130:in `process_args'
    from /builddir/build/BUILD/ruby-2.6.9/test/lib/test/unit.rb:1136:in `process_args'
    from /builddir/build/BUILD/ruby-2.6.9/test/lib/test/unit.rb:1141:in `run'
    from /builddir/build/BUILD/ruby-2.6.9/test/lib/test/unit.rb:1148:in `run'
    from ./test/runner.rb:33:in `<main>'

Setting the library name without the library path is better not to create 2 different instances of glibc loaded in memory.

See https://bugs.ruby-lang.org/issues/12666#note-11.


I tested this patch works for the unit test on the upstream Ruby 3.0.3 source code on Power 9 with optimized glibc library.

[jaruga@ibm-p9z-22 ruby]$ ldd /usr/bin/ruby
    linux-vdso64.so.1 (0x00007fff867f0000)
    libruby.so.2.6 => /lib64/libruby.so.2.6 (0x00007fff863c0000)
    libpthread.so.0 => /lib64/glibc-hwcaps/power9/libpthread-2.28.so (0x00007fff86370000)
    librt.so.1 => /lib64/glibc-hwcaps/power9/librt-2.28.so (0x00007fff86340000)
    libgmp.so.10 => /lib64/libgmp.so.10 (0x00007fff86290000)
    libdl.so.2 => /lib64/libdl.so.2 (0x00007fff86260000)
    libcrypt.so.1 => /lib64/libcrypt.so.1 (0x00007fff86210000)
    libm.so.6 => /lib64/glibc-hwcaps/power9/libm-2.28.so (0x00007fff860e0000)
    libc.so.6 => /lib64/glibc-hwcaps/power9/libc-2.28.so (0x00007fff85ec0000)
    /lib64/ld64.so.2 (0x00007fff86810000)

[jaruga@ibm-p9z-22 ruby]$ lscpu | grep '^Model name'
Model name:          POWER9, altivec supported

[jaruga@ibm-p9z-22 ruby]$ make test-all TESTS="test/fiddle/test_import.rb"
Run options: 
  --seed=20524
  "--ruby=./miniruby -I./lib -I. -I.ext/common  ./tool/runruby.rb --extout=.ext  -- --disable-gems"
  --excludes-dir=./test/excludes
  --name=!/memory_leak/

# Running tests:

Finished tests in 0.224629s, 178.0714 tests/s, 832.4836 assertions/s.                                   
40 tests, 187 assertions, 0 failures, 0 errors, 0 skips

ruby -v: ruby 3.0.3p157 (2021-11-24 revision ae804310aa) [powerpc64le-linux]

Scratch build: running
https://koji.fedoraproject.org/koji/taskinfo?taskID=80963145

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

Note Ruby 3.0.3 passes even without applying ruby-2.3.1-Rely-on-ldd-to-detect-glibc.patch and ruby-3.0.3-fiddle-1.0.8-Rely-on-hard-coded-lib-name-to-detect-glibc.patch. Because the upstream patch https://bugs.ruby-lang.org/issues/12666 has already been applied to Ruby 3.0.0. But the point of this patch is to set the glibc library name explicitly avoiding unintentionally set value.

I feel just removing ruby-2.3.1-Rely-on-ldd-to-detect-glibc.patch to align the upstream code without patch is better in Ruby 3.0.3 now.

Dropping the ruby-2.3.1-Rely-on-ldd-to-detect-glibc.patch would be fine, but I don't see any additional value in the ruby-3.0.3-fiddle-1.0.8-Rely-on-hard-coded-lib-name-to-detect-glibc.patch

Dropping the ruby-2.3.1-Rely-on-ldd-to-detect-glibc.patch would be fine, but I don't see any additional value in the ruby-3.0.3-fiddle-1.0.8-Rely-on-hard-coded-lib-name-to-detect-glibc.patch

Yes, I agree. I will rebase this PR just removing ruby-2.3.1-Rely-on-ldd-to-detect-glibc.patch.

rebased onto 75854ef8ae1896f9b22c7ac77d1470c29031fbd3

2 years ago

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.

rebased onto b24ebf5

2 years ago

OK. I rebased this PR on the latest rawhide, updating the logic, just removing the patch, and updating the changelog and commit message too.

Scratch build: running https://koji.fedoraproject.org/koji/taskinfo?taskID=81091177 .

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

Interestingly the result from the scratch build shows all the archs fails with core dump at the part of YAMLStoreTest.

https://koji.fedoraproject.org/koji/taskinfo?taskID=81091177

[21259/21271] YAMLStoreTest#test_writing_inside_readonly_transaction_raises_error = 0.00 s
[21260/21271] YAMLStoreTest#test_yaml_store_files_are_accessed_as_binary_files = 0.16 s
make: *** [uncommon.mk:801: yes-test-all] Aborted (core dumped)
error: Bad exit status from /var/tmp/rpm-tmp.qaKN5h (%check)

https://koschei.fedoraproject.org/package/ruby
I guess that the libffi-devel change causes this error.

I guess that the libffi-devel change causes this error.

I will merge this PR, as the error by libffi-devel is not related to this PR.

Pull-Request has been merged by jaruga

2 years ago

Yes, the FFI update broke Ruby :((