-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
kernel.mk: Fix regression build issue since #6002 #6089
Conversation
@hgy59 having a first fix attempt which allowed me to build |
@hgy59 confirmed, builds fine for generic arch as it used to. I'll integrate DSM 7.1 and 7.2 kernels, do a few more round of testing and that should be good to go. |
@hgy59 getting a lot closer. Besides bugs yet to be found, Besides that it would be ready for a test run if you have spare cycles. EDIT: BTW, I doubt this will build sucessfully online as it becomes really large when building for all archs for |
@th0ma7 the first issue I see in the github actions is in "Prepare for Build" job. downloading toolchain as dependency of kernel does not work as expected. the log starts with:
It first downloads the toolchain (as dependency of kernel)
and indeed, the ( content of spksrc/kernel/syno-88f6281-6.2.4/work/tc_vars.mk/tc_vars.mk:
with such a corrupt file, it is not even possible to call make clean
hopefully you can find the source of the false output redirection 😄 In this context I found another issue. In this scenario the "kernel clean" initializes the toolchain, i.e. it downloads, extracts and builds the toolchain (including the initialization of rust), before it removes the kernel-work folder
|
Thnx for testing it out. I'll have a look at that. Other issues:
|
@hgy59 I've been playing with it in and out and something doesn't please me... Looking into simplifying things a little but current complexity makes it not that easy to deal with. I'll require a few more cycles to get this over with. |
Yes another round of updates. I've greatly simplified the code (which may not be apparent at first glance but it really is). I yet have a few other bits to deal with before fixing other items mentionned above. @hgy59 one thing that is now adressed if the playing around with the
Not yet ready for prime time testing though, as mentionned, still needs some more work but now should be simpler to adapt as needed. |
@hgy59 I believe everything is now addressed and this is ready for a another review & testing. EDIT: and forgot, i still have two archs kernel to fix... EDIT2: one thing I wonder, would it be possible to enforce building DSM-7.2 as well when kernel modules are involved? |
@hgy59 I believe all issues are now fixed and this is ready for merge. Remaining BTW, it looks worst than it actually is, and really is the continuation of #6002 and simplify the whole build process significantly from prior (i.e. relatively to kernel and al). While my testing looked good there will probably be some corner cases. If any, I intend to fix them while enabling So yeah, a fresh pair of 👀 would be appreciated before merging. thnx. |
@hgy59 considering recent issues with the framework I'm suggesting waiting until the week-end to confirm existing issues are solved before I inject additional changes (while being kernel specific). Let me know if you see anything else needing my attention before I merge. |
I still vote for dedicated packages for DSM 7.1 and DSM 7.2 |
@hgy59 and you got my vote: fix for that already included in this PR. Thing I would have liked is to be able to change the |
There are only three generic archs for DSM 7.2 (x64, aarch64 and armv7) so the matrix is not much larger. At least we have to update the publish action to not only configure a specific package with |
@th0ma7 BTW I had to restart the github jobs (several times) due to an internal error: |
Are you suggesting to simply add DSM-7.2 builds into the matrix section of the
I honestly got lost in there on how to include hooks to enhance the matrix as needed. What I had in mind was to include a "new" spk variable or reuse |
IMHO this will never be possible. |
Actually, I'd need further reading but looking at https://docs.github.com/fr/actions/using-jobs/using-a-matrix-for-your-jobs there seems to be a way to dynamically set the # x64=x86_64, evansport=i686, aarch64=armv8, armv7, hi3535=armv7l, 88f6281=armv5, qoriq=ppc
# https://github.com/SynoCommunity/spksrc/wiki/Synology-and-SynoCommunity-Package-Architectures
export MATRIX_ARCH="noarch, noarch-6.1, noarch-7.0, x64-6.2.4, x64-7.1, evansport-6.2.4, evansport-7.1, aarch64-6.2.4, aarch64-7.1, armv7-6.2.4, armv7-7.1, hi3535-6.2.4, 88f6281-6.2.4, qoriq-6.2.4, comcerto2k-7.1" Then from the build:
name: Build
needs: prepare
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
arch: [${{ env.MATRIX_ARCH }}] |
@th0ma7 the prepare job (github action) has now some error logs due missing 'dependency-kernel-list' targets
|
@hgy59 thnx for letting me know, I'll have a look at it. I also noticed that |
@th0ma7 you already removed the I have now removed the usage in prepare.sh in 5fc6228 |
Description
Where was the issue, mainly due to the addition of supporting
depend
makefile rule into the toolchain build process to accomodate the pre-built rustnative
installation forqoriq
arch:WORK_DIR
was pre-set to the current build location resulting in re-installing the toolchain into the package build directory instead of the actualtoolchain/syno-<arch>-<version>
. FIXEDdepend
rule into toolchain ended-up generating a.depend_done
, which in turn made the build to entirely skip the actual dependencies which included the kernel module builds. FIXEDEnforcing the
WORK_DIR=$(TC_WORK_DIR)
inspksrc.cross-env.mk
when preparingtc_vars.cmake tc_vars.meson tc_vars.mk
solved that, ensuring that the toolchain stays in its appropriate location, resulting in kicking-off the kernel module build process.Aditionnally:
spksrc.depend.mk
to process all archs from generic build calls (e.g.make arch-<armv7|aarch64|x64|etc>-*
ormake all-supported
- FIXEDtc_vars.*
files when processing specific archs from generic - FIXEDFixes #6085
Checklist
all-supported
completed successfullyType of change