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

ruby: Update Makefile fix build error 3.3.4 using github action #25151

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

guhill1
Copy link

@guhill1 guhill1 commented Oct 15, 2024

Maintainer: @luizluca
Compile tested: (put here arch, model, OpenWrt version)
Run tested: (put here arch, model, OpenWrt version, tests done)

Description:
fix ruby build error 3.3.4 using github action:
"linking static-library libruby-static.a
LLVM ERROR: Invalid encoding"

reference from:
https://lore.kernel.org/all/[email protected]/T/

fix ruby build error 3.3.4 using github action:
"linking static-library libruby-static.a
LLVM ERROR: Invalid encoding"

Signed-off-by: gg hill <[email protected]>
@guhill1
Copy link
Author

guhill1 commented Oct 15, 2024

#25052

@luizluca
Copy link
Contributor

luizluca commented Oct 16, 2024

It is good to be merged. It will take some time to get openwrt "rusted".

However, it looks like a workaround for a ruby's bug. I think no config option should be required to avoid the build to crash. The build script should have handled it nicely.

Is there an easy way to reproduce this issue in a local build? I wonder what triggered it in github actions. Can you point me to a full build log? I would like to reproduce it and report it upstream.

@luizluca
Copy link
Contributor

I tried to find the log for a ruby action that failed but I couldn't. I also tried to build it locally with and without a rustc installed locally. In all cases, it built clearly, with all builds detecting rustc correctly and only host-build actually using it.

@luizluca
Copy link
Contributor

While disabling ygit is not a big deal as most archs still doesn't support it, I want to make sure that is is reproducible at openwrt and not only at immortalwrt/packages#1342 before merging the fix.

@guhill1
Copy link
Author

guhill1 commented Oct 16, 2024

While disabling ygit is not a big deal as most archs still doesn't support it, I want to make sure that is is reproducible at openwrt and not only at immortalwrt/packages#1342 before merging the fix.

I'm running github action for official openwrt version to re-check it and got same failure information:

2024-10-16T05:06:05.9863422Z linking static-library libruby-static.a
2024-10-16T05:06:06.0625182Z LLVM ERROR: Invalid encoding
2024-10-16T05:06:06.1531894Z make[4]: *** [Makefile:318: libruby-static.a] Aborted (core dumped)
2024-10-16T05:06:06.1532997Z make[4]: *** Deleting file 'libruby-static.a'
2024-10-16T05:06:06.1534658Z make[4]: Leaving directory '/home/runner/work/auto-build-openwrt/auto-build-openwrt/openwrt/build_dir/hostpkg/ruby-3.3.4'
2024-10-16T05:06:06.1539427Z make[3]: *** [Makefile:1189: /home/runner/work/auto-build-openwrt/auto-build-openwrt/openwrt/build_dir/hostpkg/ruby-3.3.4/.built] Error 2

And the complete job log I put link here.
https://github.com/guhill1/auto-build-openwrt/actions/runs/11358465582/job/31593012655

Maybe it is a possible solution to build a docker container in local place to try to reproduce it.

@1715173329
Copy link
Member

The commit title should be more descriptive, like ruby: disable unsupported yjit from the link you mentioned.

Also please sign the DCO with your legal name and real email.

@luizluca
Copy link
Contributor

I'll take it from here. I need to bump ruby to 3.3.5. I'll incorporate this change there.

@luizluca
Copy link
Contributor

I wonder why it didn't fail in #25171

@guhill1
Copy link
Author

guhill1 commented Oct 20, 2024

I wonder why it didn't fail in #25171

Let me check it, I remember I've tried it in 3.3.5.

@luizluca
Copy link
Contributor

@guhill1 , the CI tests uses a different script than the one failing with you. In CI, all references to "checking for rustc..." points to "no". There is no rustc installed for all targets both host and target build.

The CI uses .github/workflows/multi-arch-test-build.yml, from packages, while you uses .github/workflows/compile_official.yml , which simply does not exist in openwrt project (see: https://github.com/search?q=org%3Aopenwrt+compile_official.yml&type=code).

Something that action is doing will eventually install rustc from an old Ubuntu that might break the ruby 3.3.0 yjit.
Although that build env does not seem to be official and it seems to be a ruby autoconf bug, we should not use a feature that uses implicit local OS dependencies.

I'll try to add it as a menu option (enabled by default for supported arch: aarch64 and x86_64), adding rust as a build dependency. The ruby host build does not need yjit and I'll unconditionally disable it.

@guhill1
Copy link
Author

guhill1 commented Oct 20, 2024

@luizluca, I understand what you mean. This may be rustc's fault. Although I am not an official script, there is no problem compiling in version 3.25. Hope there will be a fix, or some option to replace old rustc with new rustc.

@luizluca
Copy link
Contributor

@luizluca, I understand what you mean. This may be rustc's fault. Although I am not an official script, there is no problem compiling in version 3.25. Hope there will be a fix, or some option to replace old rustc with new rustc.

@guhill1 , can you test your build with #25171? I guess the issue was only with the host build with an old rustc version. I disabled yjit for host build as we don't gain much with it.

yjit is only supported for x86_64 and aarch64 and now the package reflects that. For those targets, I include the rustc (from openwrt) as a build dep to avoid using the one from host machine. Even on those targets, it might still be skipped by ruby's autoconf as ruby still does not support yjit in cross-compiling.

The final result for the enduser is mostly the same from before, with ruby x86_64 built in x86_64 (or aarhc64 built in aarch64) generating a binary capable of yjit, but now independent from host os tools. For host and all other target builds, yjit will be disabled.

The package might need a minor change if ruby expands yjit support to other archs and probably no changes if cross-compiling support is fixed.

@guhill1
Copy link
Author

guhill1 commented Oct 21, 2024

@luizluca, I will integrate your patch in my action to test it.
My build with #25171
completed successfully. And now in the new build you already include rustc as a part of compile part and finally x86_64 has a capability of yjit, nice work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants