Just a thought: Ben, your log message gives a ton of useful
information, and it's importance is permanent. So probably it belongs
in a comment in the code, not [only] in a log message... :-)
One should never need the log entries to understand the current code.
If you find yourself writing a significant explanation in the log, you
should consider carefully whether your text doesn't actually belong in
a comment, alongside the code it explains.
sussman@tigris.org writes:
> User: sussman
> Date: 00/11/09 07:22:28
>
> Modified: subversion/libsvn_wc get_editor.c log.c wc.h
> Log:
> Fixed broken timestamp interface and execution.
>
> First of all, an entry should *always* receive a timestamp whenever it
> receives a revision-stamp. They are inseparable concepts; the
> timestamp represents the last time the file was known to be updated by
> a repository.
>
> Second, the log entry "<modify-entry ...>" should do *exactly* as its
> told, and not try to be smarter than its caller unless explicitly
> asked. Therefore the timestamp attribute has three potential values:
>
> 1. <modify-entry timestamp="8378128827" revision="6"/>
>
> Use the timestamp directly, as an apr_time_t.
>
> 2. <modify-entry conflict="true" reject-file="foo.c.342.34.prej"/>
>
> No mention of timestamp, so don't modify it.
>
> 3. <modify-entry timestamp="working" revision="6"/>
>
> Be smart. Get the timestamp from the working version of the
> file.
>
> All files now have timestamps. They get them when they're checked out
> or updated. (finally!)
>
> * wc.h (SVN_WC__LOG_ATTR_TIMESTAMP_WC): new `smart' value of the
> timestamp field in <modify-entry>
>
> (svn_wc__entry_merge_sync): small doc fix
>
> * log.c (start_handler): new implementation of timestamp handling, as
> described above.
>
> * get_editor.c (close_file): use `smart' value for timestamp when
> writing log entry to bump the revision number.
>
> Revision Changes Path
> 1.124 +6 -0 subversion/subversion/libsvn_wc/get_editor.c
>
> Index: get_editor.c
> ===================================================================
> RCS file: /cvs/subversion/subversion/libsvn_wc/get_editor.c,v
> retrieving revision 1.123
> retrieving revision 1.124
> diff -u -r1.123 -r1.124
> --- get_editor.c 2000/11/08 05:48:24 1.123
> +++ get_editor.c 2000/11/09 15:22:27 1.124
> @@ -683,6 +683,9 @@
> "%d",
> db->edit_baton->target_revision);
>
> + /* Because this is a directory, we're not setting the timestamp
> + here. Directories become "out of date" iff some descendant
> + does. */
> svn_xml_make_open_tag (&entry_accum,
> db->pool,
> svn_xml_self_closing,
> @@ -1217,6 +1220,9 @@
> fb->name,
> SVN_WC_ENTRY_ATTR_REVISION,
> svn_string_create (revision_str, fb->pool),
> + SVN_WC_ENTRY_ATTR_TIMESTAMP,
> + svn_string_create (SVN_WC__LOG_ATTR_TIMESTAMP_WC,
> + fb->pool), /* use wfile time */
> NULL);
>
> /* Write our accumulation of log entries into a log file */
>
>
>
> 1.57 +51 -32 subversion/subversion/libsvn_wc/log.c
>
> Index: log.c
> ===================================================================
> RCS file: /cvs/subversion/subversion/libsvn_wc/log.c,v
> retrieving revision 1.56
> retrieving revision 1.57
> diff -u -r1.56 -r1.57
> --- log.c 2000/11/08 05:59:28 1.56
> +++ log.c 2000/11/09 15:22:27 1.57
> @@ -372,13 +372,13 @@
> }
> else
> {
> + apr_time_t timestamp;
> svn_string_t *sname = svn_string_create (name, loggy->pool);
> svn_string_t *revstr = apr_hash_get (ah,
> SVN_WC_ENTRY_ATTR_REVISION,
> APR_HASH_KEY_STRING);
> svn_revnum_t new_revision = (revstr ? atoi (revstr->data)
> : SVN_INVALID_REVNUM);
> - apr_time_t timestamp = 0;
> int flags = 0;
>
> enum svn_node_kind kind = svn_node_unknown;
> @@ -416,42 +416,61 @@
> if (apr_hash_get (ah, SVN_WC_ENTRY_ATTR_CONFLICT,
> APR_HASH_KEY_STRING))
> flags |= SVN_WC_ENTRY_CONFLICT;
> -
> - /* Get the timestamp only if the working file exists. */
> +
> + /* Did the log command give us a timestamp? There are three
> + possible scenarios here. */
> {
> - /* kff todo: there is an issue here. The timestamp should
> - be done after the wfile is updated. */
> - enum svn_node_kind wfile_kind;
> - err = svn_io_check_path (wfile, &wfile_kind, loggy->pool);
> - if (err)
> - {
> - signal_error (loggy, svn_error_createf
> - (SVN_ERR_WC_BAD_ADM_LOG,
> - 0,
> - NULL,
> - loggy->pool,
> - "error checking path %s",
> - name));
> - return;
> - }
> - if (kind == svn_node_file)
> - err = svn_wc__file_affected_time (×tamp,
> - wfile,
> - loggy->pool);
> - }
> + svn_string_t *timestr = apr_hash_get (ah,
> + SVN_WC_ENTRY_ATTR_TIMESTAMP,
> + APR_HASH_KEY_STRING);
>
> - if (err)
> + /* Scenario 1: no timestamp mentioned at all */
> + if (! timestr)
> + timestamp = 0; /* this tells merge_sync to ignore the
> + field */
> +
> + /* Scenario 2: use the working copy's timestamp */
> + else if (! strcmp (timestr->data, SVN_WC__LOG_ATTR_TIMESTAMP_WC))
> {
> - signal_error (loggy, svn_error_createf
> - (SVN_ERR_WC_BAD_ADM_LOG,
> - 0,
> - NULL,
> - loggy->pool,
> - "error discovering file affected time on %s",
> - name));
> - return;
> + enum svn_node_kind wfile_kind;
> + err = svn_io_check_path (wfile, &wfile_kind, loggy->pool);
> + if (err)
> + {
> + signal_error (loggy, svn_error_createf
> + (SVN_ERR_WC_BAD_ADM_LOG,
> + 0,
> + NULL,
> + loggy->pool,
> + "error checking path %s",
> + name));
> + return;
> + }
> + if (wfile_kind == svn_node_file)
> + err = svn_wc__file_affected_time (×tamp,
> + wfile,
> + loggy->pool);
> + if (err)
> + {
> + signal_error (loggy, svn_error_createf
> + (SVN_ERR_WC_BAD_ADM_LOG,
> + 0,
> + NULL,
> + loggy->pool,
> + "error discovering file affected time on %s",
> + name));
> + return;
> + }
> }
> +
> + /* Scenario 3: use the integer provided, as-is. */
> + else
> + /* Is atol appropriate here for converting an apr_time_t
> + to a string and then back again? Or should we just use
> + our svn_wc__time_to_string and string_to_time? */
> + timestamp = (apr_time_t) atol (timestr->data);
> + }
>
> + /* Now write the new entry out */
> err = svn_wc__entry_merge_sync (loggy->path,
> sname,
> new_revision,
>
>
>
> 1.104 +5 -1 subversion/subversion/libsvn_wc/wc.h
>
> Index: wc.h
> ===================================================================
> RCS file: /cvs/subversion/subversion/libsvn_wc/wc.h,v
> retrieving revision 1.103
> retrieving revision 1.104
> diff -u -r1.103 -r1.104
> --- wc.h 2000/11/08 05:43:54 1.103
> +++ wc.h 2000/11/09 15:22:27 1.104
> @@ -323,6 +323,10 @@
> #define SVN_WC__LOG_ATTR_REVISION "revision"
> #define SVN_WC__LOG_ATTR_SAVED_MODS "saved-mods"
>
> +/** Special log attribute values **/
> +#define SVN_WC__LOG_ATTR_TIMESTAMP_WC "working"
> +
> +
> /* Starting at PATH, write out log entries indicating that a commit
> * succeeded, using REVISION as the new revision number. run_log will
> * use these log items to complete the commit.
> @@ -394,7 +398,7 @@
>
>
> /* For PATH's entries file, create or modify an entry NAME, using
> - * explicit fields and, secondarily, varargs.
> + * explicit fields and, secondarily, the attributes in ATTS.
> *
> * If NAME is null, it means the dir's own entry, as usual.
> *
>
>
>
Received on Sat Oct 21 14:36:14 2006