Update noarch build #6250

hgy59 · 2024-10-03T21:38:17Z

Description

redesign of noarch build.

noarch packages should only be build for specific TCVERSION, otherwise REQUIRED_MIN_DSM is not evaluated
noarch packages must now define override ARCH=noarch instead of override ARCH=

Replacement for #6249.
Contains a bugfix for #6247 that removed all arch specific builds in github build action.

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

Includes small framework changes
This change requires a documentation update (e.g. Wiki)

- noarch packages should only be build for specific TCVERSION, otherwise REQUIRED_MIN_DSM is not evaluated - noarch packages must now define `override ARCH=noarch` instead of `override ARCH=` - add all noarch packages to build-matrix

hgy59 · 2024-10-03T22:53:38Z

@mreid-tt, @th0ma7 I have added a workaround for non interactive builds.

but I can't run interactive builds:

th0ma7 · 2024-10-03T23:13:03Z

I can provide you assistance but I probably won't be able to look at this before Sunday? Maybe @mreid-tt can help?

.github/workflows/build.yml

mreid-tt · 2024-10-03T23:16:22Z

I'll take a closer look later tonight and perhaps suggest a better fix than the workaround presented.

EDIT: @hgy59, I've taken the liberty of adding a commit to this PR (hope you don't mind) which should resolve the issue you were trying to workaround. Have not done much testing on it.

hgy59 · 2024-10-04T07:44:45Z

EDIT: @hgy59, I've taken the liberty of adding a commit to this PR (hope you don't mind) which should resolve the issue you were trying to workaround. Have not done much testing on it.

@th0ma7 thanks, that makes sense (and it works).

oops, I wanted to thank you @mreid-tt 😄

hgy59 · 2024-10-04T08:07:17Z

Some thoughts for improvement:

In interactive builds we could optimize the prepare step by evaluating the dependencies of the requested package only.
Since we already have a list of arch and noarch packages to build in automated builds, we could disable the noarch builds at all when no noarch packages are found to build (and disable the arch builds, if only noarch packages are to build).

…d builds

mreid-tt · 2024-10-04T09:47:54Z

Since we already have a list of arch and noarch packages to build in automated builds, we could disable the noarch builds at all when no noarch packages are found to build (and disable the arch builds, if only noarch packages are to build).

It's too bad that the prepare.sh doesn't also know the required version of DSM needed for the arch packages. If it did, you could extend your approach to just enable those required DSM versions for the automated builds. As it is, when we move the default from DSM 7.1 to DSM 7.2 in the future, the build.yml changes may get a bit awkward.

hgy59 · 2024-10-04T10:51:11Z

It's too bad that the prepare.sh doesn't also know the required version of DSM needed for the arch packages. If it did, you could extend your approach to just enable those required DSM versions for the automated builds. As it is, when we move the default from DSM 7.1 to DSM 7.2 in the future, the build.yml changes may get a bit awkward.

I will have a look, and guess we can evaluate the min/max dsm requirements in the prepare script.

EDIT:
But this is a little bit shaky.
If we enable the dsm72 builds when we find a min required dsm >= 7.2 for one packge, all packages will be built for 7.2. This is by the fact that the matrix is only built for TCVERSION and ARCH and not package related.
It would need a kind of matrix entry specific list of packages to build...

hgy59 · 2024-10-04T10:55:02Z

Alas the dependeny list generation can't be optimized for interactive package build because there are additional packages evaluated to build (python, ffmpeg, video-driver).

But maybe we could speedup the dependency-list generation by parsing the Makefile(s) instead of using make.

th0ma7 · 2024-10-04T14:16:51Z

Would there be a way to detect that a 7.2 build is needed when min_dsm is set? i.e. jellyfin

mreid-tt · 2024-10-04T16:50:26Z

@hgy59, I love the direction of the last change. Would this check for DSM 7.2 be on a per-package basis? So for example if Jellyfin had an external dependency on a .NET package and that package had a minimum of DSM 7.1 would it evaluate both packages and build for DSM 7.1 and DSM 7.2?

hgy59 · 2024-10-04T17:00:36Z

@hgy59, I love the direction of the last change. Would this check for DSM 7.2 be on a per-package basis? So for example if Jellyfin had an external dependency on a .NET package and that package had a minimum of DSM 7.1 would it evaluate both packages and build for DSM 7.1 and DSM 7.2?

Yes, if any package requires dsm 7.2 (i.e. currently jellyfin only) all packages that are built are built for dsm 7.2 too.

I didn't want to built for DSM 7.2 in the arch-7.1 matrix variant in this case, since the artifacts have the name of the matrix-arch
in the build job it is unknown whether the DSM 7.2 built is manually triggered or comes from the min dsm requirement

mreid-tt · 2024-10-04T17:15:37Z

@hgy59, looks like things built well and as expected. The only suggestion I would have is perhaps the naming of the variables. min_dsm72_packages perhaps could be inc_dsm72_packages or req_dsm72_packages and has_min_dsm72_packages could just be has_dsm72_packages. But that is minor so I'm not pushing for that change.

hgy59 · 2024-10-04T19:28:54Z

@hgy59, looks like things built well and as expected. The only suggestion I would have is perhaps the naming of the variables. min_dsm72_packages perhaps could be inc_dsm72_packages or req_dsm72_packages and has_min_dsm72_packages could just be has_dsm72_packages. But that is minor so I'm not pushing for that change.

I am happy with has_min_dsm72_packages, it is abbr. of has_required_min_dsm72_packages.
The min_dsm72_packages is just the list of such packages.
I will revert the definiion of the related action variable as it is used for logging only.

hgy59 · 2024-10-04T19:37:18Z

@mreid-tt all noarch packages needs an update. I have to push the remaining.

Since you have an open PR for OwnCloud #6225, you will get a merge conflict. I can omit the update of spk/owncloud/Makefile. This will break the build of owncloud on the master branch and you will have to adjust override ARCH in your PR after updating with upstream when this redesign is integrated.

mreid-tt · 2024-10-04T19:50:33Z

Since you have an open PR for OwnCloud #6225, you will get a merge conflict. I can omit the update of spk/owncloud/Makefile. This will break the build of owncloud on the master branch and you will have to adjust override ARCH in your PR after updating with upstream when this redesign is integrated.

I can push an update to the ownCloud PR to change the override ARCH in the Makefile once this PR is merged and then do a rebase. The PR is just about done, just needed feedback from you guys on the proposed migration strategy.

EDIT: Be sure to revert the Jellyfin test before merging this PR.

th0ma7 · 2024-10-05T15:13:10Z

@mreid-tt and @hgy59 i'm guessing changes are still on-going.

My short PR here didn't kicked-off the build as expected #6253

actual build: https://github.com/SynoCommunity/spksrc/actions/runs/11194152942/job/31120397567

EDIT: Note that this PR is zero urgent and can be used to test-out github-action work as you see fit.

hgy59 · 2024-10-05T16:06:41Z

@th0ma7 none of the builds work since #6247 was merged. only noarch works.
It will be fixed with b296272 of this PR, but I suppose we need an immediate fix...

This reverts commit ebe8946.

- introduced to have build log files for noarch packages

hgy59 · 2024-10-05T16:18:27Z

@mreid-tt @th0ma7 shall we merge this?

it has some issues that could be fixed later

the syno-magnet package is built successfully but it is listed under builds with ERROR in the build status
the following noarch packages fail to build:
- plexivity
- saltpad
- subliminal

th0ma7 · 2024-10-05T16:21:36Z

One way or the other, I don't mind much as i won't have cycles to work on things for a week or two. So I'm ok at waiting a little longer so build fixes gets in.

This reverts commit 2cfc738.

- SPK_NAME must not use variable

hgy59 · 2024-10-05T18:55:06Z

all issues fixed, ready to merge.

BTW the dependency-list optimization will come in a future PR.

mreid-tt · 2024-10-05T19:10:29Z

LGTM

mreid-tt · 2024-10-05T19:22:12Z

@mreid-tt and @hgy59 i'm guessing changes are still on-going.

My short PR here didn't kicked-off the build as expected #6253

@th0ma7, you should now be able to re-base your PR and it should build correctly.

* Update ownCloud to v10.15.0 * Refine service setup scripts * Fix backup validation * Fix upgrade check * Optimise initial ownCloud setup * Use MySQL for initial setup * Set noarch build (#6250)

hgy59 added 2 commits October 3, 2024 23:35

redesign build of noarch packages

10b11e6

- noarch packages should only be build for specific TCVERSION, otherwise REQUIRED_MIN_DSM is not evaluated - noarch packages must now define `override ARCH=noarch` instead of `override ARCH=` - add all noarch packages to build-matrix

update noarch for adminer

ea4ca21

hgy59 mentioned this pull request Oct 3, 2024

Framework: Add Configurable Build Architectures and Input Flags #6247

Merged

10 tasks

workaround to build noarch/dsm71/dsm62 in non interactive workflow

c34577a

mreid-tt reviewed Oct 3, 2024

View reviewed changes

.github/workflows/build.yml Outdated Show resolved Hide resolved

Fix for non interactive workflow

b296272

add variables to define defaults for noarch/arch packages in automate…

7bd5bb5

…d builds

hgy59 added 2 commits October 4, 2024 13:06

rename github job "Generate Matrix"

601ac22

update some additional noarch packages

7852b77

add dsm72 to matrix in automated builds when package requires it

8daf14e

temp. change to force build of jellyfin

ebe8946

revert min_dsm72_packages as output variable

2f7682b

update more noarch packages

4a2546d

hgy59 added 2 commits October 5, 2024 18:10

Revert "temp. change to force build of jellyfin"

c998713

This reverts commit ebe8946.

add targets to call make noarch-{TCVERSION}

2c73b7b

- introduced to have build log files for noarch packages

hgy59 added 5 commits October 5, 2024 19:00

declare python2 dependent packages as broken

c27c532

set adminer as default package for interactive builds

cfd120a

temp. disable google-fonts package

2cfc738

Revert "temp. disable google-fonts package"

c9b1301

This reverts commit 2cfc738.

syno-magnet: fix github build

a59a81c

- SPK_NAME must not use variable

hgy59 requested a review from mreid-tt October 5, 2024 18:54

mreid-tt approved these changes Oct 5, 2024

View reviewed changes

mreid-tt mentioned this pull request Oct 5, 2024

Framework: Prevent noarch builds without DSM 5.2 #6249

Closed

10 tasks

hgy59 merged commit 099c5dd into SynoCommunity:master Oct 5, 2024
7 checks passed

hgy59 deleted the update_noarch_build branch October 5, 2024 19:11

mreid-tt added a commit to mreid-tt/spksrc that referenced this pull request Oct 5, 2024

Set noarch build (SynoCommunity#6250)

a79b926

mreid-tt added the framework label Jan 2, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update noarch build #6250

Update noarch build #6250

hgy59 commented Oct 3, 2024 •

edited by mreid-tt

Loading

hgy59 commented Oct 3, 2024

th0ma7 commented Oct 3, 2024

mreid-tt commented Oct 3, 2024 •

edited

Loading

hgy59 commented Oct 4, 2024 •

edited

Loading

hgy59 commented Oct 4, 2024

mreid-tt commented Oct 4, 2024

hgy59 commented Oct 4, 2024 •

edited

Loading

hgy59 commented Oct 4, 2024

th0ma7 commented Oct 4, 2024

mreid-tt commented Oct 4, 2024

hgy59 commented Oct 4, 2024

mreid-tt commented Oct 4, 2024 •

edited

Loading

hgy59 commented Oct 4, 2024

hgy59 commented Oct 4, 2024

mreid-tt commented Oct 4, 2024 •

edited

Loading

th0ma7 commented Oct 5, 2024 •

edited

Loading

hgy59 commented Oct 5, 2024

hgy59 commented Oct 5, 2024

th0ma7 commented Oct 5, 2024

hgy59 commented Oct 5, 2024

mreid-tt commented Oct 5, 2024

mreid-tt commented Oct 5, 2024

Update noarch build #6250

Update noarch build #6250

Conversation

hgy59 commented Oct 3, 2024 • edited by mreid-tt Loading

Description

Checklist

Type of change

hgy59 commented Oct 3, 2024

th0ma7 commented Oct 3, 2024

mreid-tt commented Oct 3, 2024 • edited Loading

hgy59 commented Oct 4, 2024 • edited Loading

hgy59 commented Oct 4, 2024

mreid-tt commented Oct 4, 2024

hgy59 commented Oct 4, 2024 • edited Loading

hgy59 commented Oct 4, 2024

th0ma7 commented Oct 4, 2024

mreid-tt commented Oct 4, 2024

hgy59 commented Oct 4, 2024

mreid-tt commented Oct 4, 2024 • edited Loading

hgy59 commented Oct 4, 2024

hgy59 commented Oct 4, 2024

mreid-tt commented Oct 4, 2024 • edited Loading

th0ma7 commented Oct 5, 2024 • edited Loading

hgy59 commented Oct 5, 2024

hgy59 commented Oct 5, 2024

th0ma7 commented Oct 5, 2024

hgy59 commented Oct 5, 2024

mreid-tt commented Oct 5, 2024

mreid-tt commented Oct 5, 2024

hgy59 commented Oct 3, 2024 •

edited by mreid-tt

Loading

mreid-tt commented Oct 3, 2024 •

edited

Loading

hgy59 commented Oct 4, 2024 •

edited

Loading

hgy59 commented Oct 4, 2024 •

edited

Loading

mreid-tt commented Oct 4, 2024 •

edited

Loading

mreid-tt commented Oct 4, 2024 •

edited

Loading

th0ma7 commented Oct 5, 2024 •

edited

Loading