bindiff icon indicating copy to clipboard operation
bindiff copied to clipboard

Use XDG basedir spec on Linux

Open mochaaP opened this issue 6 months ago • 4 comments

https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html

Why: we already use the equivalents on other platforms (Application Support, AppData, etc.) on other platforms. This behavior could be aligned on Linux. seealso: https://xdgbasedirectoryspecification.com/

Code below is attached to explain the new logic and I will make a PR once we finalize the behavior details. See the comments for further discussion.

Details

diff --git a/java/zylib/src/main/java/com/google/security/zynamics/zylib/system/SystemHelpers.java b/java/zylib/src/main/java/com/google/security/zynamics/zylib/system/SystemHelpers.java
index aa8754b9..eddf021c 100644
--- a/java/zylib/src/main/java/com/google/security/zynamics/zylib/system/SystemHelpers.java
+++ b/java/zylib/src/main/java/com/google/security/zynamics/zylib/system/SystemHelpers.java
@@ -38,7 +38,7 @@ public final class SystemHelpers {
       // Win32 function.
       result = System.getenv("ProgramData");
     } else if (isRunningLinux()) {
-      result = "/etc/opt";
+      result = "/etc/xdg";
     } else if (isRunningMacOSX()) {
       result = "/Library/Application Support";
     } else {
@@ -76,9 +76,15 @@ public final class SystemHelpers {
       // function.
       result = System.getenv("APPDATA");
     } else {
-      result = System.getProperty("user.home");
-      if (isRunningMacOSX()) {
-        result += "/Library/Application Support";
+      result = System.getenv("XDG_DATA_HOME");
+      if (result == null || !result.startsWith("/")) {
+        result = System.getProperty("user.home");
+        if (isRunningLinux()) {
+            result += "/.local/share";
+        }
+        if (isRunningMacOSX()) { // Since there isn't a standard way to get this path on macOS (at least I'm not aware of), should we also put this codepath in here?
+          result += "/Library/Application Support";
+        }
       }
     }
     return FileUtils.ensureTrailingSlash(result);
@@ -93,7 +99,7 @@ public final class SystemHelpers {
    */
   public static String getApplicationDataDirectory(final String product) {
     return getApplicationDataDirectory()
-        + (isRunningLinux() ? "." + product.toLowerCase() : product)
+        + (isRunningLinux() ? product.toLowerCase() : product)
         + File.separator;
   }
 
diff --git a/util/process.cc b/util/process.cc
index 6fdeb42..bc46b04 100644
--- a/util/process.cc
+++ b/util/process.cc
@@ -241,15 +241,25 @@ absl::StatusOr<std::string> GetOrCreateAppDataDirectory(
     absl::string_view product_name) {
   std::string path;
 #ifndef _WIN32
-  const char* home_dir = getenv("HOME");
-  if (!home_dir) {
-    return absl::NotFoundError("Home directory not set");
+  std::string data_home;
+  const char* data_home_env = getenv("XDG_DATA_HOME");
+  if (data_home_env && data_home_env[0] == '/') {
+    data_home = data_home_env;
+  } else {
+    const char* home_dir = getenv("HOME");
+    if (!home_dir) {
+      return absl::NotFoundError("Home directory not set");
+    }
+#ifdef __APPLE__
+    data_home = JoinPath(home_dir, "Library", "Application Support");
+#else
+    data_home = JoinPath(home_dir, ".local", "share");
+#endif  // __APPLE__
   }
 #ifdef __APPLE__
-  path = JoinPath(home_dir, "Library", "Application Support", product_name);
+  path = JoinPath(data_home, product_name);
 #else
-  path = JoinPath(home_dir,
-                  absl::StrCat(".", absl::AsciiStrToLower(product_name)));
+  path = JoinPath(data_home, absl::AsciiStrToLower(product_name));
 #endif  // __APPLE__
 #else
   char buffer[MAX_PATH] = {0};
@@ -270,7 +280,7 @@ absl::StatusOr<std::string> GetCommonAppDataDirectory(
 #ifdef __APPLE__
   path = JoinPath("/Library", "Application Support", product_name);
 #else
-  path = JoinPath("/etc/opt/", absl::AsciiStrToLower(product_name));
+  path = JoinPath("/etc", "xdg", absl::AsciiStrToLower(product_name));
 #endif  // __APPLE__
 #else
   char buffer[MAX_PATH] = {0};

mochaaP avatar Jun 17 '25 17:06 mochaaP

I have actually been thinking about XDG myself. However, Binary Ninja, Ghidra and IDA Pro all use plain ~/.<product_name> as their config paths. So for tool consistency I have not been pursuing this.
But I'm not opposed to this change, it shouldn't matter much in the end where this lives.

As for the code, so XDG_DATA_HOME would override the standard paths on macOS? Why is that? Is it to support a desktop environment runinng on macOS under an X server? It's otherwise unlikely to ever be set.

    data_home = JoinPath(home_dir, ".local", "share");

Should this be .config/<product_name>?

cblichmann avatar Jun 17 '25 18:06 cblichmann

Is it to support a desktop environment runinng on macOS under an X server? It's otherwise unlikely to ever be set.

This variable is usually unset on all platforms, it only exists when one wants to override the default value, and fallback to the platform default. I agree that we can ignore this variable on macOS. (does anyone on macOS actually want to override this path for whatever reason?)

Should this be .config/<product_name>?

Unless we also write logs to .local/state/bindiff on Linux, ~/Library/Logs/bindiff on macOS?

Binary Ninja, Ghidra and IDA Pro all use plain ~/.<product_name> as their config paths.

I remembered Ghidra supports XDG basedir since 11.1 release (Dec 2023).

BN uses AppData on Windows, and it has this https://github.com/Vector35/binaryninja-api/issues/654 but seems no one picked it up ever since. I will try mentioning this on their public Slack group.

IDA also uses AppData on Windows. I actually have a patch for IDAPython to work nicely with custom $IDAUSR env var, but since all my past SDK bug reports and fixes are not responded on their support portal, I doubt they will actually care about this.

mochaaP avatar Jun 17 '25 20:06 mochaaP

also: would it be possible to extract all the classes referencing yFiles, make it a standalone Gradle project, and move the proguard step in java/ui/build.gradle to there? I currently comment out all yFiles references to build modified classes and mix it with the prebuilt bindiff.jar on classpath, it's quite janky to work with 🤒

since the obfuscated jar file should be a "redistributable" according to https://www.yworks.com/products/yfiles-for-java-2.x/sla#RightsAndLimitations, it should be fine to publish this so people without a yFiles license can work on the Java code as long as they don't touch the parts involving yFiles?

mochaaP avatar Jun 17 '25 23:06 mochaaP

would it be possible to extract all the classes referencing yFiles, make it a standalone Gradle project, and move the proguard step in java/ui/build.gradle to there?

Let's open a separate issue for this. BinNavi had a yfileswrap, so this is certainly possible/doable/legal. I personally don't have time to work on this, though.

cblichmann avatar Jun 18 '25 07:06 cblichmann