search

sot / astromon

Astrometric accuracy (celestial location) monitor
0 stars 0 forks source link

Hardcode CIAO 4.5 #2

Closed jeanconn closed 10 years ago

jeanconn commented 10 years ago

CIAO 4.6 has a CFITSIO conflict with SKA. This change hardcodes CIAO 4.5 as a temporary fix.

jeanconn commented 10 years ago

This is just an idea for a temporary solution for astromon.

taldcroft commented 10 years ago

Based on discussion on ascds_help ("Bus error in dmmerge") the fix should be:

ciao = Ska.Shell.getenv('. /soft/ciao/bin/ciao.sh')
ciao['CIAO_LD_LIBRARY_PATH'] = '{root}/lib:{root}/ots/lib'.format(root=ciao['ASCDS_INSTALL'])
jeanconn commented 10 years ago

To implement that, we could use the original App::Env and set CIAO_LD_LIBRARY_PATH explicitly. I'll test the CIAO_LD_LIBRARY_PATH in this perl code.

taldcroft commented 10 years ago

Oops, didn't bother to remember that astromon is perl. I don't see why you need to revert to the original App::Env. Just set the env. var in the script any time before calling a CIAO tool and it should just work. Right?

jeanconn commented 10 years ago

I wasn't thinking of it as a "reverting" as this PR hasn't gone anywhere yet, and I thought maybe the $ENV{CIAO_LD_LIBRARY_PATH} change would be a simpler change. Either Shell::GetEnv or App::Env should be fine, it is just a question of which you prefer for maintenance.

On Tue, Dec 17, 2013 at 2:11 PM, Tom Aldcroft notifications@github.comwrote:

Oops, didn't bother to remember that astromon is perl. I don't see why you need to revert to the original App::Env. Just set the env. var in the script any time before calling a CIAO tool and it should just work. Right?

— Reply to this email directly or view it on GitHubhttps://github.com/sot/astromon/pull/2#issuecomment-30780587 .

taldcroft commented 10 years ago

OK, I'm going to slow down and take a sip of coffee. That's better. :-) So the only required change to the original code should be adding $ENV{CIAO_LD_LIBRARY_PATH} = ... in each of the two scripts. I think we are both saying this.

jeanconn commented 10 years ago

Yes :-)

More backstory you can skip:

When I was trying to explicitly call the ciao-4.5 stuff I needed to make the change from App::Env -> Shell::GetEnv because, as far as I could tell, App::Env doesn't have a convenient interface to just change the ciao source script (we'd have to update App::Env::CIAO to include an explicit 4.5). But if there's a simple environment variable fix, I don't need to change how CIAO is started in the two scripts that use it.

jeanconn commented 10 years ago

Superceded by #3