-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
SRM enhancements #6717
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
base: master
Are you sure you want to change the base?
SRM enhancements #6717
Conversation
- prevent to compare SRM version with REQUIRED_MIN_DSM version
- add SRM 1.3 to dotnet arch checks - remove unavailable TCVERSION 6.1 - cleanup for DOTNET_SERVARR_ARCHS = 2 (remove obsolete definitions already covered by ARMv7_ARCHS)
- sources are hosted as synocommunity github releases - sources are extracted from synology gpl sources since those are too large (we have similar approach for SRM 1.3 toolchains)
- add REQUIRED_MIN_SRM = 1.3 to all supported packages with REQUIRE_MIN_DSM = 7 and 7.1 - disable support for SRM with REQUIRED_MIN_SRM = 3.0 for packages with missing spk dependencies
- adjust tc_vars.mk generation for SRM 1.3 toolchains having gfortran
- fix cross/libudev_204 - add patch to fix build for newer glibc - add patch to fix cdrom_id.c (globally replacement with sed in post_configure breaks build for SRM 1.3) - adjust cross/libudev_219 - add patch to fix cdrom_id.c (avoid definition of CFLAGS -DSG_FLAG_LUN_INHIBIT=2) - update patch to fix build for newer glibc (was 003-missing-minor-major-x64-dsm72.patch)
- port 8888 is used by Synology SRM - use port 18888 instead of 8888 for demoservice - use port 18889 instead of 8889 for demowebservice (no conflict and not supported on SRM)
- cross/cpulimit: add missing toolchain headers
7dbde88
to
be1b917
Compare
# error: Cannot generate ORC metadata for CONFIG_UNWINDER_ORC=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel | ||
# with installation of elfutils and libelf-dev to the development environment, | ||
# it further fails with error: pointer may be used after ‘realloc’ [-Werror=use-after-free] | ||
UNSUPPORTED_ARCHS += epyc7002 rtd1619b |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I have an on-going PR to solve that #6646
CHANGELOG = "Initial package release" | ||
CHANGELOG = "Use service port 18889 instead of 8889" | ||
|
||
LICENSE = MIT | ||
|
||
include ../../mk/spksrc.common.mk | ||
# use the latest php version | ||
ifeq ($(call version_ge, ${TCVERSION}, 7.0),1) | ||
PHP_VERSION = 8.2 | ||
endif | ||
|
||
# only for DSM 7 we have dedicated dependencies (must match the configuration in conf/resource) | ||
ifeq ($(call version_ge, ${TCVERSION}, 7.0),1) | ||
SPK_DEPENDS = WebStation:PHP$(PHP_VERSION):Apache2.4 | ||
else ifeq ($(call version_ge, ${TCVERSION}, 6.0),1) | ||
# on DSM 6 the default web server and PHP version are used | ||
SPK_DEPENDS = WebStation | ||
endif | ||
|
||
WIZARDS_DIR = src/wizard/ | ||
SERVICE_WIZARD_SHARENAME = wizard_shared_folder_name | ||
|
||
SYSTEM_GROUP = http | ||
|
||
DSM_UI_DIR = app | ||
DSM_UI_CONFIG = src/app/config | ||
ifeq ($(call version_ge, ${TCVERSION}, 7.0),1) | ||
# webservice resource definition is for DSM 7 only | ||
CONF_DIR = src/conf_php$(PHP_VERSION)/ | ||
# The app/config file is not used on DSM 7 for web services, the app icon is created based on the resources in CONF_DIR only. | ||
# With app/config on DSM 7 (sometimes) an error occurred: System error. Unable to perform this operation. Please try again later. | ||
# Disable the creation of app/config file: | ||
NO_SERVICE_SHORTCUT = y | ||
# create firewall rules, since we add a port based "Web Portal" | ||
SERVICE_PORT = 8889 | ||
SERVICE_PORT = 18889 | ||
endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a need to update the wiki? Friendly reminder just in case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a need to update the wiki? Friendly reminder just in case.
I added a comment that it conflicts with a port used by Synology SRM and will update it when this is merged.
I updated the wiki with ports additionally used by Synology SRM
https://github.com/SynoCommunity/spksrc/wiki/Synology-Used-Ports
|
||
# Service configuration | ||
SERVICE_PORT = 8888 | ||
SERVICE_PORT = 18888 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similar to other comment, is the wiki updated?
TC_HAS_FORTRAN = 1 | ||
else ifeq ($(strip $(TC_VERS)),1.3) | ||
TC_HAS_FORTRAN = 1 | ||
else ifeq ($(strip $(TC_VERS)),6.2.4) | ||
ifeq ($(findstring $(ARCH),$(x64_ARCHS)),$(ARCH)) | ||
TC_HAS_FORTRAN = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is much larger than I though (not that there's an issue to he contrary, just takes longer to review and more than my cell phone 😄 ).
First thing that crossed my mind is, could we use REQUIRED_MIN_SRM = nop
instead of REQUIRED_MIN_SRM = 3.0
to invalidate support for SRM, similar to other nop
in used elsewhere?
Secondly, somewhere was mentioned that piece of code is missing the kernel sources? Should we instead consider adding a patch directory in the appropriate kernel directory tree instead? Or maybe I misread.
So really two items for your consideration, in anyhow, otherwise LGTM, good job.
ifneq ($(REQUIRED_MIN_SRM),) | ||
ifeq ($(ARCH),$(findstring $(ARCH),$(SRM_ARCHS))) | ||
ifeq ($(call version_lt, ${TCVERSION}, 3.0),1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your overall changes in pre-check.mk makes me reconsider why was the code like this in the first place? Maybe I'm just tired tonight but unsure that this is the best method to confirm DSM requirements based on SRM min level and so on. I do not have a better idea and mabye best to leave that for another time to revisit but something is odd from my perspective.
ifeq ($(strip $(KERNEL_DIST_SITE)),) | ||
KERNEL_DIST_SITE = https://sourceforge.net/projects/dsgpl/files/Synology%20Router%20GPL%20Source/$(KERNEL_BUILD)branch/$(KERNEL_URL_DIR) | ||
KERNEL_DIST_SITE = https://github.com/SynoCommunity/spksrc/releases/download/kernels/srm$(KERNEL_VERS) | ||
endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I get this right, that mean that all SRM (e.g. now only SRM-1.3) are now hosted on our github repo? Not now but food for thought may be to retire entirely older inexistent SRM+DSM references...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I get this right, that mean that all SRM (e.g. now only SRM-1.3) are now hosted on our github repo? Not now but food for thought may be to retire entirely older inexistent SRM+DSM references...
That is, because Synology does not provide a separate linux-* archive for SRM kernels but only the full gplsources that are 1.7..1.8 GB.
I extracted the linux-* folders from the gpl sources to get archives of ~ 80 MB.
How to get those archives is documented in the related release comments.
We already did this for the SRM 1.3 toolchains to extract those from the dev-env archives.
There are not the kernel sources but the toolchains that are missing for nortstarplus-1.2 and dakota-1.2. May be we can build kernel modules using the armv7-1.2 toolchain, but this is out of scope of this PR... |
@th0ma7 thanks for taking a look (or two)... I have added a list with all packages that can be built for SRM. The tcp_wrapper error affects DSM 7.2 too so it will be worth to dive deeper... |
Description
This is mainly a framework update to support SRM 1.3
REQUIRED_MIN_SRM
REQUIRED_MIN_SRM = 3.0
for packages not supported on SRMThis PR fixes the build for SRM 1.3 toolchains comming with
Checklist
all-supported
completed successfully (this PR triggers the build of a lot of packages, but only builds for SRM 1.2 and SRM 1.3 are affected)Type of change
All arch specific Packages that can be built for SRM
Packages marked with ❌️ require further analysis of build errors.
multiple definition
errors