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

syncthingtray: fix config persist #14230

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

qianlongzt
Copy link
Contributor

@qianlongzt qianlongzt commented Oct 16, 2024

fix config file syncthingtray.ini persist

Copy link
Contributor

All changes look good.

Wait for review from human collaborators.

syncthingtray

  • Description
  • License
  • Hashes
  • Checkver
  • Autoupdate

@Higramx
Copy link

Higramx commented Oct 25, 2024

I think what's preferable is making a symlink.
The below part could be put into pre_install:

"$config = \"syncthingtray.ini\"",
"if (!(Test-Path \"$persist_dir\\$config\")) { New-Item \"$persist_dir\\$config\" -ItemType File | Out-Null }",
"New-Item -Type SymbolicLink -Value $persist_dir\\$config -Path $dir\\$config | Out-Null"

You could then remove the post_install and pre_uninstall parts.
I also don't think the "ensure" things are necessary (in that case).

@qianlongzt
Copy link
Contributor Author

Yes, I tested the symlink, and it works correctly. It does not delete the symlink; it deletes the target file and recreates it in the target location. ensure is still necessary, or an error might occur.

But symlink need administrator privilege

image

@Higramx
Copy link

Higramx commented Oct 25, 2024

Does persist not require admin privilege? I have not tested without.

@qianlongzt
Copy link
Contributor Author

hard link and junction: normal user
symlink: admin or enable developer mode

https://github.com/ScoopInstaller/Scoop/blob/859d1db51bcc840903d5280567846ae2f7207ca2/lib/diagnostic.ps1#L59

@Higramx
Copy link

Higramx commented Oct 25, 2024

ok thanks

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