Issue 74 in casacore: casacore directory missing from include path

[casa...@googlecode.com]

Status: New
Owner: ----
Labels: Type-Defect Priority-Medium Milestone-Release1.4

New issue 74 by gijsmole...@gmail.com: casacore directory missing from
include path
https://code.google.com/p/casacore/issues/detail?id=74


currently when you want to compile python-casacore you need to manually add
the casacore header include path to the compile flags, for example
-I/usr/include/casacore. This is because the by python-cascore included
header files in casacore don't include the casacore directory.

For example casacore/python/Converters/PycBasicData.h:34

contains this include statement:

#include <python/Converters/PycArray.h>

This works for casacore, since you manually add the casacore folder to the
search list. I makes compilation of python-casacore if it can resolve the
casacore headers by looking in the default include folders.

--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[casa...@googlecode.com]

Comment #1 on issue 74 by gijsmole...@gmail.com: casacore directory missing

from include path
https://code.google.com/p/casacore/issues/detail?id=74

related python-casacore issue:

https://github.com/ska-sa/python-casacore/issues/2

[casa...@googlecode.com]

Comment #2 on issue 74 by gervandi...@gmail.com: casacore directory missing

from include path
https://code.google.com/p/casacore/issues/detail?id=74

Gijs,
The same will be true for the include of e.g. tables/Tables/TableProxy.h.
You had to supply the casacore root (or include and libdir) when building
pyrap. The build script added rootdir/include/casacore to the include path.
Don't you want to have something like CASACORE_ROOT_DIR? I certainly want
to have it to be able to build python_casacore against another casacore
than the one in /usr.
Ger

[casa...@googlecode.com]

Comment #3 on issue 74 by gijsmole...@gmail.com: casacore directory missing

from include path
https://code.google.com/p/casacore/issues/detail?id=74

Hi Ger,

AFAIK it is common practice to supply the parent directory of the header
files folder. Actually, the header files of your software don't even need
to be in a sub folder, they can be just plain header files which you
install in for example /usr/local.

Be default the compiler will look in /usr/include and /usr/local/include
for header files. You can supply the compiler with additional include paths
with the -I flag. This would mean that if the casacore libraries are
installed in /usr or /usr/local and the include files woud use (for
example):

#include /casacore/tables/Tables/TableProxy.h

You don't need to set any additional -I flags. It is still possible to
specify a non-standard location for your library headers using the -I flag,
but in the default case it would just work.

Take a look at boost, they do the same actually.

[casa...@googlecode.com]

Comment #4 on issue 74 by gijsmole...@gmail.com: casacore directory missing

from include path
https://code.google.com/p/casacore/issues/detail?id=74

Hi Ger,

AFAIK it is common practice to supply the parent directory of the header
files folder. Actually, the header files of your software don't even need
to be in a sub folder, they can be just plain header files which you
install in for example /usr/local.

Be default the compiler will look in /usr/include and /usr/local/include
for header files. You can supply the compiler with additional include paths
with the -I flag. This would mean that if the casacore libraries are
installed in /usr or /usr/local and the include files woud use (for
example):

#include "casacore/tables/Tables/TableProxy.h"

[casa...@googlecode.com]

Comment #5 on issue 74 by Malte.Marquarding: casacore directory missing

from include path
https://code.google.com/p/casacore/issues/detail?id=74

This would be a massive change to change all includes in the whole casacore
code base and every project depending on it.
boost is not the same as they already have it in all their includes.
I would say this is too big a change.

[casa...@googlecode.com]

Comment #6 on issue 74 by gervandi...@gmail.com: casacore directory missing

from include path
https://code.google.com/p/casacore/issues/detail?id=74

Although I agree it is a bit neater not having to add casacore/ to the
include path, it's also my feeling the change is too much.

[casa...@googlecode.com]

Comment #7 on issue 74 by gijsmole...@gmail.com: casacore directory missing

from include path
https://code.google.com/p/casacore/issues/detail?id=74

Hi Malte and ger,

Thanks for the answer, but I don't agree :)

Having to specify the include path this way is non-standard and breaks
various auto build tools. If we don't change this, we can't autcompile
python-casacore using pip for example.

I think modifying the include paths is not that much of work, if you want
I can do it for you. It doesn't require that much change, only
appending "casacore" in front of all the casacore specific includes. From
there I think everything should work, maybe some cmake files require a
small change.

About depending projects, people can still include casacore the old way, by
setting the include path to <prefix>/include/casacore. In the new situation
if casacore is installed with a debian package (in /usr) a end-user doesn't
need to adjust the compiler include path. The same if it is installed with
a manual build without specifying a prefix (so in /usr/local).

So effectively the only thing that would change for a end user is that
_only_ if casacore is not installed in /usr of /usr/local the user needs to
add/change the compiler include path (<casacore_prefix>/include), if they
not already did that.

I think if this is clearly stated in the changelog and/or release notes
people can deal with that. This would be for the 2.0 release right? This is
a major release, although I agree that it is very important casacore stays
backwards compatible, this change would only imply a minimal change to the
include path in some cases. As long it is clearly documented improving
compliance to the standard way of doing things should be a priority also
imho.

[casa...@googlecode.com]

Comment #8 on issue 74 by gervandi...@gmail.com: casacore directory missing

from include path
https://code.google.com/p/casacore/issues/detail?id=74

Hi Gijs,

The casacore changes are indeed not that much. A Perl command can do the
job. It also requires a minor change in the top level CMakeLists.txt to
change the include path.
However, it also requires that code is always checked out in a casacore
directory, which is currently not the case. I always do it that way and I
guess other people as well, so it is not a real problem.

Packages using casacore need to have the old and new way in their include
path, because an include of, say, casa/Array/Array.h will include
casacore/casa/Array/IPosition.h. Also this does not sound as a big problem
to me.

So I agree the changes are smaller than I originally thought. Maybe Malte
sees bigger obstacles.
I'll try to make such a change in one part of the casacore files and see if
problems arise.

Cheers,
Ger

[casa...@googlecode.com]

Comment #9 on issue 74 by Malte.Marquarding: casacore directory missing

from include path
https://code.google.com/p/casacore/issues/detail?id=74

Ger,

these were my concerns too, but after thinking about it backwards
compatibility is trivial. We will have to discuss this with NRAO however
them moving to pure third party dependency on casacore should work in
favour of this.

[casa...@googlecode.com]

Comment #10 on issue 74 by gervandi...@gmail.com: casacore directory

missing from include path
https://code.google.com/p/casacore/issues/detail?id=74

perl -p -i -e s'%(#\s*include\s+<\s*)(casa|tables|scimath|measures|lattices|
images|ms|msfits|meas|derivedmscal|coordinates|components|
fits)/%\1casacore/\2/%' `find . -name "*.h"`

will change all header files and the same can be done for *cc.

Shall we create a branch for this change, because it will break builds?

Ger

[casa...@googlecode.com]

Comment #11 on issue 74 by gervandi...@gmail.com: casacore directory

missing from include path
https://code.google.com/p/casacore/issues/detail?id=74

perl command is not entirely correct. The \s+ should be \s* because it
appears no whitespace is required between include and <
Changed include_directories line in CMakeLists.txt (appended /.. to
CMAKE_SOURCE_DIR) and now everything is happily building.

I was wondering if the include guards should also be changed by adding
CASACORE_, but I don't think that is necessary.

[casa...@googlecode.com]

Updates:
Status: Started
Owner: gervandi...@gmail.com

Comment #12 on issue 74 by gervandi...@gmail.com: casacore directory

missing from include path
https://code.google.com/p/casacore/issues/detail?id=74

- Made little script to prepend include path with casacore/
- Changed -I in CMakeLists.txt
- Changed LOFAR's CMakeLists.txt to use old and new in -I; works well