tt-rss: upgrade 20221230 => 20230828 #5862
Conversation
smaarn
force-pushed
the
feat/tt-rss/upgrade-202308
branch
from
44bd14b
to
ce2f410
Compare
|
hey @smaarn, I was doing a review of this for you and I found that I was able to successfully install, upgrade and run this under DSM 7. The only thing I noted was that the service icon was a generic one. Under DSM 6, during installation it seems to require the user to manually edit the default PHP profile. I also noted that the HTML code for the formatting on the wizard is showing up as text (which was not the case in the previous version). As for the default PHP profile modification under DSM 6, you might want to consider the workarounds I created for ownCloud and more recently for Selfoss (in #5916). These allow for the creation of PHP profiles and other web settings in DSM 6. These are automatically created in DSM 7 using the resource worker but can be manually done under DSM 6 using the workarounds. Under DSM 6, I noted however that the package does not start (may be related to #2052). The daemon which is triggered does not seem to like being run as root as per these entries in the I was able to trigger a start using the command [ EDIT: I noted that I could not start the daemon as the user |
|
@smaarn, another thing I noted was the prevalence of script files rather than consolidating most actions under the |
|
@smaarn, regarding the icon being generic in DSM 7, I believe this is because you don't have it defined in your resource file. Do take a look at the package developer guide or my example in ownCloud on its use. P.S. As a suggestion you can also upgrade the icon in your package to the larger 512px one from here. |
There was a problem hiding this comment.
my package testing found some issues:
- wrong variable for mysql call
- on DSM 6 the db cannot be created
- DSM 6 wizard files need tripple mustache brackets for
WEBSTATION_PHP_PROFILE_DESC(install and upgrade)
in service-setup.sh the ttrss user pw must be used (not the db root pw):
replace
"${MYSQL}" -u "${MYSQL_USER}" -p"${wizard_mysql_password_root}" "${MYSQL_DATABASE}" < "${WEB_DIR}/${PACKAGE}/schema/ttrss_schema_mysql.sql"
by
"${MYSQL}" -u "${MYSQL_USER}" -p"${wizard_mysql_password_ttrss}" "${MYSQL_DATABASE}" < "${WEB_DIR}/${PACKAGE}/schema/ttrss_schema_mysql.sql"
otherwise there is an installation error logged:
2023/11/10 11:50:33 Begin service_postinst
2023/11/10 11:50:33 ERROR 1045 (28000): Access denied for user 'ttrss'@'localhost' (using password: YES)
(since ttrss_schema_mysql.sql is an empty file, this did not break the installation)
On DSM 6 the ttrss database tables are not created.
installation log shows:
2023/11/10 13:14:37 Begin service_postinst
2023/11/10 13:14:43 PHP Parse error: syntax error, unexpected ':', expecting '{' in /volume1/web/tt-rss/update.php on line 23
2023/11/10 13:14:43 Parse error: syntax error, unexpected ':', expecting '{' in /volume1/web/tt-rss/update.php on line 23
2023/11/10 13:14:43 End service_postinst
extract of update.php (with line numbers):
23 function make_stampfile(string $filename): bool {
24 $fp = fopen(Config::get(Config::LOCK_DIRECTORY) . "/$filename", "w");
25
26 if (flock($fp, LOCK_EX | LOCK_NB)) {
27 fwrite($fp, time() . "\n");
28 flock($fp, LOCK_UN);
29 fclose($fp);
30 return true;
31 } else {
32 return false;
33 }
34 }
I suppose that the guess_php function does not work as expected (on my DSM 6 the default php version is 5.6.11, on my working DSM 7.2 the default php version is 8.1.9)
confirmed that guess_php = php56
and php_options = -c /var/packages/WebStation/etc/php56/php.ini
on my DSM 6 installation.
For DSM 6 php 7.4 can be hard coded (in settings.sh), but for php_options it needs some effort (or as @mreid-tt proposed a php profile should be created too).
Ok, my fault. the default php profile in WebStation was the "Default Profile ( PHP 5.6 )" profile. After switching to "Default Profile (PHP 7.4)" the tt-rss successfully runs on DSM 6. There are still installation log warnings, but the db is created sucessfully. anyway, the package should install a php profile for DSM 6 and not depend on the default profile, |
Will be having a look since there has been a few changes in the framework since but not too sure about it. |
Indeed. Will be having a look at the approach used by @mreid-tt on this topic. I thought at first it was an acceptable trade-off to be honest. Will be flagging this MR as in progress until this gets done. |
|
@smaarn another issue is, that the db backup at uninstall does not work (the db is already removed in postuninst). This can easily be fixed by moving the backup from postuninst to preuninst like this: service_preuninst ()
{
# Export database
if [ "${SYNOPKG_PKG_STATUS}" == "UNINSTALL" ]; then
if [ -n "${wizard_dbexport_path}" ]; then
${MKDIR} ${wizard_dbexport_path}
${MYSQLDUMP} -u root -p"${wizard_mysql_password_root}" ${MYSQL_DATABASE} > ${wizard_dbexport_path}/${MYSQL_DATABASE}.sql
fi
fi
}
service_postuninst ()
{
if [ "${SYNOPKG_DSM_VERSION_MAJOR}" -lt 7 ]; then
# Remove the web interface
${RM} ${WEB_DIR}/${PACKAGE}
fi
} |
smaarn
force-pushed
the
feat/tt-rss/upgrade-202308
branch
from
ce2f410
to
a24c7db
Compare
|
Ok so here goes the current status:
I'll try to see if during the week I can figure out this damn icon thing. |
|
@smaarn, so the DSM 6 install kinda works but there are a couple of issues:
For the last point, I would get rid of all the all the complexity as well as the dependencies for the daemon script. It attempts to evaluate a number of things which are not required. Additionally the function call for the daemon is expected to be very simple based on the Update daemon documentation. For testing I use a VirtualDSM 6 deployment in Virtual Machine Manager. This is the easiest way I've found to test older DSM versions that I don't use in my production NAS. If you need any assistance with the above let me know. EDIT: @hgy59, in our Wiki we have some other examples of running application binaries, would any of these help or would my recommendation to just run as root and |
I have already tried that but since it only supports btrfs file system I'm pretty stuck here (unless I decide to upgrade completely or wipe out completely my personal NAS, both options not on the table at the moment).
Would you be able to tell me what an
I think one is not necessarily related to the other but I need to check how the update daemon is now implemented to see what goes on here.
Actually the whole point was to avoid having to list the extensions lists in three different locations:
I would have expected this to be handled natively by the framework. Will be looking into it. |
|
The current version does not install correctly on DSM 6.
so the package center of my system is currently locked... install log: |
…nning as root does it need to be forked)
This Apache conf file was required as part of my workaround. The file is supposed to be placed in For the command you wanted me to run, please note the file ownership would look different because I'd already changed it to
Perhaps you are right. I just change all the files in the web directory to
You may be right. I've tried a variation where I use the Custom service process startup example to modify the service setup to look like this: ... and then have the original daemon command which ends with: ... and this seems to resolve the issue of the daemon still running after I stop the package in the Package Center.
Hmm, I do think the extensions are only required in the DSM 6 and DSM 7 config files. Launching using the recommedation in Update daemon like this has no issue:
This was an incomplete take on my part regarding modifying the daemon. The example above is a better solution. The addition of the privilege file is still required if you look at DSM 6 privilege section of the Wiki. |
|
@smaarn, I noted that you closed this PR. Would you be opening a new one or would you like me to continue your work under a new PR? |
@mreid-tt I won't be having time to look at this today. Closing the MR was an error, sorry. Regarding the extensions the way it works is that tt rss update script actually forks another php process (which is set to be virtual php in order to be able to configure the extensions) Will be trying to have a look over the coming weeks. |
|
@mreid-tt, @smaarn yesterday I had a version fully working on DSM 6.
so my final part of the working service_postinst was if [ "${SYNOPKG_DSM_VERSION_MAJOR}" -lt 7 ]; then
# permissions must be adjusted before update-schema is called
chown -R "${EFF_USER}:${GROUP}" "${WEB_DIR}/${PACKAGE}"
chown -R "${EFF_USER}:${GROUP}" "${LOGS_DIR}"
chmod -R 777 "${WEB_DIR}/${PACKAGE}/lock"
chmod -R 777 "${WEB_DIR}/${PACKAGE}/feed-icons"
chmod -R 777 "${WEB_DIR}/${PACKAGE}/cache"
fi
if [ "${SYNOPKG_PKG_STATUS}" == "INSTALL" ]; then
if [ "${SYNOPKG_DSM_VERSION_MAJOR}" -lt 7 ]; then
# run as package user when installer runs as root
SU="su ${EFF_USER} -s"
else
SU=""
fi
$SU /bin/sh -c ${SYNOPKG_PKGDEST}/bin/update-schema
fiIf we do not get the DSM 6 profile working, it would be better to depend on the default php profile. |
|
@hgy59 I was wondering if
Shouldn't be actually instead:
I reverted the changes enforcing the DSM 6 appropriate WebStation configuration to get back to a "sound" state. @mreid-tt feel free to elaborate from here if you got time. At this stage I'm going to "sleep on it" and maybe check during the week if I manage to find some spare cycles (this time for real). |
No, under DSM 7 only the privilege file defines the user (package user) to run the services and su is not executable due to restrictions. the whole block would look like: if [ -n "${service}" ]; then
if [ -n "${SVC_NO_REDIRECT}" ]; then
OUT="/dev/null"
else
OUT="${LOG_FILE}"
fi
if [ -n "${USER}" -a "$SYNOPKG_DSM_VERSION_MAJOR" -lt 7 ]; then
if [ -z "${SVC_CWD}" ]; then
SVC_CD=""
else
SVC_CD="cd ${SVC_CWD}; "
fi
if [ "$(whoami)" = "root" ]; then
SU="su ${EFF_USER} -s"
else
SU=""
fi
if [ -z "${SVC_BACKGROUND}" ]; then
$SU /bin/sh -c "${SVC_CD}${service}" >> ${OUT} 2>&1
else
$SU /bin/sh -c "${SVC_CD}${service}" >> ${OUT} 2>&1 &
fi
else
# DSM user is set by conf/privilege
if [ -n "${SVC_CWD}" ]; then
cd "${SVC_CWD}"
fi
if [ -z "${SVC_BACKGROUND}" ]; then
${service} >> ${OUT} 2>&1
else
${service} >> ${OUT} 2>&1 &
fi
fi
if [ -n "${SVC_WRITE_PID}" -a -n "${SVC_BACKGROUND}" -a -n "${PID_FILE}" ]; then
[ $i -eq 1 ] && echo -ne "$!" > ${PID_FILE} || echo -ne " $!" >> ${PID_FILE}
else
wait_for_status 0 ${SVC_WAIT_TIMEOUT:=20}
fi
fi |
@smaarn, I cloned your PR into a new one (#5923) and committed a number of changes to implement some things including:
The summary of code changes are shown here. Thus far on DSM 6 the package is able to successfully install and configure rss feeds; upgrade with the same version and retain configurations; and uninstall, cleaning up Apache and PHP configs. Once you are comfortable you can merge the new commits into your PR. Some things remain to be refined in the package based on my observation:
EDIT: I have since tested an install, configure, upgrade and uninstall on DSM 7 and all looks to be working. The caveats around the log errors and icon files above still apply. EDIT: I've pushed some additional code to my PR which amends the installation wizard to check and advise users if multiple PHP profiles are present. EDIT: I've submitted an extra commit to rectify an issue related to the configured PHP executable. An adjustment was necessary for users upgrading from the previous package version. |
|
Closed to PR #5923 |
Description
Upgrade tt-rss to latest version and migrate wizard to wizard templates
Checklist
all-supportedcompleted successfullyType of change