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

DSM6 support POC #2345

Closed
wants to merge 2 commits into from
Closed

DSM6 support POC #2345

wants to merge 2 commits into from

Conversation

Dr-Bean
Copy link
Contributor

@Dr-Bean Dr-Bean commented May 26, 2016

Motivation: This is a first attempt and more or less a POC to get DSM6 support to our packages, especially with regards to user creation. I'm using sabnzbd-testing as a testcase here.

Tested what I could on DSM5 and DSM6, but I need an extra set (or two) of eyes on this before I even think about merging/publishing this: I'm likely missing details after working on it for a while. Along with that, I don't have a spare device to test DSM upgrades with, so my tests are limited in that respect.

So, @SynoCommunity/developers, please take a close look/build/test if you can. Anyone else willing and able to check this out, please do and provide your feedback below.

For the record: Do not ask when this, or other packages, will be made available on the repository.

Linked issues: #2216

Checklist

  • Build rule all-supported completed successfully
  • Build rule all-legacy completed successfully
    ...although neither means very much in this case...

Move generic functions to common script
To include in SPK, set `COMMON_SCRIPT = ../../common/common.sh` in
spk/Makefile
- Update sabnzbd to 1.0.2
- Add unzip cross package
- Use `common` script
- Add preliminary DSM5 support
@Dr-Bean
Copy link
Contributor Author

Dr-Bean commented May 27, 2016

set_syno_permissions seems to be broken on DSM6

@ward0
Copy link

ward0 commented May 27, 2016

nice to see this going! subbed

@nickveldrin
Copy link

If a guy that has ~21tb of data that is kinda important and not backed up enough wants to test, would you recommend he back away? I put in workarounds to get all of my important applications function in DSM6, so i'm downloading my shows, parsing in sickrage, and enjoying my titles through plex. 100% perfection!

@GoodOmens83
Copy link
Contributor

GoodOmens83 commented Jun 12, 2016

@Dr-Bean broken as in how?

I noticed when playing around a while back - if you check the permissions (synoacltool -get) of a directory created with DSM 5 it would throw some sort of error. However, you could still apply permissions to that directory and the permissions would "stick" (meaning the OS would begin respecting them). If you check again after - it would show the expected result.

Meaning the line:

if [ ! "`synoacltool -get \"${DIRNAME}\"| grep \"group:${SYNO_GROUP}:allow:..x\"`" ]; then

might be failing because the directory was created in DSM 5 - you could try something like:

if [ ! "`synoacltool -get \"${DIRNAME}\"| grep \"group:${SYNO_GROUP}:allow:..x\"`"  ] || ["`synoacltool -get \"${DIRNAME}\"| grep \"ERROR MESSAGE"`"] ; then

At least that's why I think it's failing.

@JoopNL
Copy link

JoopNL commented Jun 18, 2016

Thanks for doing this @Dr-Bean. I am no expert when it comes to Synology packages, but I've read all the comments on #2216 and looked over the changes in this PR. If the approach you describe (using the privilege file to create the user) is valid, then I can't see anything wrong with the new/changed code.

I think it's important to get this merged so people can test with it. Or if you think that's too soon, maybe you can make a copy of sabnzb-testing called "sabnzbd-dsm6-temp-betatest" or something, and publish that. We need to get the ball rolling! :)

The only thing which seems a bit strange to me, is to do the DSM version checks in the functions in common.sh. It seems more logical to do the checks in installer.sh and then call the applicable functions in common.sh. When running on DSM6 it's strange to call a function called "create_legacy_user". But this doesn't affect the functionality.

@ITJamie
Copy link

ITJamie commented Jul 22, 2016

any update on this?

@piejanssens
Copy link
Contributor

@Dr-Bean What's the status for DSM 6?
Are you looking into the new DSM 6 dev guide? https://usdl.synology.com/download/Document/DeveloperGuide/DSM_Developer_Guide.pdf#page=99&zoom=auto,-159,818

@danfozzy
Copy link

danfozzy commented Oct 1, 2016

Has this stalled / been dropped or even possible now?

@GoodOmens83
Copy link
Contributor

You know now that I have time to look at this - I wonder if we should do away with the set_syno_permissions() and instead move to using a ${package}/conf/resourcefile. Check out the Data Share section:

https://developer.synology.com/developer-guide/resource_acquisition/data_share.html

This worker creates shared folder and set its permission during package startup. The share name can be hard-coded in the specification or given by user input from the UI wizard. The shared folder will not be removed after package uninstallation, since it might delete the user’s personal data as well.

Acquire(): Create shared folder and set its permission.
If the shared folder already exists, skip share creation and set the permission.
Release(): Does nothing

@Dr-Bean
Copy link
Contributor Author

Dr-Bean commented Nov 1, 2016

@GoodOmens83 We can use that for DSM6, although we'll need to still keep the function around for DSM5.
From the documentation, I can't tell if and how you're supposed to set a user-defined volume. Is the format /volume2/sharedfolder allowed in the name field, or is that field limited to sharedfolder?

@GoodOmens83
Copy link
Contributor

GoodOmens83 commented Nov 1, 2016

@Dr-Bean - agree on only for DSM6. However, since the resource file is new to DSM6, then just leave the set_syno_permissions to only run under DSM5 (e.g., if [ "${BUILDNUMBER}" -ge "7135" ]; then)

Not sure if volume specific names are allowed in share names. Some testing would be needed for that.

The other really useful section in the resource I see helping is the usr/local/ linker (e.g., for GIT).

@Dr-Bean
Copy link
Contributor Author

Dr-Bean commented Jan 11, 2017

Closing. A new branch, dsm6, has been created which will serve as a starting point for DSM6 supported packages. Currently, it contains the latest available DSM6 toolchains (6.0.2), as well as updates for nzbget-testing and sabnzbd-testing packages to make them work on DSM6. I'll work on adding packages as and when I have time, PR's using the same approach are welcome.

@Dr-Bean Dr-Bean closed this Jan 11, 2017
@Dr-Bean Dr-Bean deleted the dsm6_user_test branch January 11, 2017 12:07
@Safihre
Copy link
Contributor

Safihre commented Jan 11, 2017

@Dr-Bean So to make it clear to me as non-DSM person:
Users with DSM6 could already continue to use and update SABnzbd when they upgraded from DSM5?
And now they can also install the package 'fresh' on DSM6? And this is already publicly available to them?

@KaraokeStu
Copy link
Contributor

Hi, @Safihre the problem was that Synology changed the way that users worked in DSM 6.

Users upgrading from DSM 5 to DSM 6 were initially unaffected, but as soon as they upgraded a package, DSM 6 broke the users and the package would no longer start without quite a bit of hacking about.

Now, the packages will work regardless of which version of DSM is installed, meaning the users can upgrade with more confidence.

@Safihre
Copy link
Contributor

Safihre commented Jan 11, 2017

🎉 Great! Thanks!

@Dr-Bean
Copy link
Contributor Author

Dr-Bean commented Jan 11, 2017

#2216 may have some more details. Packages are not published yet, because the repository backend needs some work.
For the record, package upgrades on DSM6 wouldn't affect previously working packages (i.e. packages were installed on DSM5), but clean installs of packages on DSM6 wouldn't work due to the changes @KaraokeStu referred to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants