Allow both '/' and '\\' as trailing separator for LoggingDirectory

[Thomas Hallgren]

When working jointly on a project from machines with different OS'es
(Windows, Mac OSx, Linux), the constraint that the LoggingDirectory
parameter must end with a native directlry separator character becomes
very cumbersome. This commit eases this task by allowing the forward
slash to be used on all platforms (java.io.File is fine with that).

------------ src/hirondelle/web4j/webmaster/LoggingConfigImpl.java
------------
diff --git a/src/hirondelle/web4j/webmaster/LoggingConfigImpl.java b/
src/hirondelle/web4j/webmaster/LoggingConfigImpl.java
index 3a41cf2..d531ef5 100644
--- a/src/hirondelle/web4j/webmaster/LoggingConfigImpl.java
+++ b/src/hirondelle/web4j/webmaster/LoggingConfigImpl.java
@@ -95,7 +95,7 @@
}

private void validateDirectorySetting() {
- if( ! fLoggingDir.getValue().endsWith(FILE_SEPARATOR) ){
+ if( ! (fLoggingDir.getValue().endsWith(FILE_SEPARATOR) ||
(FILE_SEPARATOR.equals("\\") &&
fLoggingDir.getValue().endsWith("/")))) {
String message = "*** PROBLEM *** LoggingDirectory setting in
web.xml does not end in with a directory separator : " +
Util.quote(fLoggingDir.getValue());
logStdOut(message);
throw new IllegalArgumentException(message);

[John O'Hanley]

Could you explain more about the problem? It's not clear what exactly you mean. Are you referring to diffferent dev hosts or prod hosts? How does it become a nuisance exactly?

[Thomas Hallgren]

I'm using a Linux box. My collegue is using a Windows box. We collaborate on source checked in to a SCM and use a relative path for the log. I use 'logs/'. My collegue has to use 'logs\' or web4j will complain since \ is the path separator on windows. So every time something changes in the web.xml, we run into problems with this setting.

This is dilemma is unnecessary since the java.io.File accepts both forms on windows.

- thomas

On 04/05/2012 10:51 PM, John O'Hanley wrote:

Could you explain more about the problem? It's not clear what exactly you mean. Are you referring to diffferent dev hosts or prod hosts? How does it become a nuisance exactly?

--
You received this message because you are subscribed to the Google Groups "web4j-users" group.
To view this discussion on the web visit https://groups.google.com/d/msg/web4j-users/-/pdJD8iWIrVQJ.
To post to this group, send email to web4j...@googlegroups.com.
To unsubscribe from this group, send email to web4j-users...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/web4j-users?hl=en.

[John O'Hanley]

I see. You've stretched the class there a bit, since it's documented that the file reference should be absolute. This change is distasteful, since it's hacking on (as far as I know) undocumented behavior for the File class. Hence, I'm not enthusiastic about it.

The web.xml is a config file. If you have different needs for different OS's well, that's natural enough.

[Thomas Hallgren]

Our objective is to keep the configuration equal for all developers. A relative and OS-agnostic path for the logging is
for instance, a good thing since it's easy to document and talk about. The developer doesn't need to bother with this
setting and we can document where the directory will end up in his development environment.

I consider it bad practice to enforce OS-specific differences in web.xml. Doing so means that the distribution of your
application (the war file) becomes OS-dependent. You can no longer just drop it in you web servers 'webapps' folder and
expect it to be deployed correctly. That in turn means that your download page must either have different war's for
different systems or some instructions on how to manipulate the war deployment after the fact.

My patch doesn't cause any harm and it solves all of the above.

Sidenote: The otherwise well documented sample web.xml files doesn't mention the absolute path requirement. Digging
deeper, I found it in the LoggingConfig class. I suggest you remove it.

[Thomas Hallgren]

On 04/05/2012 11:51 PM, John O'Hanley wrote:

This change is distasteful, since it's hacking on (as far as I know) undocumented behavior for the File class. Hence, I'm not enthusiastic about it.

It's not in the java.io.File Javadoc, but I wouldn't call it "undocumented":

http://books.google.se/books?id=fu5HtixRje8C&pg=PA405&lpg=PA405&#v=onepage&q&f=false
http://books.google.se/books?id=YQLj_AsVN9QC&pg=PA86&lpg=PA86&#v=onepage&q&f=false
http://www.tutorialspoint.com/java/java_files_io.htm
http://www.java-samples.com/showtutorial.php?tutorialid=382
http://www.yaldex.com/java_tutorial/0666241118.htm

Etc...

The point I'm trying to make is that this behavior is well known and often mentioned in various books and tutorials. It's not likely to change.

[John O'Hanley]

I thank you for bringing up the topic. Sorry, but I won't be adding this change to web4j.

- LoggingConfig is provided as an interface, for this very purpose. If you don't agree with its default implementation for any reason, just provide a new one.
- undocumented behavior of File class (the only doc that matters is the javadoc)
- web.xml is a config file. It's not unexpected that it may be sensitive to the OS.
- just configure the web.xml locally; when you refresh from source control, just do a merge instead of an overwrite.

- John

[Thomas Hallgren]

I agree that the patch isn't ideal. The real problem is that keeping
logging configuration in web.xml. For reasons that I've mentioned it
should really be free from concerns that deals with locations in the
file system. Such configurations should be the responsiblity of the web
server and maintained by whoever is in charge of that. I'll use the
LoggingConfiguration interface to change this.