You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2010/06/28 20:33:05 UTC
svn commit: r958671 - /subversion/trunk/subversion/libsvn_wc/adm_ops.c
Author: rhuijben
Date: Mon Jun 28 18:33:05 2010
New Revision: 958671
URL: http://svn.apache.org/viewvc?rev=958671&view=rev
Log:
* subversion/libsvn_wc/adm_ops.c
(svn_wc_add4): Fix indentation and remove a few more unneeded ifs.
Modified:
subversion/trunk/subversion/libsvn_wc/adm_ops.c
Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=958671&r1=958670&r2=958671&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Mon Jun 28 18:33:05 2010
@@ -1120,7 +1120,6 @@ svn_wc_add4(svn_wc_context_t *wc_ctx,
svn_wc__db_status_t status;
svn_wc__db_kind_t db_kind;
svn_boolean_t exists;
- apr_hash_t *props;
SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
SVN_ERR_ASSERT(!copyfrom_url || (svn_uri_is_canonical(copyfrom_url,
@@ -1390,131 +1389,92 @@ svn_wc_add4(svn_wc_context_t *wc_ctx,
}
}
else
- { /* ### Wrong indentation - Work in progress */
- svn_wc_entry_t tmp_entry;
- int modify_flags;
-
- /* Init the modify flags. */
- tmp_entry.schedule = svn_wc_schedule_add;
- tmp_entry.kind = kind;
- modify_flags = SVN_WC__ENTRY_MODIFY_SCHEDULE | SVN_WC__ENTRY_MODIFY_KIND;
-
- /* If a copy ancestor was given, make sure the copyfrom URL is in the same
- repository (if possible) and put the proper ancestry info in the new
- entry */
- if (copyfrom_url)
- {
- if (repos_root_url
- && ! svn_uri_is_ancestor(repos_root_url, copyfrom_url))
- return svn_error_createf(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
- _("The URL '%s' has a different repository "
- "root than its parent"), copyfrom_url);
- tmp_entry.copyfrom_url = copyfrom_url;
- tmp_entry.copyfrom_rev = copyfrom_rev;
- tmp_entry.copied = TRUE;
- modify_flags |= SVN_WC__ENTRY_MODIFY_COPYFROM_URL;
- modify_flags |= SVN_WC__ENTRY_MODIFY_COPYFROM_REV;
- modify_flags |= SVN_WC__ENTRY_MODIFY_COPIED;
- }
+ {
+ svn_wc_entry_t tmp_entry;
+ int modify_flags;
+ apr_hash_t *props = NULL;
+
+ /* Store the pristine properties to install them on working, because
+ we might delete the base table */
+ if (exists && status != svn_wc__db_status_not_present)
+ {
+ if (!is_replace && copyfrom_url != NULL)
+ SVN_ERR(svn_wc__db_read_pristine_props(&props, db, local_abspath,
+ scratch_pool,
+ scratch_pool));
+ else
+ props = apr_hash_make(scratch_pool);
+ }
- /* Store the pristine properties to install them on working, because
- we might delete the base table */
- if ((exists && status != svn_wc__db_status_not_present)
- && !is_replace && copyfrom_url != NULL)
- {
- /* NOTE: the conditions to reach here *exactly* match the
- conditions used below when PROPS is referenced.
- Be careful to keep these sets of conditionals aligned to avoid
- an uninitialized PROPS value. */
- SVN_ERR(svn_wc__db_read_pristine_props(&props, db, local_abspath,
- scratch_pool, scratch_pool));
- }
+ /* Init the modify flags. */
+ tmp_entry.schedule = svn_wc_schedule_add;
+ tmp_entry.kind = kind;
+ modify_flags = SVN_WC__ENTRY_MODIFY_SCHEDULE | SVN_WC__ENTRY_MODIFY_KIND;
+
+ /* If a copy ancestor was given, make sure the copyfrom URL is in the same
+ repository (if possible) and put the proper ancestry info in the new
+ entry */
+ if (copyfrom_url)
+ {
+ if (repos_root_url
+ && ! svn_uri_is_ancestor(repos_root_url, copyfrom_url))
+ return svn_error_createf(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
+ _("The URL '%s' has a different "
+ "repository root than its parent"),
+ copyfrom_url);
+ tmp_entry.copyfrom_url = copyfrom_url;
+ tmp_entry.copyfrom_rev = copyfrom_rev;
+ tmp_entry.copied = TRUE;
+ modify_flags |= SVN_WC__ENTRY_MODIFY_COPYFROM_URL;
+ modify_flags |= SVN_WC__ENTRY_MODIFY_COPYFROM_REV;
+ modify_flags |= SVN_WC__ENTRY_MODIFY_COPIED;
+ }
- if (modify_flags)
- {
- if (kind == svn_node_dir)
- SVN_ERR(svn_wc__entry_modify_stub(db, local_abspath,
- &tmp_entry, modify_flags,
- scratch_pool));
- else
- SVN_ERR(svn_wc__entry_modify(db, local_abspath, kind,
- &tmp_entry, modify_flags,
- scratch_pool));
- }
+ SVN_ERR(svn_wc__entry_modify_stub(db, local_abspath,
+ &tmp_entry, modify_flags,
+ scratch_pool));
+
+ /* We're making the same mods we made above, but this time we'll
+ force the scheduling. Also make sure to undo the
+ 'incomplete' flag which svn_wc__internal_ensure_adm() sets by
+ default.
+
+ This deletes the erroneous BASE_NODE for added directories and
+ adds a WORKING_NODE. */
+ modify_flags |= SVN_WC__ENTRY_MODIFY_FORCE;
+ tmp_entry.schedule = (is_replace
+ ? svn_wc_schedule_replace
+ : svn_wc_schedule_add);
+ SVN_ERR(svn_wc__entry_modify(db, local_abspath, svn_node_dir,
+ &tmp_entry, modify_flags, scratch_pool));
- if (kind == svn_node_dir) /* scheduling a directory for addition */
- {
- /* We're making the same mods we made above, but this time we'll
- force the scheduling. Also make sure to undo the
- 'incomplete' flag which svn_wc__internal_ensure_adm() sets by
- default.
-
- This deletes the erroneous BASE_NODE for added directories and
- adds a WORKING_NODE. */
- if (modify_flags)
- {
- modify_flags |= SVN_WC__ENTRY_MODIFY_FORCE;
- tmp_entry.schedule = (is_replace
- ? svn_wc_schedule_replace
- : svn_wc_schedule_add);
- SVN_ERR(svn_wc__entry_modify(db, local_abspath, svn_node_dir,
- &tmp_entry, modify_flags,
- scratch_pool));
- }
+ SVN_ERR(svn_wc__db_temp_op_set_working_incomplete(db, local_abspath,
+ FALSE, scratch_pool));
- SVN_ERR(svn_wc__db_temp_op_set_working_incomplete(db, local_abspath,
- FALSE, scratch_pool));
+ if (is_wc_root)
+ {
+ /* If this new directory has ancestry, it's not enough to
+ schedule it for addition with copyfrom args. We also
+ need to rewrite all its BASE nodes to move them into WORKING.
- if (is_wc_root)
- {
- /* If this new directory has ancestry, it's not enough to
- schedule it for addition with copyfrom args. We also
- need to rewrite its ancestor-url, and rewrite the
- ancestor-url of ALL its children!
-
- We're doing this because our current commit model (for
- hysterical raisins, presumably) assumes an entry's URL is
- correct before commit -- i.e. the URL is not tweaked in
- the post-commit bumping process. We might want to change
- this model someday. */
-
- /* ### copy.c will copy .svn subdirs, which means the whole
- ### subtree is already "versioned". we now need to rejigger
- ### the metadata to make it Proper for this location. */
+ Currently we still handle this by setting copied on all
+ subnodes. */
- /* Recursively add the 'copied' existence flag as well! */
+ SVN_ERR(mark_tree_copied(db, local_abspath,
+ exists ? status : svn_wc__db_status_added,
+ scratch_pool));
- SVN_ERR(mark_tree_copied(db, local_abspath,
- exists ? status : svn_wc__db_status_added,
- scratch_pool));
+ /* Clean out the now-obsolete dav cache values. */
+ /* ### put this into above walk. clear all cached values. */
+ SVN_ERR(svn_wc__db_base_set_dav_cache(db, local_abspath, NULL,
+ scratch_pool));
+ }
- /* Clean out the now-obsolete dav cache values. */
- /* ### put this into above walk. clear all cached values. */
- SVN_ERR(svn_wc__db_base_set_dav_cache(db, local_abspath, NULL,
+ /* Set the WORKING_NODE properties from the values we saved earlier */
+ if (props != NULL)
+ SVN_ERR(svn_wc__db_temp_working_set_props(db, local_abspath, props,
scratch_pool));
- }
- }
-
- /* Set the pristine properties in WORKING_NODE, by copying them from the
- deleted BASE_NODE record. Or set them to empty to make sure we don't
- inherit wrong properties from BASE */
- if (exists && status != svn_wc__db_status_not_present)
- {
- if (!is_replace && copyfrom_url != NULL)
- {
- /* NOTE: the conditions to reach here *exactly* match the
- conditions that were used to initialize the PROPS localvar.
- Be careful to keep these sets of conditionals aligned to avoid
- an uninitialized PROPS value. */
- SVN_ERR(svn_wc__db_temp_working_set_props(db, local_abspath, props,
- scratch_pool));
- }
- else
- SVN_ERR(svn_wc__db_temp_working_set_props(db, local_abspath,
- apr_hash_make(scratch_pool),
- scratch_pool));
- }
- } /* ### /Wrong indentation - /Work in progress */
+ }
/* Report the addition to the caller. */
if (notify_func != NULL)
Re: svn commit: r958671 - /subversion/trunk/subversion/libsvn_wc/adm_ops.c
Posted by Greg Stein <gs...@gmail.com>.
On Mon, Jun 28, 2010 at 15:24, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
>...
>> > (on s.c.n viewvc's diffs used to ignore whitespace... doesn't seem to be
>> > the case on s.a.o though)
>>
>> Whitespace can be important (e.g Python scripts), so keeping simple
>> whitespace changes in the commit email is a Good Thing.
>
> +1. (I'm used to being able to just pipe a commit mail to 'svn patch'.)
>
>> I hadn't realized that s.c.n did that.
>>
>
> In its default view, yes. (it also had a 'view raw unidiff' mode)
Whoops. I missed the "viewvc" part (rather than commit emails).
Yeah... whitespace elimination isn't bad there. Tho it could probably
examine the file, detect python code, and keep whitespace for those
files.
*shrug*
Thanks,
-g
Re: svn commit: r958671 -
/subversion/trunk/subversion/libsvn_wc/adm_ops.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Greg Stein wrote on Mon, 28 Jun 2010 at 22:16 -0000:
> On Mon, Jun 28, 2010 at 14:59, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> > Greg Stein wrote on Mon, 28 Jun 2010 at 21:50 -0000:
> >> How am I supposed to review this? I can't tell what is indentation,
> >> versus actual code change.
> >>
> >
> > svn diff -x-w ?
>
> Sure, I could do that, but reviewing commit emails is much easier.
Agreed. Just saying what you could do for this revision (which is
already committed).
(last time it happened *I* forgot about the -x-w option, that's why I'm
mentioning it here now)
> No context switching. No cut/paste of revision numbers. I can actually
> reply in-line with comments/suggestions. etc.
>
> (tho I expect I'm speaking to the choir here :-P )
>
> > (on s.c.n viewvc's diffs used to ignore whitespace... doesn't seem to be
> > the case on s.a.o though)
>
> Whitespace can be important (e.g Python scripts), so keeping simple
> whitespace changes in the commit email is a Good Thing.
+1. (I'm used to being able to just pipe a commit mail to 'svn patch'.)
> I hadn't realized that s.c.n did that.
>
In its default view, yes. (it also had a 'view raw unidiff' mode)
> Cheers,
> -g
>
Re: svn commit: r958671 - /subversion/trunk/subversion/libsvn_wc/adm_ops.c
Posted by Greg Stein <gs...@gmail.com>.
On Mon, Jun 28, 2010 at 14:59, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Greg Stein wrote on Mon, 28 Jun 2010 at 21:50 -0000:
>> On Mon, Jun 28, 2010 at 14:33, <rh...@apache.org> wrote:
>> > Author: rhuijben
>> > Date: Mon Jun 28 18:33:05 2010
>> > New Revision: 958671
>> >
>> > URL: http://svn.apache.org/viewvc?rev=958671&view=rev
>> > Log:
>> > * subversion/libsvn_wc/adm_ops.c
>> > (svn_wc_add4): Fix indentation and remove a few more unneeded ifs.
>>
>> Argh!!!
>>
>> How am I supposed to review this? I can't tell what is indentation,
>> versus actual code change.
>>
>
> svn diff -x-w ?
Sure, I could do that, but reviewing commit emails is much easier. No
context switching. No cut/paste of revision numbers. I can actually
reply in-line with comments/suggestions. etc.
(tho I expect I'm speaking to the choir here :-P )
> (on s.c.n viewvc's diffs used to ignore whitespace... doesn't seem to be
> the case on s.a.o though)
Whitespace can be important (e.g Python scripts), so keeping simple
whitespace changes in the commit email is a Good Thing. I hadn't
realized that s.c.n did that.
Cheers,
-g
Re: svn commit: r958671 -
/subversion/trunk/subversion/libsvn_wc/adm_ops.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Greg Stein wrote on Mon, 28 Jun 2010 at 21:50 -0000:
> On Mon, Jun 28, 2010 at 14:33, <rh...@apache.org> wrote:
> > Author: rhuijben
> > Date: Mon Jun 28 18:33:05 2010
> > New Revision: 958671
> >
> > URL: http://svn.apache.org/viewvc?rev=958671&view=rev
> > Log:
> > * subversion/libsvn_wc/adm_ops.c
> > (svn_wc_add4): Fix indentation and remove a few more unneeded ifs.
>
> Argh!!!
>
> How am I supposed to review this? I can't tell what is indentation,
> versus actual code change.
>
svn diff -x-w ?
(on s.c.n viewvc's diffs used to ignore whitespace... doesn't seem to be
the case on s.a.o though)
> Please, please, please only do big whitespace change. Or functional
> change. Mixing them makes it unreviewable :-(
>
+1
> >...
>
> -g
>
Re: svn commit: r958671 - /subversion/trunk/subversion/libsvn_wc/adm_ops.c
Posted by Greg Stein <gs...@gmail.com>.
On Mon, Jun 28, 2010 at 14:33, <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Mon Jun 28 18:33:05 2010
> New Revision: 958671
>
> URL: http://svn.apache.org/viewvc?rev=958671&view=rev
> Log:
> * subversion/libsvn_wc/adm_ops.c
> (svn_wc_add4): Fix indentation and remove a few more unneeded ifs.
Argh!!!
How am I supposed to review this? I can't tell what is indentation,
versus actual code change.
Please, please, please only do big whitespace change. Or functional
change. Mixing them makes it unreviewable :-(
>...
-g