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

Isolate DSM5 branch #3917

Closed
wants to merge 2 commits into from
Closed

Isolate DSM5 branch #3917

wants to merge 2 commits into from

Conversation

th0ma7
Copy link
Contributor

@th0ma7 th0ma7 commented Mar 10, 2020

Motivation: Isolate DSM5 from master.
Linked issues: Relates to discussion from PR #3908 ans #3919

Checklist

  • Build rule all-supported completed successfully
  • Package upgrade completed successfully
  • New installation of package completed successfully

@hgy59
Copy link
Contributor

hgy59 commented Mar 10, 2020

Two suggestions

  1. keep all < 5.0 versions to not loose them (will result in a "pre DSM 6" instead of a "DSM 5 only" branch)
  2. the SRM-versions (1.x) can be removed too, since these are DSM 6.x compatible

@ymartin59
Copy link
Contributor

@hgy59 I am not satisfied about the way I added SRM toolchains... which prevents to re-use DSM_REQUIRED variable in package... Any proposal for theses?

@ymartin59
Copy link
Contributor

@th0ma7 WARNING this PR targets "master" branch which is not "expected"

@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 10, 2020

@hgy59 I am not satisfied about the way I added SRM toolchains... which prevents to re-use DSM_REQUIRED variable in package... Any proposal for theses?

Ideas on top of my head:

  1. make setup now also returns a SRM variable:
$ make setup
Creating local configuration "local.mk"...
Setting default toolchain version to DSM-6.1 & SRM-1.1
  1. create a tree under toolchain/dsm|srm to split both types
  2. create a SRM-1.1 sort of "compatible to" >= DSM-6.1
  3. add make all-supported-dsm & make all-supported-srm on top of make all-supported

@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 10, 2020

@th0ma7 WARNING this PR targets "master" branch which is not "expected"

@ymartin59 Thnx for the WARNING!!
I have no intent for now to merge anything, this is more preview for discussions at this stage.
(and anyway my git skillset ain't there yet for such a change).

Would it be possible to create a new branch, perhaps named dsm-legacy ? I could then work & merge this type of material into afterwards. Eventually this could lead into migrating dropped versions such as DSM-6, DSM-6.1 once we add DSM-6.2.2 and DSM-7...

@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 10, 2020

@hgy59

Two suggestions
1. keep all < 5.0 versions to not loose them (will result in a "pre DSM 6" instead of a "DSM 5 only" branch)

I strongly recomment something like dsm-legacy (see previous comment on this)

2. the SRM-versions (1.x) can be removed too, since these are DSM 6.x compatible

agreed, will ensure to remove them once a new branch gets created so my work doesn't apply on master but rather on the new branch

@hgy59
Copy link
Contributor

hgy59 commented Mar 10, 2020

@hgy59 I am not satisfied about the way I added SRM toolchains... which prevents to re-use DSM_REQUIRED variable in package... Any proposal for theses?

I already introduced a different variable SRM_REQUIRED to define a minimal SRM-Version and separated DSM_REQUIRED to match DSM versions only.

see commit 5a67e15

@ymartin59
Copy link
Contributor

Many packages properly build for DSM 5.2 and DSM 6 without "source change"... Moving DSM 5 out-of master branch will require to maintain these packages in both branches to deliver them.

So I agree to "move out" DSM < 5 toolchains from master but would keep DSM 5.2 for a while.

Another point, at least one arch has not received DSM 6.2 but only DSM 6.1 update. For most packages compilation, it changes nothing to build against DSM 6.1 toolchain instead of DSM 6.2.

For my own "minimal effort", I select to build and package either for DSM 5.2 (all archs) or for DSM 6.1 according to source requirements.
It would be easier to decide what archs to target if we had some statistics about downloads and installations.

@hgy59
Copy link
Contributor

hgy59 commented Mar 11, 2020

It would be easier to decide what archs to target if we had some statistics about downloads and installations.

for this, we need download counters in spkrepo

@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 11, 2020

@ymartin59

So I agree to "move out" DSM < 5 toolchains from master but would keep DSM 5.2 for a while.

Makes sense.

Another point, at least one arch has not received DSM 6.2 but only DSM 6.1 update. For most packages compilation, it changes nothing to build against DSM 6.1 toolchain instead of DSM 6.2.

From a compilation point of view, fully agreed. Although from a security perspective probably not as I would assume that some fixes went into both the kernel & compiler to address various intel flaws resulting in 6.2.2 release.

For my own "minimal effort", I select to build and package either for DSM 5.2 (all archs) or for DSM 6.1 according to source requirements.

Would make all-supported self-adjust and use latest for each "supported" archs?

Also, this will not address my original PR to drop ppc853x arch from default supported build process #3908

@hgy59
Copy link
Contributor

hgy59 commented Mar 12, 2020

Many packages properly build for DSM 5.2 and DSM 6 without "source change"... Moving DSM 5 out-of master branch will require to maintain these packages in both branches to deliver them.

The idea to isolate DSM < 6 into a separate branch is to not maintain and deliver such versions anymore.
We should keep noarch packages compatible for DSM < 6 but all arch specific packages we should discontinue to officially support.
For reference and manually builds by interested users we separate such a branch.

The following we want to get rid of.

  • keep the shell scripts compatible with sh shell
  • regard pre DSM 6 users and groups in installation and update code
  • maintain migration from DSM 5 to DSM 6 on package upgrades

As deadline to accomplish, I see the day DSM 7 gets available for building thirdparty packages (alpha, beta, pre-release - what ever will get available first), and this may take another couple of months from now.
This corresponds with keeping DSM 5.2 (and ppc853x) for a while ...

@ymartin59
Copy link
Contributor

ymartin59 commented Mar 15, 2020

From a compilation point of view, fully agreed. Although from a security perspective probably not as I would assume that some fixes went into both the kernel & compiler to address various intel flaws resulting in 6.2.2 release.

@th0ma7 I see no security issue here. Compilation is done against kernel headers, (not implementation version for runtime, Synology has to take care of) and it is rare to hear about security fixes in compiler itself.

Synology packages both toolchain (compiler and kernel headers) and SDK (with full DSM libraries' headers in addition to toolchain) but as far as I noticed until now, only SDK really differs between DSM minor versions.

By the way, we may compare compiler versions in toolchains for each architecture and DSM minor versions but my opinion is that Synology is quite lazy with it and never update it.

@hgy59
Copy link
Contributor

hgy59 commented Mar 15, 2020

By the way, we may compare compiler versions in toolchains for each architecture and DSM minor versions but my opinion is that Synology is quite lazy with it and never update it.

Yes, we may compare (and document) the compiler and glibc versions of the toolchains.

With this, we could avoid:

# archs lacking C++11 compiler (ppc archs except QorIQ)
UNSUPPORTED_ARCHS = powerpc ppc824x ppc853x ppc854x

And write something like this instead:

MIN_GCC_VERS = 4.6

@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 15, 2020

Ok, let's try to recap and align future work on this, actions are:

  1. cleanup/delete old dsm6 branch
  2. create new branch for older DSM versions
  3. branch name proposals: dsm-legacy (any other?)
  4. Add to framework functionality for something similar to MIN_GCC_VERS = 4.6
  5. enable "download counter" Support for "Download count" in DSM 6.2 spkrepo#22 to make a decision based on factual data

To be refined, options are:

Seems to lean towards keeping DSM 5.2, DSM 6.1, SRM 1.x and ppc853x to master until DSM 7 and use UNSUPPORTED_ARCH #3873 (comment) where needed so make all-supported always work, perhaps in conjunction where needed of a new variable made available within the spksrc framework: MIN_GCC_VERS = 4.6

Unanswered question: Should we only keep "latest" DSM of each of each starting with 5.2? This would then mean also migrating to legacy version 6.0.2 when 6.1 exists (likewise for 6.1 by adding 6.2.2)?

As for the question of differences on the toolchain from 6.1 vs 6.2. I had a quick look at it and indeed they do look quite similar. Although libc was updated between the two. The update dates back from 2017 which mean that no other recent mitigations where added since then (possibly around the time of mitigate speculative execution side-channel):

-rwxr-xr-x 1 spksrc spksrc 1993008 aoû 28  2017 sys-root/lib/libc-2.20-2014.11.so*

vs 6.1:

-rwxr-xr-x 1 spksrc spksrc 1980696 aoû  3  2016 sys-root/lib/libc-2.20-2014.11.so*

@ymartin59
Copy link
Contributor

I have just deleted dsm6 branch from master repository as its goal was to migration packages to DSM 6 support. Job is over. Anyway I still have a copy of that reference in my own forks if needed to restore later.

As far as I know (so to be confirmed), all DSM 6.0 architectures have received updates to 6.1.x, so if I am right, there is no trouble to expect removing all DSM 6.0.2 toolchains from master.
And only qoriq arch has not received update from 6.1 to 6.2

I am OK with proposals but I really wonder what is "motivation" to cleanup master... maintenance runs that way since years and it is convenient for anyone to get latest package code and just "try to build with luck" for his target architecture, even if no longer supported (either too old, or simply not updated)

If we decide to create a legacy branch with current master state before cleanup, it may mean we should "maintain it", of instance moving DSM 5.2 toolchains from master to legacy... but what should we do with related/unrelated "package code"?

Notice I have created tag pre-dsm6 on master before merging dsm6 so that anyone may retrieve packages and toolchains for DSM <= 5.2

@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 15, 2020

I am OK with proposals but I really wonder what is "motivation" to cleanup master... maintenance runs that way since years and it is convenient for anyone to get latest package code and just "try to build with luck" for his target architecture, even if no longer supported (either too old, or simply not updated)

Original motivation came from here #3908 (comment) . Personally I think it's a great opportunity to clean-up the code for eventual push on dsm7.

If we decide to create a legacy branch with current master state before cleanup, it may mean we should "maintain it", of instance moving DSM 5.2 toolchains from master to legacy... but what should we do with related/unrelated "package code"?

Interesting question. There may be a tipping point where some specific arch or DSM version becomes unsupported. Download counter could help on this but mostly for updated packages against such arch & version. My understanding is that current proposal would keep for now 5.2, 6.1 and 6.2.2 and move everything else to legacy which in turn for me would mean "unsupported" but available if people still wish to play with it.

@hgy59
Copy link
Contributor

hgy59 commented Mar 15, 2020

I would not call such a branch legacy. I would propose to name the branch pre-dsm6 (and not to keep DSM 5.2 on the master).
When we call the branch legacy, what will we call the pre-dsm7 branch in the far future, when DSM 6 has reached it's end of life?

The main motivation for me on this topic is to cleanup the installer code from the pre dsm 6 user management and the dsm 5 to 6 migration code.
As I expect required DSM 6 to 7 migrations, it would get too complex to support DSM 5 to 6 to 7 migration in each package.

@ymartin59
Copy link
Contributor

for me would mean "unsupported" but available if people still wish to play with it

That is a point of view from "toolchains support", but what about "package code" support?

In case recent "package code" still works on a no-longer supported toolchain, it becomes more difficult to "play with it", except if we decide and maintain that "unsupported" branch - which means replicating any package code update there too.

From my point of view, it is "useless effort", even if package-code-only PR may merge without conflict/work on that "unsupported" branch, it may be simply broken if no tests are driven. Without speaking about PR mixing package code and toolchain-or-framework code, and additional process for merge, which may simple lead to forget-or-mistake to replicate to "unsupported" branch.

@ymartin59
Copy link
Contributor

Notice that "pre dsm 6" user management is included in generic installer framework support. So as far as a package uses generic installer, it should work as-is on DSM 4 or DSM 5.

It is correct that some package service-setup.sh functions contain code for dsm 5 to 6 migration BUT after 2 years, we may consider anyone willing/able to upgrade already did, so it sounds me relevant to discard that specific 5-to-6 migration code when writing any 6-to-7 code if needed.

And "with luck", first package installation (without upgrade) should still works on unsupported target DSM version someone has just built for his own needs.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 15, 2020

Really interesting discussion and also clearly we are not entirely in agreement with expected outcome. Looking at things differently, what if we where to define a rule of thumb for spksrc such as, using a rule of two:

  1. Keep the two latest major versions supported for each arch (ex: 5.x + 6.x where applicable)
  2. Keep the two latest minor versions of latest major version (e.g: arch dependant, 6.0.2/6.1 or 6.1/6.2.2)
  3. Once an arch is behind both rule 1 & 2, move to unsupported (how to be defined)
  4. Code specific to versions not included by rule 1 & 2 to be discarded
  5. make all-supported matches rule 1 & 2 otherwise must have a strict UNSUPPORTED_ARCH, REQUIRED_DSM, REQUIRED_SRM, OS_MIN_VERS

Otherwise, current ideas being shared (and please correct me if I'm wrong):

@th0ma7

  • Add 6.2.2 DSM
  • Move older arch and mark them as unsupported into a legacy tree. To me this also means no more updates & backports, code to be frozen.
  • Eventually migrate "to become unsupported" trees into that same legacy branch (e.g. possibly 5.2 once 7.x is considered stable)

@hgy59

  • Create a pre-dsm6 tree and move everything prior to dsm6 under that tree
  • eventually create a pre-dsm7 tree for next upcoming release

@ymartin59

  • Not in disagreement with the general clean-up idea but unsure of it's actual value
  • expect issue with maintenance of "unsupported" branch
  • discard specific 5-to-6 migration code when writing any 6-to-7 code

@ymartin59
Copy link
Contributor

Just an idea: if toolchains, framework and package code were disjointed into separate Git repositories, we would have more options how/what to branch, allowing to mix but again more difficult to manage.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 15, 2020

Just an idea: if toolchains, framework and package code were disjointed into separate Git repositories, we would have more options how/what to branch, allowing to mix but again more difficult to manage.

I fear this would be become way harder to maintain... On the other hand, creating a branch for each toolchains+kernel (single containing both) such as:

  • master (current dsm6.x
  • toolchain-dsm5
  • toolchain-dsm4

So action could be to git checkout master, something like "merge" branch associated to needed toolchain to compile against it...

@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 15, 2020

Further thinking about it.... Looking at things with a workflow perspective and doing additional mileage from your previous comment @ymartin59 :

  1. Assuming master has the latest DSM toolkit & kernel, always
  2. Default allows building against latest DSM, always (e.g. make all-supported)
  3. All legacy toolkits & kernels in branches, tags or parallel git, using DSM version basis (e.g. dsm4, dsm5, etc)
  4. Ability to checkout (and perhaps bind-mount or something) a legacy toolkit version to build against it - to do so under toolkit it could be subdivided by dsm4, dsm5, dsm6 directories, where we create under both a subdir for kernel & toolkit to ease bind-mounts?

It may sound a bit crazy but doing so might allow meeting all/most comments made previously.... ?

@publicarray
Copy link
Member

Can I propose to create a new repository for legacy branches? it makes for smaller repository sizes and makes it super clear form a users pint of view that they are deprecated but are keep for historical reasons.

@ymartin59
Copy link
Contributor

ymartin59 commented Apr 12, 2020

@publicarray Creating a "fork" has similar troubles as "creating a branch" in existing spksrc... that is first to "not maintain" content with packages/frameworks. By the way, DSM 7 is coming and it is too much effort to maintain three targets DSM 5, 6 and 7. And I really doubt that are many DSM 5 still running out there. Our effort has to be put in migrating current packages to DSM 7, when we know how that version looks like.

@hgy59
Copy link
Contributor

hgy59 commented Apr 12, 2020

For my own "minimal effort", I select to build and package either for DSM 5.2 (all archs) or for DSM 6.1 according to source requirements.
It would be easier to decide what archs to target if we had some statistics about downloads and installations.

What about to separate all noarch packages to a different spk folder like noarch-spk? As these do not depend on a toolchain they will remain on the main branch for all DSM versions already supported.
With this approach a new target all-noarch can be introduced too.

@ymartin59
Copy link
Contributor

@th0ma7 @hgy59 @publicarray I would let all toolchains "in place"... that cost nothing and it still allows users to build packages for their own usage on old arch/dsm versions if interested - with same concept as "diyspk/". So I propose to close without merging.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Jun 23, 2020

@th0ma7 @hgy59 @publicarray I would let all toolchains "in place"... that cost nothing and it still allows users to build packages for their own usage on old arch/dsm versions if interested - with same concept as "diyspk/". So I propose to close without merging.

Definitively this thread will end-up being closed without merging. This really was more about the asking ourselves what do we want to do (or not) about it. Personally I still think we should create a legacy branch and move everything < 6.1 down there, even 5.2. This would clean-up the room for upcoming 7.x and sort of freeze that side of the code.

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