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

ps3netsrv: Update to v1.47.46e with ps3netsrv 20240210a #6090

Merged
merged 15 commits into from
Jul 19, 2024

Conversation

Hirador
Copy link
Contributor

@Hirador Hirador commented May 3, 2024

Update to v1.47.46e with ps3netsrv 20240210a for DSM's versions: DSM-6.2.4 -> DSM-7.1

Description

  1. Updated the variables found on spksrc/spk/ps3netsrv and spksrc/cross/ps3netsrv to match the current version (v1.47.46e with ps3netsrv 20240210a) of aldostools's ps3netsrv
  2. made "make all-supported"
  3. created .spk for my Architecture (arch-armada370)
  4. Made some tests with the updated version of the ps3netsrv
  5. Cleaned everything up for the pull request (make clean on both spksrc/spk/ps3netsrv and spksrc/cross/ps3netsrv)

Checklist

  • Build rule all-supported completed successfully
  • New installation of package completed successfully
  • Package upgrade completed successfully (Manually install the package again)
  • Package functionality was tested
  • Any needed documentation is updated/created

Type of change

  • Bug fix
  • New Package
  • Package update
  • Includes small framework changes
  • This change requires a documentation update (e.g. Wiki)

Updated the variables found on "spksrc/spk/ps3netsrv" and " spksrc/cross/ps3netsrv" to match the current version (v1.47.46b with ps3netsrv 20240210) of aldostools's ps3netsrv
@Hirador
Copy link
Contributor Author

Hirador commented Jun 1, 2024

@hgy59 Can everything be merged in the next commit now that I have created a dedicated branch?

@hgy59
Copy link
Contributor

hgy59 commented Jun 1, 2024

@hgy59 Can everything be merged in the next commit now that I have created a dedicated branch?

I can see an official release 1.47.46 in https://github.com/aldostools/webMAN-MOD/tags
it was created on Mar 21 2024.
Probably only the changelog is outdated?
CHANGELOG = "Update to v1.47.46b with ps3netsrv 20240210."

@Hirador
Copy link
Contributor Author

Hirador commented Jun 1, 2024

@hgy59 when I was working on this commit the version was 1.47.46b, even if the code should be the same the tar.gz could have different md5, I'll make some checks and see if everything it's okay. Just to be as precise as possible I would in any case update the comment and description, if so I think that this pull request should be closed and recreated again, right?

@mreid-tt
Copy link
Contributor

mreid-tt commented Jun 1, 2024

@Hirador, opening a new PR for version changes isn't always necessary. You can simply update the PR description and the change log to reflect the current versions being committed. This approach not only saves time but also makes it easier for reviewers to track minor changes like version updates, rather than reviewing a new PR from scratch.

@Hirador Hirador changed the title ps3netsrv: Update to v1.47.46b with ps3netsrv 20240210 ps3netsrv: Update to v1.47.46e with ps3netsrv 20240210a Jun 1, 2024
@Hirador
Copy link
Contributor Author

Hirador commented Jun 1, 2024

@mreid-tt @hgy59 I've made all the checks, apparently the md5 is the same, i've changed the changelog with the updated values and changed the pull request title and description, i think everything is good to go now

@mreid-tt
Copy link
Contributor

mreid-tt commented Jun 6, 2024

@Hirador there was an initiative previously to move away from wizard_data_volume and wizard_data_directory variables in the wizard (see: #5649). If you feel up to it perhaps you can also implement the changes to move toward the new shared folder handling.

EDIT: A good example is found in #5929

@Hirador
Copy link
Contributor Author

Hirador commented Jun 7, 2024

@mreid-tt With a little support it shouldn't be a problem, i'll work on it today :D

Hirador added 10 commits June 7, 2024 12:32
Makefile updated so to meet requirements for the redesigned shared folder handling
ps3netsrv.conf updated so to meet requirements for the redesigned shared folder handling
service-setup.sh updated so to meet requirements for the redesigned shared folder handling
install_uifile so to meet requirements for the redesigned shared folder handling
install_uifile_fre updated so to meet requirements for the redesigned shared folder handling
install_uifile_rus updated so to meet requirements for the redesigned shared folder handling
adjusted "step_title"
Fixed syntax error found in Fixed install_uifile
Fixed syntax error found in install_uifile_fre
Fixed syntax error found on install_uifile_rus
@Hirador
Copy link
Contributor Author

Hirador commented Jun 7, 2024

@mreid-tt @hgy59 I've made the required changes, please check everything out to be sure that i've made them properly (May have missed the DSM version check, don't know if it is mandatory in my case). If everything is fine I'll test the .spk on my DiskStation DS414slim (Marvell Armada 370) with DSM 7.1.1 later today, if you could test it on DSM 5.X , 6.X, and SRM 1.X it would be great!

adjusted sed funztion
@Hirador
Copy link
Contributor Author

Hirador commented Jun 7, 2024

@hgy59 @mreid-tt Made a few tests on DSM 7.1.1, everything seems to be working as expected, i think we may be ready to merge after your further tests!

@mreid-tt
Copy link
Contributor

mreid-tt commented Jun 8, 2024

@Hirador great work! I did some brief testing under DSM6 and this release definitely fixes a share issue with the existing release. When I install the existing release I get the following error:

2024/06/08 07:08:24	Configuring PS3
2024/06/08 07:08:24	Get Share defualt config failed.
2024/06/08 07:08:24	Lastest SynoErr=[share_db_get.c:31]
2024/06/08 07:08:24	SYNOShareGet failed. synoerr=[0x1400]

As a result, the PS3 share is not created. Interestingly enough, the app seems to run fine but I have no way to test beyond this:

Sat Jun  8 07:08:27 -04 2024
Starting ps3netsrv command /volume1/@appstore/ps3netsrv/ps3netsrv-starter.sh

The challenge is that when you try to upgrade with this version it fails because of the missing share:

2024/06/08 07:13:00	upgrade ps3netsrv 1.47.46-9 Begin postupgrade
2024/06/08 07:13:01	Begin initialize_variables
2024/06/08 07:13:01	SHARE_NAME is not an existing share [PS3].
2024/06/08 07:13:01	Shared folder configured with SHARE_NAME [PS3] and SHARE_PATH [PS3]
2024/06/08 07:13:01	End initialize_variables
2024/06/08 07:13:01	===> Step postupgrade. STATUS=UPGRADE USER=ps3netsrv GROUP= SHARE_PATH=PS3
2024/06/08 07:13:01	Granting 'sc-ps3netsrv' unix ownership on /volume1/@appstore/ps3netsrv/var
2024/06/08 07:13:01	upgrade ps3netsrv 1.47.46-9 End postupgrade ret=[0]
2024/06/08 07:13:01	upgrade ps3netsrv 1.47.46-9 Begin /bin/rm -rf /volume1/@tmp/pkginstall
2024/06/08 07:13:01	upgrade ps3netsrv 1.47.46-9 End /bin/rm -rf /volume1/@tmp/pkginstall ret=[0]

While I don't see a specific error message, there is a dialog presented in DSM that the update failed and the app it put in a repair state. For existing users, I expect this will be a rare scenario since some folder should exist otherwise the app wouldn't work for them. In either case, a complete uninstall a re-install with the new version had the folder created successfully:

2024/06/08 07:14:00	Begin initialize_variables
2024/06/08 07:14:00	SHARE_NAME is not an existing share [PS3].
2024/06/08 07:14:00	Shared folder configured with SHARE_NAME [PS3] and SHARE_PATH [PS3]
2024/06/08 07:14:00	End initialize_variables

An in-place upgrade over the new version was also successful so on the surface everything looks good to me under DSM6. And as always, the service log is very minimal:

Sat Jun  8 07:29:45 -04 2024
Starting ps3netsrv command /volume1/@appstore/ps3netsrv/ps3netsrv-starter.sh

@mreid-tt mreid-tt requested a review from hgy59 June 22, 2024 10:59
@Hirador
Copy link
Contributor Author

Hirador commented Jun 29, 2024

@hgy59 did you manage to check up everything so to proceed in merging?

@hgy59
Copy link
Contributor

hgy59 commented Jun 29, 2024

The migration of installer-variables isn't correct. The upgrade might be missing something.

With the default share "PS3" the etc/installer-variables file looks like this:

former package version:

SHARE_PATH=PS3

new installation of new version:

SHARE_PATH=/volume1/PS3
SHARE_NAME=PS3

upgrade from former to new version:

SHARE_PATH=PS3
SHARE_NAME=PS3

As we can see, the SHARE_PATH variable is not migrated to the full name /volume1/PS3

AFAICR this was fixed by @mreid-tt for some other packages...

EDIT:
This error is on DSM 7 only (for DSM 6 see below)

@hgy59
Copy link
Contributor

hgy59 commented Jun 29, 2024

package upgrade on DSM 6 fails ❗

The issue with etc/installer-variable reported above occurs with DSM 7 (tested with DSM 7.2.1-69057 Update 5).

On DSM 6.2.4 the package upgrade from former version fails. (on DSM 6 there is no "Repair" button provided).

The /var/log/messages file reports an error Failed to acquire resource after install ps3netsrv

2024-06-29T11:59:52+02:00 ##### synoscgi_SYNO.Core.Package_2_list[15440]: list.cpp:329 SYNO.Core.Package.list rollback to direct call
2024-06-29T11:59:58+02:00 ##### synoscgi_SYNO.Core.Package.Installation_1_install[17667]: resource_api.cpp:289 Release service-cfg for ps3netsrv when 0x0002 (done)
2024-06-29T11:59:58+02:00 ##### synoscgi_SYNO.Core.Package.Installation_1_install[17673]: resource_api.cpp:289 Release port-config for ps3netsrv when 0x0002 (done)
2024-06-29T11:59:59+02:00 ##### synoscgi_SYNO.Core.Package.Installation_1_install[17730]: resource_api.cpp:190 Acquire data-share for ps3netsrv when 0x0002 (done)
2024-06-29T11:59:59+02:00 ##### synoscgi_SYNO.Core.Package.Installation_1_install[17799]: resource_api.cpp:190 Acquire service-cfg for ps3netsrv when 0x0002 (done)
2024-06-29T11:59:59+02:00 ##### synoscgi_SYNO.Core.Package.Installation_1_install[17799]: resource_api.cpp:190 Acquire data-share for ps3netsrv when 0x0002 (fail)
2024-06-29T11:59:59+02:00 ##### synoscgi_SYNO.Core.Package.Installation_1_install[17799]: resource_api.cpp:190 Acquire port-config for ps3netsrv when 0x0002 (done)
2024-06-29T12:00:00+02:00 ##### synoscgi_SYNO.Core.Package.Installation_1_install[11011]: pkginstall.cpp:1221 Failed to acquire resource after install ps3netsrv [0xD900 manager.cpp:204]
2024-06-29T12:00:11+02:00 ##### synoscgi_SYNO.Core.Package_2_list[17933]: list.cpp:329 SYNO.Core.Package.list rollback to direct call
2024-06-29T12:00:45+02:00 ##### synoscgi_SYNO.Core.Package_2_list[20468]: list.cpp:329 SYNO.Core.Package.list rollback to direct call

That must be related to the package upgrade since a new installation on DSM 6.2.4 works.

REMARKS:
on DSM 6 the migration of etc/installer-variables is correct.

@mreid-tt mreid-tt self-requested a review June 29, 2024 10:45
@mreid-tt
Copy link
Contributor

It's been a while since I've worked on this. Perhaps #5962 may have some hints of how to resolve this?

@mreid-tt mreid-tt removed their request for review June 29, 2024 13:07
@hgy59
Copy link
Contributor

hgy59 commented Jun 29, 2024

It's been a while since I've worked on this. Perhaps #5962 may have some hints of how to resolve this?

@mreid-tt after testing with cops, I can tell that cops package provides a dialog in upgrade wizard for the share name, when installer-variables for share are not already populated.

For cops since 1.5.4-9 the installer variables SHARE_NAME and SHARE_PATH are added. until cops 1.4.2-7 the installer-variables file contained GROUP=http only.

On DSM 6.2.4 I was able to get the error Failed to acquire resource after install cops by adding SHARE_NAME=calibre to the installer-variables file before the update to v1.5.4-9.
When SHARE_NAME is defined, the upgrade wizard page for the share is not shown and I guess the error is caused by lacking variable wizard_calibre_share.

Further information on shared folder migration can be found in the discussions of #5968

ps3netsrv on DSM 6.2.4:

This is really a mess:
After new installation of ps3netsrv v1.47.37-7 or v1.47.45-8 the installer-variables file contains one entry SHARE_PATH=PS3 (the variable defines the share name as SHARE_PATH and not SHARE_NAME !)
But it is even worse:
With an update of ps3netsrv from v1.47.37-7 or v1.47.45-8 the installer-variables file is removed.

For DSM 6 I propose to always show the upgrade wizard page for the share (or we have to correctly populate the variable wizard_shared_folder_name from SHARE_PATH variable since the current solution in cops does not solve this)


todo: analysis for DSM 7

@hgy59
Copy link
Contributor

hgy59 commented Jun 29, 2024

The migration of installer-variables isn't correct. The upgrade might be missing something.

With the default share "PS3" the etc/installer-variables file looks like this:

former package version:

SHARE_PATH=PS3

new installation of new version:

SHARE_PATH=/volume1/PS3
SHARE_NAME=PS3

will be fixed with #6163

- add upgrade page to configure shared folder when not recoverable from former installation
- inspired by spk/sabnzbd/src/wizard/upgrade_uifile.sh but with priority on package config over installer-variables
- correct the installer-variables file defining SHARE_PATH="name of the shared folder" on package updates
- fixes DSM 6 update error "Failed to acquire resource after install ps3netsrv"
@mreid-tt
Copy link
Contributor

@mreid-tt after testing with cops, I can tell that cops package provides a dialog in upgrade wizard for the share name, when installer-variables for share are not already populated.

Interesting, with the change you just made with #6163, does it resolve the issue with COPS as well? Should a new version be published?

@hgy59
Copy link
Contributor

hgy59 commented Jun 29, 2024

@mreid-tt, @Hirador all installer issues are fixed now.

The upgrade wizard page for the rare case of unrecoverable share folder configuration is currently in english only.
If we want multiple languages we should migrate to wizard_templates...

@hgy59
Copy link
Contributor

hgy59 commented Jun 29, 2024

@mreid-tt after testing with cops, I can tell that cops package provides a dialog in upgrade wizard for the share name, when installer-variables for share are not already populated.

Interesting, with the change you just made with #6163, does it resolve the issue with COPS as well? Should a new version be published?

There is currently no issue in cops.
I intentionally patched the installer-variables file to get an error.

The main reason that cops does not have this issue is the initialization of the CONFIGURED_SHARE_NAME variable from the package configuration and not from installer-variables.

But there might be an issue in sabnzb

Other packages with shared folders don't have an upgrade wizard (at least none with optional shared folder configuration) but I didn't hear about upgrade issues for those. so far..

  • transmission
  • gitea
  • kiwix
  • aria2
  • mpd
  • owncloud

@Hirador
Copy link
Contributor Author

Hirador commented Jun 29, 2024

@mreid-tt, @Hirador all installer issues are fixed now.

The upgrade wizard page for the rare case of unrecoverable share folder configuration is currently in english only.

If we want multiple languages we should migrate to wizard_templates...

No worries, I was planning to just keep English for future releases. But If you wish me to migrate to wizard_templates anyway just give me an example and I'll try to figure it out as soon as I can

@Hirador
Copy link
Contributor Author

Hirador commented Jul 9, 2024

@hgy59 Let me know how we can proceed

@hgy59 hgy59 merged commit b51439b into SynoCommunity:master Jul 19, 2024
17 checks passed
@hgy59 hgy59 added the status/published Published and activated (may take up to 48h until visible in DSM package manager) label Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/published Published and activated (may take up to 48h until visible in DSM package manager)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants