On Wed, Jan 07, 2004 at 03:38:50AM -0800, Greg Stein wrote:
> I'm only -0 on the concept of a date parser rewrite. If we were at another
> time in our "release curve", then it would be no big deal. However, that
I believe the exact opposite; it was never given priority because there
were better things to do, but now that we are where we are in the "release
curve" (or, more optimistically: the "adoption curve"), it becomes very
important that we fix certain things that need fixing before the toothpaste
is out of the tube.
> isn't the case here, so this kind of rewrite introduces a measure of risk
> into the release. Today, getdate.y has not changed since we imported it
> into the repository back in August, 2001; no idea about its history within
> the old CVS repos. During that whole time, I don't recall any "big"
> problems that people have actually experienced. It seems the largest issue
> is missing the ISO formats, rather than a defect, per se. Against that
> stability, we're talking about introducing new, untried code with some
> number of potential issues.
As Erik Huelsmann has pointed out, it's possible that it hasn't changed
because no one has dared change it. If the comments at the top are to
believed, it hasn't changed in 13 years, potentially because people were
too afraid. Personally, I dont believe that it was because people didn't
want to; I know that I had so much trouble with cvs's -D option that
I nearly gave up on it completely, rather than trying to sort out which
date formats actually did what I expected.
> The current patch still seems to have a few open issues on the recognized
> formats (e.g. cvs/rcs formats, or use of a slash). At this point in the
> release, I'd hope we would have a more solid closure/consensus on the
> formats before application.
One nice thing about this matcher is that it makes it easy for us to support
new formats. We can have a quick vote on the formats that should go in,
and use and document those.
> My second issue is that the patch is incomplete. I recognize that is
> simply because it is meant as a "talking reference", but that is too loose
> for application to the 1.0 branch. The Windows build still makes reference
> to getdate.*, and there aren't any regression tests around this
> functionality to give some confidence that we aren't introducing bugs.
C. Scott Ananian's patches came with a set of tests. I can integrate the
ones that are applicable.
>
> The final point kind of balances itself out: this code doesn't get used or
> exercised that much, so there isn't a lot of need to "fix" it. But by the
> same token, that also means that introducing problems here will have a
> very low impact.
>
> I worry that even if each of these things were addressed, then at that
> point, we'll be even further down the road where the risk/benefit tradeoff
> is even more weighed against changes.
Right, except as I noted before, I believe the opposite. :-)
>
> I believe the proper resolution is to recognize that the date formats are
> just another "contract" (to borrow ghudson's terms w.r.t APIs). SVN will
> make certain guarantees about specific forms of input. Pass something else
> at your own risk. If you depend upon behavior outside of the contract,
> then too bad if we need to alter code. We do this in many areas of the
> code. Where we find implicit contracts, then we try to document those to
> make them explicit. I'd like to posit that we're in the exact same
> situation here. Our cmdline client has got a lot of implicit behaviors
> that we try to document, and I believe that the acceptable formats are
> simply one of those situations.
>
This particular contract would be difficult to honor. It is not a matter
of "foo must not be NULL" or even "you must have called apr_initialize()
before you call apr_pool_create()"; it has to do with a difficult to
express limitation on the form of a text string, which is likely to have
been provided by a user (not generated algorithmically). Because the
data is likely to be user-provided, consumers of our APIs have three
choices:
1.) Pass the limitations of our contract on to their users. Or worse,
forget to do so (or even worse, forget to do "obvious" things like
add a mutex to protect both svn_opt_parse_revision and
svn_opt_args_to_target_array).
2.) Enforce the contract on their users (or worse, misenforce it). This
will require that they implement a validator of similar algorithmic
complexity to the one proposed.
3.) Eschew svn_opt_parse_revision and svn_opt_args_to_target_array in
favor of hand-rolled implementations under their control. This
could lead to several mutually incompatible svn revision date-format
domains, each with its own qualities (or lack thereof).
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jan 7 16:42:46 2004