Re: [Flightgear-devel] apt.dat - add local version ?

[Florent R. <f.r...@fr...>]

Hi James,

James Turner <zak...@ma...> wrote:

> Hmm, I’m really not sure this a good direction to be proceeding in :/
>
> I would much prefer we solve this by generating new apt.dat offline and
> distributing them, rather than ending up with a large number of small apt.dat
> files, which is not what the maintainers of the main file expect. This won’t
> help with custom scenery-add ons but I think that really needs to be handled
> through a different mechanism, or just accepting that a single centralised
> scenery build works much better than small custom-scenery add-ons.

I'm not sure I follow you very well. The small apt.dat files are what
people developing/improving airport layouts naturally get from WED,
therefore they can't be avoided short of centrally merging them by some
FG infrastructure. But in such a case, the terrain would have to match,
and we have seen getting this updated takes a loooong time (not
criticizing anyone---I know it is a hard problem)...

The code I posted will allow users to easily install custom sceneries
that add or modify airport layouts. Currently, this requires modifying
apt.dat.gz by hand for every such installation, which is too difficult
for most users and a maintainance nightmare for those who know how to do
it (patch-unfriendly binary format, file in Git repo).

Or are you proposing a separate, for-development-only, scenery system
with central, frequently updated apt.dat.gz and non-seamless joints
between tiles (necessary evil for reasonably fast updates)? I fear this
would add burden to the infrastructure. Less importantly maybe, if that
central apt.dat.gz is updated too often, the "rebuiding navdata cache"
at FG startup may be annoying. But once every day or so is probably not
so much of a problem for people wanting to follow scenery development.

I also thought of a variant of my proposal: FG could automatically use
apt.dat or apt.dat.gz files found in some defined place inside each
FG_SCENERY component (files from earlier components overriding those
from later ones, as expected with FG_SCENERY). That would avoid the need
to pass --apt-dat in most cases, while still loading custom sceneries in
a correct way (as opposed to what happens when people naively install
them without updating their apt.dat.gz).

> However, given that I’m very time constrained, and hence unlikely to work on
> an alternative solution, I don’t mind this going in, so long as it’s
> sufficiently robust.

I don't intend to make things any less robust than they currently are.
See my previous apt.dat parsing changes for reference, and
<https://sourceforge.net/p/flightgear/flightgear/merge-requests/42/>
which is fixing a few small bugs. I also wanted to add precise error
reporting with the new simgear::strutils::error_string() function, but
wanted to do it for both apt.dat and fix.dat at the same time for
simplicity, which can't be done before this (#42) is merged.
Alternatively, I could file a new request using
simgear::strutils::error_string() right from the beginning, but thought
that maybe Torsten wouldn't appreciate it since he has already read this
one.

> In particular we somehow need to capture the state of /all/ the .dat files
> used by a nav-cache, so that if any change, or the set of files change, we
> force a rebuild. Otherwise we’ll be fighting really inexplicable bugs.

This is exactly what I did in
<https://sourceforge.net/u/frougon/flightgear-flightgear/ci/5b10dc0c4d0bb3adb6d404bab7c216021cb3ff4b/>.
The code is open for review...

As I explained, I introduced two fields for that in the existing
'stat_cache' table of the navdb:
  - the 'group_' field allows to identify cached files that belong
    together (apt.dat pieces, possibly fix.dat pieces, etc.);
  - the 'seq_num' field (sequence number) allows the navdb to remember a
    precise order for the various files in a given group (e.g.,
    precedence order between the various apt.dat pieces ).

Relevant excerpts brought by the above commit:

flightgear/src/Navaids/NavDataCache.hxx:
----------------------------------------
  /**
   * Groups used in the database to mark cached files that should be treated
   * together.
   *
   * In order to somehow merge several files, they should be declared in the
   * same group, with appropriate sequence numbers to define the precedence
   * order. For instance, partial apt.dat(.gz) files should all be put in the
   * CACHED_FILE_GROUP_APT_DAT group. Files that are not to be grouped in any
   * particular way should be put in the default group, CACHED_FILE_GROUP_DFLT
   * (and the sequence number doesn't matter in this case).
   *
   * If codes are changed in this enum, one should make sure that the database
   * is rebuilt before it is used again.
   */
  enum CachedFileGroup
  {
    CACHED_FILE_GROUP_DFLT = 0,
    CACHED_FILE_GROUP_APT_DAT
  }; // To be kept in sync with cachedFileGroupDesc() right below

  static const char *cachedFileGroupDesc(enum CachedFileGroup group)
  {
    static const char *desc[] = {"default", "apt.dat"};

    return desc[group];
  }

[...]

  /**
   * Tell whether a group of cached files should be considered modified.
   *
   * @param group       group identifier
   * @param refFilesIt  input iterator yielding SGPath instances
   * @param refFilesEnd iterator pointing to the past-the-end element in the
   *                    sequence
   * @return true if the group of cached files is considered modified
   *
   * See the definition of NavDataCachePrivate::isCachedGroupModified() for
   * more details.
   */
  template<class InputIter>
  bool isCachedGroupModified(CachedFileGroup group,
                             InputIter refFilesIt,
                             InputIter refFilesEnd) const;
  /**
   * Insert or replace info about a cached file in the database.
   *
   * @param path   file to insert or replace info about
   * @param group  group identifier
   * @param seqNum sequence number for the file inside the group
   *
   * XXX Maybe the method should be renamed, because it does a bit more than
   *     updating the timestamp.
   */
  void stampCacheFile(const SGPath& path,
                      CachedFileGroup group = CACHED_FILE_GROUP_DFLT,
                      unsigned int seqNum = 0);

(I have indeed renamed this one to updateCachedFileMetadata() in my
working copy)


flightgear/src/Navaids/NavDataCache.cxx:
----------------------------------------

// Tell whether a group of cached files should be considered modified.
//
// To be considered unmodified, the two lists of files must have the same
// number of elements and, when comparing each SGPath object 'p' yielded by
// the 'refFilesIt' iterator to the corresponding element from the navdata
// cache (according to its 'seq_num' field), p.realpath() and p.modTime() have
// to exactly match the 'path' and 'stamp' fields present in the database.
//
// Return true if the group of cached files is considered modified.
//
// In short, this means that what matters is: the number and the order of the
// files in 'group', their SGPath::realpath() as well as their
// SGPath::modTime(). The elements yielded by the 'refFilesIt' iterator are
// typically SGPath instances corresponding to on-disk files such as apt.dat.
// The notion of group here allows to merge several apt.dat files (possibly
// gzip-compressed and with arbitrary paths) in a deterministic way thanks to
// the sequence number recorded for each file. Other groups could be used to
// merge multiple fix.dat files together, etc.
template<class InputIter>
bool NavDataCache::NavDataCachePrivate::isCachedGroupModified(
  CachedFileGroup group, InputIter refFilesIt, InputIter refFilesEnd,
  bool verbose)

[...]

bool NavDataCache::isRebuildRequired()
{
  const PathList& apt_dat_paths = globals->get_apt_dat_paths();

    if (d->readOnly) {
        return false;
    }

    if (flightgear::Options::sharedInstance()->isOptionSet("restore-defaults")) {
        SG_LOG(SG_NAVCACHE, SG_INFO, "NavCache: restore-defaults requested, will rebuild cache");
        return true;
    }

  if (d->isCachedGroupModified(CACHED_FILE_GROUP_APT_DAT,
                               apt_dat_paths.begin(), apt_dat_paths.end(),
                               true) ||
      d->isCachedFileModified(d->metarDatPath, true) ||
      d->isCachedFileModified(d->navDatPath, true) ||
      d->isCachedFileModified(d->fixDatPath, true) ||
      d->isCachedFileModified(d->carrierDatPath, true) ||

[...]

> Right now it’s easy because we know there is exactly one of each type
> of file (apt, fix, etc), but I’m not clear from the description if
> this change is storing the list of file paths to compare against when
> determining cache validity.

I hope it is clear now... I even posted a sample sqlite3 session
exploring my ~/.fgfs/navdata_2016_3.cache in the message you are
replying to, that shows the paths of the two apt.dat(.gz) files I used
in the example I gave, along with their group, sequence number and
timestamp.

Regards

-- 
Florent