You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Scott Ananian" <ca...@lesser-magoo.lcs.mit.edu> on 2001/08/23 02:32:30 UTC

[PATCH] replacement for getdate.y

The attached patch replaces getdate.y and getdate.cw with the new
svn_date.c code.  Instructions:
 1) apply appended patch.
 2) untar attached svn.tgz from root subversion directory.
    % cd ~/src/subversion
    % tar xzvf svn.tgz
    subversion/libsvn_subr/svn_date.c
    %
 3) remove old getdate.* files
    % rm subversion/libsvn_subr/getdate.*
 4) regenerate the make files.
    % ./autogen.sh
    % `sed -ne '7,7s/^#//p'  config.status`
 5) remake!
    % make

ta-da!
 --s

p.s. you should then 'cvs remove' the getdate files, commit the changes,
and close issue #408.  Note that the appended patch already removed
the last two items from the 'bite-sized tasks' list for you.

Indonesia Delta Force TASS DC COBRA JUDY RNC Rijndael COBRA JANE IDEA 
EZLN Saddam Hussein planning Cheney Albanian assassination postcard 
              ( http://lesser-magoo.lcs.mit.edu/~cananian )

Index: autogen.sh
===================================================================
RCS file: /usr/local/tigris/data/helm/cvs/repository/subversion/autogen.sh,v
retrieving revision 1.47
diff -u -p -r1.47 autogen.sh
--- autogen.sh	2001/08/16 19:45:44	1.47
+++ autogen.sh	2001/08/23 02:17:45
@@ -102,23 +102,6 @@ cp $ltfile ac-helpers/libtool.m4
 # any old aclocal.m4 left over from prior build so it doesn't cause errors.
 rm -f aclocal.m4
 
-# Produce getdate.c from getdate.y.
-# Again, this means that "developers" who run autogen.sh need either
-# yacc or bison -- but not people who compile sourceballs, since `make
-# dist` will include getdate.c.
-echo "Creating getdate.c..."
-bison -o subversion/libsvn_subr/getdate.c subversion/libsvn_subr/getdate.y
-if [ $? -ne 0 ]; then
-    yacc -o subversion/libsvn_subr/getdate.c subversion/libsvn_subr/getdate.y
-    if [ $? -ne 0 ]; then
-        echo
-        echo "   Error:  can't find either bison or yacc."
-        echo "   One of these is needed to generate the date parser."
-        echo
-        exit 1
-    fi
-fi
-
 # Create the file detailing all of the build outputs for SVN.
 #
 # Note: this dependency on Python is fine: only SVN developers use autogen.sh
Index: subversion/clients/cmdline/main.c
===================================================================
RCS file: /usr/local/tigris/data/helm/cvs/repository/subversion/subversion/clients/cmdline/main.c,v
retrieving revision 1.15
diff -u -p -r1.15 main.c
--- subversion/clients/cmdline/main.c	2001/08/10 20:40:37	1.15
+++ subversion/clients/cmdline/main.c	2001/08/23 02:17:45
@@ -21,6 +21,7 @@
 #include <string.h>
 #include <assert.h>
 #include <locale.h>
+#include <time.h>
 
 #include <apr_strings.h>
 #include <apr_tables.h>
@@ -243,14 +244,29 @@ main (int argc, const char * const *argv
         opt_state.revision = (svn_revnum_t) atoi (opt_arg);
         break;
       case 'D':
-        /* svn_parse_date() originates in getdate.y; while I'd love to
-           change it to const char *, that turns out to be a little
-           more complex than just adding the qualifier.  So for now,
-           I'm casting to get rid of the compilation warning, and have
-           filed issue #408 so we don't forget about this.  -kff  */
-        apr_ansi_time_to_apr_time (&opt_state.date,
-                                   svn_parse_date ((char *) opt_arg, NULL));
-        break;
+	{
+	  time_t now, then;
+	  /* XXX: If we evaluate -D multiple times in the course of
+	   * an operation, we probably want to use the same 'now'
+	   * value every time, instead of always using the current time. */
+	  now = time(NULL);
+	  apr_err = svn_parse_date(opt_arg, now, &then);
+	  if (APR_STATUS_IS_SUCCESS (apr_err))
+	    {
+	      apr_ansi_time_to_apr_time (&opt_state.date, then);
+	    }
+	  else
+	    {
+	      err = svn_error_createf (SVN_ERR_CL_ARG_PARSING_ERROR,
+				       0, NULL, pool,
+				       "Invalid date specification `%s'",
+				       opt_arg);
+	      svn_handle_error (err, stderr, FALSE);
+	      /* XXX: this should be fatal?  Otherwise we may
+	       * commit/checkout wrong versions? */
+	    }
+	  break;
+	}
       case 'v':
         opt_state.version = TRUE;
       case 'h':
@@ -305,6 +321,10 @@ main (int argc, const char * const *argv
                                      "The locale `%s' can not be set",
                                      opt_arg);
             svn_handle_error (err, stderr, FALSE);
+	    /* XXX: this should be fatal?  Otherwise we may
+	     * commit/checkout wrong versions? (because we specified
+	     * date strings etc according to a locale which wasn't actually
+	     * set? */
           }
         break;
       default:
Index: subversion/include/svn_time.h
===================================================================
RCS file: /usr/local/tigris/data/helm/cvs/repository/subversion/subversion/include/svn_time.h,v
retrieving revision 1.2
diff -u -p -r1.2 svn_time.h
--- subversion/include/svn_time.h	2001/07/04 12:12:20	1.2
+++ subversion/include/svn_time.h	2001/08/23 02:17:45
@@ -38,16 +38,15 @@ svn_stringbuf_t *svn_time_to_string (apr
 apr_time_t svn_time_from_string (svn_stringbuf_t *timestr);
 
 
-/* Needed by getdate.y parser */
-struct getdate_time {
-  time_t time;
-  short timezone;
-};
-
-/* The one interface in our getdate.y parser;  convert human-readable
-   date TEXT into a standard C time_t.  The 2nd argument is unused;
-   we always pass NULL. */
-time_t svn_parse_date (char *text, struct getdate_time *now);
+/* The one public interface of the date parser:  convert human-readable
+   date TEXT into a standard C time_t.  Note that 'now' is passed as
+   a parameter so that you can use this routine to find out how SVN
+   *would have* parsed some string at some *arbitrary* time: relative
+   times should always parse the same even if svn_parse_date is called
+   multiple times during a computation of finite length.  For this reason,
+   the 'now' parameter is *mandatory*. Returns 0 on success. */
+apr_status_t svn_parse_date (const char *text, const time_t now,
+                            time_t * result);
 
 #endif /* SVN_TIME_H */
 
Index: www/project_tasks.html
===================================================================
RCS file: /usr/local/tigris/data/helm/cvs/repository/subversion/www/project_tasks.html,v
retrieving revision 1.8
diff -u -p -r1.8 project_tasks.html
--- www/project_tasks.html	2001/07/25 15:53:08	1.8
+++ www/project_tasks.html	2001/08/23 02:31:01
@@ -119,46 +119,6 @@ Here are the tasks:
    </li>
    <p>
 
-   <!-- ---------------------------------------------------------- -->
-
-   <li> <b>Fix up date parsing library issues</b> <p>
-   </li>
-   This task is probably small, but will require some investigation
-   and list discussion first probably.  The basic issue is this: Ben
-   took the getdate.y date grammar file from CVS (that file has always
-   been in the public domain) and imported it into Subversion.  So now
-   Subversion has CVS's date parsing capabilities, which are good, but
-   not perfect.  Aside from the functionality issues, there's also the
-   problem that getdate.c needs to be automatically generated from
-   getdate.y, and it would be better to have a .c file that we edit
-   directly, than a .y file which causes Subversion developers to be
-   dependent on having the correct version of Yacc/Bison/Whatever
-   installed.
-   <p>
-   This message from Branko summarizes the issues pretty well; read
-   it, then move back and forth in the thread to get some context and
-   a sense of what people see as the solution domain right now:
-   <p>
-   <a
-  href="http://subversion.tigris.org/servlets/ReadMsg?msgId=31147&listName=dev"
-   >http://subversion.tigris.org/servlets/ReadMsg?msgId=31147&listName=dev</a>
-   <p>
-
-   <!-- ---------------------------------------------------------- -->
-
-   <li> <b>Constify svn_parse_date()'s first parameter</b> <p>
-   </li>
-   This is
-   <a
-   href="http://subversion.tigris.org/issues/show_bug.cgi?id=408">issue
-   #408</a>, the description is:
-   <p>
-   The first argument of svn_parse_date() should be const.  However,
-   because the function originates in getdate.y and that parameter is
-   related to the global yyInput variable, there may be more to this
-   change than just adding the qualifier...
-   <p>
-
 <!-- template for further items: -->
 <!--
 

Re: [SVN-DEV] Re: [PATCH] replacement for getdate.y

Posted by kf...@collab.net.
"C. Scott Ananian" <ca...@lesser-magoo.lcs.mit.edu> writes:
> > Until then: *nothing* regarding i18n goes in. (well, unless you want to call
> > choosing UTF-8 as a subtle nod to i18n charset issues...)
> 
> I think this is a mistake.  I'm thinking about i18n now, and so I'd like
> to write my code correct the first time.  It's going to be much harder for
> one of you guys to go back, figure out how my code works, and *then* deal
> with the i18n issues, which might mean rewriting large chunks if I happen
> to Do Things Wrong.

Let me translate Greg's statement: if doing something for i18n now
means risking delaying 1.0, then no.  But if it's a matter of "Do it
this way and make it harder for i18n, do it that way and make it
easier", and this and that are basically equivalent, then by all means
we should do "that".

And of course, if you're volunteering to help with such things, then
that makes a difference.

(However, until M3 there won't be much activity even on "that"s.)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [SVN-DEV] Re: [PATCH] replacement for getdate.y

Posted by "C. Scott Ananian" <ca...@lesser-magoo.lcs.mit.edu>.
On Thu, 23 Aug 2001, Greg Stein wrote:

> We will come back to the localization and i18n issue after 1.0. At that
> point, we'll do a comprehensive sweep of the entire code base after we
> choose how we want to approach the problem.
> 
> Until then: *nothing* regarding i18n goes in. (well, unless you want to call
> choosing UTF-8 as a subtle nod to i18n charset issues...)

I think this is a mistake.  I'm thinking about i18n now, and so I'd like
to write my code correct the first time.  It's going to be much harder for
one of you guys to go back, figure out how my code works, and *then* deal
with the i18n issues, which might mean rewriting large chunks if I happen
to Do Things Wrong.

How about a compromise: I'll leave in the gettext calls, but
  #define gettext(x) x
in the first line of the file, with a big comment.  Then when y'all get
around to i18n, it will be easier because I'll have done most of the work
already --- anyone who's doing i18n will know how gettext is supposed to
work, even if you end up choosing a different i18n system.  All the hard
work will still have been done.

I thought i18n issues were one of the big reasons to ditch getdate.y,
after all.
 --s

Honduras Richard Tomlinson Cocaine chemical agent TASS Yeltsin Delta Force 
Sudan DNC RUCKUS Indonesia SSBN 731 Nader OVER THE HORIZON RADAR Ortega 
              ( http://lesser-magoo.lcs.mit.edu/~cananian )
 --
 "These students are going to have to find out what law and order is
 all about."  -- Brig. General Robert Canterbury, Noon, May 4, 1970,
 minutes before his troops shot 13 unarmed Kent State students, killing 4.
 --
            [http://www.cs.cmu.edu/~dst/DeCSS/Gallery/]
#!/usr/bin/perl -w
# 526-byte qrpff, Keith Winstein and Marc Horowitz <si...@mit.edu>
# MPEG 2 PS VOB file on stdin -> descrambled output on stdout
# arguments: title key bytes in least to most-significant order
$_='while(read+STDIN,$_,2048){$a=29;$c=142;if((@a=unx"C*",$_)[20]&48){$h=5;
$_=unxb24,join"",@b=map{xB8,unxb8,chr($_^$a[--$h+84])}@ARGV;s/...$/1$&/;$d=
unxV,xb25,$_;$b=73;$e=256|(ord$b[4])<<9|ord$b[3];$d=$d>>8^($f=($t=255)&($d
>>12^$d>>4^$d^$d/8))<<17,$e=$e>>8^($t&($g=($q=$e>>14&7^$e)^$q*8^$q<<6))<<9
,$_=(map{$_%16or$t^=$c^=($m=(11,10,116,100,11,122,20,100)[$_/16%8])&110;$t
^=(72,@z=(64,72,$a^=12*($_%16-2?0:$m&17)),$b^=$_%64?12:0,@z)[$_%8]}(16..271))
[$_]^(($h>>=8)+=$f+(~$g&$t))for@a[128..$#a]}print+x"C*",@a}';s/x/pack+/g;eval


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [SVN-DEV] Re: [PATCH] replacement for getdate.y

Posted by Greg Stein <gs...@lyra.org>.
On Thu, Aug 23, 2001 at 03:10:59PM -0400, C. Scott Ananian wrote:
> On Thu, 23 Aug 2001, Branko [ISO-8859-2] �ibej wrote:
>...
>    work.  I assume we've got SVN guys in close with the APR developers so
>    that I could get a apr_strptime API committed if written?

Most of us have commit access to APR. I'm probably the only active APR
developer, but Karl, Ben, Fitz, and Mike all have commit access, too.

> >     * I wouldn't start using gettext just now. Maybe just use it as a
> >       marker. I'd like to take time to review the gettext usage, anyway.
> 
>  - okay, you should do this ASAP, because I don't like the idea of ripping
>    out gettext for no good reason.  You've got two choices: catgets or
>    gettext.  Neither are POSIX, and there are LGPL'ed as well as GPLed
>    implementations of both.  I don't care which, just choose one.

Third choice, and the one we *are* choosing: leave it all out.

We will come back to the localization and i18n issue after 1.0. At that
point, we'll do a comprehensive sweep of the entire code base after we
choose how we want to approach the problem.

Until then: *nothing* regarding i18n goes in. (well, unless you want to call
choosing UTF-8 as a subtle nod to i18n charset issues...)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] replacement for getdate.y

Posted by Branko Čibej <br...@xbc.nu>.
C. Scott Ananian wrote:

>The attached patch replaces getdate.y and getdate.cw with the new
>svn_date.c code.  Instructions:
>

This is great work.

Unfortunately, IMO it not ready for commit yet. Here's what I think 
needs doing:

    * Loose <time.h>, <ctype.h>, and use the APR equivalents. 99.9% of
      the time, we'll have an apr_exploded_time_t available, and it
      makes no sense to convert that to and from struct tm. Youl'' also
      be able to toss mktime_UTC, and use apr_explode_gmt() instead.
    * Given the above, you obviously won't be able to use strptime. We
      need a replacement, anyway because strptime isn't available,
      e.g.,  on Windows. That probably means writing an apr_strptime()
      function, and using that. (I /said/ this was no longer a
      bite-sized task.)
    * I strongly recommend using strtol instead of atoi, because you can
      use the returned end pointer to check the correctness of the
      format. That'll make some of the parsing easier.
    * I wouldn't start using gettext just now. Maybe just use it as a
      marker. I'd like to take time to review the gettext usage, anyway.
      At first glance, it's too simplistic. (For instance, in Slovenian,
      having only a choice of "%d <whatever> ago" and "%d <whatever>s
      ago" is not enough. See `info gettext', the bit about mgettext().
      Welcome to i18n. :-)


Given that replacing strptime is anything mut trivial, you might 
consider dropping support for parsing locale-specific (%X) dates for 
now. I think the sscanf should be enough for the other forms you 
support, or at least for a subset?



-- 
Brane �ibej   <br...@xbc.nu>            http://www.xbc.nu/brane/




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org