SynoCommunity / spksrc

Cross compilation framework to create native packages for the Synology's NAS
https://synocommunity.com
Other
3.03k stars 1.23k forks source link

Framework DSM7: shared folder worker fails #4766

Closed publicarray closed 3 years ago

publicarray commented 3 years ago

Hi, I'm having exactly the same issue @clemthi is having. Same version, same logs etc…

I tried using the GUI with the install wizard, chose the default directory /volume1/downloads, still having the error Acquire data-share for transmission when 0x0002 (fail)

I tried after adding read/write permissions to sc-transmission user on the directory, still having the same error.

Originally posted by @dremixam in https://github.com/SynoCommunity/spksrc/issues/4313#issuecomment-893065151

publicarray commented 3 years ago

I created a new issue as It became hard to follow the original thread.

publicarray commented 3 years ago

Might be related to https://github.com/SynoCommunity/spksrc/pull/4748#issuecomment-892452496 different error/install log though: errno=0x1200 Failed to get defualt status

BenjV commented 3 years ago

@publicarray I cannot reproduce the problem.

I installed your DSM7 transmission package for ARM7 two times. Once choosing the default download location (volume1/download) which already existed and the package added read/write permissions to the "system internal user" sc-transmission. Secondly I removed the package and installed it again, now using a new download location and this also worked fine. The share was created and sc-transmission got read/write permissons.

By the way: The way the wizard is still talking about sc-downloads and DSM 6 which of course is not valid for DSM 7 anymore.

publicarray commented 3 years ago

Thanks @BenjV like you, I've been unable to reproduce the problem.

The way the wizard is still talking about sc-downloads and DSM 6 which of course is not valid for DSM 7 anymore.

I've been clicking thought the wizards so many times that I stopped reading them. Thanks for the heads-up.

BenjV commented 3 years ago

@publicarray Did some more testing and found out that filling in the "watched" and 'incomplete" folder results in a message that those folder do not exists. This is due to the fact that the package center scripts are running as a user (in this case "sc-transmission") which does not have read/write access to those already existing folders and so will not see them.

So either this test has to be removed from the script or change the structure so everything is part of the same share created by the resource file.

I would suggest that instead of creating a "downloads" share to create a "transmission" share ( fixed name). And on that share create subfolders like "downloads", "watched", "incomplete". Because that share is owned by "sc-transmission" the scripts can create those subfolders. This way the "system internal user" "sc-transmission" wil have all the correct permissions.

Also in the service setup script there are some permission settings in the post _upgrade. They are not possible for existing folders because the script has no root privs to do that and if you implement my suggestion it is not needed.

My last suggestion would be change the pre_upgrade and add a copy of all the existing torrents (*.torrent) from:

/volume1/@appstore/transmission/var/torrents

to

${SYNOPKG_PKGINST_TEMP_DIR}

and move them in the post-upgrade phase to

${SYNOPKG_PKGDES T_VOL}/@appdata/transmission/torrents

This will fail if the old package is not on volume1 so you could add a question in the upgrade_uifile to ask for that volume.

I was not able to test this because I would need a DSM 6 installation and upgrade it to DSM 7, but it should work because the old folder was owned by "sc-transmission". Of course all data downloaded for that torrent will be lost but the torrents will still be there to start over downloading.

publicarray commented 3 years ago

Did some more testing and found out that filling in the "watched" and 'incomplete" folder results in a message that those folder do not exists.

That was always the case (there was never a mkdir command in the script) The user was supposed to make sure that they exist. My assumption is the intent was that the feature was a convenience to change the conf file only.

This is due to the fact that the package center scripts are running as a user (in this case "sc-transmission") which does not have read/write access to those already existing folders and so will not see them.

Thanks I had another look at the behaviour and fixed a few small issues, so thanks for making me look :smile: Hm, the user is responsible to read the warning. I have a feeling you tested an older version of the package than the one here

4719. I've updated the messages to be more explicit anyway. Thanks. Maybe I should remove the ones here https://seby.io/download/synology-7.0 (they were always for devs, but I have a feeling that someone blogged about them and advertised them ready for production use which they defiantly are not! Furthermore, I've already uploaded a slightly more recent transmission version recently to help fix people loosing their torrent files)

I would suggest that instead of creating a "downloads" share to create a "transmission" share ( fixed name).

This is user configurable, so I don't see any improvement here. But I think what you are saying is to remove the "watched" and "incomplete" wizard options and let the package manage them. I thought about that too, I'm just not sure if it's worth it. The package would have to migrate existing installations and would break integrations with sonarr etc. The watched folder can be used by other packages.

My last suggestion would be change the pre_upgrade and add a copy of all the existing torrents (*.torrent) from:

My fist transmission package did not correctly migrate package data up upgrades, the framework did not have the feature at the time, so this I think this is solved by recompiling.

So from my point of view shared folder not being created is an issue with synology. or wrong/old versions

BenjV commented 3 years ago

@publicarray I can only test complete packages so I used https://seby.io/download/synology-7.0 to test, so I don't know if this is changed.

This is the part of the service-setup script of the package with the problem:

service_preinst ()
{
    if [ "${SYNOPKG_PKG_STATUS}" == "INSTALL" ]; then
        # If chosen, they need to exist
        if [ -n "${wizard_watch_dir}" -a ! -d "${wizard_watch_dir}" ]; then
            echo "Watch directory ${wizard_watch_dir} does not exist."
            exit 1
        fi
        if [ -n "${wizard_incomplete_dir}" -a ! -d "${wizard_incomplete_dir}" ]; then
            echo "Incomplete directory ${wizard_incomplete_dir} does not exist."
            exit 1
        fi
    fi

    exit 0
}

So if someone installs the package for the first time and he will want to use existing location for "watched" and "incomplete" then the installation will faill because the installation script does not have permissions to access those locations (even if they exist) and will return the message that the location does not exist and the installation will stop.

A user cannot give those permissions before the installation of the package due to the fact that the "system internal user" "sc-transmission" does not yet exists. So effectively you cannot use those fields.

publicarray commented 3 years ago

Fair enough, I've updated them again. On dsm7 the package attempts to create the folder otherwise prompts to reinstall the package.

Thanks, I'm a bit slow sometimes. Ah, the classic :chicken: and :egg: problem.

publicarray commented 3 years ago

Regarding to remove the watch and incomplete folder and letting the package manage them. I think if I can make the wizard changes DSM7 only, for new installations that would be an improvement.

gemmmo commented 3 years ago

I am using a RTD1296 synology (ARMV7?) on DSM6 + Transmission, but don't have much experience with SSH and other custom tweaks. Is there anything I can help with testing? If not I'll hang on to DSM6 for now.

publicarray commented 3 years ago

hang on to DSM6 for now

Yes please do, until the packages you need are published see #4524

publicarray commented 3 years ago

@BenjV I've push another update would you be willing to test it? https://seby.io/download/synology-7.0

BenjV commented 3 years ago

@publicarray Ok nice update!

I tested it and found this working as expected.

The incomplete and watched folders are now created as subfolders of the chosen download share. If that share does not exists it will be created. In both cases the system internal user of transmission (sc-transmission) will have read/write permissions on that share.

Doing this the right way is important, because the gui of this Transmission version does not have the possibility to create or change the location of the watched- and incomplete folders. So if they were created in the data folder of transmission (e.g. /volume1/@appdata/transmission) they cannot be used at all.

gemmmo commented 3 years ago

Yes please do

Alright! When it publishes should users update Transmission in DSM6 before updating to DSM7? Thank you for your help.

publicarray commented 3 years ago

@gemmmo that should not be necessary. Besides I don't think we need to publish a new DSM6 version

clemthi commented 3 years ago

I don't know what I'm doing wrong but I still get this damn Acquire data-share for transmission when 0x0001 (fail) error in /var/log/sunopkg.log when trying to install. I've done so far:

Is there any way to have a more detailed error message? Or how to debug this?

publicarray commented 3 years ago

@clemthi I'm surprised you still get the error, did you grab the newer version from https://seby.io/download/synology-7.0 ? And can you confirm that you let the installer create a shared folder without creating it first by other means?

clemthi commented 3 years ago

I've tried another time with the latest package (which I re-downloaded, just in case). I've set install to /volume1/data/torrent (which doesn't exist)

I've checked access of /volume1/data in the shell

dr-xr-xr-x+  13 root            root                 4096 Mar 30  2020 data

I've also verified in DSM: https://i.imgur.com/4uL7ctC.png

Everything looks good. But ... install fails again (and the folder /volume1/data/torrent haven't been created). It's obviously and access issue but I have no idea what I should fix now.

publicarray commented 3 years ago

Oh thanks for the image, Transmission expects full access to a shared folder it won't create a sub folder. I thought I had a regex to validate this and block any / in the text input.

publicarray commented 3 years ago

To clarify something like /volume1/my_transmission_data would work but /volume1/my_transmission_data/anything_else not. It's hard to support sub folders like this in DSM7 (and allow seamless migration from DSM6), unfortunately.

clemthi commented 3 years ago

I've created a /volume1/torrent share, granted rw access to sc-transmission and the install (without specifiying watched and working folders) still fails with the same error.

BenjV commented 3 years ago

@publicarray The package for ARM is OK. The wizard does not allow to proceed when a / is present in the "download shared folder" field, although it only block going further but does not display the error message.

The Package for X64 still has the watched and incomplete fields in the wizard.

publicarray commented 3 years ago

The Package for X64 still has the watched and incomplete fields in the wizard.

Thanks @BenjV, I've uploaded the correct version now. Might take some time for it to be reflected by the CDN

publicarray commented 3 years ago

Oh The wizard files have not been translated to french, so I need to removed them, you are still using the old wizard.

clemthi commented 3 years ago

That's weird, the files online have a last modified date from yesterday, august 17.

publicarray commented 3 years ago

I'm recompiling without the old wizard now.

clemthi commented 3 years ago

just in case, I've forgot to say that I was using the transmission_aarch64-7.0_3.00-19.spk package.

publicarray commented 3 years ago

@clemthi You can give it a try now, the files have been modified but may take time to be distributed by the CDN

clemthi commented 3 years ago

I does work now. Yeepee :)

publicarray commented 3 years ago

Thanks for confirming.