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

bump(main/gnushogi): update to commit 5bb0b5b from Nov 2014 #21967

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

Conversation

robertkirkman
Copy link
Contributor

@robertkirkman robertkirkman commented Oct 24, 2024

Fixes #21966 by adding improved support for cross-compiling, preventing errors from happening during the hostbuild step.
Inspired by the version of gnushogi packaged by Fedora.

@robertkirkman robertkirkman changed the title TEST COMMIT do not merge [WIP] possible future gnushogi version test Oct 24, 2024
@TomJo2000
Copy link
Member

I understand that termux-packages does not accept packages of unreleased versions, so waiting for ydirson to officially release an update to gnushogi will be required.

Exceptions and backports can be made.

@robertkirkman
Copy link
Contributor Author

robertkirkman commented Oct 25, 2024

That's interesting, thanks for explaining. Fedora packages the exact same version, resolving to the exact same source URL, that I did in my example here, but I guess I hadn't noticed that there are some packages in this repo that do also package a git commit hash.

_COMMIT=90d1f7f199cc55b13c7fdb5839d1409806633fdb
TERMUX_PKG_VERSION=2020.12.07
TERMUX_PKG_REVISION=1
TERMUX_PKG_SRCURL=git+https://github.com/rui314/chibicc
TERMUX_PKG_SHA256=9cb136d4713c8003122e8b637730a15808dd102dc2b54a5f96f33053a34a8171

Every one I can find uses the _COMMIT variable and the git+ source URL prefix to do that. Do you think the way I have this PR now is appropriate for the situation, or should I change it to look more like all the other packages I see here using the git commit hash?

I see now that the commit guidelines here say that TERMUX_PKG_VERSION should contain the "commit date" so I'll try to revise this to comply with the guidelines wherever possible. I guess the _COMMIT variable and use of the git+ prefix is appropriate - even though Fedora doesn't do that and it's not mentioned in the guidelines, that's what all the other packages I see here do.

@robertkirkman robertkirkman changed the title [WIP] possible future gnushogi version test bump(main/gnushogi): update to commit 5bb0b5b from Nov 2014 Oct 25, 2024
@robertkirkman
Copy link
Contributor Author

robertkirkman commented Oct 25, 2024

I tested the resulting package on my device and it appears to be working, though I have to admit I am not highly experienced with using the gnushogi program.

I noticed that another package with slow upstream development was removed partly because a log inside packages.termux.dev showed that it was downloaded 0 times in a 2 week period. Maybe that log needs to be checked for this package also, to determine whether this should be merged or the package should also be dropped.

@robertkirkman robertkirkman marked this pull request as ready for review October 25, 2024 01:40
@truboxl
Copy link
Contributor

truboxl commented Oct 26, 2024

_COMMIT are used to verify when git repo is the SRCURL albeit we have to do it in a manual step git checkout as shown in many other packages. TERMUX_PKG_SHA256 is not used and can be removed in this case.

@robertkirkman robertkirkman marked this pull request as draft October 26, 2024 21:27
@robertkirkman
Copy link
Contributor Author

robertkirkman commented Oct 26, 2024

Thank you that is correct, and on closer inspection, based on what you said about the TERMUX_PKG_SHA256, I noticed that my potential migration of the package to the git+ format is missing xtkoba's implementation of termux_step_post_get_source() found in 7883ba6 which remains the pattern for most of the other git+ packages, so I should also copy and paste that to this package.

termux_step_post_get_source() {
git fetch --unshallow
git checkout $_COMMIT
local version="$(git log -1 --format=%cs | sed 's/-/./g')"
if [ "$version" != "$TERMUX_PKG_VERSION" ]; then
echo -n "ERROR: The specified version \"$TERMUX_PKG_VERSION\""
echo " is different from what is expected to be: \"$version\""
return 1
fi
local s=$(find . -type f ! -path '*/.git/*' -print0 | xargs -0 sha256sum | LC_ALL=C sort | sha256sum)
if [[ "${s}" != "${TERMUX_PKG_SHA256} "* ]]; then
termux_error_exit "Checksum mismatch for source files."
fi
}

@robertkirkman robertkirkman marked this pull request as ready for review October 26, 2024 21:49
@robertkirkman
Copy link
Contributor Author

robertkirkman commented Oct 26, 2024

Summary, which also outlines roughly how most of the relatively straightforward types of hostbuild-steps work in termux-packages:

  • all errors in gnushogi fixed in cross-compilation mode
    • in this mode, the hostbuild-step uses gcc which is a real copy of Ubuntu GCC for amd64 targets. That produces binaries for the crossbuild host to run during the part of the build that requires executing build artifacts to generate more code for compilation.
    • in this mode, the final make step uses aarch64-linux-android-clang which is the cross-compiler provided by official NDK.
  • in non-cross-compilation mode, the whole build of this PR also works fine exactly the same without modifying anything, which surprised me but makes sense when checking the build step behavior:
    • in this mode, the hostbuild-step also runs, but uses gcc which is a symbolic link to /data/data/com.termux/files/usr/bin/clang-19
    • in this mode, the final make step uses aarch64-linux-android-clang which is a symbolic link to /data/data/com.termux/files/usr/bin/clang-19.
    • Therefore, in non-cross-compilation mode, the hostbuild step still runs and creates working binaries to run to generate the code for the final make step, but the same compiler is used for both steps, to achieve a fairly consistent result between both build codepaths.

This summary tends to apply generally to packages that are primarily built from C and sometimes C++ code. Packages whose sources contain moderate to large amounts of Rust, Python, Zig, or other languages can sometimes deviate heavily from this pattern and may "swap between" various compilers during their builds in a much more complicated way, so they are considered separately. gnushogi is all C, so the summary above applies to it pretty accurately.

packages/gnushogi/build.sh Outdated Show resolved Hide resolved
@robertkirkman robertkirkman force-pushed the gnushogi-test-upstream-master branch 2 times, most recently from 646c7fd to de31300 Compare October 27, 2024 18:27
Fixes termux#21966 by adding improved support for cross-compiling, preventing errors from happening during the hostbuild step.
Inspired by the version of gnushogi packaged by Fedora: https://src.fedoraproject.org/rpms/gnushogi/blob/rawhide/f/gnushogi.spec
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.

[Bug]: archaic hostbuild-step of gnushogi 1.4.2 throws errors
4 participants