Re: [gpsd-dev] gpsd_config.h in release tarballs can break rebuild

[Fred Wright]

On Sun, 15 Jan 2017, Hal Murray wrote:
> address@hidden said:
> > 2283:    distfiles += generated_sources
> > Anyone know why it's doing this?
>
> I don't know anything about this example, but there is a class of files that
> get built from source but don't change with options or environment.  If the
> tools to do that are not common, it might make sense to include those files
> in the tarball so the end user doesn't have to install those tools.

Indeed, for files in that class, it's *acceptable* to include them in the
tarball, though whether it's necessary or desirable is another matter.
That depends mainly on what tools are needed to recreate them.

On Mon, 16 Jan 2017, Gregory wrote:
> Let's look at each other item in generated_sources:
>
>     generated_sources = ['packet_names.h', 'timebase.h', 'gpsd.h', 
> "ais_json.i",
>                          'gps_maskdump.c', 'revision.h', 'gpsd.php',
>                          'gpsd_config.h']
>
>
> - packet_names.h: created from packet_states.h using sed
> - timebase.h: uses AlwaysBuild

Yes.  This one is potentially special since it's derived from leap-second
information which may or may not be downloadable at the time of the build.
But that's already handled by including leapseconds.cache in the tarball.
Personally, I think it would have been cleaner just to keep a copy of
tai-utc.dat in the repo/tarball, but that's another story.

> - gpsd.h: created from gpsd.h-head, gpsd_config.h, and gpsd.h-tail
> - ais_json.i: created using `python jsongen.py`
> - gps_maskdump.c: created using `python maskgen.py`
> - revision.h: for releases, use `git describe --tags`

Actually, no.  For releases, it's derived purely from the version number
in SConstruct.  It's *non*-releases (where the version number contains
"dev") that expand the version via "git describe", or fall back to using
a date/time when run outside a git repo.

There is some logic that seems to be worrying about possible
inconsistencies between the included revision.h and the version number
used for the tarball name and/or the tag, but it's not clear to me why
that would even be relevant if revision.h weren't included in the tarball
in the first place.

> - gpsd.php: created uses substituter (defined in SConstruct)
>
> Of these, revision.h seems to be the only one that makes sense in the
> release tarball, it's safe to assume that the tools required to make all
> of the other files are available.

Based on the above, I don't see why *anything* in generated_sources should
be in the tarball, though it might be safer to leave revision.h for now,
pending further analysis.

There is something a bit fishy here, however.  It's not clear to me why it
was ever necessary to include gpsd_config.h in generated_sources just for
cleaning purposes.  While it is true that SCons's autogenerated cleaning
list can be fragile with respect to files whose creation can depend on
options, I don't see why that ever would have been a problem for the
unconditionally generated gpsd_config.h.  And indeed, if I check out the
code immediately before Sanjeev's fix, then build and clean, I don't see a
leftover gpsd_config.h (though I do see a leftover libQgpsmm.dylib).
Meanwhile, generated_sources is no longer added to the explicit clean list
in recent versions, anyway (I think as part of some cleanup of the clean
stuff I did a while back).

Then again, reloading sources without cleaning is probably asking for
trouble, anyway, so we're really talking about competing corner cases in
terms of actual behavior. :-)

That being said, I'm inclined to accept the proposed change (and perhaps
revisit the revision.h question later), unless someone sees a reason not
to.  It's still used to exclude them from the "true source" list for
timestamp purposes, as well as to use them in "deheader".  The manpages
can stay, due to the tool dependencies.

However, I'd have to sort out a couple of issues here that prevent testing
the prerelease steps.

Fred Wright