This is a comment for the people that will be reviewing this patch. I
think that Paul took the "conservative" route with this patch based on our
best knowledge on how these sorts of changes have been made in the past.
Personally, I think there is a better way to do this, but it may be a bit
more controversial.
The heart of this patch is this change to the svn_wc_status2_t structure.
> + /** Out of Date Info.
> + *
> + * If the working copy item is out of date compared to the repository
> + * the following five fields represent the HEAD state of the item in
> + * the repository. If not out of date, the items are set as
described
> + * below.
> + */
> +
> + /* If not out of date set to SVN_INVALID_REVNUM. */
> + svn_revnum_t ood_last_cmt_rev;
> +
> + /* If not out of date set to 0. */
> + apr_time_t ood_last_cmt_date;
> +
> + /* If not out of date set to svn_wc_status_none. */
> + svn_node_kind_t ood_kind;
> +
> + /* If not out of date set to NULL. */
> + const char *ood_url;
> +
> + /* If not out of date set to NULL. */
> + const char *ood_last_cmt_author;
> +
Let's start with the URL and kind. The main issue with these two is that
currently if there is a new item in the repository these fields (like most
of the fields in the structure) are null/empty or -1. So one change would
be to drop these fields from the new structure and just change the
behavior so that the existing fields in the existing structure are always
filled in with the values from the repository. In the case of a mod to an
existing file, I think that the values in these fields would always be the
same so it seems like a bit of a waste to add these two new fields.
Perhaps in the future if "true renames" are implemented the URL's could be
different?
That leaves us with three new fields. The revision, date and author of
the last commit for this item in the repository. Again, there are already
existing fields for these items in the existing structures. Currently,
the values of those fields always reflect the values that are cached in
the working copy, even when the -u switch. I would propose that this is
another case where a behavior change could be warranted. If the -u switch
is passed, I think that the fields in the existing structure should just
contain the values from the repository. I cannot see any reason why
someone would want to know the local values in that scenario. If we were
to make this change, then we would not have to make *any* changes to the
existing structure. We would simply be modifying the existing behavior
when the -u switch is used. I think that this would be an improvement in
the existing behavior and if we simply updated the existing structure
everything that uses it would get the new behavior automatically.
For example, I have an out-of-date WC for the svn /www folder. If I run
svn st -u -v I see this:
* 15585 15427 kfogel project_links.html
15585 14400 maxb inconveniences.html
* 15585 15502 fitz faq.html
15585 15382 kfogel project_development.html
15585 15382 kfogel project_status.html
15585 15038 kfogel testimonials.html
15585 14400 maxb propaganda.html
15585 14009 maxb db42-support-patch.txt
15585 15382 kfogel mailing-list-guidelines.html
15585 14400 maxb license-1.html
15585 14400 maxb cvs-changelog.html
15585 15382 kfogel getting_subversion.html
* license-for-python-bindings.html
15585 15382 kfogel roadmap.html
15585 14009 maxb robots.txt
15585 14400 maxb variance-adjusted-patching.html
15585 15335 fitz svn-dav-securityspace-survey.html
15585 14009 maxb httpd-win32.patch.txt
15585 14400 maxb project_faq.html
15585 15382 kfogel svn_1.1_releasenotes.html
15585 15328 maxb svn_1.2_releasenotes.html
15585 14974 maxb validate.sh
15585 14009 maxb security\CAN-2004-0749-advisory.txt
15585 14009 maxb security\mod_authz_svn-copy-advisory.txt
15585 14009 maxb security\CAN-2004-0413-advisory.txt
15585 14009 maxb security
* 15585 15392 maxb hacking.html
Status against revision: 15884
For the files that are modified in the repository it is showing the local
HEAD revision as well as the cached local last changed revision/author. In
this scenario, it seems like the latter is not a lot of value. With my
proposed change the output would look like this, which seems more useful.
* 15585 15847 kfogel project_links.html
15585 14400 maxb inconveniences.html
* 15585 15814 kfogel faq.html
15585 15382 kfogel project_development.html
15585 15382 kfogel project_status.html
15585 15038 kfogel testimonials.html
15585 14400 maxb propaganda.html
15585 14009 maxb db42-support-patch.txt
15585 15382 kfogel mailing-list-guidelines.html
15585 14400 maxb license-1.html
15585 14400 maxb cvs-changelog.html
15585 15382 kfogel getting_subversion.html
* 15848 djames license-for-python-bindings.html
15585 15382 kfogel roadmap.html
15585 14009 maxb robots.txt
15585 14400 maxb variance-adjusted-patching.html
15585 15335 fitz svn-dav-securityspace-survey.html
15585 14009 maxb httpd-win32.patch.txt
15585 14400 maxb project_faq.html
15585 15382 kfogel svn_1.1_releasenotes.html
15585 15328 maxb svn_1.2_releasenotes.html
15585 14974 maxb validate.sh
15585 14009 maxb security\CAN-2004-0749-advisory.txt
15585 14009 maxb security\mod_authz_svn-copy-advisory.txt
15585 14009 maxb security\CAN-2004-0413-advisory.txt
15585 14009 maxb security
* 15769 15392 dlr hacking.html
Status against revision: 15884
This output seems more useful to me since it tells me the revision when
the item was changed in the repository. Since I specified the -u switch
it seems like that is the info I would want.
In summary, I would propose that at a minimum we ditch the new URL and
kind fields from this structure and instead update the existing fields in
the cases that we currently do not (new files in repos). I also think it
is worthy of consideration that we do the same for the other three fields
(revision, date, and author). However, since changing those fields would
visibly change the output of status, I can see where some people would be
against that. To me, it seems like a better way to change this.
Thanks
Mark
_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. by IBM Email Security Management Services powered by MessageLabs.
_____________________________________________________________________________
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Aug 23 16:39:49 2005