On 21 July 2017 at 12:32, Mark Taylor <[log in to unmask]> wrote:
> On Fri, 21 Jul 2017, David Berry wrote:
>
>> Ah yes - thanks for keeping your eye on this Mark. When I looked over
>> this change I missed the point that jniast carries its own (rather
>> old) AST source files round with it in src/ast, and does not use an
>> external AST library. So as you say there is no need to make any
>> changes to jniast.
>
> Not quite - the src/ast/*.c files are never compiled, they are
> only used for their comments (to auto-generate javadocs).
> If you execute the build-native ant target it does use an
> external AST library (which must match the API it's expecting),
> but that's not a normal part of the build process, it's only
> done when generating the src/lib/*/jniast_libs.jar files,
> which are checked in to the repository.

Ah - I was fooled by the fact that the jniast sharable library defines
all the AST internal symbols itself rather than using libast.so -
shows my poor knowledge of ldd etc.
So it could be a little tricky to do a build-native as we'd need to
dig out and build a suitable old version of AST first.

>> Having said that, if the src/ast files *had* been updated to the new
>> versions, Graham's changes to (e.g.)  src/jni/SlaMap.c do not change
>> the API or functionality of the Java_uk_ac_starlink_ast_SlaMap_add
>> function so I'm not sure I understand why it would break anything.
>
> Oops - you are quite right.  I misread the changes, as you say
> they are just to the implementation and not to the API of
> Java_uk_ac_starlink_ast_SlaMap_add etc.  So much of the detail
> of what I wrote was wrong, sorry.  But, the point remains that
> this commit can't stand on its own (it would need rebuilding of
> all the system-specific shared libraries), and as you also say
> it's not necessary in any case.
>
>> However, as you say, it's probably better to back out of these
>> changes. Especially given that we've not incorporated any other AST
>> changes into jniast for a long time. At some point it would be nice to
>> bring jniast up to date wrt AST, but there seems to be little demand
>> for it (as far as I can tell), so motivation is low.
>
> ... and required effort probably quite high.

True.

David