You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by "Robin Bankhead (JIRA)" <ji...@apache.org> on 2013/10/02 18:16:41 UTC

[jira] [Comment Edited] (MAILBOX-199) Maildir support for Windows

    [ https://issues.apache.org/jira/browse/MAILBOX-199?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13784112#comment-13784112 ] 

Robin Bankhead edited comment on MAILBOX-199 at 10/2/13 4:14 PM:
-----------------------------------------------------------------

OK, well each of the blocks I've changed to try calling System.gc() does so only when the original code (various file move or delete operations) fails. For example, here's the first section of the current patch, from MaildirMailboxMapper.java, comments inserted to clarify:

{{
Index: src/main/java/org/apache/james/mailbox/maildir/mail/MaildirMailboxMapper.java
--- src/main/java/org/apache/james/mailbox/maildir/mail/MaildirMailboxMapper.java Base (BASE)
+++ src/main/java/org/apache/james/mailbox/maildir/mail/MaildirMailboxMapper.java Locally Modified (Based On LOCAL)
@@ -82,8 +82,19 @@
                 }
                 else {
                     // We simply delete all the folder for non INBOX mailboxes.
+					byte tries = 0;
+					while (true) {
+						try {
                     FileUtils.deleteDirectory(folder);
+							break;//No error, so move on
+						} catch (IOException e) {
+							if(File.pathSeparator == ":" || ++tries >= 5) {
+								throw e;//Rethrow if not on Windows or have tried 5 times already
                 }
+							System.gc();//Only ever fires on Windows
+						}
+					}
+                }
             } catch (IOException e) {
                 e.printStackTrace();
                 throw new MailboxException("Unable to delete Mailbox " + mailbox, e);
}}

The other three I've done so far all follow the same pattern; I can annotate all like the above if necessary but hopefully one was enough to make it clearer what they do.

Is it the usage of File.pathSeparator that's the issue? I agree and have stated that it isn't a very elegant approach (and may not be an accurate test for all I know), but I am not sure of the best approach, or exactly what you may want to see in order to satisfy "logic".

I could use the approach of the OsDetector.isWindows() method (in the tests dir) to define an IS_WINDOWS var as well as FLAG_PREFIX, and then use that in code so it's more obvious, like
{{
	public static final boolean IS_WINDOWS = (System.getProperty("os.name").toLowerCase().indexOf( "win" ) >= 0);
	public static final String FLAG_PREFIX = IS_WINDOWS ? ";" : ":";
}}
Then the code I added could use "if(MaildirMessageName.IS_WINDOWS) ..." which would be clearer, I guess.

Is that the sort of thing you were getting at?


was (Author: headbank):
OK, well each of the blocks I've changed to try calling System.gc() does so only when the original code (various file move or delete operations) fails. For example, here's the first section of the current patch, from MaildirMailboxMapper.java, comments inserted to clarify:

+					byte tries = 0;
+					while (true) {
+						try {
							FileUtils.deleteDirectory(folder); //If successful, nothing else happens, method proceeds
+							break;
+						} catch (IOException e) {
+							if(File.pathSeparator == ":" || ++tries >= 5) { //If we're not on Windows or it's the 5th try...
+								throw e; //Stop trying and re-throw the exception to be caught further up
							}
+							System.gc(); //This only fires if we're on Windows and haven't tried 5 times already
							//Loop now continues for next try
+						}
+					}
+                }

The other three I've done so far all follow the same pattern; I can annotate all like the above if necessary but hopefully one was enough to make it clearer what they do.

Is it the usage of File.pathSeparator that's the issue? I agree and have stated that it isn't a very elegant approach (and may not be an accurate test for all I know), but I am not sure of the best approach, or exactly what you may want to see in order to satisfy "logic".

I could use the approach of the OsDetector.isWindows() method (in the tests dir) to define an IS_WINDOWS var as well as FLAG_PREFIX, and then use that in code so it's more obvious, like

	public static final boolean IS_WINDOWS = (System.getProperty("os.name").toLowerCase().indexOf( "win" ) >= 0);
	public static final String FLAG_PREFIX = IS_WINDOWS ? ";" : ":";

Then the code I added could use "if(MaildirMessageName.IS_WINDOWS) ..." which would be clearer, I guess.

Is that the sort of thing you were getting at?

> Maildir support for Windows
> ---------------------------
>
>                 Key: MAILBOX-199
>                 URL: https://issues.apache.org/jira/browse/MAILBOX-199
>             Project: James Mailbox
>          Issue Type: New Feature
>          Components: maildir
>         Environment: Windows 7 Professional
>            Reporter: Robin Bankhead
>         Attachments: james-server-winmaildir-v5.patch
>
>
> Propose that Maildir be supported under the Windows platform. To my knowledge, the only technical bar to this is that maildir uses the colon (":") character, which is an illegal character on the NTFS (and I believe also FAT*) filesystem.
> The Maildir standard document[1] that this project refers to, specifies a colon (although quite obliquely). The "standard" clearly was never aimed at use on Windows, but its benefits as a format strike me as sufficiently great that a small deviation is justifiable in order to include it with James and thus increase platform-portability.
> The below patch alters message-naming so that, instead of a literal colon, the local system's path separator character (java.io.File.pathSeparator) is used in each case, giving a colon on *NIX and a semicolon (";") on Windows. (Critique of this particular methodology is welcome.)
> It also includes a quick fix for an issue encountered with file locking on Windows, which calls System.gc() on that platform. There may be other issues like this; I'm searching for them currently.
> [1] http://cr.yp.to/proto/maildir.html
> Discussion about this issue: http://www.mail-archive.com/server-user@james.apache.org/msg14542.html



--
This message was sent by Atlassian JIRA
(v6.1#6144)

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org