GothenburgBitFactory / timewarrior

Timewarrior - Commandline Time Tracking and Reporting
https://timewarrior.net
MIT License
1.26k stars 95 forks source link

Using timew config replaces symlink with physical file #546

Closed crossbone-magister closed 1 year ago

crossbone-magister commented 1 year ago

Report

If file ~/.config/timewarrior.cfg is a symlink, running timew config to update the configuration replaces it with a physical file.

Example steps to reproduce

wsl@wsl:~/.config/timewarrior$ ln -s /path/to/config/timewarrior.cfg timewarrior.cfg

wsl@wsl:~/.config/timewarrior$ ls -la
extensions
timewarrior.cfg -> /path/to/config/timewarrior.cfg

wsl@wsl:~/.config/timewarrior$ timew config test test
Are you sure you want to add 'test' with a value of 'test'? (yes/no) yes
Config file '/home/wsl/.config/timewarrior/timewarrior.cfg' modified.

wsl@wsl:~/.config/timewarrior$ ls -la
total 4
drwx------ 1 wsl wsl 512 Jun 13 18:43 .
drwx------ 1 wsl wsl 512 Jun  9 18:42 ..
drwx------ 1 wsl wsl 512 May 10 09:51 extensions
-rw-r--r-- 1 wsl wsl 411 Jun 13 18:43 timewarrior.cfg

Details

Timewarrior version

$ timew --version
1.5.0

OS Info

Ubuntu 22.04 on wsl

$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 22.04.2 LTS
Release:        22.04
Codename:       jammy

Python version

$ python --version
Python 3.11.2
lauft commented 1 year ago

I assume this is due to the usage of AtomicFile which we use to prevent corruption of our database. Here we write to a temporary file first and, on success, swap this with the original one. It seems as if here the link is not followed, so it gets replaced and not the target.

@sruffell can you confirm this?

sruffell commented 1 year ago

I assume this is due to the usage of AtomicFile which we use to prevent corruption of our database. Here we write to a temporary file first and, on success, swap this with the original one. It seems as if here the link is not followed, so it gets replaced and not the target.

@sruffell can you confirm this?

Yes, this is the case here. The AtomicFile implementation is not link aware. I can take a look to see if link-awareness can be easily added.

sruffell commented 1 year ago

For symlinks that have valid targets, this should be fixed in #547.