> Sory for this late reply, but I have a few questions
> regarding this half-year-old commit.
No problem :-) Better late than never and it's before a new minor
release, which probably means early enough.
> > Index: log.c
> > ===================================================================
> > --- log.c (revision 21722)
> > +++ log.c (revision 21723)
> > @@ -1343,15 +1288,74 @@
> > (pick_error_code(loggy), err,
> > _("Error replacing text-base of '%s'"), name);
> >
> > -
> > + if ((err = svn_io_stat(&finfo, full_path, APR_FINFO_MIN | APR_FINFO_LINK,
> > + pool)))
> > + return svn_error_createf(pick_error_code(loggy), err,
> > + _("Error getting 'affected time' of '%s'"),
> > + svn_path_local_style(full_path, pool));
> > /* If the working file was overwritten (due to re-translation)
> > or touched (due to +x / -x), then use *that* textual
> > timestamp instead. */
> > if (overwrote_working)
> > - if ((err = svn_io_file_affected_time(&text_time, full_path, pool)))
> > - return svn_error_createf(pick_error_code(loggy), err,
> > - _("Error getting 'affected time' of '%s'"),
> > - svn_path_local_style(full_path, pool));
> > + text_time = finfo.mtime;
> > + else
> > + {
> > + /* The working copy file hasn't been overwritten, meaning
> > + we need to decide which timestamp to use. */
> > +
> > + /* For file commit items, we need to "install" the user's working
>
> This installation is already done above, right?
Yes.
> > + file as the new `text-base' in the administrative area. A copy
> > + of this file should have been dropped into our `tmp/text-base'
> > + directory during the commit process. Part of this process
> > + involves setting the textual timestamp for this entry. We'd like
> > + to just use the timestamp of the working file, but it is possible
> > + that at some point during the commit, the real working file might
> > + have changed again. If that has happened, we'll use the
> > + timestamp of the copy of this file in `tmp/text-base'. */
> > +
> > + const char *tmpf;
> > + svn_boolean_t modified = FALSE;
> > + apr_finfo_t tmpf_finfo;
> > +
> > + /* Assume for now our working file copy
> > + is present in the temp area. */
> > + tmpf = svn_wc__text_base_path(full_path, 1, pool);
> > +
>
> However, install_committed_file moved the temporary text base
> into its final position, so this will never be the case.
Exactly.
> > + /* If it isn't, we'll error out here... */
>
> We're not exactly erroring out if the file isn't present, are we?
Nope.
> > + err = svn_io_stat(&tmpf_finfo, tmpf, APR_FINFO_MIN | APR_FINFO_LINK,
> > + pool);
> > + if (err)
> > + {
> > + if (APR_STATUS_IS_ENOENT(err->apr_err)
> > + || APR_STATUS_IS_ENOTDIR(err->apr_err))
> > + svn_error_clear(err);
> > + else
> > + return svn_error_createf
> > + (pick_error_code(loggy), err,
> > + _("Error getting 'affected time' for '%s'"),
>
> This error description seems a bit out of date.
Ok, but since we'll always get ENOENT, there's no reason for that code
to be executed.
> > + svn_path_local_style(tmpf, pool));
> > + }
> > + else
> > + {
>
> If my assertions above are correct, then the below code never gets
> executed.
Yes, that's correct.
> > + /* Verify that the working file is the same as the tmpf file. */
> > + modified = finfo.size == tmpf_finfo.size;
>
> So the file is considered modified if the sizes are the
> same and not modified if the sizes are different.
> Isn't that backwards?
Ouch!
> > + if (finfo.mtime != tmpf_finfo.mtime && ! modified)
> > + {
> > + err = svn_wc__versioned_file_modcheck(&modified, full_path,
> > + loggy->adm_access,
> > + tmpf, FALSE, pool);
> > + if (err)
> > + return svn_error_createf
> > + (pick_error_code(loggy), err,
> > + _("Error comparing '%s' and '%s'"),
> > + svn_path_local_style(full_path, pool),
> > + svn_path_local_style(tmpf, pool));
> > + }
> > + /* If they are the same, use the working file's timestamp,
> > + else use the tmpf file's timestamp. */
> > + text_time = modified ? tmpf_finfo.mtime : finfo.mtime;
> > + }
> > + }
> > }
> >
> > /* Files have been moved, and timestamps have been found. It is now
>
> Testing with the latest trunk code, it turns
> out that commits don't update timestamps correctly.
> Committing an add file doesn't add a timestamp
> and commits of an existing file leaves the timestamp
> at the old value. I haven't revert this revision
> to se if it is the one that causes
> this behaviour.
Well, it probably is. I'm working in Windows now, so, I'm unable to
build a client with a reverted trunk right now, but it looks like I
botched the commit quite seriously. I'm sorry. I'll work on finding
out more about it as soon as I can reboot into Windows and have some
time to investigate. Maybe sometime this weekend.
bye,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Mar 6 21:44:55 2007