Patch for issue 730 (my way)

Discuss issues related to PS3 Media Server development (only for programmers)

Patch for issue 730 (my way)

Postby Thibaut » Thu Jul 28, 2011 7:06 pm

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.
Thibaut
 
Posts: 6
Joined: Sat Jun 18, 2011 2:42 pm

Re: Patch for issue 730 (my way)

Postby chocolateboy » Thu Jul 28, 2011 10:09 pm

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?
chocolateboy
Project Member
 
Posts: 2579
Joined: Wed Sep 16, 2009 10:05 am

Re: Patch for issue 730 (my way)

Postby Thibaut » Fri Jul 29, 2011 12:54 am

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...
Thibaut
 
Posts: 6
Joined: Sat Jun 18, 2011 2:42 pm


Return to Developers

Who is online

Users browsing this forum: No registered users and 5 guests