DSM7 method to manage @appdata, @apptemp and @apphome directories

[th0ma7]

Motivation:
The package.tgz only contains /var/packages/<package>/target directory (refereed as @appstore). This PR aim at working around that in order to allow installing files also into @appdata, @apptemp and @apphome directories.

A good example is with the tvheadend package where a few mandatory files needs to be contained under the new @appdata directory for the application to function. With the DSM7 new directory structure there are no workaround to allow this. Usage example available here 90985a4

With the DSM7 migration, a quick count shows more than 60x packages needing to drop files into the new /var/packages/<package>/var instead of the previous /var/packages/<package>/target/var directory. There are another set of 27x cross/* using etc and var directories at build time.

This PR creates the following new variable to be used at build time (backward compatible <= DSM6):

• INSTALL_PREFIX_ETC = target/../etc
• INSTALL_PREFIX_VAR = target/../var
• INSTALL_PREFIX_TMP = target/../tmp
• INSTALL_PREFIX_HOME = target/../home

And also creates the following new variables to be used at package creation time (backward compatible <= DSM6):

• STAGING_SPKETC -> staging/@etc
• STAGING_SPKVAR -> staging/@var
• STAGING_SPKTMP -> staging/@tmp
• STAGING_SPKHOME -> staging/@home

Linked issues: #4545

TODO

• Define wether or not we keep original config files under target/@app*. Currently they are being discarded post-rsync.
• Add support for etc
• Migrate preupgrade to using rsync + delete
• Create a backup .tgz copy prior to rsync + delete at preupgrade. OR if size to big do a rsync + mv of target/var as target/var.backup.DSM7-upgrade so it is kept and not reprocessed at next upgrade.
• Should then the target/env used for python wheels be migrated to $HOME leading to @apphome ? Differ for another PR if needed.
• Allow using non-target path at build time (NEW)

Checklist

• Build rule all-supported completed successfully
• Package upgrade completed successfully
• New installation of package completed successfully

[th0ma7]

@hgy59 and @publicarray unless you think of another approach I believe I pretty much nailed it. This should allow migrating all packages needing to drop files under the new var directory structure in a relatively trivial manner.

A pending question is: should we keep or not the source files under target? Current behavior deletes them.

Ready for your review.

[publicarray]

I don't think you should use @appdata etc. since that is where stuff is stored internally and can also change per volume. Please only use /var/packages/[packagename]/[etc/var/home/tmp/target] on DSM7. I also think it's conceptually easier to browse the files that way

/var/packages/[package_name]
├── etc -> /usr/syno/etc/packages/[package_name]
├── var -> /volume[volume_number]/@appdata/[package_name]
├── tmp -> /volume[volume_number]/@apptemp/[package_name]
├── home -> /volume[volume_number]/@apphome/[package_name]
└── target -> /volume[volume_number]/@appstore/[package_name]

[publicarray]

With the DSM7 migration, a quick count shows more than 60x packages needing to drop files into the new /var/packages//var instead of the previous /var/packages//target/var directory.

This is already migrated. On fresh DSM7 install the files are copied from /target/var to /var.

This is why correcting 72be928 was so important. If you want you can change this behaviour, but I don't follow why you need to...

With the DSM7 new directory structure there are no workaround to allow this.

[th0ma7]

This is why correcting 72be928 was so important. If you want you can change this behaviour, but I don't follow why you need to...

Well interestingly the postinst part of the code didn't worked with a fresh install leading me to find another approach. Also, it does not address any "new" files added into var part of subsequent package upgrades, something my patch address.

This new approach does also work for all other @app* directories if we ever need to make use of them.

Behavior is simple, using new variables it create "temporary" @app* directories under staging. Only if not empty the directories will be added to package.tgz. Upon postisntall and postupgrade they are then migrated from target over to their final "real" @app* destination (with no overwrite)

[th0ma7]

I don't think you should use @appdata etc. since that is where stuff is stored internally and can also change per volume. Please only use /var/packages/[packagename]/[etc/var/home/tmp/target] on DSM7. I also think it's conceptually easier to browse the files that way

I'm not really using it as I'm only referring ${SYNOPKG_PKGDEST} and move from there relatively such as ${SYNOPKG_PKGDEST}/../../@app*

Also, re-using the @app naming from staging then to migrate from target to destination directory greatly simplifies the shell code by using direct printout @${appdir##*@}

[th0ma7]

@publicarray also, the preupgrade copy would benefit from using rsync as well...

EDIT: I would actually suggest doing a var-preDSM7.backup.tgz first, then rsync and delete.

[publicarray]

Thanks, using rsync is a good idea 👍

code didn't work

I've tested it thoroughly and it works as designed. I don't know why it failed for you. It maybe that you had left over user files there, so the code was skipped (my design decision). I thought users files should not be overridden/added, but maybe I was wrong. Using rsync would fix that... 🤷

This new approach does also work for all other @app* directories if we ever need to make use of them.

Yes adding more folders to be supported (home, etc) is appreciated.

(with no overwrite)

Hm, The chances of a package adding new config files is very slim so the wrong behaviour that you experienced is only apparent when you don't have a clean new install (you had a broken package with missing files). So maybe we should overwrite? This is the current behaviour in DSM6 I think. And store user files in home? So that way packages that update their default config will be updated automatically. Unfortunately all of this is package dependent. I.E. we don't want to overwrite the password file for tvheadend we would have to move it to home.

I'm not really using it as I'm only referring ${SYNOPKG_PKGDEST} and move from there relatively such as ${SYNOPKG_PKGDEST}/../../@app*

I think that's a bad idea AFAIK the path is not documented. Why not use ${SYNOPKG_PKGDEST}/../var (SYNOPKG_PKGVAR), ${SYNOPKG_PKGDEST}/../home (SYNOPKG_PKGHOME), ${SYNOPKG_PKGDEST}/../tmp (SYNOPKG_PKGTMP) ... as documented? Simpler code is not a good reason in this case IMHO

[publicarray]

Personally I would ignore any @app* stuff as it's mainly Synology's implementation detail, I would go so far as to ignore its existence. IMHO packages should only modify files in /var/packages/[packagename]/[etc/var/home/tmp/target] or files/folder accessible via SYNOPKG_* variables for consistency and compatibility.

Packages can also have the following structure if it is installed on a system partition:

/var/packages/[package_name]
├── etc -> /usr/syno/etc/packages/[package_name]
├── var -> /usr/local/packages/@appdata/[package_name]
├── tmp -> /usr/local/packages/@apptemp/[package_name]
├── home -> /usr/local/packages/@apphome/[package_name]
└── target -> /usr/local/packages/@appstore/[package_name]

[th0ma7]

Thanks, using rsync is a good idea +1

code didn't work

I've tested it thoroughly and it works as designed. I don't know why it failed for you. It maybe that you had left over user files there, so the code was skipped (my design decision). I thought users files should not be overridden/added, but maybe I was wrong. Using rsync would fix that... shrug

I can confirm that I tested it using clean install/reinstall/etc. I really tried figuring out why it didn't worked. My best guess is [ "$(find ${SYNOPKG_PKGVAR} -mindepth 1 -not -name '*.log' -print)" = "" ] which probably failed somehow on my platform (running official + license synology kvmx64 DSM7 image)

Yes adding more folders to be supported (home, etc) is appreciated.

etc is actually missing from the code... to add I guess?

(with no overwrite)

So maybe we should overwrite? This is the current behavior in DSM6 I think.

Indeed in DSM6 it gets overwritten because target is fully rewritten anyway leading us to backup-copy over the previous config files after upgrading. On DSM7 the files in var are untouched... this changes and simplify the behavior significantly.

And store user files in home? So that way packages that update their default config will be updated automatically.

Not sure I understand how home can be that useful, can you elaborate a bit further?

Unfortunately all of this is package dependent. I.E. we don't want to overwrite the password file for tvheadend we would have to move it to home.

If I understand the behavior correctly, config files will not be overwritten in var by default, thus leading to copy-no-overwrite when rsynching afterwards. This way only "new" config files gets added, if any. To me the question is more: do we keep a copy of the "original" configuration files par of target or discard them after synching over to var? Currently discarding entirely but not sure its the right approach.

I'm not really using it as I'm only referring ${SYNOPKG_PKGDEST} and move from there relatively such as ${SYNOPKG_PKGDEST}/../../@app*

I think that's a bad idea AFAIK the path is not documented. Why not use ${SYNOPKG_PKGDEST}/../var (SYNOPKG_PKGVAR), ${SYNOPKG_PKGDEST}/../home (SYNOPKG_PKGHOME), ${SYNOPKG_PKGDEST}/../tmp (SYNOPKG_PKGTMP) ... as documented? Simpler code is not a good reason in this case IMHO

You are right. When playing with this I ended-up sending an echo 1>&2 to understand what was going on. I noticed that the ${SYNOPKG_PKGDEST} is actually expended to its final destination instead of the symlink. Therefore it requires a ../../ to be relative from there. Extract from the log:

/bin/rsync -avh --ignore-existing --remove-source-files /volume1/@appstore/tvheadend/@appdata/* /volume1/@appstore/tvheadend/../../@appdata/tvheadend

[th0ma7]

Personally I would ignore any @app* stuff as it's mainly Synology's implementation detail, I would go so far as to ignore its existence. IMHO packages should only modify files in /var/packages/[packagename]/[etc/var/home/tmp/target] or files/folder accessible via SYNOPKG_* variables for consistency and compatibility.

Currently using relative to ${SYNOPKG_PKGDEST}. Could probably use another approach but to the detriment of overly complexifying the code.

Packages can also have the following structure if it is installed on a system partition:

/var/packages/[package_name]
├── etc -> /usr/syno/etc/packages/[package_name]
├── var -> /usr/local/packages/@appdata/[package_name]
├── tmp -> /usr/local/packages/@apptemp/[package_name]
├── home -> /usr/local/packages/@apphome/[package_name]
└── target -> /usr/local/packages/@appstore/[package_name]

Which currently will also work due to its relativeness to ${SYNOPKG_PKGDEST} is kept. Except for etc which requires a different approach, which could lead in adapting the existing to meet this, duno. I will see what I can come up with.

[th0ma7]

@publicarray btw, from the dev manual in theory etc should be used for configuration files. This could require compile options to work out in some cases. The tvheadend is one that can't move as relative to the var (thus web) structure by default but I guess many other could use that.

EDIT: Another point, should then the target/env used for python wheels be migrated to $HOME leading to @apphome.

[th0ma7]

@publicarray moving back to [WIP] and added some comments about TODO changes. Let me know what you see fit.

[th0ma7]

@publicarray I've migrated to dsm6 -> dsm7 migration to rsync + backup and support for etc.
Unit testing looks good on my end but would benefit of additional testing.
Items left aside for now:

• delete or not the files for target/@* after rsyncing over to their final destination etc, var, home, tmp?

As for target/env I'd leave that up for a different PR as may need per package solution.

[th0ma7]

@publicarray I recall you where going to be out for a while. Re-raising this to confirm wetter it is of interest or not?

[th0ma7]

@hgy59 and @ymartin59 , in lack of having @publicarray around, would you mind providing feedback on this PR?

We need a solution for use cases such as mentioned in #4545 (review)

[hgy59]

We need a solution for use cases such as mentioned in #4545 (review)

IMHO we don't.
I am happy with /var folder in package.tgz that is extracted to /var/packages/{package}/target/var (and on DSM7: moved to /var/packages/{package}/var).

As we already discussed, the installer must not use any "@*" name but only folders starting with /var/packages.

And IMHO we do not need to provide generic support for etc and home folders as those can be left in the care of the packages.

[th0ma7]

IMHO we don't.

I agree to disagree and really think we have to. Here's a try at explaining the reasoning behind this :)

I am happy with /var folder in package.tgz that is extracted to /var/packages/{package}/target/var (and on DSM7: moved to /var/packages/{package}/var).

There was a bug I was faced with at the time: a sort of race condition happens somewhere between pre and post where "new" files ended-up still residing under former target/var directory.

Side effect was silent failing of sed command at tvheadend post-installation time when modifying non-existing file for managing password and thus, inability to login to the web application (e.g. broken installation and unusable application, ref #4545 (comment)). Directory structure looked similar to follow:

# tree -d -L 1 target/var
target/var
├── accesscontrol
└── passwd
# tree -d -L 1 var
var
├── backup
├── bouquet
├── caclient
├── channel
├── codec
├── dvr
├── epggrab
├── esfilter
├── imagecache
├── input
├── profile
├── service_mapper
└── timeshift

As we already discussed, the installer must not use any "@*" name but only folders starting with /var/packages.

Thnx for the reminder, I now recall that discussion. And as mentioned previously, current patchset never made any reference to /var/package/*/@.. directories on DSM. It always referred to relative path from target such as <package>/target/../<destination>.

The delimiter was just convenient at package creation time under the staging directory tree while matching DSM7 naming. Commit 697425d alter that to using _ instead of @ (but really that's just cosmetic).

And IMHO we do not need to provide generic support for etc and home folders as those can be left in the care of the packages.

I must admit that tmp is not that much of a use, and value of home is limited. Although:

1. currently missing is the --localstatedir=$(INSTALL_PREFIX_VAR) for var so application be in a readiness state by default to using this new path at compilation time (otherwise some will fall under the default prefix being target/var).
2. patchset currently enforce etc with the usage of --sysconfdir=$(INSTALL_PREFIX_ETC) at compile time as well
3. home is actually the new $HOME for application instead of the previously <application>/target and could end-up being convenient:

th0ma7@DSM7:/$ cd ~sc-tvheadend
th0ma7@DSM7:/var/packages/tvheadend/home$ pwd
/var/packages/tvheadend/home

Overall I really think we miss an opportunity here of"

1. rigorously follow Synology's directory structure
2. distinguishing the "upgrade" action from the direct installation of "new" files under this directory structure
3. need to adjust compilation flags to make usage of that new directory structure by default

[th0ma7]

@hgy59 just a thought...

Wouldn't (or shouldn't) python wheels ends-up going under $HOME?
And would keeping access to $HOME (which has now moved) be of interest for app user environment/profile?

Also, I noticed that some packages make usage of target/etc such as ejaberd, irssi, lirc, etc. With this patchset we could then provide a "real" etc path to accommodate such packages properly.

[th0ma7]

Thnx @ymartin59 for this approving this PR, but really the "first" step in achieving this starts with #4797
If you have time to go over that one instead would be much appreciated, Thnx in advance.

[burghy86]

@hgy59 just a thought...

Wouldn't (or shouldn't) python wheels ends-up going under $HOME? And would keeping access to $HOME (which has now moved) be of interest for app user environment/profile?

Also, I noticed that some packages make usage of target/etc such as ejaberd, irssi, lirc, etc. With this patchset we could then provide a "real" etc path to accommodate such packages properly.

yes please. implemented irssi in a dsm7.x