[SPARK-7945][CORE]Do trim to values in properties file

[WangTaoTheTonic]

https://issues.apache.org/jira/browse/SPARK-7945

Now applications submited by org.apache.spark.launcher.Main read properties file without doing trim to values in it.
If user left a space after a value(say spark.driver.extraClassPath) then it probably affect global functions(like some jar could not be included in the classpath), so we should do it like Utils.getPropertiesFromFile.

[srowen]

Yeah, the way Properties parses is a little weird, in that whitespace doesn't matter around keys and values except at the end of the value. On the one hand I'm reluctant to make Spark's use of Properties nonstandard, but I also can't think of a case where trailing whitespace is valuable to preserve.

This only affects one usage of Properties though which seems inconsistent, so I wonder if this is just causing more confusion. It's also not directly addressing the stated problem, which is theoretical, that this is causing JARs to not load.

[WangTaoTheTonic]

What more confusion would this cause? And I cann't think of another solution to address this perfectly so did same as Utils.getPropertiesFromFile.

BTW the problem this causes really hurts us as whitespace behind some value is really hard to be detected.

[srowen]

You expect Properties syntax but get something slightly different? A pretty theoretical problem, but that's what I meant.

I suppose we can't use Utils.getPropertiesFromFile because is in the launcher, but are these the only places Properties are parsed this way? then at least this change would make Spark consistent.

I suppose if Utils is already doing this then the cat's out of the bag, so no point in restricting this change to the identified problem I guess.

[SparkQA]

Test build #33732 has finished for PR 6496 at commit 2c053a1.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[WangTaoTheTonic]

If you look at this change #3916, then will find actually we did use Utils.getPropertiesFromFile before.

I think it is a miss by carelessness. @vanzin Could you help check this either?

[vanzin]

Yeah, it's a theoretical problem, but to be consistent with the rest of the code, I guess it doesn't hurt.

[vanzin]

This is way more complicated than it needs to be. Just do:

for (Map.Entry<Object, Object> e : props.entrySet()) {
e.setValue(e.getValue().toString().trim());
}

[WangTaoTheTonic]

I'm at home now and commit this without compiling locally, hope it all be good.

[SparkQA]

Test build #33742 has finished for PR 6496 at commit bb41b4b.

• This patch fails MiMa tests.
• This patch merges cleanly.
• This patch adds no public classes.

[WangTaoTheTonic]

Jenkins, retest this please.

[SparkQA]

Test build #33745 has finished for PR 6496 at commit bb41b4b.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[WangTaoTheTonic]

I've done a test with current code, it worked out fine.

[srowen]

LGTM