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

spksrc: Fully use DSM7 var directory #5163

Merged
merged 4 commits into from
Apr 10, 2022

Conversation

th0ma7
Copy link
Contributor

@th0ma7 th0ma7 commented Mar 21, 2022

Description

Motivation: Follow-up to to both #4579 and #4797 with focus only on var. This now uses previously merged PR such as:

Current framework simply move files from target/var to target/../var at package installation/upgrade time. While it's probably fine for most use-cases, this doesn't adapt all configuration files at build time using the "real" end-state path and most importantly the application isn't "aware" by default of the expected var directory path.

Solving this require using localstatedir option at configuration time. Doing so enforces the application to have an awareness and use by default the new DSM7 var directory and thus, installs files at the proper location and adjust paths in configurations at build time. Not doing so leaves a bit of unknown on actual application state & behavior.

This PR aims at solving the current limitations by doing the following on top of rsync work (#4997):

  1. enforcing the usage of localstatedir for DSM7 builds. At package generation time, files installed under <workdir>/install/var/package/<package>/var are now being handled in order to be packaged and installed properly.

Details

This new implementation is a major step above previous attempt as it:

  1. requires NO changes at all in any packages.
  2. is key to demonstrate feasibility to also migrate etc in a later work

diag-var-spksrc

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)

@Safihre
Copy link
Contributor

Safihre commented Mar 23, 2022

I tried to review, since I know you put a lot of work in it, but I am not fully understanding (even after reading the previous PR) what this change will do?
Maybe you could explain it again shortly?

@th0ma7
Copy link
Contributor Author

th0ma7 commented Mar 23, 2022

Thnx @Safihre for looking into it. It is now ready for review (you where a few minutes to early). I've added a much better description + a diagram that explains it hopefully well.

Copy link
Contributor

@Safihre Safihre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on your diagram, it looks good. All changes seem logical.
But @hgy59 or someone else is more an expert.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Apr 4, 2022

@SynoCommunity/developers Friendly reminder for reviews. Unless there is opposition to this PR I'll look into merging in the coming days.

@th0ma7 th0ma7 merged commit c2a1590 into SynoCommunity:master Apr 10, 2022
@th0ma7 th0ma7 deleted the use-var-default branch April 10, 2022 23:04
@hgy59
Copy link
Contributor

hgy59 commented Apr 19, 2022

I have a package under development (urbackup) where the cross packages create config files in the var folder. PLIST contains entries like rsc:var/urbackup/dataplan_db.txt.

After rebase to include this PR, DSM 7 packages cannot be created anymore.

  • The install folder for DSM 7 does not contain the {packagename}/target/var but the {packagename}/var with the required config data.
  • The staging folder for DSM 7 is empty, and does not contain the directory structure of {packagename}/var in the install folder, so there is an error:
    ===>  [DSM7+] Copy & merge var and target/var to /spksrc/spk/urbackup/work-x64-7.0/staging/var
    tar: Cowardly refusing to create an empty archive
    

@th0ma7 any idea how to handle this?

@th0ma7
Copy link
Contributor Author

th0ma7 commented Apr 19, 2022

@hgy59 can you point me to your development branch i'll look into it probably tomorrow.

hgy59 added a commit to hgy59/spksrc that referenced this pull request Apr 19, 2022
- fix "Copy and merge var and target/var to $(STAGING_DIR)/var"
@hgy59
Copy link
Contributor

hgy59 commented Apr 19, 2022

@th0ma7 the copy and merge of different var folders needs two command lines, as tar must be executed within each var folder separately.

hgy59 added a commit to hgy59/spksrc that referenced this pull request Apr 19, 2022
- rework fix "Copy and merge var and target/var to $(STAGING_DIR)/var"
@hgy59
Copy link
Contributor

hgy59 commented Apr 19, 2022

The following existing cross packages have var folders in PLIST

  • domoticz
  • ejabberd
  • mutt
  • rsnapshot
  • saltgui (spk: salt-master)
  • zap2epg (spk: tvheadend)

@th0ma7 shall I create a dedicated PR to merge this fix to master?

@th0ma7
Copy link
Contributor Author

th0ma7 commented Apr 19, 2022

Yes please do @hgy59 , I'll have a second look at it as well.

EDIT: I can't reproduce your issue... I tried with rsnapshot and mutt and all is good with the current code.

hgy59 added a commit to hgy59/spksrc that referenced this pull request Apr 20, 2022
- avoid .. in INSTALL_PREFIX_VAR
@hgy59 hgy59 mentioned this pull request Apr 20, 2022
10 tasks
hgy59 added a commit to hgy59/spksrc that referenced this pull request Aug 5, 2022
- use resource worker to create shared folder
- adjust wizards for shared folder creation
- create readme.html that can be opened in the browser
- create dummy file as for DSM7 we cannot create packages without content (since SynoCommunity#5163: Fully use DSM7 var directory)
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.

3 participants