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

Define var using localstatedir and use rsync #4797

Closed
wants to merge 42 commits into from

Conversation

th0ma7
Copy link
Contributor

@th0ma7 th0ma7 commented Aug 18, 2021

Motivation: Follow-up to #4579 with focus only on var. This includes migrating to using rsync for this new installation method into spksrc.service.installer.dsm7.

Current framework simply move files from target/var to target/../var at installation/upgrade time. While it's probably fine for most use-cases, in order to generate proper configuration files at compile time the "real" path must be given at configure time using localstatedir option. Doing so enforces the application to use by default the new DSM7 var directory and install and adjust path in configurations accordingly at build time. Not doing so leaves a bit of unknown on actual application state & behavior.

Current limitations of spksrc fixed by this PR:

  1. application not set to use var properly by default (e.g. at configure/compile time)
  2. package updated configuration files under var during package upgrade overwriting existing target/../var
  3. inability to provide updated configuration file examples under var for user reference

This PR aims at solving the current limitations by:

  1. enforcing the usage of localstatedir for DSM7 builds. At package generation time, files installed under <workdir>/install/var/package/<package>/var are now be handled differently in order to be packaged and installed properly.
  2. Uses rsync to synchronize folders at installation time
  3. Add a functionality that allows generating .new suffix to configuration new files when previous ones already exist. User can then refer to them and adapt their own configuration as needed.

Changes

1. @appdata (e.g. var)

The package.tgz only contains target directory (referred as @appstore). The framework currently copies over the target/var to target/../var directory. The limitation is that it doesn't change the configure options to make usage of localstatedir so the application isn't built by default to be in a readiness state for using this new directory structure. Further more, it doesn't allow to provide updated files with proper path that the user could refer to during upgrades.

This PR aim at working around that in order to fully use var (referred as @appdata):

  1. Build all packages using localstatedir configure option pointing to this /var/packages/<package>/var by default
  2. Allow packaging and installing files in this new directory by "staging" them at packaging under _var

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

  • INSTALL_PREFIX_VAR = /var/packages/<package/target/../var-> Used for setting up the localstatedir path
  • STAGING_INSTALL_PREFIX_VAR = work-*/install/....../target/../var -> Used under cross/* installation
  • STAGING_SPKVAR -> Used for copying files directly to staging under work-*/staging/_var at spk pre-packaging time

2. spksrc.service.installer.dsm7 (rsync usage, merged part of #4997)

A few changes where made to the dsm7 installer script in order to

  1. Allow installing files directly in .../target/../var
  2. Allow installing with ".new" suffix when files already exists under .../target/../var in order to never overwrite user-configurations while still providing updated examples for reference.
  3. Use a tmp directory when upgrading packages from DSM6 -> DSM7 where var files gets lost due to missing @appdata

At the lower level it works such as:

  1. Use rsync instead of cp to copy files
  2. When upgrading a package, at preupgrade, copy any "residual" DSM6 files from .../target/var/* to a tmp directory
  3. When upgrading a package, at postupgrade, copy any "residual" DSM6 files from tmp directory back to the new .../target/../var. As theses files where pre-existing from a previous version of the package, it uses the --backup flag to move any conflicting files with .new suffix and puts back the original user files in its location.
  4. When installing, copy-no-overwrite all var files located under .../target/_var to .../target/../var. Any conflicting file gets renamed to .new and sync'ed to .../target/../var ensuring to keep the original user configuration while providing fresh updated configuration examples.

3. Migrate SYNOPKG_PKGDEST/var to SYNOPKG_SPKVAR in all spk/*/src/service_setup.sh

As identified in #4895 (comment) there are many service_setup.sh scripts still using SYNOPKG_PKGDEST/var which will necessarely break under DSM7. All of them are being migrated over to using SYNOPKG_SPKVAR

Linked issues: #4545, #4758, #4895, #4910

Checklist

  • Build rule all-supported completed successfully

All packages cannot be tested, tested successfully:

  • tvheadend
  • ffmpeg
  • chromaprint
  • comskip
  • python38
  • Package upgrade completed successfully
  • New installation of package completed successfully

@th0ma7 th0ma7 changed the title Define var and use rsync [WIP] define var and use rsync Aug 18, 2021
@th0ma7 th0ma7 self-assigned this Aug 18, 2021
@th0ma7
Copy link
Contributor Author

th0ma7 commented Aug 18, 2021

Hi @publicarray and @hgy59, finally slowly coming back and revisiting this PR.
As requested I've stripped out all other directories, leaving only var to be managed by default.
I still have to run a few tests to confirm code-porting is OK but early comments would be much appreciated.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Sep 8, 2021

@publicarray issue #4758 is due to "new" files being deployed under the target/var instead of the new var.
Rebuilding against this PR to confirm that it does solves that.

EDIT: This PR do solve the issue for tvheadend and been working well, additional testers would be great to confirm all is ok.

@th0ma7 th0ma7 changed the title [WIP] define var and use rsync Define var using localstatedir and use rsync Sep 8, 2021
@th0ma7 th0ma7 requested a review from hgy59 September 8, 2021 15:23
@th0ma7 th0ma7 force-pushed the etc_rsync branch 2 times, most recently from 668f8df to 4b326e3 Compare September 10, 2021 12:56
@th0ma7
Copy link
Contributor Author

th0ma7 commented Sep 10, 2021

@publicarray All changes are now in and all packages adjusted accordingly.

I'll be running a few more tests of my own to confirm all is still ok for my regular packages. But in theory this should make all packages fully using target/../var directory for DSM => 7, from configure (with all proper switches) , to build then to install time.

@hgy59 you where right, indeed it was worth splitting things over in separate PR as var on its own is already quite significant once all packages are adapted to using that.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Sep 13, 2021

Now that #4854 is completed, I'm back to this pending PR.
I've rebased to master and peding github-action build to see any hicups.
A re-review from you guys @publicarray and @hgy59 would much appreciated, thnx in advance.

@th0ma7 th0ma7 changed the title Define var using localstatedir and use rsync [WIP] Define var using localstatedir and use rsync Sep 17, 2021
@th0ma7
Copy link
Contributor Author

th0ma7 commented Sep 17, 2021

There is a caveat in the spksrc.copy.mk that I have to figure out... I got some ideas to solve it but that will require a few cycles. Will ping you back guys for a review.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Sep 28, 2021

@hgy59 and @publicarray I was able to solve my last blocker to properly inherit copying while managing var properly. I'm now quite please with the output. With theses changes all DSM7 packages builds are now behaving as follow:

  1. natively built to use the /var/packages/<package>/var directory
  2. at staging file copying time, var files are being copied over to work-<arch>-7.0/staging/_var
  3. at package installation time the resulting /var/packages/<package>/target/_var is being rsync'ed to /var/packages/<package>/var

I've ran quite a few tests with the demoservice that I enhanced to add README files in other typical directories to watch the behaviour. demoservice output for DSM7 is as follow, notice the _var:

$ tar -ztvf package.tgz 
drwxr-xr-x root/root         0 2021-09-28 02:11 ./bin/
-rw-r--r-- root/root        17 2021-09-28 02:11 ./bin/README.bin
drwxr-xr-x root/root         0 2021-09-28 02:11 ./etc/
-rw-r--r-- root/root        17 2021-09-28 02:11 ./etc/README.etc
drwxr-xr-x root/root         0 2021-09-28 02:11 ./app/
-rw-r--r-- root/root        99 2021-09-28 02:11 ./app/demoservice.sc
-rw-r--r-- root/root       455 2021-09-28 02:11 ./app/config
drwxr-xr-x root/root         0 2021-09-28 02:11 ./app/images/
-rw-r--r-- root/root      1238 2021-09-28 02:11 ./app/images/demoservice-24.png
-rw-r--r-- root/root      4148 2021-09-28 02:11 ./app/images/demoservice-64.png
-rw-r--r-- root/root     32372 2021-09-28 02:11 ./app/images/demoservice-256.png
-rw-r--r-- root/root       941 2021-09-28 02:11 ./app/images/demoservice-16.png
-rw-r--r-- root/root      4859 2021-09-28 02:11 ./app/images/demoservice-72.png
-rw-r--r-- root/root      1743 2021-09-28 02:11 ./app/images/demoservice-32.png
-rw-r--r-- root/root      2865 2021-09-28 02:11 ./app/images/demoservice-48.png
-rw-r--r-- root/root        17 2021-09-28 02:11 ./README.target
drwxr-xr-x root/root         0 2021-09-28 02:11 ./_var/
-rw-r--r-- root/root        17 2021-09-28 02:11 ./_var/README.var
drwxr-xr-x root/root         0 2021-09-28 02:11 ./tmp/
-rw-r--r-- root/root        17 2021-09-28 02:11 ./tmp/README.tmp

demoservice for DSM6:

drwxr-xr-x root/root         0 2021-09-28 02:11 ./bin/
-rw-r--r-- root/root        17 2021-09-28 02:11 ./bin/README.bin
drwxr-xr-x root/root         0 2021-09-28 02:11 ./app/
-rw-r--r-- root/root        99 2021-09-28 02:11 ./app/demoservice.sc
-rw-r--r-- root/root       455 2021-09-28 02:11 ./app/config
drwxr-xr-x root/root         0 2021-09-28 02:11 ./app/images/
-rw-r--r-- root/root      4148 2021-09-28 02:11 ./app/images/demoservice-64.png
-rw-r--r-- root/root       941 2021-09-28 02:11 ./app/images/demoservice-16.png
-rw-r--r-- root/root      1238 2021-09-28 02:11 ./app/images/demoservice-24.png
-rw-r--r-- root/root      1743 2021-09-28 02:11 ./app/images/demoservice-32.png
-rw-r--r-- root/root      4859 2021-09-28 02:11 ./app/images/demoservice-72.png
-rw-r--r-- root/root     32372 2021-09-28 02:11 ./app/images/demoservice-256.png
-rw-r--r-- root/root      2865 2021-09-28 02:11 ./app/images/demoservice-48.png
drwxr-xr-x root/root         0 2021-09-28 02:11 ./etc/
-rw-r--r-- root/root        17 2021-09-28 02:11 ./etc/README.etc
-rw-r--r-- root/root        17 2021-09-28 02:11 ./README.target
drwxr-xr-x root/root         0 2021-09-28 02:11 ./tmp/
-rw-r--r-- root/root        17 2021-09-28 02:11 ./tmp/README.tmp
drwxr-xr-x root/root         0 2021-09-28 02:11 ./var/
-rw-r--r-- root/root        17 2021-09-28 02:11 ./var/README.var

This is the next step into using natively the Synology DSM7 var directory as official localstatedir. For your consideration, testing & review which would be much appreciated.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Oct 6, 2021

@publicarray and @hgy59 suggestion for migrating src/installer.sh use cases to using SYNOPKG_PKGVAR, brutal but would be a start.

sed -e 's?^INSTALL_DIR=.*$?SYNOPKG_PKGVAR="${SYNOPKG_PKGDEST}/var"?g' \
    -e 's?${INSTALL_DIR}/var?${SYNOPKG_PKGVAR}?g' \
    -e 's?${INSTALL_DIR}?${SYNOPKG_PKGDEST}?g' src/installer.sh

Unless all packages using INSTALLER_SCRIPT are simply declared broken or not compatible until migrated to service-setup.sh for DSM7 ?

EDIT: Revisiting this, makes no sense... too many use cases. Suggesting to declare them as not compatible for DSM7 until migrated to service-setup.sh.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Oct 6, 2021

@hgy59 and @publicarray proposing the following change in pre-check.mk to stop building non-DSM7 compatible packages cc966a7

th0ma7 added 22 commits December 8, 2021 00:59
When testing the package creation with spk/demoservice it appeared
that files in staging/README was not being processed.  This is due
to the "-type d" option which only include directories, thus leaving
files location at root of staging (e.g. target/) to be skipped.
Otherwise ends-up getting Cannot open: No such file or directory
Use tmp directory for DSM6-7 migration instead of moving
right away to permanent storage as it is not available
immediately when upgrading.

Also, when copying over new target/_var/* files, any existing
will be added suffix .new and rsync a second time so user
will keep his existing but will have access to any new versions
of the configuration file.
postupgrade now use the --backup in conjunction with --suffix=.new
which will update any changed files with the previous copy and
rename the file in place to .new so it can be used as reference
for the user.
@th0ma7
Copy link
Contributor Author

th0ma7 commented Dec 8, 2021

Hi fellow @SynoCommunity/developers , winter is coming and I would really much like to get this merged before spring break.

Every smaller bits have been merged into other PR. Now whats left is the framework changes to fully handle Synology's new var directory schema (e.g. making mileage on what @publicarray had done previously). This also sets the pathway to handle other directories in the future as needed.

So overall I'm now begging to get a thumbs-up before merging so I can focus on other things afterwards (e.g. rustc support). Thnx in advance.

@th0ma7 th0ma7 mentioned this pull request Mar 21, 2022
10 tasks
@th0ma7 th0ma7 closed this Mar 23, 2022
@th0ma7 th0ma7 deleted the etc_rsync branch March 23, 2022 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants