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

Clobbers symlinked config file #1628

Open
gustaphe opened this issue Sep 7, 2023 · 2 comments
Open

Clobbers symlinked config file #1628

gustaphe opened this issue Sep 7, 2023 · 2 comments
Labels

Comments

@gustaphe
Copy link

gustaphe commented Sep 7, 2023

Describe the bug

If ~/.config/plover/plover.cfg is a symlink to somewhere else, config changes made in Plover delete the symlink and create a new ~/.config/plover/plover.cfg.

To Reproduce

Steps to reproduce the behavior:

  1. $ mv ~/.config/plover/plover.cfg ~/.dotfiles/plover.cfg
  2. $ ln -s ~/.dotfiles/plover.cfg ~/.config/plover/plover.cfg
  3. Open Plover, edit config
  4. diff ~/.dotfiles/plover.cfg ~/.config/plover/plover.cfg
  5. There's a difference between the two files, ~/.config/plover/plover.cfg is no longer a symlink, and ~/.dotfiles/plover.cfg does not have the latest edits

Expected behavior

I expect Plover to follow the symlink and update the indicated config file, rather than create a new one. My guess is the suggested solution in #823 would fix this too.

Operating system

  • OS: Arch Linux
  • Plover Version 4.0.0.dev12+8.g90575048
@gustaphe gustaphe added the bug label Sep 7, 2023
@user202729
Copy link
Member

user202729 commented Sep 8, 2023

Not really, the cause is different.
Plover writes to a temporary configuration file, and if that is successful, it atomically (I assume?) moves the new configuration file to the original file.

tempfile = NamedTemporaryFile(delete=False, dir=directory,

Maybe we can do something like this.

--- a/plover/resource.py
+++ b/plover/resource.py
@@ -35,7 +35,7 @@ def resource_timestamp(resource_name):
 def resource_update(resource_name):
     if resource_name.startswith(ASSET_SCHEME):
         raise ValueError(f'updating an asset is unsupported: {resource_name}')
-    filename = resource_filename(resource_name)
+    filename = os.path.realpath(resource_filename(resource_name))
     directory = os.path.dirname(filename)
     extension = os.path.splitext(filename)[1]
     tempfile = NamedTemporaryFile(delete=False, dir=directory,

It's important that the temporary file is on the same file system as the target file to be moved to (TODO check)

If the user intentionally create a symbolic link, this seems to be more desirable. However, if in some case a symbolic link is unintentionally created, problem happens.

@gustaphe
Copy link
Author

gustaphe commented Sep 8, 2023

What problems would an unintentional symlink cause?

Fwiw, for my usecase a $PLOVERCONFIG environment variable would solve the same underlying problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants