You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by solo turn <so...@yahoo.com> on 2002/08/24 19:20:05 UTC

svn -R ... how to get it really into svn

some time ago i proposed a patch to make svn -R recurse over all
directories, and does not stop if a directory already exists.

there were 2 small comments on it (like use .. != .. instead of !(..
== ..)

but for a reason i still don't understand, the patch made it to the
mailing list, noone objecting to it, but also noone seemed to care to
really apply it to the source tree.

what do i have to do to get a patch into the source tree?

for us in a stadium of switching to svn, the -R switch is kind of a
showstopper, cause it makes a BIG difference if you do a 100 adds, or
one add -R. we are not switching everything at once, but little by
little, so top-directories are there and get filled later.

also, if you have java code distributed over many directories, with
one file per class, you add classes to different directories
constantly, and the main work is to issue all the add commands.


__________________________________________________
Do You Yahoo!?
Yahoo! Finance - Get real-time stock quotes
http://finance.yahoo.com

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

Re: svn -R ... how to get it really into svn

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
solo turn <so...@yahoo.com> writes:
> some time ago i proposed a patch to make svn -R recurse over all
> directories, and does not stop if a directory already exists.
> 
> there were 2 small comments on it (like use .. != .. instead of !(..
> == ..)
> 
> but for a reason i still don't understand, the patch made it to the
> mailing list, noone objecting to it, but also noone seemed to care to
> really apply it to the source tree.
> 
> what do i have to do to get a patch into the source tree?

You have to wait for people to finish the 0.14.2 release process and
then get time to review pending patches, basically :-).

There's nothing wrong with posting a reminder, as you just did.  But
don't worry, your patch is not lost.  It's archived with about fifty
other patches, and we will get to them as soon as we can.

I've been (personally) planning a big patch review day for a week; it
got pushed due to some other stuff, right now it's scheduled for
Tuesday.  I can't *promise* that it will get done that day, but it
seems likely.

It helps if you include your entire patch (and log message!) with each
reminder post.  And if you've incorporated anyone's comments, for
example Philip Martin's, into the patch, then say so in the
introduction, so we know that reviewer X's feedback has been taken
care of.

Are you planning to repost the patch, or should we just use the last
copy we can find?

> if i wrote these 2 lines of code, and it worked, then i don't think
> it is using your spare time, or changes anything else in the tracked
> issues ...

Every patch we apply takes a certain minimum amount of time.  We have
to read and understand it, and make sure it passes the regression
tests.  It doesn't matter how trivial it is.  I have applied patches
that did *nothing* except change the usage message for some option,
and it still broke a regression test (getopt_tests.py).  Did you run
"make check" to make sure your patch passes, by the way?

Also, the priority developers give a patch is not necessarily related
to the size or complexity of the patch, as Mike Pilato explained.

-K

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

Re: svn -R ... how to get it really into svn

Posted by solo turn <so...@yahoo.com>.
the reason is very simple: i tried to convice 4 people to use it (the
4 open minded ones in our teams). the result was the same for all of
them:

the reasons for failure:

1. windows does not allow to do "add -R *", 
   so "add -R bla" 10 times for every single directory entry
2. they added some files in their tree,
   and then same story. do "svn add" for every single file.

got a "that is crap", and that was it. with the patch, they did NOT
refuse. working style was:
- cd ..
- add -R directory

if i wrote these 2 lines of code, and it worked, then i don't think
it is using your spare time, or changes anything else in the tracked
issues ...


--- cmpilato@collab.net wrote:
> solo turn <so...@yahoo.com> writes:
> 
> > some time ago i proposed a patch to make svn -R recurse over all
> > directories, and does not stop if a directory already exists.
> > 
> > there were 2 small comments on it (like use .. != .. instead of
> !(..
> > == ..)
> > 
> > but for a reason i still don't understand, the patch made it to
> the
> > mailing list, noone objecting to it, but also noone seemed to
> care to
> > really apply it to the source tree.
> > 
> > what do i have to do to get a patch into the source tree?
> > 
> > for us in a stadium of switching to svn, the -R switch is kind of
> a
> > showstopper, cause it makes a BIG difference if you do a 100
> adds, or
> > one add -R. we are not switching everything at once, but little
> by
> > little, so top-directories are there and get filled later.
> > 
> > also, if you have java code distributed over many directories,
> with
> > one file per class, you add classes to different directories
> > constantly, and the main work is to issue all the add commands.
> 
> Patience, homey.  
> 
> As a regular poster to this list, I assume you also read what
> others
> post.  You might have gathered that there are currently (in fact,
> almost always) several important threads going on, plus a 0.14.2
> tarball release, plus patches being submitted at a ridiculous rate.
> 
> It's nothing personal against you or your patch, but we have an
> issue
> tracker for prioritizing tasks to be done, and there are several of
> us
> who use just that prioritized list to determine what we need to be
> working on.  If you feel that the list is prioritized wrongly, make
> a
> stronger case for different ordering on the list.  That is, explain
> to
> us why 'svn -R' is more important than upgrading the FS schema, or
> tweaking the property filenames, or ... you know, the other things
> that we spent our time for this last release.
> 
> Also, I believe Karl is planning another batch-patch-processing day
> soon.
> 


__________________________________________________
Do You Yahoo!?
Yahoo! Finance - Get real-time stock quotes
http://finance.yahoo.com

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

Re: svn -R ... how to get it really into svn

Posted by cm...@collab.net.
solo turn <so...@yahoo.com> writes:

> some time ago i proposed a patch to make svn -R recurse over all
> directories, and does not stop if a directory already exists.
> 
> there were 2 small comments on it (like use .. != .. instead of !(..
> == ..)
> 
> but for a reason i still don't understand, the patch made it to the
> mailing list, noone objecting to it, but also noone seemed to care to
> really apply it to the source tree.
> 
> what do i have to do to get a patch into the source tree?
> 
> for us in a stadium of switching to svn, the -R switch is kind of a
> showstopper, cause it makes a BIG difference if you do a 100 adds, or
> one add -R. we are not switching everything at once, but little by
> little, so top-directories are there and get filled later.
> 
> also, if you have java code distributed over many directories, with
> one file per class, you add classes to different directories
> constantly, and the main work is to issue all the add commands.

Patience, homey.  

As a regular poster to this list, I assume you also read what others
post.  You might have gathered that there are currently (in fact,
almost always) several important threads going on, plus a 0.14.2
tarball release, plus patches being submitted at a ridiculous rate.

It's nothing personal against you or your patch, but we have an issue
tracker for prioritizing tasks to be done, and there are several of us
who use just that prioritized list to determine what we need to be
working on.  If you feel that the list is prioritized wrongly, make a
stronger case for different ordering on the list.  That is, explain to
us why 'svn -R' is more important than upgrading the FS schema, or
tweaking the property filenames, or ... you know, the other things
that we spent our time for this last release.

Also, I believe Karl is planning another batch-patch-processing day
soon.


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

Re: svn -R ... how to get it really into svn

Posted by Philip Martin <ph...@codematters.co.uk>.
solo turn <so...@yahoo.com> writes:

> and if i add the tree locking in svn_client_add:
>   SVN_ERR (svn_wc_adm_open (&adm_access, NULL, parent_path, TRUE,
> TRUE, pool));

I said set TREE_LOCK to RECURSIVE not TRUE, although RECURSIVE is
usually TRUE.

> it does not do any good, but seems to break other things.

That's not a good description of the problem.

> should it not look kind of (pseudocode):
> svn_wc_add()
> if (err == SVN_ERR_ENTRY_EXISTS) {
>   if directory
>     recurse_normal
>   }
> else
>   svn_wc_adm_retrieve()

No.

svn_wc_add is a rather complicated function.  It gets used during add,
checkout, update, copy and merge.  It adds new versioned files and
directories and also adds ancestor history to existing versioned
items.  I see I neglected to document OPTIONAL_ADM_ACCESS in svn_wc.h,
and it should be called PARENT_ACCESS :-(

When adding PATH, PARENT_ACCESS needs to be an access baton with a
write lock for the parent directory of PATH.  If PATH is a new
directory then when svn_wc_add completes it will have added an access
baton for PATH to the set that contains PARENT_ACCESS.

If you look at svn_client_add it only opens the parent directory, it
relies on svn_wc_add to open the access baton for each directory that
gets added and so builds up its tree of locks.  In your case when
svn_wc_add in add_dir_recursive fails there will be no access baton
for the existing directory so continuing won't work.  I would have
expected that causing svn_client_add to open all the existing
directories under parent would have solved this problem, you say it
doesn't but you don't say why.

You could use an svn_wc_adm_open in add_dir_recursive when svn_wc_add
fails, but I would want to know why setting TREE_LOCK to RECURSIVE in
svn_client_add didn't work.

       svn_wc_add
       if SVN_ERR_ENTRY_EXISTS
         svn_wc_adm_open
       else
         svn_wc_adm_retrieve

-- 
Philip Martin

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

Re: svn -R ... how to get it really into svn

Posted by cm...@collab.net.
solo turn <so...@yahoo.com> writes:

> i'm a little unsure about this locking stuff.

[...]

> $ svn -R *
>   svn: Working copy not locked
>   svn: directory not locked (a)

I'm a little unsure of this magical Subversion command `*' ? :-)


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

Re: svn -R ... how to get it really into svn

Posted by solo turn <so...@yahoo.com>.
i'm a little unsure about this locking stuff.

an example wc:
a/b
a/b/c.txt
a/b/d.txt
a/b/e
a/b/e/f.txt
b

newly added to the wc are: file d, directory e including contents.
the following happens:

$ svn -R *
  svn: Working copy not locked
  svn: directory not locked (a)

at what level it is expected to lock? lock tree a, lock the whole
repository, or lock just the newly added things (d,e)? and when does
the unlock happen?

the code:
  /* Add this directory to revision control. */
  err = svn_wc_add (dirname, adm_access,
                       NULL, SVN_INVALID_REVNUM,
                       notify_added, notify_baton, pool);

  if (err) {
    if (err->apr_err != SVN_ERR_ENTRY_EXISTS) {
      /* ignore "exists" errors, but nothing else */
      return err;
    }
    printf("%s\n",err->message); 
    /* reset err */
    svn_error_clear_all (err);
  }

  SVN_ERR (svn_wc_adm_retrieve (&dir_access, adm_access, dirname,
pool));


and if i add the tree locking in svn_client_add:
  SVN_ERR (svn_wc_adm_open (&adm_access, NULL, parent_path, TRUE,
TRUE, pool));
it does not do any good, but seems to break other things.


should it not look kind of (pseudocode):
svn_wc_add()
if (err == SVN_ERR_ENTRY_EXISTS) {
  if directory
    recurse_normal
  }
else
  svn_wc_adm_retrieve()


--- Philip Martin <ph...@codematters.co.uk> wrote:
> solo turn <so...@yahoo.com> writes:
> 
> > some time ago i proposed a patch to make svn -R recurse over all
> > directories, and does not stop if a directory already exists.
> > 
> > there were 2 small comments on it (like use .. != .. instead of
> !(..
> > == ..)
> > 
> > but for a reason i still don't understand, the patch made it to
> the
> > mailing list, noone objecting to it, but also noone seemed to
> care to
> > really apply it to the source tree.
> > 
> > what do i have to do to get a patch into the source tree?
> 
> Your original patch didn't have a log message. It helps to include
> one
> for two reasons: first, it helps with the patch review, and second
> a
> log message is required to commit, if you don't write it the person
> committing it has to do it.  Your patch broke the indentation of
> surrounding code.  As a reviewer pointed out, your patch didn't
> call
> svn_error_clear_all when suppressing an error.  Your patch didn't
> include a regression test.
> 
> Now, none of the above means that your patch won't get applied. 
> You
> do not have to supply a complete patch/log/regression test, however
> the more complete your patch, meaning less work for the person
> applying it, the more likely it is that your patch will be
> committed.
> 
> I don't think your patch works with the current code, although I
> haven't tried it so I may be wrong.  I think this case will fail
> 
> $ svn mkdir foo
> $ touch foo/zig.c
> $ svn add -R foo
> 
> I think the solution is to modify the svn_wc_adm_open call in
> svn_client_add, to set the TREE_LOCK argument to value of
> RECURSIVE.
> 
> Are you interested in updating your patch?
> 
> -- 
> Philip Martin
> 
>
---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
> 


__________________________________________________
Do You Yahoo!?
Yahoo! Finance - Get real-time stock quotes
http://finance.yahoo.com

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

Re: svn -R ... how to get it really into svn

Posted by Philip Martin <ph...@codematters.co.uk>.
solo turn <so...@yahoo.com> writes:

> some time ago i proposed a patch to make svn -R recurse over all
> directories, and does not stop if a directory already exists.
> 
> there were 2 small comments on it (like use .. != .. instead of !(..
> == ..)
> 
> but for a reason i still don't understand, the patch made it to the
> mailing list, noone objecting to it, but also noone seemed to care to
> really apply it to the source tree.
> 
> what do i have to do to get a patch into the source tree?

Your original patch didn't have a log message. It helps to include one
for two reasons: first, it helps with the patch review, and second a
log message is required to commit, if you don't write it the person
committing it has to do it.  Your patch broke the indentation of
surrounding code.  As a reviewer pointed out, your patch didn't call
svn_error_clear_all when suppressing an error.  Your patch didn't
include a regression test.

Now, none of the above means that your patch won't get applied.  You
do not have to supply a complete patch/log/regression test, however
the more complete your patch, meaning less work for the person
applying it, the more likely it is that your patch will be committed.

I don't think your patch works with the current code, although I
haven't tried it so I may be wrong.  I think this case will fail

$ svn mkdir foo
$ touch foo/zig.c
$ svn add -R foo

I think the solution is to modify the svn_wc_adm_open call in
svn_client_add, to set the TREE_LOCK argument to value of RECURSIVE.

Are you interested in updating your patch?

-- 
Philip Martin

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