resultspace: adding explicit exec paths

[jbohren]

No description provided.

[wjwwood]

You need to make sure *_EXECUTABLE is not None (not found), otherwise you'll get an unintended failure mode.

Also, I think you should either use env | sort or just cmake -E environment. Using cmake -E environment implies you want to stay portable, but combining it with sort sort of ruins that. So whether you choose to make it portable or dependent on Unix, either way mixing cmake -E environment with sort doesn't really make sense. Any reason you decided to call out to get the env rather than use os.environ?

[jbohren]

You need to make sure *_EXECUTABLE is not None (not found), otherwise you'll get an unintended failure mode.

Sure.

Also, I think you should either use env | sort or just cmake -E environment. Using cmake -E environment implies you want to stay portable, but combining it with sort sort of ruins that. So whether you choose to make it portable or dependent on Unix, either way mixing cmake -E environment with sort doesn't really make sense.

I don't mind moving sort into the python code, but I think this fix should go in regardless. Then we can open an enhancement ticket.

Any reason you decided to call out to get the env rather than use os.environ?

Because it's getting the environment from the subshell after sourcing the setup file. The only other option is to write your own script that needs to get called.

[wjwwood]

I don't mind moving sort into the python code, but I think this fix should go in regardless. Then we can open an enhancement ticket.

Sounds good to me.

Because it's getting the environment from the subshell after sourcing the setup file. The only other option is to write your own script that needs to get called.

Makes sense, I didn't notice how it was getting called before.

[wjwwood]

Just to be clear, I'm waiting on the check for None on the *_EXECUTABLE variables before merging this, but the other enhancement of not using sort can wait till later.

[jbohren]

Yep.

On Thu, Mar 26, 2015, 19:44 William Woodall [email protected]
wrote:

Just to be clear, I'm waiting on the check for None on the *_EXECUTABLE
variables before merging this, but the other enhancement of not using sort
can wait till later.

Reply to this email directly or view it on GitHub
#174 (comment).

[jbohren]

@wjwwood Should be good for review.

[wjwwood]

lgtm