On 24 July 2017 at 12:32, Peter W. Draper <[log in to unmask]> wrote:
> On Mon, 24 Jul 2017, Mark Taylor wrote:
>
>> On Mon, 24 Jul 2017, David Berry wrote:
>>
>>> On 22 July 2017 at 05:21, Graham Bell <[log in to unmask]> wrote:
>>>>>
>>>>> [...] the src/lib/*/jniast_libs.jar native files need to be recompiled
>>>>> and
>>>>> committed for all supported architectures.
>>>>
>>>>
>>>>
>>>> Hello,
>>>>
>>>> I've done that for Linux and hopefully fixed the errors which were
>>>> preventing us compiling them on Mac OS X.
>>>>
>>>> The Starlink release build process (as on the wiki page
>>>> "StarlinkRelease")
>>>> needs the update for astSlaAdd, astSpecAdd and astTimeAdd as otherwise
>>>> an
>>>> error occurs at the build-native step due to the new nargs parameter not
>>>> being supplied. If you want to be able to run "ant ... test" with either
>>>> version of AST, we could just take away the two new
>>>> "assertTrue(raised);"
>>>> lines from the test -- they check something that wasn't being tested
>>>> before.
>>>>
>>>> Best regards,
>>>> Graham
>>>
>>>
>>> I'm not sure I understand. What is it that requires the new nargs
>>> parameters, and why are they required? What is the problem with just
>>> leaving jniast as it is? Just picking up one change from the current
>>> version of AST and ignoring all the countless other changes that have
>>> happened since the last rebuild of the native jniast libraries seems a
>>> bit odd.
>
>
> The main reason to rebuild these libraries for official EAO releases is to
> pick up internal changes like leap seconds, important bug fixes and any
> changes to the AST encodings (otherwise SPLAT cannot read newer NDFs). The
> JNI libraries are essentially static (so that the Java release is somewhat
> independent of the classic one) so these changes can only be picked up by a
> rebuild.
>
> Most other SPLAT users do not care about any of this, so having slightly
> stale builds on master isn't a big issue
>
>> I agree with David, this doesn't look right to me, though I don't
>> know what the actual changes in AST requiring this are.
>
>
> David has changed the public API for several functions. So here we are.
The three functions that have changed are very unlikely to be used
anywhere except inside AST.
>> In any case, it's not enough to just rebuild and commit
>> jniast/src/lib/amd64/jniast_libs.jar, all the other architectures
>> (i386, ppc, sparc, x86, x86_64) have to be rebuilt as well
>> otherwise we have an inconsistent repository.
>
>
> To be fair it has been inconsistent for quite a while, the only libraries
> being updated are the x86_64 and OS X ones these days. I have occasionally
> caught the Windows one up, but only when necessary. The rest are pretty much
> dead.
>
>> It seems to me that the right thing to do here is just to remove
>> the build-native step from the starlink build process, and back
>> out of all these JNIAST changes. The build-native step is not
>> intended to be done during a normal build from source, which is
>> why the native libraries are checked in to the repository.
>> The alternative is to make a proper update at least of all the
>> built native libraries and commit them back to the repository,
>> preferably alongside a full review and update of what's changed
>> since the last build-native was done, at least to check that
>> recent AST updates won't have broken anything. On the other
>> hand after so long in java-land I'm pretty ignorant about the
>> requirements for platform-specific code, so it's possible that
>> I'm missing some essential reason why build-native has to get
>> done in a starlink release
>>
>> Since SPLAT is the thing most affected here Peter might have
>> relevant opinions.
>
>
> As far as I can tell these updated functions are not used, so making these
> changes is no big deal. The nasty part is that this internal change is not
> seen in the Java API, that requires an update to the internal AST capture so
> the Java API can be regenerated.
>
> So the question is, have we new leap seconds or any encoding changes? If not
> we don't need to update.
The main change from EAO's point of view is that the number of leap
seconds at the epoch associated with a given Frame can now be set
externally using a new Frame attribute called DTAI, instead of looking
it up in an internal hard-wired table. This is to solve the problem of
people trying to process new data with old AST libraries that do not
include recent leap-seconds within the hard-wired table.
NDFs holding JCMT data now include a header specifying the number of
leap seconds that should be used when processing the data (don't ask
what happens if an observation spans a leap second). The FitsChan
class reads this header and stores it as the DTAI value for the WCS
Frame, which then gets used in preference to the internal hard-wired
table of leap seconds.
This will not provide any immediate benefit, but may do after future
leap seconds have occurred. That is, people will no longer need to
update their AST library every time a leap second occurs.
So I suppose it probably is worth re-building the native jniast
libraries - or as many of them as are still used - so that splat picks
up this facility. In which case Graham is probably right to make the
changes in 345e9d63.
David
|