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

Add generic spksrc.supported.mk #6002

Merged
merged 42 commits into from
Apr 1, 2024
Merged

Conversation

th0ma7
Copy link
Contributor

@th0ma7 th0ma7 commented Feb 1, 2024

Description

TL;DR;

Rework, optimize and simplify generic mk calls involving a major refactory but most importantly major clean-up of the code in hope to simplify future maintenance:

  • remove tons of duplicated code by using spksrc.common-rules.mk. This invovles *clean, dependency-*, changelog and a new rustup such as in getting the rustc info using make rustup show for instance.
  • allow using make all-supported and make all-latest from anywhere applicable (i.e. cross, spk and diyspk)
  • add a framework self-testing facility (to be incorporated into github-action later to trigger on any changes to mk/*)
  • allow DEPENDS from toolchain

Self-test facility calls (as of March 6th, more to be included):

  • make testall
  • make test clean
  • make test depend
  • make test digests
  • make test download
  • make test toolchain
  • make test test-dependency-*

TODO:

  • make test rust
  • make test cmake
  • make test gnu-configure
  • make test meson
  • make test native
  • ...

Superseeds #5998

Checklist

  • Build rule all-supported completed successfully
  • New installation of package completed successfully
  • Package upgrade completed successfully (Manually install the package again)
  • Package functionality was tested
  • Any needed documentation is updated/created

Type of change

  • Bug fix
  • New Package
  • Package update
  • Includes small framework changes
  • This change requires a documentation update (e.g. Wiki)

TODO

  • cc
  • cmake
  • dotnet
  • go
  • install-resources
  • meson
  • rust
  • spk

Testing

Toolchain -> COMPLETE

  • ~/spksrc$ make -C toolchain/syno-x64-7.1
  • ~/spksrc$ make -C toolchain/syno-x64-7.1 clean
  • ~/spksrc/toolchain/syno-x64-7.1$ make
  • ~/spksrc/toolchain/syno-x64-7.1$ make clean
  • ~/spksrc/toolchain/syno-x64-7.1$ make changelog
git log --pretty=format:"- %s" -- /home/spksrc/all-supported-fix/spksrc/toolchain/syno-x64-7.1
- synocli-devel: New package - debugging, target python wheel building (#5225)
- update generic x64 archs (#5347)
- Add toolchains for DSM 7.1 (#5320)

native -> COMPLETE

  • ~/spksrc$ make -C native/nasm
  • ~/spksrc$ make -C native/nasm clean
  • ~/spksrc/native/nasm$ make
  • ~/spksrc/native/nasm$ make clean
  • ~/spksrc/native/elixir$ make changelog
git log --pretty=format:"- %s" -- /home/spksrc/all-supported-fix/spksrc/native/elixir
- erlang: update to 24.3 (#5814)
- use make_install_options (#5350)
- Update erlang and rabbitmq with openssl 1.1.1l (#4913)
- rabbitmq: update to v3.8.16 (#4648)
- Add RabbitMQ and dedicated Erlang package (#4437)
  • ~/spksrc/native/elixir$ make dependency-list
elixir: native/elixir native/erlang 
  • ~/spksrc/native/elixir$ make dependency-flat
native/elixir
native/erlang
  • ~/spksrc/native/elixir$ make dependency-tree
+ elixir 1.14.5
\ + erlang 24.3

cross -> COMPLETE

cc -> test with xz
rust -> test with bat
cmake -> test with lz4
cmake + ninja -> test with zstd
cmake + main-depends.mk + unsupported -> test with intel-gmmlib

  • ~/spksrc/cross/<app>$ make arch-qoriq-6.2.4
  • ~/spksrc$ make -C cross/<app> arch-qoriq-6.2.4
  • ~/spksrc/cross/<app>$ make TCVERSION=6.2.4 ARCH=qoriq
  • ~/spksrc$ make -C cross/<app> TCVERSION=6.2.4 ARCH=qoriq
  • ~/spksrc/cross/<app>$ PARALLEL_MAKE=max make -j4 all-supported
  • ~/spksrc$ PARALLEL_MAKE=max make -C cross/<app> -j4 all-supported
    ===>> confirmed native get built first
    ===>> confirmed status-build.log with system-time at end of build (conditional to ~/spksrc/make setup)
  • ~/spksrc/cross/<app>$ make clean
  • ~/spksrc$ make -C cross/bat clean
  • ~/spksrc/cross/<app>$ make changelog
  • ~/spksrc/cross/<app>$ make dependency-flat
  • ~/spksrc/cross/<app>$ make dependency-list
  • ~/spksrc/cross/<app>$ make dependency-tree
  • ~/spksrc/cross/<app>$ make rustup show
Default host: x86_64-unknown-linux-gnu
rustup home:  /home/spksrc/all-supported-fix/spksrc/cross/bat/../../distrib/rustup

installed toolchains
--------------------

stable-x86_64-unknown-linux-gnu (default)
stage2-powerpc-unknown-linux-gnuspe

installed targets for active toolchain
--------------------------------------

aarch64-unknown-linux-gnu
armv5te-unknown-linux-gnueabi
armv7-unknown-linux-gnueabi
armv7-unknown-linux-gnueabihf
i686-unknown-linux-gnu
powerpc-unknown-linux-gnu
x86_64-unknown-linux-gnu

active toolchain
----------------

stable-x86_64-unknown-linux-gnu (default)
rustc 1.75.0 (82e1608df 2023-12-21)

@th0ma7
Copy link
Contributor Author

th0ma7 commented Feb 1, 2024

@mreid-tt and @hgy59 while trickier than it looks this code fixes the dependency evaluation duration (current 3m20s as expected).

I'll pursue this question to remove code duplication in makefiles and overall simplification where possible. My intent is to include other targets, spk.mk, cross-cmake.mk and cross-rust.mk so make all-supported works out always.

@th0ma7 th0ma7 self-assigned this Feb 1, 2024
@th0ma7
Copy link
Contributor Author

th0ma7 commented Feb 1, 2024

@hgy59 any interest in updating dotnet as well? As that would require more in-depth changes to it...

@th0ma7
Copy link
Contributor Author

th0ma7 commented Feb 3, 2024

@hgy59 and @mreid-tt working on long overdue major simplifications to the spk.mk file. Current woks locally and needs further testing, mainly kernel related, github-action and publishing (although preliminary local tests works ok).

Basically it does the following:

  1. extracts all the kernel, build and publishing code from spk and redirect them into other files, with hopefully a better and simplified narrative for exit calls and dependencies.
  2. remove duplicates of clean and smart-clean being in everywhere
  3. enable tracing

As mentionned, there's still testing to be done, along with further optimizing the code. But your initial thoughts on it would be welcomed to see if any gaps have been overlooked.

My hope from here is that future maintenance will be simplified thus allowing easier inclusion of other longer term goals, mainly related to python wheel building complexity for instance (while not related, overall code simplification and clean-up will ease integrating subsequent changes).

Thoughts on this welcomed.

@mreid-tt
Copy link
Contributor

mreid-tt commented Feb 3, 2024

@th0ma7, big thanks for your awesome work on optimizing spksrc's core. Just a heads up, I don't have a local environment for testing. I rely solely on building and publishing from GitHub. But hey, I'm here to help out in any way I can with a little guidance from you. Even though some of the kernel-level stuff goes over my head, I'm still keen to chip in where possible.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Feb 4, 2024

You really first need an environment to play with. Worst case if you get to install an Ubuntu using WSL on windows would be a start, from there following the lxc howto on our wiki. I can assist over discord as needed.

@mreid-tt
Copy link
Contributor

mreid-tt commented Feb 4, 2024

You really first need an environment to play with. Worst case if you get to install an Ubuntu using WSL on windows would be a start, from there following the lxc howto on our wiki. I can assist over discord as needed.

Thank you for your support. I primarily use a Mac with Apple Silicon for my computing needs. Consequently, when I install Ubuntu via VMware, it's the ARM version. While this setup works well for spkrepo, I've discovered that spksrc necessitates an x86 platform, as indicated in the wiki. Hence, I currently lack a local environment for spksrc. If there's a workaround for ARM, I'd welcome further discussion on Discord.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Feb 4, 2024

Currently broken part of my last set of changes yesterday night, will let you know once resolved.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Feb 6, 2024

Now fixed, testing undergoing. There is most probably still a few loose-ends to be catched-up.

Also started documenting every calls to confirm things are working as expected...

If anyone has some idea on how to do this, having a github-action rule that "TEST" all the common calls when an mk/* gets modified would be really awesome. Thus ensuring API stays always functional.

@th0ma7 th0ma7 changed the title Add generic spksrc.supported.mk [DRAFTAdd generic spksrc.supported.mk Feb 6, 2024
@th0ma7 th0ma7 changed the title [DRAFTAdd generic spksrc.supported.mk [DRAFT] Add generic spksrc.supported.mk Feb 6, 2024
@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 6, 2024

@hgy59 and @mreid-tt and @SynoCommunity/developers I'd appreciate if you could try-out building a few packages for testing using the current set of changes this PR involves. It is a major re-factor of the code to simplify and standardize when ever possible.

The latest bits I'm including is a self-test mechanism to confirm all functionalities still work with no error. My hope is to eventually automate it through github-action so when ever a change is identified in mk/* it does a make testall to self-check its own health.

Besides adding to the testing facility I believe I'm now really close to the finishing line. I do expect to identify a few remaining issues as I add features to the testing facility (which is the reason for) but overall things looks fairly good now.

All in all, if you can test this and let me know remaining bits that needs adressing would be great. Also if you have additional ideas for more in-depth self-testing facility would be nice to hear from you (i.e. spksrc.test-rules.mk). Thnx in advance.

@Safihre
Copy link
Contributor

Safihre commented Mar 6, 2024

@th0ma7 Could you provide a TLDR version of what this does? And what to test?

@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 6, 2024

@Safihre certainly, here's a few highlights (to be added in top summary):

  • remove tons of duplicated code by using spksrc.common-rules.mk. This invovles *clean, dependency-*, changelog and a new rustup such as in getting the rustc info using make rustup show for instance.
  • allow using make all-supported and make all-latest from anywhere applicable (i.e. cross, spk and diyspk)
  • add a framework self-testing facility (to be incorporated into github-action later to trigger on any changes to mk/*)
  • allow DEPENDS from toolchain

So all in all, major refactory but most importantly major clean-up of the code in hope to simplify future maintenance.

@hgy59
Copy link
Contributor

hgy59 commented Mar 6, 2024

  • allow using make all-supported and make all-latest from anywhere applicable (i.e. cross, spk and diyspk)

regarding the from anywhere there is an issue in spksrc.python.mk.

working on homeassistant package I wanted to create dedicated wheels with (temp) diyspk Makefiles. When using pre-built python in spk/python311 it did not work for packages in diyspk folder.
This is caused by line 8 in spksrc.python.mk

PYTHON_PACKAGE_ROOT = $(realpath $(shell pwd)/../$(PYTHON_PACKAGE)/work-$(ARCH)-$(TCVERSION))

And can be fixed like this:

PYTHON_PACKAGE_ROOT = $(realpath $(shell pwd)/../../spk/$(PYTHON_PACKAGE)/work-$(ARCH)-$(TCVERSION))

In the context of this PR you should fix this too, as it could probably benefit of the new variable $(BASEDIR) defined in spksrc.common.mk.


Another cleanup proposal:
There are several statements $(shell pwd) but some mk files use $(PWD) (defined in spksrc.directories.mk as PWD := $(shell pwd).
Find candidates by grep pwd *.mk.

@hgy59
Copy link
Contributor

hgy59 commented Mar 6, 2024

@th0ma7 thanks for your huge work.

can you please merge current master (or rebase) to include #6028?
I will then build and test all packages for ppc853x-5.2.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 6, 2024

@hgy59 now rebased against master - done.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 6, 2024

regarding the from anywhere there is an issue in spksrc.python.mk.

@hgy59 will definitively look at it.

There are several statements $(shell pwd) but some mk files use $(PWD) (defined in spksrc.directories.mk as PWD := $(shell pwd).

Well, further reading, it should all be replaced with $(CURDIR) which in theory always work in makefiles... something else added to the pile.

Let me know if you find other things that doesn't work on this branch in comparison with master. Considering the depth of the changes I'll take all the testing assistance possible (in hope the testing facility help in finding them).

@hgy59
Copy link
Contributor

hgy59 commented Mar 7, 2024

@th0ma7 one issue found:

cross/openjpeg does not build anymore.

this affects all ffmpeg and the imagemagick package.

*** No targets specified and no makefile found.  Stop.

it is using spksrc.cross-cmake.mk with CMAKE_USE_NINJA = 1 but I have no idea...

@hgy59
Copy link
Contributor

hgy59 commented Mar 7, 2024

@th0ma7 another issue, not related to this PR

spk/gateone fails to build the wheels. This is a python2 only package and since we delcared spk/python2 as BROKEN, we should mark spk/gateone as BROKEN too.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 7, 2024

@hgy59 both meson + cmake using ninja are now fixed, thnx for identifying it. Also fixed gateone while at it.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 10, 2024

There are several statements $(shell pwd) but some mk files use $(PWD) (defined in spksrc.directories.mk as PWD := $(shell pwd).

@hgy59 I've now migrated the framework to using makefile default $(CURDIR). Note that I haven't changed cross/* packages that make use of $(PWD) variable as this would trigger too many packages to be built. As such for the moment I've kept :

# For legacy reasons keep $(PWD) call
PWD := $(CURDIR)

regarding the from anywhere there is an issue in spksrc.python.mk.

Also now fixed.

EDIT: And also, while I had fixed meson + cmake using ninja there where long-standing issues with cmake I wanted to address. Now done which finally allowed me to migrate x265 to using it, accelerating the build significantly as now fully parallel.

@th0ma7 th0ma7 changed the title [DRAFT] Add generic spksrc.supported.mk Add generic spksrc.supported.mk Mar 10, 2024
@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 31, 2024

@hgy59 I'm now ready to merge this PR. I cannot test publishing although code in theory did not change. This will then allow me to pursue work on the python311 update (#6040).

Note that I expect that something, somehow, will not work as expected as this PR is quite invasive. Although the current make test all which build 25GB+ with no error is a good indicative that problems, if any, should be relatively minor.

Any objections before I merge this?

@hgy59
Copy link
Contributor

hgy59 commented Apr 1, 2024

@hgy59 I'm now ready to merge this PR. I cannot test publishing although code in theory did not change. This will then allow me to pursue work on the python311 update (#6040).

Note that I expect that something, somehow, will not work as expected as this PR is quite invasive. Although the current make test all which build 25GB+ with no error is a good indicative that problems, if any, should be relatively minor.

Any objections before I merge this?

@th0ma7 thanks for your huge and great work.
The only issue i found so far, is that after a make native-clean (or clean in native/rust-qoriq) it does not build rust-qoriq when building a package depending on rust for qoriq.
It probably depends on the rust installation related to building the toolchain(s). I often face this issue because I cache the toolchains for all my different work copies but do not cache the native packages. And it was an issue when testing dedicated versions of rust in #6050.
This is all related to rust and should be addressed in a dedicated PR.

BTW I didn't figure out what you really solved with "allow DEPENDS from toolchain"

Copy link
Contributor

@hgy59 hgy59 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@th0ma7 th0ma7 merged commit ecf67dd into SynoCommunity:master Apr 1, 2024
2 checks passed
@th0ma7
Copy link
Contributor Author

th0ma7 commented Apr 1, 2024

thanks for your huge and great work.

Thnx, now fingers crossed nothing breaks.

The only issue i found so far, is that after a make native-clean (or clean in native/rust-qoriq) it does not build rust-qoriq when building a package depending on rust for qoriq. It probably depends on the rust installation related to building the toolchain(s). I often face this issue because I cache the toolchains for all my different work copies but do not cache the native packages. And it was an issue when testing dedicated versions of rust in #6050. This is all related to rust and should be addressed in a dedicated PR.

This is because when rust-qoriq is "installed" it uses rustup toolchain link which creates, well, a link between the distrib/rustup/toolchains/stage2-powerpc-unknown-linux-gnuspe and native/rust-qoriq/work-native/install/usr/local. Therefore when cleaning-up native, it removes that symlink'ed folder. Further, the .rustc_done status file is still showing as complete in the toolchain/syno-qoriq-6.2.4/work thus it will never re-try to install. The way to fix this temporarely is to clean-up the toolchain/syno-qoriq-6.2.4 folder and re-invoke make. The more permanent way would be to force copying over the rust toolchain files into the distrib/rustup folder when building native (instead of the rustup link), so subsequent cleanup doesn't affect the installation.

BTW I didn't figure out what you really solved with "allow DEPENDS from toolchain"

Instead of enforcing within the code a make -C native/rust-qoriq when RUST_BUILD_TOOLCHAIN = 1, I've added the depend functionality to the toolchain makefile so I now use the following (and much simpler) directly into toolchain/syno-qoriq-6.2.4/Makefile:

OPTIONAL_DEPENDS = native/rust-qoriq

ifneq ($(RUST_BUILD_TOOLCHAIN),1)
DEPENDS = native/rust-qoriq
endif

So with this train of thoughts, you could probably now delete the .depend_done and re-invoke make from syno-qoriq-6.2.4 directory to enforce re-installing the rust toolchain.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Apr 1, 2024

Just so we know as reference, invoking a full make test all takes a lot of disk space:

spksrc@spksrc:~$ du -sh all-supported-fix
83G	all-supported-fix

And while at it, @hgy59 thnx for the review.

@hgy59 hgy59 mentioned this pull request Apr 5, 2024
10 tasks
hgy59 added a commit to hgy59/spksrc that referenced this pull request Apr 6, 2024
- make publish-arch-* must include make arch-* (broken by SynoCommunity#6002)
@hgy59 hgy59 mentioned this pull request Apr 6, 2024
6 tasks
hgy59 added a commit that referenced this pull request Apr 6, 2024
- make publish-arch-* must include make arch-* (broken by #6002)
th0ma7 added a commit to th0ma7/spksrc that referenced this pull request Apr 30, 2024
th0ma7 added a commit to th0ma7/spksrc that referenced this pull request Jun 4, 2024
@hgy59 hgy59 mentioned this pull request Jun 10, 2024
6 tasks
th0ma7 added a commit to th0ma7/spksrc that referenced this pull request Jun 11, 2024
hgy59 added a commit to hgy59/spksrc that referenced this pull request Jun 14, 2024
- avoid the make command and the list of all packages in the output
- this is a leftover of PR SynoCommunity#6002
@hgy59 hgy59 mentioned this pull request Jun 14, 2024
1 task
th0ma7 added a commit that referenced this pull request Jun 16, 2024
* kernel.mk: First attempt to fix build issue since #6002

* kernel.mk: Include changes from PR #5790 to include new kernels

* kernel.mk: Update to using actual DSM-7.2 kernel files

* kernel.mk: Filter out unsupported kernel archs

* kernel: Remove DSM-7.1 fixes not applicable anymore on DSM-7.2

* synokernel-cdrom: Bump package version and mark arch as unsupported

* synokernel-usbserial: Bump spk vers. and mark arch as unsupported

* kernel.mk: Yet another round of fixes

* kernel.mk: Other round of fixes simplifying the overall processing

* spk.mk: Oversight missing tc_vars.meson into the spkclean call

* kernel.mk: Other round of fixes and optimizations

* kernel.mk: Fix kernel source tree remove post module build

* kernel.mk: Fix building generic arch -> skip at toolchain

* kernel.mk: Fix kernel build-tree removal after modules are built

* kernel.mk: Remove dependency-kernel-list call

* kernel.mk: Fix re-installing toolchain at clean call

* kernel.mk: Layout fix

* kernel.mk: Remove now unecessary SUPPORTED_KERNEL_VERSIONS

* kernel.mk: Fix building rtd1619b (and partial fix for epyc7002)

* kernel.mk: Fix epyc7002 and rtd1619b kernel configuration path

* synokernel-*: Only remain epyc7002 which does not build yet
hgy59 added a commit that referenced this pull request Jun 17, 2024
* fix output of make dependency-list
- avoid the make command and the list of all packages in the output
- this is a leftover of PR #6002

* remove global dependency-flat target
- dependency-flat is an internal target used dependency-list, dependency-tree and for some internal targets

* znc: fix for dependency-list

* avoid duplicates in dependency-flat

* build-action: fix prepare
- fix rename of spk names: remove only package with exact name from list

* remove dependency-kernel-list
- this was introduced in #5095
- BUT: we must not download kernel sources in prepare.sh, just as we don't do with toolchains
- if we want to optimize the download, we could cache the (extracted) kernel sources as we do for toolchains
  but since the cache is already at its limit, it is not possible to cache kernel sources without
  dropping other cached data.
hgy59 added a commit to hgy59/spksrc that referenced this pull request Oct 13, 2024
- build with CMake (disable ninja)
- mympd build was broken since CMAKE_USE_NINJA default value changed from 0 to 1 in SynoCommunity#6002
@hgy59 hgy59 mentioned this pull request Oct 13, 2024
6 tasks
hgy59 added a commit that referenced this pull request Oct 13, 2024
* mympd: fix build
- build with CMake (disable ninja)
- mympd build was broken since CMAKE_USE_NINJA default value changed from 0 to 1 in #6002

* fix log output of meson build path
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants