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

Fix aarch/arm64 Architecture Issues #240

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from
Open

Conversation

jxmx
Copy link
Contributor

@jxmx jxmx commented Dec 1, 2024

This pull request addresses two things:

@guysoft
Copy link
Owner

guysoft commented Dec 10, 2024

Hey, before I merge this I want to try and take all the logic out to a function and add a unit test, this area has been getting lots of attention and testing it is a little complex, I think. unit test would help out. so didn't forget it yet

@guysoft
Copy link
Owner

guysoft commented Jan 6, 2025

Ok, I put that section of code in to a unit test.
The tested options are:

    # aarch64 host
    # ["aarch64:armv7l:gentoo"]="not using qemu"
    ["aarch64:armv7l:ubuntu"]="using qemu-arm-static"
    # ["aarch64:armhf:gentoo"]="not using qemu"
    ["aarch64:armhf:ubuntu"]="using qemu-arm-static"
    ["aarch64:aarch64:gentoo"]="not using qemu"
    ["aarch64:aarch64:ubuntu"]="not using qemu"
    ["aarch64:arm64:gentoo"]="not using qemu"
    ["aarch64:arm64:ubuntu"]="not using qemu"

What target are you trying to build to on aarch64? And what distribution (if docker then its debian/Ubuntu)?

@jxmx
Copy link
Contributor Author

jxmx commented Jan 7, 2025

I'm not sure I'm fully understanding what you're trying to do. However I've tested my code changes on both aarch64 and amd64 generating aarch64 PI images. No Docker.

@guysoft
Copy link
Owner

guysoft commented Jan 7, 2025

What I am doing is adding a unit test. It takes the code in the src/custompios script and tests all inputs to make ure they work correctly. Try not to focus on the code, but what you actually want it to do. If you are running on aarch64 and building aarch64, you dont need qemu-aarch64-static, no emulation is required.
I think the change is not required there. Since I ran the unit test with all the possibilities and they do want I expect, the question is do you want a different behavior?
2.
Only the change in src/build seems like its tackling an issue:

What its saying is that you need is to source the distro/config before AND after. It might make more sense only to get the BASE_BOARD variable.

Here is the execution order documentation:
https://github.com/guysoft/CustomPiOS/wiki/CustomPiOS-internals#execution-scripts-order

nightly_build_scripts/custompios_nightly_build
    ${CUSTOM_PI_OS_PATH}/build_custom_os "${WORKSPACE_SUFFIX}"
        ${DIR}/build "$1"
            source ${CUSTOM_PI_OS_PATH}/config "${1}" "${EXTRA_BOARD_CONFIG}" ${@}
            source ${CUSTOM_OS_PATH}/custompios ${@}

What this PR is suggesting is:

nightly_build_scripts/custompios_nightly_build
    ${CUSTOM_PI_OS_PATH}/build_custom_os "${WORKSPACE_SUFFIX}"
        ${DIR}/build "$1"
            source ${DISTRO}/config ${@} <-- SOURCE HERE            
            source ${CUSTOM_PI_OS_PATH}/config "${1}" "${EXTRA_BOARD_CONFIG}" ${@} <-- AND HERE because source ${DISTRO}/config  is sourced at the end here too.
            source ${CUSTOM_OS_PATH}/custompios ${@}

@jxmx
Copy link
Contributor Author

jxmx commented Jan 7, 2025

Without my patch, src/custompios will try to use qemu-arm-static on an aarch64 box to build an aarch64 Pi image.

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.

2 participants