search

msys2 / MINGW-packages

Package scripts for MinGW-w64 targets to build under MSYS2.
https://packages.msys2.org
BSD 3-Clause "New" or "Revised" License
2.31k stars 1.23k forks source link

ruby: update to 3.3.6 #22422

Closed raedrizqie closed 1 week ago

raedrizqie commented 3 weeks ago
lazka commented 3 weeks ago

the commits contain much more than you mention without any explanation why it was changed, or why it is OK to remove certain things. I can't really review that.

All the package changes look fine though.

raedrizqie commented 3 weeks ago

@lazka i have updated the commit change

ognevny commented 1 week ago

seems fine now, doesn't it?

mmuetzel commented 1 week ago

Is it ok to remove all of the provides?

raedrizqie commented 1 week ago

Is it ok to remove all of the provides?

With those provides, it's like creating meta packages with each and every python internal modules. I do not see any point of that.. For example, if any external ruby gems need any of those internal gems, pacman will try to pull in the ruby package itself.. ruby is already a dependency for all ruby gems anyway..

mmuetzel commented 1 week ago

If I understand correctly, the gems have been added as separate provides with this change: https://github.com/msys2/MINGW-packages/commit/dc27816c5835037ae7d79e7a85efa7d8ec7eb8a1

@JPeterMugaas: Is the reasoning you gave in the commit message still valid? Do you think they could be removed now?

JPeterMugaas commented 1 week ago

I think the reasoning behind the provides is still valid.

ognevny commented 1 week ago

@raedrizqie can you do chmod +x for updpkgprovs? it had 0755 mode before

raedrizqie commented 1 week ago

clang32 errors:

2024-11-20T19:13:03.6049616Z building spec/ruby/optional/capi/ext/boolean_spec.so
2024-11-20T19:13:03.6503807Z ld.lld: error: undefined symbol: __initialize_onexit_table
2024-11-20T19:13:03.6574225Z >>> referenced by C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtdll.c:64
2024-11-20T19:13:03.6619124Z >>>               D:/M/msys64/clang32/lib/dllcrt2.o:(_pre_c_init)
2024-11-20T19:13:03.6714090Z 
2024-11-20T19:13:03.6724723Z ld.lld: error: undefined symbol: __execute_onexit_table
2024-11-20T19:13:03.6753377Z >>> referenced by C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtdll.c:130
2024-11-20T19:13:03.6765942Z >>>               D:/M/msys64/clang32/lib/dllcrt2.o:(__CRT_INIT@12)
2024-11-20T19:13:03.6769057Z >>> referenced by C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtdll.c:130
2024-11-20T19:13:03.6770571Z >>>               D:/M/msys64/clang32/lib/dllcrt2.o:(___DllMainCRTStartup)
2024-11-20T19:13:03.6834005Z >>> referenced by C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtdll.c:130
2024-11-20T19:13:03.6853689Z >>>               D:/M/msys64/clang32/lib/dllcrt2.o:(___DllMainCRTStartup)
2024-11-20T19:13:03.6865933Z 
2024-11-20T19:13:03.6876865Z ld.lld: error: undefined symbol: __initterm
2024-11-20T19:13:03.6890593Z >>> referenced by C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtdll.c:100
2024-11-20T19:13:03.6893630Z >>>               D:/M/msys64/clang32/lib/dllcrt2.o:(__CRT_INIT@12)
2024-11-20T19:13:03.6926614Z >>> referenced by C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtdll.c:104
...

mingw-w64 crt bugs?

ognevny commented 1 week ago

IMO it's better to just drop clang32 here

mmuetzel commented 1 week ago

IMO it's better to just drop clang32 here

If we prefer to do that, we'd need to drop all reverse dependencies including the build dependencies.

If the consensus is that the CLANG32 environment as currently distributed by MSYS2 is broken beyond repair, it would be better imho to remove it in one fowl swoop instead of continue to distribute something that is broken.

This is a larger decision than updating Ruby.

@lazka: What do you think?

raedrizqie commented 1 week ago

One of the reason I want to update ruby is because of this warning when configuring qtwebkit:

...
2024-11-17T16:21:20.8217076Z -- Found Perl: D:/M/msys64/usr/bin/perl.exe (found suitable version "5.38.2", minimum required is "5.10.0")
2024-11-17T16:21:20.8599084Z -- Found PythonInterp: D:/M/msys64/clang64/bin/python3.exe (found suitable version "3.12.7", minimum required is "2.7.0")
2024-11-17T16:21:22.8290085Z -- Could NOT find Ruby (missing: Ruby_LIBRARY) (found suitable version "3.1.5", minimum required is "1.9")
...
mmuetzel commented 1 week ago

It looks like the linker doesn't pull in any CRT libraries on CLANG32 for some reason. (I wonder why that is the only affected environment.)

The following change fixes the build for me: 0001-ruby-link-UCRT.patch

raedrizqie commented 1 week ago

It looks like the linker doesn't pull in any CRT libraries on CLANG32 for some reason. (I wonder why that is the only affected environment.)

The following change fixes the build for me: 0001-ruby-link-UCRT.patch

Let me try something first.. if it failed, I will try your fix..

mmuetzel commented 1 week ago

Let me try something first.. if it failed, I will try your fix..

Nice. So, this was caused by a new hunk of a patch?

raedrizqie commented 1 week ago

Let me try something first.. if it failed, I will try your fix..

Nice. So, this was caused by a new hunk of a patch?

Yeah.. but the dll naming for i386 ucrt should be ucrt-ruby330.dll, without the extra prefx i386- We can force it by adding;

    $( [[ ${MSYSTEM} == CLANG32 ]] && \
      echo '--with-soname=ucrt-$(RUBY_BASE_NAME)$(MAJOR)$(MINOR)0' || true )

to configure (like before)

mmuetzel commented 1 week ago

In that case, should the same be done for CLANG64 which also has the extra prefixes now (from the file-diff section of the CI)?

  -/clang64/bin/ruby310.dll
   /clang64/bin/rubyw.exe
  +/clang64/bin/syntax_suggest
  +/clang64/bin/syntax_suggest.bat
   /clang64/bin/typeprof
   /clang64/bin/typeprof.bat
  +/clang64/bin/x64-ucrt-ruby330.dll
raedrizqie commented 1 week ago

In that case, should the same be done for CLANG64 which also has the extra prefixes now (from the file-diff section of the CI)?

  -/clang64/bin/ruby310.dll
   /clang64/bin/rubyw.exe
  +/clang64/bin/syntax_suggest
  +/clang64/bin/syntax_suggest.bat
   /clang64/bin/typeprof
   /clang64/bin/typeprof.bat
  +/clang64/bin/x64-ucrt-ruby330.dll

For x64 ucrt based dll, the name is correct with x64-ucrt-ruby330.dll. Only i386 should strip the i386- prefix, same as MINGW32 (msvcrt-ruby330.dll)

mmuetzel commented 1 week ago

For x64 ucrt based dll, the name is correct with x64-ucrt-ruby330.dll. Only i386 should strip the i386- prefix, same as MINGW32 (msvcrt-ruby330.dll)

I don't know why the naming pattern should be different depending on the architecture. But I trust you know the reason.

raedrizqie commented 1 week ago

For x64 ucrt based dll, the name is correct with x64-ucrt-ruby330.dll. Only i386 should strip the i386- prefix, same as MINGW32 (msvcrt-ruby330.dll)

I don't know why the naming pattern should be different depending on the architecture. But I trust you know the reason.

Yeah.. it is because of historical reasons.. see here: https://github.com/ruby/ruby/pull/11364#issuecomment-2283960114

ognevny commented 1 week ago

I wonder, can Rust be used for YJIT here?

mmuetzel commented 1 week ago

Would you like to take over again and restore that configure flag? (I could also amend your last commit if you prefer that.)

raedrizqie commented 1 week ago

Would you like to take over again and restore that configure flag? (I could also amend your last commit if you prefer that.)

Ok.. its easier if you could do it..

raedrizqie commented 1 week ago

I wonder, can Rust be used for YJIT here?

Sorry, I know nothing about rust :(

ognevny commented 1 week ago

Sorry, I know nothing about rust :(

from what I see the only required thing is to add rust into makedepends. I can try it in separate PR

mmuetzel commented 1 week ago

Would you like to take over again and restore that configure flag? (I could also amend your last commit if you prefer that.)

Ok.. its easier if you could do it..

Trying to do so leads to the old issue where no CRT seems to have been linked.

Tbh, I didn't understand from the link you showed earlier why the naming convention needs to differ for i386. What would be the issue if we just used the naming convention from upstream?

Edit: If I drop the ucrt- prefix, it seems to work...

raedrizqie commented 1 week ago

Would you like to take over again and restore that configure flag? (I could also amend your last commit if you prefer that.)

Ok.. its easier if you could do it..

Trying to do so leads to the old issue where no CRT seems to have been linked.

Ah.. I see.. It may be the way they hardcoded soname when compiling..

Tbh, I didn't understand from the link you showed earlier why the naming convention needs to differ for i386.

Sorry, it should be this one.. https://github.com/ruby/ruby/commit/576b2e64cdc5ea42ad345dd3c1c215e006c06fca

What would be the issue if we just used the naming convention from upstream?

No issue actually.. upstream don't have CI job for testing x86 ucrt anyway..

Edit: If I drop the ucrt- prefix, it seems to work...

Ok

mmuetzel commented 1 week ago

What would be the issue if we just used the naming convention from upstream?

No issue actually.. upstream don't have CI job for testing x86 ucrt anyway..

So, could this be merged like it currently is?

raedrizqie commented 1 week ago

What would be the issue if we just used the naming convention from upstream?

No issue actually.. upstream don't have CI job for testing x86 ucrt anyway..

So, could this be merged like it currently is?

I think so, yes

raedrizqie commented 1 week ago

Thank you!

ognevny commented 1 week ago

from what I see the only required thing is to add rust into makedepends. I can try it in separate PR

it's not possible currently: https://bugs.ruby-lang.org/issues/19325. I currently don't have a time to test it :( (building with --enable-yjit fails with lots of mismatched types error)