Handle relative includes (#2535)

* Handle relative includes

We change to the directory of the given config file before parsing.
This allows us to handle relative includes.
TODO: Maybe improve the name of the change_dir() function.

* Fix unused result warning

* Add `relative_to` parameter to expand()

If the path is relative, we resolve it by prepending
dirname(config) to the path.

Add dirname() - Returns the parent directory of the file or an empty
string.

* Resolve relative paths

Handle paths relative to the current file being parsed.

* Remove unneeded change_dir()

* Fix expand()

`is_absolute` is calculated after we expand the path.
`relative_to` must be a directory.

Add test for expand() with relative paths

* Recalculate `is_absolute` after expanding `path`

* Add more file_util::expand tests

* Add changelog

Co-authored-by: patrick96 <p.ziegler96@gmail.com>

CHANGELOG.md

@@ -164,6 +164,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Polybar can now be run without passing the bar name as argument given that
   the configuration file only defines one bar
   ([`#2525`](https://github.com/polybar/polybar/issues/2525))
+- `include-directory` and `include-file` now support relative paths. The paths
+  are relative to the folder of the file where those directives appear.
+  ([`#2523`](https://github.com/polybar/polybar/issues/2523))
 
 ### Fixed
 - `custom/script`: Concurrency issues with fast-updating tailed scripts.

include/utils/file.hpp

@@ -88,9 +88,10 @@ namespace file_util {
   void write_contents(const string& filename, const string& contents);
   bool is_fifo(const string& filename);
   vector<string> glob(string pattern);
-  const string expand(const string& path);
+  string expand(const string& path, const string& relative_to = {});
   string get_config_path();
   vector<string> list_files(const string& dirname);
+  string dirname(const string& path);
 
   template <typename... Args>
   decltype(auto) make_file_descriptor(Args&&... args) {

src/components/config_parser.cpp

@@ -161,6 +161,8 @@ void config_parser::parse_file(const string& file, file_list path) {
     throw application_error("Failed to open config file " + file + ": " + strerror(errno));
   }
 
+  auto dirname = file_util::dirname(file);
+
   while (std::getline(in, line_str)) {
     line_no++;
     line_t line;
@@ -174,9 +176,9 @@ void config_parser::parse_file(const string& file, file_list path) {
     }
 
     if (!line.is_header && line.key == "include-file") {
-      parse_file(file_util::expand(line.value), path);
+      parse_file(file_util::expand(line.value, dirname), path);
     } else if (!line.is_header && line.key == "include-directory") {
-      const string expanded_path = file_util::expand(line.value);
+      const string expanded_path = file_util::expand(line.value, dirname);
       vector<string> file_list = file_util::list_files(expanded_path);
       sort(file_list.begin(), file_list.end());
       for (const auto& filename : file_list) {

src/utils/file.cpp

@@ -237,15 +237,16 @@ namespace file_util {
 
   /**
    * Path expansion
+   *
+   * `relative_to` must be a directory
    */
-  const string expand(const string& path) {
-    string ret;
+  string expand(const string& path, const string& relative_to) {
     /*
      * This doesn't capture all cases for absolute paths but the other cases
      * (tilde and env variable) have the initial '/' character in their
      * expansion already and will thus not require adding '/' to the beginning.
      */
-    bool is_absolute = path.size() > 0 && path.at(0) == '/';
+    bool is_absolute = !path.empty() && (path.at(0) == '/');
     vector<string> p_exploded = string_util::split(path, '/');
     for (auto& section : p_exploded) {
       switch (section[0]) {
@@ -257,11 +258,17 @@ namespace file_util {
           break;
       }
     }
-    ret = string_util::join(p_exploded, "/");
+    string ret = string_util::join(p_exploded, "/");
     // Don't add an initial slash for relative paths
     if (ret[0] != '/' && is_absolute) {
       ret.insert(0, 1, '/');
     }
+
+    is_absolute = !ret.empty() && (ret.at(0) == '/');
+
+    if (!is_absolute && !relative_to.empty()) {
+      return relative_to + "/" + ret;
+    }
     return ret;
   }
 
@@ -333,6 +340,15 @@ namespace file_util {
     }
     throw system_error("Failed to open directory stream for " + dirname);
   }
+
+  string dirname(const string& path) {
+    const auto pos = path.find_last_of('/');
+    if (pos != string::npos) {
+      return path.substr(0, pos + 1);
+    }
+
+    return string{};
+  }
 }  // namespace file_util
 
 POLYBAR_NS_END

tests/unit_tests/utils/file.cpp

@@ -1,16 +1,46 @@
+#include "utils/file.hpp"
+
 #include <iomanip>
 #include <iostream>
 
 #include "common/test.hpp"
 #include "utils/command.hpp"
-#include "utils/file.hpp"
+#include "utils/env.hpp"
 
 using namespace polybar;
 
-TEST(File, expand) {
-  auto cmd = command_util::make_command<output_policy::REDIRECTED>("echo $HOME");
-  cmd->exec();
-  cmd->tail([](string home) {
-      EXPECT_EQ(home + "/test", file_util::expand("~/test"));
-      });
+using expand_test_t = pair<string, string>;
+class ExpandTest : public testing::TestWithParam<expand_test_t> {};
+
+vector<expand_test_t> expand_absolute_test_list = {
+    {"~/foo", env_util::get("HOME") + "/foo"},
+    {"$HOME/foo", env_util::get("HOME") + "/foo"},
+    {"/scratch/polybar", "/scratch/polybar"},
+};
+
+INSTANTIATE_TEST_SUITE_P(Inst, ExpandTest, ::testing::ValuesIn(expand_absolute_test_list));
+
+TEST_P(ExpandTest, absolute) {
+  EXPECT_EQ(file_util::expand(GetParam().first), GetParam().second);
+}
+
+TEST_P(ExpandTest, relativeToAbsolute) {
+  EXPECT_EQ(file_util::expand(GetParam().first, "/scratch"), GetParam().second);
+}
+
+using expand_relative_test_t = std::tuple<string, string, string>;
+class ExpandRelativeTest : public testing::TestWithParam<expand_relative_test_t> {};
+
+vector<expand_relative_test_t> expand_relative_test_list = {
+    {"../test", "/scratch", "/scratch/../test"},
+    {"modules/battery", "/scratch/polybar", "/scratch/polybar/modules/battery"},
+    {"/tmp/foo", "/scratch", "/tmp/foo"},
+};
+
+INSTANTIATE_TEST_SUITE_P(Inst, ExpandRelativeTest, ::testing::ValuesIn(expand_relative_test_list));
+
+TEST_P(ExpandRelativeTest, correctness) {
+  string path, relative_to, expected;
+  std::tie(path, relative_to, expected) = GetParam();
+  EXPECT_EQ(file_util::expand(path, relative_to), expected);
 }