Re: [gpsd-dev] [PATCH 1/2] Moves Linux-only ppscheck program back to contrib/.

[Fred Wright]

On Mon, 8 Aug 2016, Gary E. Miller wrote:
> On Sat, 6 Aug 2016 09:47:13 -0700 (PDT)
> Fred Wright <address@hidden> wrote:
> > On Fri, 5 Aug 2016, Gary E. Miller wrote:

> > > Got a patch that selectively compiles only on Linux?  Or, better
> > > yet, a patch to make it work on more OS?
> >
> > Those things are possible, but with more work and hence more time.
>
> Yup, and I'm gonna spend the time, when I get it, if no one beats me to it.
>
> I'm sure you could make a patch in less time that your email took to write.

Make a patch?  Perhaps.  *Test* a patch across all OSes?  No way.

Those of us who used to write boot-critical code that had to run on every
single Google server (not to mention all StreetView cars) tend to pay
attention to details like testing. :-)

> > The priority should be unbreaking the build ASAP.
>
> Yup.  But not by trading one breakage for another.

In any area where people actually care about reliability, the first
response to discovering that a change broke something is *always* to roll
back the change first, and figure out how to redo it so that it doesn't
break things later.

> > Breaking all
> > non-Linux builds just so that a small subset of Linux users have a
> > slightly easier way to run a program that they can still run when
> > it's in contrib/ doesn't seem like a good tradeoff.
>
> I can;t test everthing, and I'm pretty sure I did not break all non-Linux
> builds.

It's unlikely that you didn't.  Feel free to post evidence to the
contrary.

> > Software development should obey the Hippocratic oath:  "First do no
> > harm."
>
> Which only applies after you know what may cause harm.

Or after one has thought about what *might* cause harm.

> > This issue might be partly Eric's fault, due to the following highly
> > misleading comment in ppscheck.c:
> >
> >  * This code requires only ANSI/POSIX. If it doesn't compile and run
> >  * on your Unix there is something very wrong with your Unix.
>
> Got proof that is not true?  If so, I'll take a patch to fix it.

The proof is the nonexistence of TIOCMIWAIT on most "ANSI/POSIX"
platforms.

> > In the case of clock_gettime(), I believe that
>
> Belief is not a proof.  And how about we worry about making it
> portable instead of worrying about water over the dam.

If that belief isn't true, it would make clock_gettime() even *less*
portable.

> > There's also the matter of testing.  It requires a serial adapter
> > with a full complement of modem-control signals, and driver support
> > in all supported OSes.
>
> Which is sort that point?  To test the modem control signals!

I mean testing the *code*.

> > Meanwhile, let's just fix the build, and worry about convenience of
> > building ppscheck later.
>
> I'll take patches, as long as the do not break the new functionality
> where it is currently working.

It's kind of a stretch to call something "broken" just because it has to
be built from contrib/.  The whole point of contrib/ is to have a place
for code that isn't ready for prime-time.

On Mon, 8 Aug 2016, Hal Murray wrote:
> address@hidden said:
> > I can;t test everthing, and I'm pretty sure I did not break all non-Linux
> > builds.
>
> I can confirm that you broke NetBSD and FreeBSD.

And I'd be extremely surprised if OpenBSD were different, though I haven't
checked.

It certainly broke OSX, which is where I got bitten in the first place.

> As far as I know, the ioctl it needs is only available on Linux.

That's my understanding as well.

> It might work to replace that with a short wait - as long as the rest of the
> code ignores extra wakeups.

Indeed heavy-handed polling isn't necessarily unreasonable for a special
test program, but note that if the build procedure finds both TIOCMIWAIT
and kernel PPS support absent, it turns off PPS support in gpsd itself.
PPS support in the kernel can be tested via the ppstest program that's
part of the pps-tools package, so there's not a lot of motivation to have
a kernel-independent PPS test that doesn't use TIOCMIWAIT, just so that
you could test a PPS signal that gpsd won't use.

The "short" wait would need to be no wait at all if it's trying to catch
really narrow PPS pulses without the help of hardware edge detection.

Fred Wright