Patch for issue 730 (my way)

[Thibaut]

Hi
Here are some patches for the issue 730, this is how I think it is better to do, but I'm not God almighty...

Code: Select all

--- PMS.sh   (revision 771)
+++ PMS.sh   (working copy)
@@ -20,17 +20,17 @@
 fi
 
 # Setup PMS_HOME
-if [ "x$PMS_HOME" = "x" ]; then
+if [ -z "$PMS_HOME" ]; then
    PMS_HOME=$DIRNAME
 fi
 
 export PMS_HOME
 # XXX: always cd to the working dir: https://code.google.com/p/ps3mediaserver/issues/detail?id=730
-cd $PMS_HOME
+#cd $PMS_HOME
 
 # Setup the JVM
-if [ "x$JAVA" = "x" ]; then
-   if [ "x$JAVA_HOME" != "x" ]; then
+if [ -z "$JAVA" ]; then
+   if [ -n "$JAVA_HOME" ]; then
       JAVA="$JAVA_HOME/bin/java"
    else
       JAVA="java"
@@ -40,7 +40,7 @@
 # Setup the classpath
 # since we always cd to the working dir, these a) can be unqualified and b) *must*
 # be unqualified: https://code.google.com/p/ps3mediaserver/issues/detail?id=1122
-PMS_JARS="update.jar:pms.jar"
+PMS_JARS="$PMS_HOME/update.jar:$PMS_HOME/pms.jar"
 
 # For Cygwin, switch paths to Windows format before running java
 if $cygwin; then


Code: Select all

--- net/pms/configuration/RendererConfiguration.java   (revision 771)
+++ net/pms/configuration/RendererConfiguration.java   (working copy)
@@ -45,7 +45,7 @@
       } catch (ConfigurationException e) {
       }
 
-      File renderersDir = new File("renderers");
+      File renderersDir = new File(System.getenv("PMS_HOME") + System.getProperty("file.separator") + "renderers");
 
       if (renderersDir.exists() && renderersDir.isDirectory()) {
          logger.info("Loading renderer configurations from " + renderersDir.getAbsolutePath());


I don't know if it works under cygwin.

[chocolateboy]

Patches/fixes are always welcome. But...

Why move a working fix out of the flexible, generic, easily-patched/tweaked/modified shell script to the compiled Java code?

Also, it's never clear what "sh" will be. On Ubuntu, it's /bin/dash. Does it support -z? Do all possible "sh" shells support it? You haven't tested it on Cygwin. Have you also not tested it on Solaris? The BSDs?

Code: Select all

"x$PMS_HOME" = "x"

- AFAIK, this is the standard/portable way that works on the largest number of shells/platforms. Are you saying there's a better way that is more portable?

And what's the rationale for changing/breaking the PMS working directory? What does it gain?

What specific problem does this patch fix that hasn't already been fixed?

[Thibaut]

For me the script PMS.sh is just to adjust some variable like the quantity of memory you are asking for java, but accessing to some configuration file, it's the program itself that needs to know where to look. It's only my point of view.

No I did not test on all OS possible because I don't have them. In the original script -n was already used so I did not thought that -z would not be supported.

I don't see where I change/break the working directory.

If I posted these "patches" it's because I liked what has been done and I wanted to help a bit but if you think that what I have done is not worth to add, then don't, I won't kill myself...