Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#729 closed enhancement (fixed)

on OSX ~/.i2p -> ~/Library/Application Support/i2p

Reported by: zab Owned by: meeh
Priority: minor Milestone: 0.9.7
Component: package/other Version: 0.9.2
Keywords: OSX integration Cc: meeh@…, zab@…
Parent Tickets:

Description

The Mac Way(TM) and should be very easy to do from java

This is not just a cosmetic change as far as the environment is concerned; it affects many system and third-party tools that expect preferences to be stored in that location

Subtickets

Change History (23)

comment:1 Changed 7 years ago by zzz

Should be straightforward to add a case for Mac in WorkingDir?.java.

Questions:

  • whether to have a different location if a service
  • net.i2p.router.plist.template refers to .i2p

The bigger effort is testing and documentation. Make sure it finds an existing install in ~/.i2p. Find and update all applicable docs on website. Testing Testing Testing.

Can this be implemented without doing #730, or is it unacceptable to store snark downloads in preferences?

comment:2 Changed 7 years ago by meeh

It should be ~/Library/Application? Support/i2p. Preferences path does not exists. ~/Library/Preferences? is only used for single plist config files, so that's not a option.

Now the service uses the same directory as if you start it in a regular way. It is possible to have the service config in /Library/Application? Support/i2p, but that would just confuse a user more. Since the service runs as the user executing the service install script, I see no harm having the config inside the user home dir.

The latest versions of OSX (including 10.7) hide the Library folder from the user in Finder (default file manager), so the config and i2psnark download will still be hidden for the user.

The way I see it, both .i2p and ~/Library/Application? Support/i2p is a config/preferences path. So the "harm" is already done regarding the downloads. It should be moved to a separate directory, but that's a case for all OS.

I got a few questions;

  • On a upgrade, what in i2p should move/migrate old .i2p to the new path? (a method? a script?)
  • If service is installed, the plist file needs a update on config path and a reload. Should i2p do this automatic, or should the user reinstall the service with a new plist template? or something else.
  • Should it make a symlink back to .i2p, to ensure homebrew scipts/plugins don't break? (backward compatible)

comment:3 Changed 7 years ago by zzz

On a upgrade, what in i2p should move/migrate old .i2p to the new path? (a method? a script?)

Noooo. Don't move an existing .i2p anywhere. The new place would be only for new installs.

comment:4 Changed 7 years ago by meeh

Ok, I've made a solution and it worked as expected when tested. The router check if a .i2p folder exists in home, if it does it will use that. If not, ~/Library/Application? Support/i2p will be used if Application Support folder is found. (unsure if really old mac version have that folder) if not found, it will use .i2p in home folder as a fallback.

The wrapper/service install made it a bit more tricky, but it works. I noticed a code for installing osx service inside i2prouter wrapper script, but that will not work because of the missing params in launchctl. Also it reuqires the user to edit i2prouter manually setting I2P_CONFIG_DIR and RUN_AS_USER, and that you run it with sudo. Removing .command files will remove usability, also having a not working function in i2prouter wrapper isn't good, it need a cleanup.

A solution might be fixing the i2prouter function, making the .command files check the wrapper for RUN_AS_USER and I2P_CONFIG_DIR, if RUN_AS_USER isn't set, then it set those variables matching current user, and then call the function, installing the service.

Any suggestions, ideas, comments?

Would like someone to look at the code before I commit.

--- installer/resources/i2prouter	8b10158ef18444d8ad65089ae80f930fc6b5deec
+++ installer/resources/i2prouter	aaa7ff6e98c7029125e50e5d74b6b1296b8f8b83
@@ -29,7 +29,15 @@ I2P="%INSTALL_PATH"
 # should have been replaced by the izpack installer.
 # If you did not run the installer, replace them with the appropriate paths.
 I2P="%INSTALL_PATH"
-I2P_CONFIG_DIR="%USER_HOME/.i2p"
+if [ "`uname -s`" == "Darwin" ]; then
+	if [ -d "%USER_HOME/Library/Application\ Support" ]; then
+		I2P_CONFIG_DIR="%USER_HOME/Library/Application\ Support/i2p"
+	else
+		I2P_CONFIG_DIR="%USER_HOME/.i2p"
+	fi
+else
+    I2P_CONFIG_DIR="%USER_HOME/.i2p"
+fi
 I2PTEMP="%SYSTEM_java_io_tmpdir"
 # PORTABLE installation:
 # Use the following instead.
============================================================
--- router/java/src/net/i2p/router/startup/WorkingDir.java	5a96330de3806af8d24ac9cbc646529deedc7e94
+++ router/java/src/net/i2p/router/startup/WorkingDir.java	e6fec9cfe9daeef51a73d89b4fc5c0610afbcf87
@@ -49,6 +49,7 @@ public class WorkingDir {
     private final static String PROP_BASE_DIR = "i2p.dir.base";
     private final static String PROP_WORKING_DIR = "i2p.dir.config";
     private final static String WORKING_DIR_DEFAULT_WINDOWS = "I2P";
+    private final static String WORKING_DIR_DEFAULT_MAC = "i2p";
     private final static String WORKING_DIR_DEFAULT = ".i2p";
     private final static String WORKING_DIR_DEFAULT_DAEMON = "i2p-config";
     /** we do a couple of things differently if this is the username */
@@ -81,6 +82,14 @@ public class WorkingDir {
                 if (appdata != null)
                     home = appdata;
                 dirf = new SecureDirectory(home, WORKING_DIR_DEFAULT_WINDOWS);
+            } else if (SystemVersion.isMac()) {
+                String appdata = "/Library/Application Support/";
+                if (new File(home+appdata).exists()&&false==(new File(home+"/"+WORKING_DIR_DEFAULT).exists())) {
+                    home = home+appdata;
+                    dirf = new SecureDirectory(home, WORKING_DIR_DEFAULT_MAC);
+                } else {
+                    dirf = new SecureDirectory(home, WORKING_DIR_DEFAULT);
+                }
             } else {
                 if (DAEMON_USER.equals(System.getProperty("user.name")))
                     dirf = new SecureDirectory(home, WORKING_DIR_DEFAULT_DAEMON);
============================================================
--- installer/resources/net.i2p.router.plist.template	f414066db4ef16a8fe1ae84d10a02625f4298b31
+++ installer/resources/net.i2p.router.plist.template	9757ad83e9157569474fad912627ca7ff525ee98
@@ -7,9 +7,9 @@
     <key>OnDemand</key>
     <false/>
     <key>StandardOutPath</key>
-    <string>~/.i2p/wrapper.log</string>
+    <string>~/Library/Application Support/i2p/wrapper.log</string>
     <key>StandardErrorPath</key>
-    <string>~/.i2p/wrapper.log</string>
+    <string>~/Library/Application Support/i2p/wrapper.log</string>
     <key>ProgramArguments</key>
     <array>
         <string>COMMAND</string>

comment:5 Changed 7 years ago by zzz

Logic-wise, the WorkingDir? changes look right. Except for the && false thing, not sure what you're trying to do there. You want something like (pseudocode): if (~/Library/Application? Support exists && ! ~/.i2p exists)

Style-wise, when making a new File or SecureDirectory?, the 2-arg constructor new File(parent, child) is preferable to new File(parent + '/' + child). Do this twice - once to make dir=(home,appdata), check that for exists(), then use that to make (dir, i2p)

In i2prouter, you don't need a \ inside a quoted string.

Let's hold off on this until after 0.9.3 is out, don't want to throw this in at the last minute.

comment:6 Changed 7 years ago by zzz

  • Milestone set to 0.9.4
  • Owner set to meeh
  • Status changed from new to assigned

comment:7 Changed 7 years ago by zab

  • Cc zab@… added

Just catching up with this.. trac didn't email me any of these comments even though I opened the ticket !?!?

In general you want to use File.separator instead of hardcoding "/" or "\". Otherwise patch looks fine.

comment:8 Changed 7 years ago by meeh

Summary: It checks if ~/Library/Application? Support/ exists (I don't know if Application Support exists in OSX 10.4>), and if ~/.i2p exist. If ~/.i2p is found, it uses it since it's a upgrade. On new installs if the Application Support folder don't exists, it has a fallback to ~/.i2p, if Application Support exists, it uses it..

In my patch I think maybe it's best to change WORKING_DIR_DEFAULT_MAC to the whole Application Support path, and use it instead of the "home = home+appdata;" logic. home should be home if SecureDirectory? will use the home variable in the future.. Seems more logic to do it this way.

comment:9 Changed 6 years ago by meeh

Ok, I've done a extended test on this. I installed it on a clean OSX where i2p have never been installed before. Build 0.9.3-14.

Reseed worked. i2psnark worked, I downloaded a torrent without problems. eepsite works also (jetty). Should make something in the default help page about it's a new path in OSX, and not .i2p witch is in the help page. (eepsite)

However, my first test was with runplain.sh. When I tried with i2prouter wrapper I got a error I need to solve:

2012/12/13 19:34:35 | Launching a JVM...
2012/12/13 19:34:36 | JVM exited while loading the application.
2012/12/13 19:34:36 | WrapperManager: Initializing...
2012/12/13 19:34:36 | Exception in thread "main" java.lang.UnsatisfiedLinkError: org.tanukisoftware.wrapper.WrapperManager.nativeInit(Z)V
2012/12/13 19:34:36 | 	at org.tanukisoftware.wrapper.WrapperManager.nativeInit(Native Method)
2012/12/13 19:34:36 | 	at org.tanukisoftware.wrapper.WrapperManager.initializeNativeLibrary(WrapperManager.java:1448)
2012/12/13 19:34:36 | 	at org.tanukisoftware.wrapper.WrapperManager.privilegedClassInit(WrapperManager.java:811)
2012/12/13 19:34:36 | 	at org.tanukisoftware.wrapper.WrapperManager.access$000(WrapperManager.java:125)
2012/12/13 19:34:36 | 	at org.tanukisoftware.wrapper.WrapperManager$1.run(WrapperManager.java:466)
2012/12/13 19:34:36 | 	at java.security.AccessController.doPrivileged(Native Method)
2012/12/13 19:34:36 | 	at org.tanukisoftware.wrapper.WrapperManager.<clinit>(WrapperManager.java:463)
2012/12/13 19:34:36 | 	at java.lang.Class.forName0(Native Method)
2012/12/13 19:34:36 | 	at java.lang.Class.forName(Class.java:169)
2012/12/13 19:34:36 | 	at org.tanukisoftware.wrapper.WrapperSimpleApp.class$(WrapperSimpleApp.java:145)
2012/12/13 19:34:36 | 	at org.tanukisoftware.wrapper.WrapperSimpleApp.<init>(WrapperSimpleApp.java:145)
2012/12/13 19:34:36 | 	at org.tanukisoftware.wrapper.WrapperSimpleApp.main(WrapperSimpleApp.java:569)
2012/12/13 19:34:36 | Exception in thread "Wrapper-Shutdown-Hook" java.lang.NoClassDefFoundError: Could not initialize class org.tanukisoftware.wrapper.WrapperManager
2012/12/13 19:34:36 | 	at org.tanukisoftware.wrapper.WrapperManager$2.run(WrapperManager.java:669)

This resulting in the service install not working too ofc.

I used a updated patch:

--- installer/resources/i2prouter	259940f54704bcc710d593a24fd2124f1e0bbdb2
+++ installer/resources/i2prouter	30494f073010200b1129ce3b3f797af6d7edb95d
@@ -29,7 +29,15 @@ I2P="%INSTALL_PATH"
 # should have been replaced by the izpack installer.
 # If you did not run the installer, replace them with the appropriate paths.
 I2P="%INSTALL_PATH"
-I2P_CONFIG_DIR="%USER_HOME/.i2p"
+if [ "`uname -s`" == "Darwin" ]; then
+   if [ -d "%USER_HOME/Library/Application Support" ]; then
+       I2P_CONFIG_DIR="%USER_HOME/Library/Application Support/i2p"
+   else
+       I2P_CONFIG_DIR="%USER_HOME/.i2p"
+   fi
+else
+    I2P_CONFIG_DIR="%USER_HOME/.i2p"
+fi
 I2PTEMP="%SYSTEM_java_io_tmpdir"
 # PORTABLE installation:
 # Use the following instead.
============================================================
--- router/java/src/net/i2p/router/startup/WorkingDir.java	5a96330de3806af8d24ac9cbc646529deedc7e94
+++ router/java/src/net/i2p/router/startup/WorkingDir.java	99a109a4f318805353d5c8660f25044a90224639
@@ -49,6 +49,7 @@ public class WorkingDir {
     private final static String PROP_BASE_DIR = "i2p.dir.base";
     private final static String PROP_WORKING_DIR = "i2p.dir.config";
     private final static String WORKING_DIR_DEFAULT_WINDOWS = "I2P";
+    private final static String WORKING_DIR_DEFAULT_MAC = "i2p";
     private final static String WORKING_DIR_DEFAULT = ".i2p";
     private final static String WORKING_DIR_DEFAULT_DAEMON = "i2p-config";
     /** we do a couple of things differently if this is the username */
@@ -81,6 +82,14 @@ public class WorkingDir {
                 if (appdata != null)
                     home = appdata;
                 dirf = new SecureDirectory(home, WORKING_DIR_DEFAULT_WINDOWS);
+            } else if (SystemVersion.isMac()) {
+                String appdata = "/Library/Application Support/";
+                if (new File(home,appdata).exists()&&false==(new File(home,WORKING_DIR_DEFAULT).exists())) {
+                    home = home+appdata;
+                    dirf = new SecureDirectory(home, WORKING_DIR_DEFAULT_MAC);
+                } else {
+                    dirf = new SecureDirectory(home, WORKING_DIR_DEFAULT);
+                }
             } else {
                 if (DAEMON_USER.equals(System.getProperty("user.name")))
                     dirf = new SecureDirectory(home, WORKING_DIR_DEFAULT_DAEMON);
============================================================
--- installer/resources/net.i2p.router.plist.template	f414066db4ef16a8fe1ae84d10a02625f4298b31
+++ installer/resources/net.i2p.router.plist.template	9757ad83e9157569474fad912627ca7ff525ee98
@@ -7,9 +7,9 @@
     <key>OnDemand</key>
     <false/>
     <key>StandardOutPath</key>
-    <string>~/.i2p/wrapper.log</string>
+    <string>~/Library/Application Support/i2p/wrapper.log</string>
     <key>StandardErrorPath</key>
-    <string>~/.i2p/wrapper.log</string>
+    <string>~/Library/Application Support/i2p/wrapper.log</string>
     <key>ProgramArguments</key>
     <array>
         <string>COMMAND</string>

comment:10 Changed 6 years ago by meeh

I also suggest to remove the broken OSX service installer in i2prouter with the one I added, as in i2prouter just call the .command file. The old one requires editing of i2prouter for setting RUN_AS_USER variable. Mine uses the user that loads it. Also the old version would require sudo ./i2prouter to work. And the .command extension allows you to double click on the file and it open a terminal with the script running. The .command script ask you for the password, instead of requiring sudo ./*.command witch would require a user to open a terminal before using it, and then the .command double click loses it's feature.

comment:11 Changed 6 years ago by zzz

  • Milestone changed from 0.9.4 to 0.9.5

Still kind of a mishmosh of string concatentation, multiple objects for the same thing, etc. My stab at it:

+            } else if (SystemVersion.isMac()) {
+                File appdata = new File(home, "Library/Application Support");
+                File oldDir = new SecureDirectory(home, WORKING_DIR_DEFAULT);
+                if (appdata.exists() && !oldDir.exists()) {
+                    dirf = new SecureDirectory(appdata, WORKING_DIR_DEFAULT_MAC);
+                } else {
+                    dirf = oldDir;
+                }

comment:12 Changed 6 years ago by zab

  • Summary changed from on OSX ~/.i2p -> ~/Preferences/Application Support/i2p to on OSX ~/.i2p -> ~/Library/Application Support/i2p

Hmm, I realize that I'm not sure if the "Library/Application Support" part of the path does not get translated in internationalized versions of OSX. Can someone with spare mac hardware test if that path stays the same on a non-English OSX installation?

If it does get translated, this may require some JNI

This is starting to look like a bigger task, but we either play by Apple's rules or face the consequences

comment:13 follow-up: Changed 6 years ago by meeh

The UNIX path "Library/Application? Support" have the same name. However, IIRC in older versions of Finder, it report the translated value of "library" to the user.. As in, Finder say the folder is named "Biblotek", but after a "ls -lsa" in terminal, you see it's named "Library" in reality.

From OSX 10.7 (I think?) the Library folder became hidden in Finder. Path is still valid from a shell/terminal POV.
Tested on a Norwegian model (OSX 10.8).

comment:14 follow-up: Changed 6 years ago by zab

Syndie suffers from the same issue in syndie.db.TextEngine::getRootPath().

comment:15 in reply to: ↑ 13 Changed 6 years ago by zab

Replying to meeh:

The UNIX path "Library/Application? Support" have the same name. However, IIRC in older versions of Finder, it report the translated value of "library" to the user.. As in, Finder say the folder is named "Biblotek", but after a "ls -lsa" in terminal, you see it's named "Library" in reality.

From OSX 10.7 (I think?) the Library folder became hidden in Finder. Path is still valid from a shell/terminal POV.
Tested on a Norwegian model (OSX 10.8).

Excellent. Then the logic on OSX can become:

 if ( ~/.i2p/ does not exist && 
      "~/Library/Application Support" exists )
    create an i2p folder and use it
 else
    use ~/.i2p/ 

comment:16 in reply to: ↑ 14 Changed 6 years ago by zab

Replying to zab:

Syndie suffers from the same issue in syndie.db.TextEngine::getRootPath().

To kill 2 birds with 1 changeset, and avoid code duplication, we could move the logic from WorkingDir to somewhere in the i2p.jar and make WorkingDir delegate.

comment:17 Changed 6 years ago by zzz

  • Milestone changed from 0.9.5 to 0.9.6

iirc Meeh had a patch ready to go, but for a couple release cycles in a row, he asked to check it in just days before the release. Of course we asked him to wait until just after the release so there was plenty of time for testing... but nothing yet.

Meeh, if you're ready, please push it in the next week... or else wait, again, until after the 0.9.6 release. Or push to your own branch.

re: i2p.jar, not sure it's general-purpose enough for that. Not overly concerned about making things easy for Syndie.

comment:18 Changed 6 years ago by zab

I've committed the patch to branch "i2p.i2p.729" and will continue working on that branch

comment:19 Changed 6 years ago by zab

Did some limited testing on my mac, it works fine. If there is no response from Meeh I will merge this to trunk later this week.

comment:20 Changed 6 years ago by meeh

I can continue on that patch, but I suggest waiting until next release because I got limited time of testing because I'm in the process of moving in RL. I feel it need more testing before I'm confortable with pushing it into trunk.

comment:21 Changed 6 years ago by zab

  • Milestone changed from 0.9.6 to 0.9.7
  • Resolution set to fixed
  • Status changed from assigned to closed

Did some limited testing on my mac, it works fine. If there is no response from Meeh I will merge this to trunk later this week.

comment:22 Changed 6 years ago by meeh

I'm gonna test it too. I'll report back. Guessing I got the time for it tomorrow. Gonna test on OSX 10.7.5.

comment:23 Changed 6 years ago by meeh

Tested most of the senarios now. Except the startup script, I will test it later, if it doesn't work, I'll fix it and commit.

Note: See TracTickets for help on using tickets.