ralphbean / taskw

python taskwarrior api
http://threebean.org
GNU General Public License v3.0
175 stars 47 forks source link

RcFile: _read: try taskrc directory when trying to load includes #151

Open smemsh opened 3 years ago

smemsh commented 3 years ago

Taskwarrior can have 'include' directives in the rcfile which use pathnames relative to the rcfile location. Currently in taskw library, these will fail because the file is opened without attempting to search for it or qualify the path beyond expanding tildes and normalizing the path. It only can work if given as absolute path or if the file is in cwd of process executing the library code.

With this PR, if we try to load the file and it doesn't exist, we will try it again as a path in the rcfile directory. This will work in most cases of a relative path and follows the same algorithm as TaskWarrior itself (partial implementation: TaskWarrior also tries other places, see commit message for details).

Fixes #150

smemsh commented 2 years ago

@ralphbean could we merge this trivial patch, so relative includes work in taskrc?

smemsh commented 2 years ago

rebased for 1.3.1, looks like @coddingtonbear is the one to merge these?

coddingtonbear commented 2 years ago

Can, theoretically, but it's going to take some time before I can reasonably have a look at this.

ryaminal commented 1 year ago

Hi all. Wondering if this fix is still up to date or if there is another solution? seeing this problem emerge from bugwarrior.

RaitoBezarius commented 11 months ago

Hi there, @coddingtonbear is there something we can do to help you review this change? It is a very sharp edge currently with taskw :/.

RaitoBezarius commented 11 months ago

Here's an alternative patch making use of os.path and fixing the case where TaskRc.rcdir is None.

diff --git c/taskw/taskrc.py w/taskw/taskrc.py
index 1b6f8e5..b72dee6 100644
--- c/taskw/taskrc.py
+++ w/taskw/taskrc.py
@@ -1,5 +1,6 @@
 import logging
 import os

 from taskw.fields import (
     ChoiceField,
@@ -39,6 +40,7 @@ class TaskRc(dict):

     """

+    rcdir = None
     UDA_TYPE_MAP = {
         'date': DateField,
         'duration': DurationField,
@@ -54,6 +56,8 @@ class TaskRc(dict):
                     path
                 )
             )
+            if not self.rcdir:
+                TaskRc.rcdir = os.path.dirname(os.path.realpath(self.path))
             config = self._read(self.path)
         else:
             self.path = None
@@ -92,6 +96,17 @@ class TaskRc(dict):

     def _read(self, path):
         config = {}
+        if not os.path.exists(path) and TaskRc.rcdir is not None:
+            # include path may be given relative to dir of rcfile
+            oldpath = path
+            path = os.path.join(TaskRc.rcdir, oldpath)
+            if not os.path.exists(path):
+                logger.error(
+                    "rcfile does not exist, tried %s and %s",
+                    oldpath, path
+                )
+                raise FileNotFoundError
+
         with open(path, 'r') as config_file:
             for raw_line in config_file.readlines():
                 line = sanitize(raw_line)
RaitoBezarius commented 11 months ago

I also sent a PR downstream in nixpkgs to apply the patch for the time being: https://github.com/NixOS/nixpkgs/pull/265899.

RaitoBezarius commented 11 months ago

Actually, you need at least:

diff --git c/taskw/taskrc.py w/taskw/taskrc.py
index 1b6f8e5..9a6610e 100644
--- c/taskw/taskrc.py
+++ w/taskw/taskrc.py
@@ -1,5 +1,6 @@
 import logging
 import os
+import shutil

 from taskw.fields import (
     ChoiceField,
@@ -39,6 +40,7 @@ class TaskRc(dict):

     """

+    rcdir = None
     UDA_TYPE_MAP = {
         'date': DateField,
         'duration': DurationField,
@@ -48,16 +50,31 @@ class TaskRc(dict):

     def __init__(self, path=None, overrides=None):
         self.overrides = overrides if overrides else {}
+        binary_path = shutil.which("task")
+        share_rc_path = None
+        if binary_path is not None:
+            binary_path = os.path.realpath(binary_path) # Normalizes any potential symlinks.
+            prefix = os.path.dirname(os.path.dirname(binary_path)) # $prefix/bin/task → $prefix
+            share_rc_path = os.path.join(prefix, "share", "doc", "task", "rc") # $prefix/share/doc/task/rc contains additional files.
+
+        self.fallback_prefixes = list(filter(None, [
+            share_rc_path
+        ]))
+
         if path:
             self.path = os.path.normpath(
                 os.path.expanduser(
                     path
                 )
             )
+            if not self.rcdir:
+                TaskRc.rcdir = os.path.dirname(os.path.realpath(self.path))
+                self.fallback_prefixes.append(TaskRc.rcdir)
             config = self._read(self.path)
         else:
             self.path = None
             config = {}
+
         super(TaskRc, self).__init__(config)

     def _add_to_tree(self, config, key, value):
@@ -92,6 +109,28 @@ class TaskRc(dict):

     def _read(self, path):
         config = {}
+
+        if not os.path.exists(path):
+            # include path may be given relative to dir of rcfile
+            oldpath = path
+            tries = []
+            for fallback_prefix in self.fallback_prefixes:
+                path = os.path.join(fallback_prefix, oldpath)
+                if not os.path.exists(path):
+                    tries.append(path)
+                    logger.warn(
+                        "tried %s and %s with no success",
+                        oldpath, path
+                    )
+                else:
+                    break
+            if len(tries) == self.fallback_prefixes:
+                logger.error(
+                    "failed to find a fallback prefix for %s",
+                    oldpath
+                )
+                raise FileNotFoundError
+
         with open(path, 'r') as config_file:
             for raw_line in config_file.readlines():
                 line = sanitize(raw_line)

to make this work for cases where the relative path is related to the default share data installed as part of TaskWarrior.

bergercookie commented 8 months ago

Hi all, I'm attempting to fix this issue in my taskw-ng fork (since AFAICT development has stalled unfortunately in this repo)

Anyone knows what' the use of the .rcdir variable mentioned in this PR ?

            if not self.rcdir:
                TaskRc.rcdir = os.path.dirname(os.path.realpath(self.path))

https://github.com/ralphbean/taskw/pull/151/files#diff-eb2e93c64ee7759abb84b6b1a876071bc5c87c146daf0c99d5dce59bd63168a9R42-R59

smemsh commented 3 months ago

@bergercookie not sure what you mean by "what's the use," rcdir is a class attribute which is populated with the self.path dir in TaskRc instance constructor. The code tries both as an absolute, and relative path later in _read() method.