Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Meake measures data search path smaller #277

Merged
merged 3 commits into from
Dec 16, 2015
Merged

Meake measures data search path smaller #277

merged 3 commits into from
Dec 16, 2015

Conversation

tammojan
Copy link
Contributor

Philosophy: only two paths are searched.

  1. A user path, set in .casarc
  2. A system path, set at compile time (actually cmake time)

This change removes hardcoded paths like /share/casacore/data. For compatibility, I kept the special paths casahome() and casaroot(), which can be specified at compile time with %CASAHOME% and %CASAROOT% and will be expanded at run time.

@tammojan
Copy link
Contributor Author

This would fix #273. To repeat: the issue is that if you would have more than one instance of measures data installed, it would be unclear to users which data instance would be picked. Making the search path much smaller would make it clearer.

@tammojan
Copy link
Contributor Author

This effectively reverts PR #271 by @schiebel . His use case would be covered by specifying -DDATA_DIR=%CASAROOT%/share/casacore/data/ at compile time.

@schiebel, could you comment on this PR?

@dpetry I think you should review this.

@dpetry
Copy link
Contributor

dpetry commented Dec 15, 2015

Looking at it now. But we need to wait for Darrell's opinion.

@schiebel
Copy link
Contributor

CASA distributions can be relocated at will by users. For this reason, we require dynamic paths which could change from one startup to the next.

@tammojan
Copy link
Contributor Author

CASA distributions can be relocated at will by users. For this reason, we require dynamic paths which could change from one startup to the next.

Agreed. So that is why I put the option %CASAROOT% in. That would solve this issue, wouldn't it?

@schiebel
Copy link
Contributor

CASA is distributed as a tar file containing a binary distribution. The users download the tar file and unpacks it in an arbitrary directory (without compiling anything). At runtime, we discover the directory, set CASAPATH, and then exec the binary which actually runs CASA. So any solution must permit finding the data et al. directories based upon an environment variable.

@tammojan
Copy link
Contributor Author

So any solution must permit finding the data et al. directories based upon an environment variable.

I claim that this solution does permit that. The packager (you) needs to set -DDATA_DIR=%CASAROOT%/share/casacore/data at compile time. The %CASAROOT% is replaced by the result of casaRoot() at run time, which should find the current path from where casa is run (I didn't change this code).

@schiebel
Copy link
Contributor

OK, got it, and I'm satisfied. I took the comment that this is %CASAROOT% is filled from .casarc too literally.

@schiebel
Copy link
Contributor

Hi Dirk,

I just created a pull request #278 to fix the CASA builds WRT this change,
could you review it (after tests complete), please.

thanks,
Darrell

On Tue, Dec 15, 2015 at 10:51 AM, Tammo Jan Dijkema <
notifications@github.com> wrote:

So any solution must permit finding the data et al. directories based upon
an environment variable.

I claim that this solution does permit that. The packager (you) needs to
set -DDATA_DIR=%CASAROOT%/share/casacore/data at compile time. The
%CASAROOT% is replaced by the result of casaRoot() at run time, which
should find the current path from where casa is run (I didn't change this
code).


Reply to this email directly or view it on GitHub
https://github.com/casacore/casacore/pull/277#issuecomment-164805008.[image:
Web Bug from
https://github.com/notifications/beacon/AIBOqY6zHinmjcVW2fyNvMunf4X8hGVrks5pQC5mgaJpZM4G1fbC.gif]

dpetry added a commit that referenced this pull request Dec 16, 2015
Meake measures data search path smaller
@dpetry dpetry merged commit 952adad into casacore:master Dec 16, 2015
@vsuorant
Copy link
Contributor

Hello,

This caused trouble with our Jenkins build which runs unit tests before the application is packaged. I fixed this by setting the DATA_DIR to a fixed location, but this raised a question about running these tests a part of our nightly builds (which does the packaging). If I set the DATA_DIR at the compile time, is the %CASAROOT% going to work at runtime? If not, what is the proper way of setting the DATA_DIR?

--Ville

@tammojan
Copy link
Contributor Author

@vsuarant this should be solved by #278, which should be merged. If @dpetry doesn't do that soon, I'll do it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants