Add libtool patch for cross compile.#6963
Conversation
Summary of ChangesHello @Redbeanw44602, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a targeted patch for Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a patch for the libtool script to improve cross-compilation support for non-GCC toolchains like clang and zig. The patch is applied dynamically after the configure step. My review focuses on making this patching process more robust. I've suggested adding error handling to the file replacement operations to ensure that any patching failures are caught early, preventing potential cryptic build errors down the line.
|
|
||
| -- @see https://github.com/xmake-io/xmake-repo/pull/8165#issuecomment-3354492014 | ||
| function _apply_libtool_patch_for_cross(package, opt) | ||
| if package:is_cross() and os.isfile("libtool") then |
There was a problem hiding this comment.
怪怪的，为啥要这里改？目前看 99% 的 autoconf 包交叉编译都没遇到问题么，如果只是特定包的问题，应该在特定包里打 patch
如果是 libtool 特定版本的问题，应该在 libtool 包里打 patch 。。
这里全局替换不可靠，很容易 break ，不可能兼容所有包，反而容易出其他问题，比如替换错误。
There was a problem hiding this comment.
这里交叉编译指的是需要传 -target 的工具链，GCC 自己没有这个问题，所以主要影响的是基于 Clang 的工具链。
xmake-repo 里 cross 测试用的是 GCC，所以一直都没出过问题
不仅仅是 libtool 特定版本的问题，是影响所有 libtool，包括 2.6.0 也影响，因为 2.6.0 修了但没完全修，libtool 这块真是太草台了，让人难以相信
如果替换不出错的话，应该不会 break 现有包吧？其实就是改了三个 case，也没有动libtool 里其他功能，case 的语法的话 bash 这块还是比较宽松的…
但是替换出错的风险的确是有，这个 patch 我在 libtool 2.4.6/2.5.4 测试过了，如果担心出错，感觉可以换成 git patch 这样可靠一些？
There was a problem hiding this comment.
跟 git apply 没啥关系。主要是 libtool 的版本是不确定的，你这写死的模式匹配，很容易匹配错，导致错误替换
比如后续新版本 libtool 中多加一个 "-Z*) 类似的字符串在各种地方，你一替换就乱了，这种都是不确定的。
There was a problem hiding this comment.
确实有这个风险，所以不模式匹配了嘛，用git patch的话上下文就都能匹配到了
There was a problem hiding this comment.
最好做一些约束，比如 从 libtool 提前版本，限制下版本
另外，尽可能减小替换影响范围，比如仅仅 clang 编译器才需要，默认的 gcc 交叉编译，就根本不需要替换。
There was a problem hiding this comment.
准备支持 libtool 2.4.3 ~ 2.6.0 修补（2014年至今）
2.4.3.patch 2.4.4.patch 2.4.7.patch 2.5.0.patch 2.6.0.patch
这些补丁放到哪比较好呢？🤔
There was a problem hiding this comment.
af75be8 to
7674ce8
Compare
感觉弄好了👀 |
xmake/core/sandbox/modules/import/lib/detect/find_programver.lua
Outdated
Show resolved
Hide resolved
This reverts commit 4dade56.
|
|
||
| if #suitable_patch_versions == 0 then | ||
| if option.get("verbose") then | ||
| wprint("warning: no suitable cross-compilation patch was found for libtool for this project.") |
There was a problem hiding this comment.
wprint 本来就是打印 warning 的，warning: 可以去掉。
另外，如果是高版本 libtool ，原本就不需要 patch ，那完全可以直接返回不处理，没必要还去打印 warning
There was a problem hiding this comment.
暂时不对>2.6.0 warning吧，也不知道他们下次发版能不能解决，我等会到邮件列表去问问
Like: xmake-io/xmake-repo#8165 (comment)
The deficiencies in libtool make it almost impossible to cross-compile with non-gcc toolchains(clang, zig...).
2.6.0 does not solve the problem. Although autotools-mirror/libtool@9c7772f forwards the
--targetparameter, but-target *is still not forwarded🙃.Moreover, a large number of packages are still using old versions of libtool, making it almost impossible to fix this problem from the root in a short period of time.
So, I hope to apply a patch for libtool on the xmake side. This patch will not break existing packages (including those using libtool 2.6.0).
When the time is right, we can remove this patch.