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

Fail on broken symlink to avoid potential data loss #359

Open
owenh000 opened this issue Sep 16, 2021 · 3 comments
Open

Fail on broken symlink to avoid potential data loss #359

owenh000 opened this issue Sep 16, 2021 · 3 comments

Comments

@owenh000
Copy link

When the user attempts to run an add-on action that does not exist, todo.txt-cli prints a usage message and fails. However, todo.txt-cli supports add-ons that replace built-in actions. In this case, it is important to fail if there is something wrong with the add-on. Otherwise the user thinks they are still running the add-on while in reality they are only running the built-in action.

Here is an example scenario. This is based on real cases; see the again add-on README and the dorecur add-on README (of which I am the author). Other add-ons may be affected also.

  1. The user installs an add-on for handling recurring tasks. The add-on hooks into (overrides) the built-in do action, calling command do to access the built-in action as required. This is necessary to prevent the user from accidentally running do and discarding a recurring task.
  2. To facilitate updates, and because of limitations in how add-ons can be installed (see Allow for addons to be in subdirectories #214, for example), the add-on is installed as a local Git repository clone with a symlink in ~/.todo.actions.d/.
  3. At some point in the future, this Git repository is moved or the file is renamed, breaking the symlink.
  4. Instead of failing with an error, todo.txt-cli ignores the now-broken symlink, running the built-in action instead. Now recurring actions are being discarded (not replaced) without the user being aware of it.

Do you want to request a feature or report a bug?

That's open to interpretation, but I'll call this a bug.

What is the current behavior?

todo.txt-cli silently ignores an add-on that is a broken symbolic link.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem.

$ cd ~/.todo.actions.d
$ ln` -s ~/repos/todo.txt-cli-dorecur/dorecur.py do
$ todo add "Pay rent t:2021-09-28 due:2021-10-02 rec:+1m
TODO: 1 added.
$ todo do 1
1 (A) Pay rent t:2021-10-28 due:2021-11-02 rec:+1m
TODO: 1 added.
## Time passes and the user moves their Git repositories
$ mv ~/repos ~/git-repos
$ todo do 1
## todo.txt-cli silently fails to run the add-on and the recurring task is not replaced.

What is the expected behavior?

If:

  • the user attempts to run a built-in action,
  • there is a matching add-on overriding it, and
  • the overriding add-on file is a broken symbolic link,

then todo.txt-cli should fail with an error.

It seems reasonable to also apply this to other cases where an add-on is apparently installed but cannot be used, for example when an add-on file or directory lacks whatever read/execute permissions may be required.

Which versions todo.sh are you using?

$ todo-txt -V
TODO.TXT Command Line Interface v2.11.0

Which Operating System are you using?

Debian GNU/Linux Bullseye

Which version of bash are you using?

$ bash --version
GNU bash, version 5.1.4(1)-release (x86_64-pc-linux-gnu)
inkarkat added a commit to inkarkat/todo.txt-cli that referenced this issue Sep 16, 2021
Instead of potentially falling back to the built-in action that a custom action was intended to override, but (e.g. due to file system reorganizations) now results in a broken link. The extension functionality that is then skipped may result in undesired results, but this may not be immedately obvious to the user (if the extension is not particularly verbose), so some data corruption could occur if this remains undetected.
To avoid duplicating (or somehow extracting) all the built-in actions, simply detect _any_ broken symlink; i.e. offer a superset of the required functionality. So this would also complain about a broken symlink to a non-executable custom (auxiliary) file (rarely used) if that is mistakenly passed as a custom action (unlikely).

Fixes todotxt#359
@inkarkat
Copy link
Member

That's indeed a potential problem, though it would only affect power users with customizations that extend the built-in commands. I'm using such symlinks myself, and I also have done such reorganizations that required the update of them. If we don't limit this check to built-in commands, a small additional logic can detect and deal with it, as in my PR.

@owenh000
Copy link
Author

@inkarkat, thanks!

If I understand correctly, applying the check to all commands simply results in a more explicit error (versus a general failure with usage output) in the case of a non-built-in custom add-on. Thus the way you did it seems optimal to me, in addition to reducing required logic.

Also I tested your changes (e1c1c32) and it looks good to me!

@inkarkat
Copy link
Member

@owenh000 thanks for testing my changes!

Yes, you're right, in case of a custom add-on that's not overriding a built-in command, it now also complains about the broken symlink instead of assuming such an add-on command does not exist, and just printing the generic usage help.

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

No branches or pull requests

2 participants