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

make all-supported and all-default #4200

Merged
merged 22 commits into from
Oct 12, 2020
Merged

Conversation

th0ma7
Copy link
Contributor

@th0ma7 th0ma7 commented Oct 1, 2020

Motivation: With the addition of all latest toolchains the make all-supported behavior expects to always use latest available version available for all targets. This PR propose the following changes:

  • revert to the previous behavior where all-supported uses DSM-6.1
  • adjust code behavior to allow all-supported to be enhanced with additional specific targets we intend to support
  • add a new target called all-default which always use the latest version available
  • bump legacy version to 5.2 instead of current 4.3 and include SRM-1.1
  • remove all-archs-latest as currently broken and now superseded by all-default

Linked PR: #4195

Checklist

  • Build rule all-supported completed successfully
  • Build rule all-default completed successfully
  • Build rule all-legacy completed successfully

Latest targets uses latest version available:

$ make all-latest
├── work-88f6281-6.2.2
├── work-aarch64-6.2.2
├── work-armv7-6.2.2
├── work-evansport-6.2.2
├── work-hi3535-6.2.2
├── work-ppc853x-5.2
├── work-qoriq-6.2.2
└── work-x64-6.2.2

Latest targets when using GO environment:

$ make all-latest
├── work-88f6281-6.2.2
├── work-aarch64-6.2.2
├── work-alpine-6.2.2
├── work-armada370-6.2.2
├── work-armada375-6.2.2
├── work-armada38x-6.2.2
├── work-armadaxp-6.2.2
├── work-comcerto2k-6.2.2
├── work-dakota-1.2
├── work-evansport-6.2.2
├── work-hi3535-6.2.2
├── work-ipq806x-1.2
├── work-monaco-6.2.2
├── work-northstarplus-1.2
└── work-x64-6.2.2

Legacy targets now uses DSM-5.2 + SRM-1.1 by default:

$ make all-legacy
├── work-88f6281-5.2
├── work-alpine-5.2
├── work-armada370-5.2
├── work-armada375-5.2
├── work-armada38x-5.2
├── work-armadaxp-5.2
├── work-avoton-5.2
├── work-braswell-5.2
├── work-bromolow-5.2
├── work-cedarview-5.2
├── work-comcerto2k-5.2
├── work-evansport-5.2
├── work-ipq806x-1.1
├── work-monaco-5.2
├── work-northstarplus-1.1
├── work-ppc853x-5.2
├── work-qoriq-5.2
├── work-x64-5.2
└── work-x86-5.2

Supported targets uses DSM-6.1 by default with extras for hardware acceleration purpose (apollolake, purley, geminilake):

all-supported
├── work-88f6281-6.1
├── work-aarch64-6.1
├── work-apollolake-6.2
├── work-armv7-6.1
├── work-evansport-6.1
├── work-geminilake-6.2
├── work-hi3535-6.1
├── work-ppc853x-5.2
├── work-purley-6.2
├── work-qoriq-6.1
└── work-x64-6.1

Supported targets when using GO environment:

├── work-88f6281-6.1
├── work-aarch64-6.1
├── work-alpine-6.1
├── work-apollolake-6.2
├── work-armada370-6.1
├── work-armada375-6.1
├── work-armada38x-6.1
├── work-armadaxp-6.1
├── work-comcerto2k-6.1
├── work-dakota-1.2
├── work-evansport-6.1
├── work-geminilake-6.2
├── work-hi3535-6.1
├── work-ipq806x-1.2
├── work-monaco-6.1
├── work-northstarplus-1.2
├── work-purley-6.2
└── work-x64-6.1

@th0ma7 th0ma7 requested a review from hgy59 October 1, 2020 00:01
@th0ma7
Copy link
Contributor Author

th0ma7 commented Oct 1, 2020

@hgy59 in reference to our chat on PR #4195 in particular #4195 (comment)

I believe all-general is still of use. Perhaps renaming it to something closer to all-github or all-action ?!?! But that's only semantics.

@th0ma7 th0ma7 self-assigned this Oct 1, 2020
@hgy59
Copy link
Contributor

hgy59 commented Oct 1, 2020

@hgy59 in reference to our chat on PR #4195 in particular #4195 (comment)

I believe all-general is still of use. Perhaps renaming it to something closer to all-github or all-action ?!?! But that's only semantics.

I don't really like additional all-* targets. It should be as simple as all-supported was - to build the archs and TC versions that usually are built to publish to the repo. The github build action was designed to do the same as all-supported does.
May be a better approach would be to adjust the all-supported rule to behave as before the 6.2.x toolchains were added.

And don't forget the noarch packages. AFAIK noarch packages build with os_min_ver=5.0 (os_min_ver defines whether a package is installable on a certain DSM version).

To solve this, we must consider (and evaluate) the correlations of TC version and os_min_ver and whether these are specefic for the packages to build.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Oct 1, 2020

The github build action was designed to do the same as all-supported does.

It never totally did, for instance the armv7-6.1 and armv7-1.2 in github action are not present into the all-supported builds.

May be a better approach would be to adjust the all-supported rule to behave as before the 6.2.x toolchains were added.

This is exactly the intent of this PR:

  • all-supported : back to the exact same behavior as before (e.g. defaults to DSM6.1 + ppc853x-5.2)
  • all-default (NEW): uses the default latest version available for each arches (based on previous all-supported behavior)

And don't forget the noarch packages. AFAIK noarch packages build with os_min_ver=5.0 (os_min_ver defines whether a package is installable on a certain DSM version).

To solve this, we must consider (and evaluate) the correlations of TC version and os_min_ver and whether these are specefic for the packages to build.

This PR currently does not try to address this but rather only bring back the previous output of all-supported.

If I follow you correctly, perhaps related to this there was the DSM vs SRM issue related to minimum version that does not really work currently (old thread reference #3917 (comment))?

And lastly, as far as all-general in your other PR, yes, no, maybe? I thought this was to replicate the github-action build output which differs from the all-supported output. If we end-up further aligning github-action with all-supported output then perhaps it then become less useful?

@th0ma7 th0ma7 requested a review from ymartin59 October 2, 2020 13:09
@th0ma7 th0ma7 added framework enhancement request to enhance existing package status/needs-review labels Oct 2, 2020
@hgy59
Copy link
Contributor

hgy59 commented Oct 3, 2020

@th0ma7 what about usage of generic archs - at least for all-supported?

This will reduce the number of packages:
all-supported
├── work-88f6281-6.1
├── work-aarch64-6.1
├── work-alpine-6.1 => work-armv7-6.1
├── work-apollolake-6.2 => work-x64-6.1
├── work-armada370-6.1 => work-armv7-6.1
├── work-armada375-6.1 => work-armv7-6.1
├── work-armada38x-6.1 => work-armv7-6.1
├── work-armadaxp-6.1 => work-armv7-6.1
├── work-comcerto2k-6.1 => work-armv7-6.1
├── work-dakota-1.2 => work-armv7-1.2
├── work-evansport-6.1
├── work-geminilake-6.2 => work-x64-6.1
├── work-hi3535-6.1 => work-armv7-6.1
├── work-ipq806x-1.2 => work-armv7-1.2
├── work-monaco-6.1 => work-armv7-6.1
├── work-northstarplus-1.2 => work-armv7-1.2
├── work-ppc853x-5.2
├── work-purley-6.2 => work-x64-6.1
├── work-qoriq-6.1
└── work-x64-6.1

to have only
all-supported
├── work-88f6281-6.1
├── work-aarch64-6.1
├── work-armv7-6.1
├── work-armv7-1.2
├── work-evansport-6.1
├── work-ppc853x-5.2
├── work-qoriq-6.1
└── work-x64-6.1

@hgy59
Copy link
Contributor

hgy59 commented Oct 3, 2020

@th0ma7 What about renaming all-default to all-latest as this is what it is for (which always use the latest version available).
Or maybe a better naming would be default-latest because not all archs are built but only the defaults.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Oct 3, 2020

@hgy59 good ideas!

  • I'm okay with renaming all-default to all-latest which would be somewhat similar to previous "latest" behavour and naming.
  • As for the generic I must not change the all-supported as this is really what is needed for publishing packages along with officially supported archs. A new all-generic call could be added later on in a subsequent PR unless I find cycles to look at it shortly. I'll come back to you on this one in particular as I see benefit to simulate the github-action output and I like the generic wording as well.

@hgy59
Copy link
Contributor

hgy59 commented Oct 3, 2020

As for the generic I must not change the all-supported as this is really what is needed for publishing packages along with officially supported archs

I think that @ymartin59 uses the generic x64-6.1, aarch64-6.1 and armv7-6.1 to publish packages.
And for packages built with go the additional hi3535-6.1 is published.

@ymartin59
Copy link
Contributor

@th0ma7
Copy link
Contributor Author

th0ma7 commented Oct 5, 2020

I found out something rather interesting where the ifeq for GO was actually reverted and should have been ifneq all along solving the ARMv7 builds for all targets! I've updated the output for every targets including with GO support.

@hgy59 this should be closer to what you were expecting and following @ymartin59 update policy.

Note that I did includ additional targets in all-supported intentionally for the need of newer x64 platform hardware acceleration where default toolchain used isn't sufficient for newer targets. Perhaps this could be reviewed at another time with DSM7 arrival and dropping really old platforms.

@th0ma7 th0ma7 requested review from ymartin59 and hgy59 and removed request for hgy59 October 5, 2020 10:41
@th0ma7
Copy link
Contributor Author

th0ma7 commented Oct 8, 2020

@hgy59 and @ymartin59 I finally manage to add the last remaining bit I wanted on this PR - parallelization!

I've revamped the code for make all-supported and make all-latest in order for it to parallelize using -j option.
This does not affect publishing as .NOTPARALLEL is being set and forces running in serial mode.

There is a major bunch of packages to be published post-PR #4155 and I believe this last addition will be quite useful.

Things look really good from my testing but I would really much appreciate if you could test the code before merging in order to shake any remaining bugs. And yes, one odd expected behavior which always existed is the wget download portion in parallel mode that may fail due to multiple instances running concurrently.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Oct 8, 2020

I ended figuring out a way to manage concurrent wget so parallel builds runs smoothly.
Again, testing would be much appreciated.
Thnx in advance.

@th0ma7 th0ma7 mentioned this pull request Oct 10, 2020
55 tasks
@ymartin59 ymartin59 mentioned this pull request Oct 10, 2020
3 tasks
@th0ma7
Copy link
Contributor Author

th0ma7 commented Oct 10, 2020

There is still one special case to handle that I'd like to work on on a subsequent PR: native/xxx builds.
That portion needs to be serialized and has always failed when invoking parallel builds as many parallel threads tries to interject themselves in the unique native build. I already have a good idea on how to resolve that but suggesting to address that later.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Oct 11, 2020

It took me quite a few trial & errors but I managed to find a way to force to serialize "native" then parallelize arch* builds.
Again, a bit of testing @ymartin59 and @hgy59 or any other to confirm it works all-right would be appreciated but my initial tests tends toward working quite well :)

@th0ma7 th0ma7 merged commit 678435a into SynoCommunity:master Oct 12, 2020
@th0ma7 th0ma7 deleted the all-supported branch October 12, 2020 12:44
@hgy59 hgy59 mentioned this pull request Dec 19, 2020
6 tasks
@hgy59 hgy59 mentioned this pull request Sep 15, 2023
6 tasks
AlexPresso pushed a commit to AlexPresso/spksrc that referenced this pull request Jan 30, 2025
* mk: Bump legacy archs from 4.3 to 5.2

* mk: Force all-supported to match expected supported arches

* mk: Revamp creation of supported, legacy archs lists

* toolchains: Missing 88f6281 and aarch64 DSM 6.2 and 6.2.2 targets

* mk: Include SRM-1.1 into all-legacy targets

* kernel: Add DSM-6.2 and 6.2.2 kernels for 88f6281 arch

* toolchain: Add ARMv7 for DSMi-6.2 and DSM-6.2.2

* spk.mk: make all-supported and latest are now parallel

* download.mk: use flock for concurrent wget for parallel builds

* spk.mk: Force building native in serial mode first then parallel

* tc.mk: Set TC_OS_MIN_VER and avoid TC_FIRMWARE (from @hgy59)

* tc.mk: Add DSM-6.2.3 build number 25423

* tc.mk: If TC_FIRMWARE is unset use TC_OS_MIN_VER
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement request to enhance existing package framework status/needs-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants