You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Joe Swatosh <jo...@gmail.com> on 2007/09/15 23:40:51 UTC

Re: svn commit: r26607 - in trunk/subversion: include libsvn_client libsvn_wc svn tests/cmdline

Hi Karl,

On 9/15/07, kfogel@tigris.org <kf...@tigris.org> wrote:
> Author: kfogel
> Date: Sat Sep 15 00:54:39 2007
> New Revision: 26607
>
> Log:
> Add --depth support to 'svn add' and 'svn import', as part of issue #2847.
> This is also part of issue #2284 ("TODO(sd)" cleanup).
>
> * subversion/include/svn_client.h
>  (svn_client_add4, svn_client_import3): Take depth argument, instead of
>    recursive or nonrecursive.
>  (svn_client_add3, svn_client_import2): Doc adjustments for above.
>
> * subversion/libsvn_client/add.c
>  (add): Take depth instead of recursive.
>  (add_dir_recursive): Take depth instead of recursive, use it to
>    descend appropriately for the given depth.  Simplify an error message.
>  (svn_client_add4): Take depth instead of recursive.  Also, clean up
>    some code by re-using the path parameter and thereby allowing
>    other things to be unduplicated.
>  (svn_client_add3): Convert recursive to depth.
>  (svn_client__make_local_parents): Pass empty depth to svn_client_add4.
>
> * subversion/libsvn_client/commit.c
>  (svn_client_import3): Take depth instead of nonrecursive, pass it along
>    to import.
>  (import): Take depth, pass it along.
>  (import_dir): Take depth instead of nonrecursive, use it to descend
>    appropriately for the given depth.
>  (svn_client_import2): Convert nonrecursive to depth.
>
> * subversion/svn/add-cmd.c
>  (svn_cl__add): Pass depth directly to svn_client_add4.
>
> * subversion/svn/import-cmd.c
>  (svn_cl__import): Pass depth directly to svn_client_import3.
>
> * subversion/libsvn_wc/adm_ops.c
>  (svn_wc_add2): Convert "TODO(sd)" comment to explanatory comment.
>
> * subversion/tests/cmdline/commit_tests.py
>  (nested_dir_replacements, commit_uri_unsafe, commit_nonrecursive):
>    Use --depth=empty instead of -N, for adds and imports.
>



> Modified: trunk/subversion/libsvn_client/add.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/add.c?pathrev=26607&r1=26606&r2=26607
> ==============================================================================
> --- trunk/subversion/libsvn_client/add.c        (original)
> +++ trunk/subversion/libsvn_client/add.c        Sat Sep 15 00:54:39 2007


> @@ -549,7 +555,8 @@
>                 svn_client_ctx_t *ctx,
>                 apr_pool_t *pool)
>  {
> -  return svn_client_add4(path, recursive, force, no_ignore, FALSE, ctx,
> +  return svn_client_add4(path, SVN_DEPTH_FROM_RECURSE(recursive),
> +                         force, no_ignore, FALSE, ctx,
>                          pool);
>  }
>

I've been trying to catch the Ruby bindings up with HEAD today, but when I
apply r26607 I get two test failures that I can't explain.  I think if you can
help me understand this one, the other will fall into place.  Or perhaps the
quoted bit above is causing the regression?  The Ruby bindings are currently
wrapping svn_client_add3.

  def test_add_not_recurse
    log = "sample log"
    file = "hello.txt"
    src = "Hello"
    dir = "dir"
    dir_path = File.join(@wc_path, dir)
    path = File.join(dir_path, file)
    uri = "#{@repos_uri}/#{dir}/#{file}"

    ctx = make_context(log)
    FileUtils.mkdir(dir_path)
    File.open(path, "w") {|f| f.print(src)}

# Boilerplate up to here.  Now add the directory containing a file created
# above, without recursing.

    ctx.add(dir_path, false)
    ctx.commit(@wc_path)

# This assertion fails.  We are now finding the path in the repo where we
# before we didn't.

    assert_raise(Svn::Error::FS_NOT_FOUND) do
      ctx.cat(uri)
    end
  end

I checked the definition of SVN_DEPTH_FROM_RECURSE, and since it returns
svn_depth_files, I'm wondering if that is the correct behavior for add?

I hope I'm at least half coherent.

--
Joe

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

Re: svn commit: r26607 - in trunk/subversion: include libsvn_client libsvn_wc svn tests/cmdline

Posted by Joe Swatosh <jo...@gmail.com>.
On 9/15/07, Karl Fogel <kf...@red-bean.com> wrote:
> "Joe Swatosh" <jo...@gmail.com> writes:

>
> Yup, that's the intended behavior now, and the r26607 transcript looks

> > Furthermore, assuming this was intentional, I started playing with

Obviously I guessed...

>
> Ah, that's a bug -- it shouldn't complain, it should just do the right
> thing.  I've filed issue #2931, will fix.  Thanks!
>
Glad something worthwhile came of you explaining to me.
--
Joe

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

Re: svn commit: r26607 - in trunk/subversion: include libsvn_client libsvn_wc svn tests/cmdline

Posted by Karl Fogel <kf...@red-bean.com>.
"Joe Swatosh" <jo...@gmail.com> writes:
> So r26607 adds the directory, but not the file, r26607 adds the directory and
> the file.  Basically, "svn add -N" meant add the directory only before r26607
> and now it appears to mean add the directory and any files in it, but no
> subdirectories.  As long as this is intentional, no problem.  If it is not
> intentional....

Yup, that's the intended behavior now, and the r26607 transcript looks
correct to me (and the r26606 transcript looks as I would have
expected, too -- the old -N behavior).

> Furthermore, assuming this was intentional, I started playing with
> this to see about modifying the test.  Just added an intermediate
> directory, and it worked as I expected, but I was suprised by the
> message "Can't detect MIME type of non-file" in the "svn add":
>
> D:\SVN\26607>set MD=D:/SVN/26607
>
> D:\SVN\26607>svnadmin create repos
>
> D:\SVN\26607>svn mkdir -m "" file:///D:/SVN/26607/repos/wc
>
> Committed revision 1.
>
> D:\SVN\26607>svn co file:///D:/SVN/26607/repos/wc wc
> Checked out revision 1.
>
> D:\SVN\26607>md wc\dir
>
> D:\SVN\26607>md wc\dir\dir
>
> D:\SVN\26607>echo content  1>wc\dir\dir\file.txt
>
> D:\SVN\26607>svn add -N wc\dir
> A         wc\dir
> svn: Can't detect MIME type of non-file 'wc\dir\dir'

Ah, that's a bug -- it shouldn't complain, it should just do the right
thing.  I've filed issue #2931, will fix.  Thanks!

-Karl

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

Re: svn commit: r26607 - in trunk/subversion: include libsvn_client libsvn_wc svn tests/cmdline

Posted by Joe Swatosh <jo...@gmail.com>.
Hi Karl

On 9/15/07, Joe Swatosh <jo...@gmail.com> wrote:
> Karl,
>
> On 9/15/07, Karl Fogel <kf...@red-bean.com> wrote:
> > "Joe Swatosh" <jo...@gmail.com> writes:


>
> command line transcript approximating the test actions (stupid typos edited)

Okay, only most of the stupid typos.  :-)  Corrected below.

> r26606
D:\SVN\26606>svnadmin create repos

D:\SVN\26606>svn mkdir -m "" file:///D:/SVN/26606/repos/wc

Committed revision 1.

D:\SVN\26606>svn co file:///D:/SVN/26606/repos/wc wc
Checked out revision 1.

D:\SVN\26606>md wc\dir

D:\SVN\26606>echo content  1>wc\dir\file.txt

D:\SVN\26606>svn add -N wc\dir
A         wc\dir

D:\SVN\26606>svn commit -m "" wc
Adding         wc\dir

Committed revision 2.

D:\SVN\26606>svn ls file:///D:/SVN/26606/repos/wc/dir

> r26607
D:\SVN\26607>svnadmin create repos

D:\SVN\26607>svn mkdir -m "" file:///D:/SVN/26607/repos/wc

Committed revision 1.

D:\SVN\26607>svn co file:///D:/SVN/26607/repos/wc wc
Checked out revision 1.

D:\SVN\26607>md wc\dir

D:\SVN\26607>echo content  1>wc\dir\file.txt

D:\SVN\26607>svn add -N wc\dir
A         wc\dir
A         wc\dir\file.txt

D:\SVN\26607>svn commit -m "" wc
Adding         wc\dir
Adding         wc\dir\file.txt
Transmitting file data .
Committed revision 2.

D:\SVN\26607>svn ls file:///D:/SVN/26607/repos/wc/dir
file.txt

> So r26607 adds the directory, but not the file, r26607 adds the directory and
> the file.  Basically, "svn add -N" meant add the directory only before r26607
> and now it appears to mean add the directory and any files in it, but no
> subdirectories.  As long as this is intentional, no problem.  If it is not
> intentional....
>

Furthermore, assuming this was intentional, I started playing with
this to see about modifying the test.  Just added an intermediate
directory, and it worked as I expected, but I was suprised by the
message "Can't detect MIME type of non-file" in the "svn add":

D:\SVN\26607>set MD=D:/SVN/26607

D:\SVN\26607>svnadmin create repos

D:\SVN\26607>svn mkdir -m "" file:///D:/SVN/26607/repos/wc

Committed revision 1.

D:\SVN\26607>svn co file:///D:/SVN/26607/repos/wc wc
Checked out revision 1.

D:\SVN\26607>md wc\dir

D:\SVN\26607>md wc\dir\dir

D:\SVN\26607>echo content  1>wc\dir\dir\file.txt

D:\SVN\26607>svn add -N wc\dir
A         wc\dir
svn: Can't detect MIME type of non-file 'wc\dir\dir'

D:\SVN\26607>svn commit -m "" wc
Adding         wc\dir

Committed revision 2.

D:\SVN\26607>svn ls file:///D:/SVN/26607/repos/wc/dir/dir
svn: URL 'file:///D:/SVN/26607/repos/wc/dir/dir' non-existent in that revision


--
Joe

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

Re: svn commit: r26607 - in trunk/subversion: include libsvn_client libsvn_wc svn tests/cmdline

Posted by Joe Swatosh <jo...@gmail.com>.
Karl,

On 9/15/07, Karl Fogel <kf...@red-bean.com> wrote:
> "Joe Swatosh" <jo...@gmail.com> writes:
> > I've been trying to catch the Ruby bindings up with HEAD today, but when I
> > apply r26607 I get two test failures that I can't explain.  I think if you can
> > help me understand this one, the other will fall into place.  Or perhaps the
> > quoted bit above is causing the regression?  The Ruby bindings are currently
> > wrapping svn_client_add3.
> >
> >   def test_add_not_recurse
> >     log = "sample log"
> >     file = "hello.txt"
> >     src = "Hello"
> >     dir = "dir"
> >     dir_path = File.join(@wc_path, dir)
> >     path = File.join(dir_path, file)
> >     uri = "#{@repos_uri}/#{dir}/#{file}"
> >
> >     ctx = make_context(log)
> >     FileUtils.mkdir(dir_path)
> >     File.open(path, "w") {|f| f.print(src)}
> >
> > # Boilerplate up to here.  Now add the directory containing a file created
> > # above, without recursing.
> >
> >     ctx.add(dir_path, false)
> >     ctx.commit(@wc_path)
>
> Hmm, which depth does "without recursing" imply here?
>
> > # This assertion fails.  We are now finding the path in the repo where we
> > # before we didn't.
> >
> >     assert_raise(Svn::Error::FS_NOT_FOUND) do
> >       ctx.cat(uri)
> >     end
> >   end
> >
> > I checked the definition of SVN_DEPTH_FROM_RECURSE, and since it returns
> > svn_depth_files, I'm wondering if that is the correct behavior for add?
> >
> > I hope I'm at least half coherent.
>
> I confess I don't understand the recipe.  Could you spell it out,
> showing exactly what tree is on the client side, exactly what command
> is run (from where and with what arguments), exactly what you expect
> to see in the repository before and after, and exactly what you
> actually see?
>
> Ideally, the bindings would wrap svn_client_add4() and prefer it to
> svn_client_add3(), of course.  However, as long as they're using add3,
> the conversion of recurse=false to svn_depth_files is correct.  That
> may have resulted in the behavior of 'svn add' changing slightly;
> we've accepted that mapping -N to --depth results in some subtle
> behavior changes for some commands.
>


Thanks for the prompt reply.  I was just about to reply to myself to say that
of course the test could be wrong.  The important bit to me was that there
seemed to be a change of behavior.  If that means that the behavior of 'svn
add' has changed slightly then that's fine.  Again, I was most concerned about
a regression.  <later>  sorry for the delay.  My night to make dinner.
</later>

command line transcript approximating the test actions (stupid typos edited)
r26606
        D:\SVN\26606>svnadmin create repos

        D:\SVN\26606>svn mkdir -m "" file:///d:/SVN/26606/repos/wc

        Committed revision 1.

        D:\SVN\26606>svn co file:///d:/SVN/26606/repos/wc wc
        Checked out revision 1.

        D:\SVN\26606>md wc\dir

        D:\SVN\26606>echo content > wc\dir\file.txt

        D:\SVN\26606>svn add -N wc\dir
        A         wc\dir

        D:\SVN\26606>svn commit -m "" wc
        Adding         wc\dir

        Committed revision 2.

        D:\SVN\26606>svn ls file:///d:/SVN/26607/repos/wc/dir
        file.txt

        D:\SVN\26606>svn ls file:///d:/SVN/26606/repos/wc/dir

r26607
        D:\SVN\26607>svnadmin create repos

        D:\SVN\26607>svn mkdir -m "" file:///d:/SVN/26607/repos/wc

        Committed revision 1.

        D:\SVN\26607>svn co file:///d:/SVN/26607/repos/wc wc
        Checked out revision 1.

        D:\SVN\26607>md wc\dir

        D:\SVN\26607>echo content > wc\dir\file.txt

        D:\SVN\26607>svn add -N wc\dir
        A         wc\dir
        A         wc\dir\file.txt

        D:\SVN\26607>svn commit -m "" wc
        Adding         wc\dir
        Adding         wc\dir\file.txt
        Transmitting file data .
        Committed revision 2.

        D:\SVN\26607>svn ls file:///d:/SVN/26607/repos/wc/dir
        file.txt

So r26607 adds the directory, but not the file, r26607 adds the directory and
the file.  Basically, "svn add -N" meant add the directory only before r26607
and now it appears to mean add the directory and any files in it, but no
subdirectories.  As long as this is intentional, no problem.  If it is not
intentional....

--
Joe

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

Re: svn commit: r26607 - in trunk/subversion: include libsvn_client libsvn_wc svn tests/cmdline

Posted by Karl Fogel <kf...@red-bean.com>.
"Joe Swatosh" <jo...@gmail.com> writes:
> I've been trying to catch the Ruby bindings up with HEAD today, but when I
> apply r26607 I get two test failures that I can't explain.  I think if you can
> help me understand this one, the other will fall into place.  Or perhaps the
> quoted bit above is causing the regression?  The Ruby bindings are currently
> wrapping svn_client_add3.
>
>   def test_add_not_recurse
>     log = "sample log"
>     file = "hello.txt"
>     src = "Hello"
>     dir = "dir"
>     dir_path = File.join(@wc_path, dir)
>     path = File.join(dir_path, file)
>     uri = "#{@repos_uri}/#{dir}/#{file}"
>
>     ctx = make_context(log)
>     FileUtils.mkdir(dir_path)
>     File.open(path, "w") {|f| f.print(src)}
>
> # Boilerplate up to here.  Now add the directory containing a file created
> # above, without recursing.
>
>     ctx.add(dir_path, false)
>     ctx.commit(@wc_path)

Hmm, which depth does "without recursing" imply here?

> # This assertion fails.  We are now finding the path in the repo where we
> # before we didn't.
>
>     assert_raise(Svn::Error::FS_NOT_FOUND) do
>       ctx.cat(uri)
>     end
>   end
>
> I checked the definition of SVN_DEPTH_FROM_RECURSE, and since it returns
> svn_depth_files, I'm wondering if that is the correct behavior for add?
>
> I hope I'm at least half coherent.

I confess I don't understand the recipe.  Could you spell it out,
showing exactly what tree is on the client side, exactly what command
is run (from where and with what arguments), exactly what you expect
to see in the repository before and after, and exactly what you
actually see?

Ideally, the bindings would wrap svn_client_add4() and prefer it to
svn_client_add3(), of course.  However, as long as they're using add3,
the conversion of recurse=false to svn_depth_files is correct.  That
may have resulted in the behavior of 'svn add' changing slightly;
we've accepted that mapping -N to --depth results in some subtle
behavior changes for some commands.

-Karl

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