You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by ep...@google.com on 2008/05/20 23:55:49 UTC
Yet more DAV bugs
Both ra-serf and neon misbehave when doing get_file on a
directory or get_dir on a file.
# ra-svn (right)
0 svn% ./get_dir-file svn://svnfe-test/r 84
doing get_dir2 on a file (welcome)
subversion/libsvn_fs_fs/dag.c:425: (apr_err=160016)
foo: Can't get entries of non-directory
# ra-neon (wrong)
1 svn% ./get_dir-file https://svnfe-test/r 84
doing get_dir2 on a file (welcome)
bad, should have died but got 0x8084970 0x807dfd0 (1 dirents)
d welcome
# ra-serf (different wrong)
0 svn% ./get_dir-file https://svnfe-test/r 84
doing get_dir2 on a file (welcome)
bad, should have died but got 0x80c81c0 0x80c80a0 (0 dirents)
# ra-svn
0 svn% ./get_file-dir svn://svnfe-test/r 84
doing get_file on a dir (changes)
subversion/libsvn_fs_fs/dag.c:984: (apr_err=160017)
foo: Attempted to get checksum of a *non*-file node
# ra-neon (wrong)
1 svn% ./get_file-dir https://svnfe-test/r 84
doing get_file on a dir (changes)
subversion/libsvn_ra_neon/util.c:549: (apr_err=175011)
foo: Repository moved permanently to 'https://svnfe-test/r/!svn/bc/119447/changes/'; please relocate
# ra-serf (different wrong)
1 svn% ./get_file-dir https://svnfe-test/r 84
doing get_file on a dir (changes)
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>301 Moved Permanently</title>
</head><body>
<h1>Moved Permanently</h1>
<p>The document has moved <a href="https://svnfe-test/r/changes/">here</a>.</p>
</body></html>
bad, should have died
Re: Yet more DAV bugs
Posted by ep...@google.com.
Karl Fogel <kf...@red-bean.com> writes:
> epg@google.com writes:
> > Both ra-serf and neon misbehave when doing get_file on a
> > directory or get_dir on a file.
>
> I saw you filed issue #3206 for this. IMHO, these should be fixed (and
> are probably easy to fix), but do not _need_ to be fixed for 1.5.0.
> Would you concur? They're bugs, but not showstopper bugs.
These bugs have been around since at least 2003, if not longer.
I should have said that before, my bad. Noted in issue.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Yet more DAV bugs
Posted by Karl Fogel <kf...@red-bean.com>.
epg@google.com writes:
> Both ra-serf and neon misbehave when doing get_file on a
> directory or get_dir on a file.
I saw you filed issue #3206 for this. IMHO, these should be fixed (and
are probably easy to fix), but do not _need_ to be fixed for 1.5.0.
Would you concur? They're bugs, but not showstopper bugs.
-Karl
> # ra-svn (right)
> 0 svn% ./get_dir-file svn://svnfe-test/r 84
> doing get_dir2 on a file (welcome)
> subversion/libsvn_fs_fs/dag.c:425: (apr_err=160016)
> foo: Can't get entries of non-directory
>
> # ra-neon (wrong)
> 1 svn% ./get_dir-file https://svnfe-test/r 84
> doing get_dir2 on a file (welcome)
> bad, should have died but got 0x8084970 0x807dfd0 (1 dirents)
> d welcome
>
> # ra-serf (different wrong)
> 0 svn% ./get_dir-file https://svnfe-test/r 84
> doing get_dir2 on a file (welcome)
> bad, should have died but got 0x80c81c0 0x80c80a0 (0 dirents)
>
> # ra-svn
> 0 svn% ./get_file-dir svn://svnfe-test/r 84
> doing get_file on a dir (changes)
> subversion/libsvn_fs_fs/dag.c:984: (apr_err=160017)
> foo: Attempted to get checksum of a *non*-file node
>
> # ra-neon (wrong)
> 1 svn% ./get_file-dir https://svnfe-test/r 84
> doing get_file on a dir (changes)
> subversion/libsvn_ra_neon/util.c:549: (apr_err=175011)
> foo: Repository moved permanently to 'https://svnfe-test/r/!svn/bc/119447/changes/'; please relocate
>
> # ra-serf (different wrong)
> 1 svn% ./get_file-dir https://svnfe-test/r 84
> doing get_file on a dir (changes)
> <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
> <html><head>
> <title>301 Moved Permanently</title>
> </head><body>
> <h1>Moved Permanently</h1>
> <p>The document has moved <a href="https://svnfe-test/r/changes/">here</a>.</p>
> </body></html>
> bad, should have died
>
>
> #include <errno.h>
> #include <stdio.h>
> #include <stdlib.h>
>
> /* for svn initialization */
> #include <apr_errno.h>
> #include <apr_general.h>
> #include <apr_strings.h>
> #include <svn_auth.h>
> #include <svn_cmdline.h>
> #include <svn_config.h>
> #include <svn_dso.h>
> #include <svn_nls.h>
> #include <svn_pools.h>
> /* for svn_ra_get_dir2 */
> #include <apr_hash.h>
> #include <svn_error.h>
> #include <svn_io.h>
> #include <svn_path.h>
> #include <svn_ra.h>
>
> static svn_error_t *
> open_tmp_file(apr_file_t **fp, void *baton, apr_pool_t *pool)
> {
> const char *base_fn, *base_path, *fn;
> SVN_ERR(svn_io_temp_dir(&base_fn, pool));
> base_path = svn_path_join(base_fn, "foo", pool);
> return svn_io_open_unique_file2(fp, &fn, base_path, ".tmp",
> svn_io_file_del_on_close, pool);
> }
>
> #define ERR(expr) \
> do { \
> svn_error_t *err = (expr); \
> if (err) { \
> fprintf(stderr, "%s:%ld: (apr_err=%d)\nfoo: %s\n", \
> err->file, err->line, err->apr_err, \
> err->message); \
> svn_error_clear(err); \
> svn_pool_destroy(pool); \
> return 1; \
> } \
> } while (0)
>
> int
> main(int argc, char *argv[])
> {
> apr_pool_t *pool;
> apr_hash_t *cfg_hash;
> long revision;
> svn_ra_callbacks2_t *callbacks;
>
> if (argc < 3 || (argv[1] && argv[1][0] == '-')) {
> fputs("usage: foo URL REVISION\n", stderr);
> if (argv[1] && argv[1][0] == '-') {
> return 0;
> }
> return 2;
> }
>
> errno = 0;
> revision = strtol(argv[2], NULL, 10);
> if (errno != 0) {
> fprintf(stderr, "foo: strtol(%s): ", argv[2]);
> perror("");
> return 3;
> }
>
> /* Initialize svn. */
> {
> apr_status_t apr_status;
> char buf[1024];
>
> if ((apr_status = apr_initialize()) != APR_SUCCESS) {
> apr_strerror(apr_status, buf, sizeof(buf) - 1);
> fprintf(stderr, "foo: cannot initialize APR: %s\n", buf);
> return 1;
> }
>
> svn_dso_initialize();
>
> if (atexit(apr_terminate) != 0) {
> fputs("atexit registration failed\n", stderr);
> return 1;
> }
>
> pool = svn_pool_create(NULL);
> svn_utf_initialize(pool);
> ERR(svn_nls_init());
>
> ERR(svn_config_get_config(&cfg_hash, NULL, pool));
>
> ERR(svn_ra_initialize(pool));
>
> ERR(svn_ra_create_callbacks(&callbacks, pool));
> callbacks->open_tmp_file = open_tmp_file;
> ERR(svn_cmdline_setup_auth_baton(&(callbacks->auth_baton),
> TRUE, /* non_interactive */
> NULL, /* username */
> NULL, /* password */
> NULL, /* config_dir */
> FALSE, /* no_auth_cache */
> apr_hash_get(cfg_hash,
> SVN_CONFIG_CATEGORY_CONFIG,
> APR_HASH_KEY_STRING),
> NULL, /* cancel_func */
> NULL, /* cancel_baton */
> pool));
> }
>
> /* Try something simple enough. */
> {
> svn_ra_session_t *ra;
> svn_revnum_t head;
> apr_hash_t *dirents;
> apr_hash_t *props;
> ERR(svn_ra_open2(&ra, argv[1], callbacks, NULL, cfg_hash, pool));
> ERR(svn_ra_get_latest_revnum(ra, &head, pool));
> puts("doing get_dir2 on a file (welcome)");
> ERR(svn_ra_get_dir2(ra, &dirents,
> NULL, /* fetched_rev */
> &props,
> "welcome", /* path */
> SVN_INVALID_REVNUM,
> 0, /* dirent_fields */
> pool));
> printf("bad, should have died but got %p %p (%d dirents)\n",
> props, dirents,
> apr_hash_count(dirents));
> {
> apr_hash_index_t *hi;
> const char *dirname;
> svn_dirent_t *dirent;
> for (hi = apr_hash_first(pool, dirents); hi; hi = apr_hash_next(hi)) {
> apr_hash_this(hi, (const void **)&dirname, NULL,
> (void **)&dirent);
> printf("%s %s\n", dirent->kind == svn_node_file ? "-" : "d",
> dirname);
> }
> }
> }
>
> svn_pool_destroy(pool);
> return 0;
> }
> #include <errno.h>
> #include <stdio.h>
> #include <stdlib.h>
>
> /* for svn initialization */
> #include <apr_errno.h>
> #include <apr_general.h>
> #include <apr_strings.h>
> #include <svn_auth.h>
> #include <svn_cmdline.h>
> #include <svn_config.h>
> #include <svn_dso.h>
> #include <svn_nls.h>
> #include <svn_pools.h>
> /* for svn_ra_get_dir2 */
> #include <apr_hash.h>
> #include <svn_error.h>
> #include <svn_io.h>
> #include <svn_path.h>
> #include <svn_ra.h>
>
> static svn_error_t *
> open_tmp_file(apr_file_t **fp, void *baton, apr_pool_t *pool)
> {
> const char *base_fn, *base_path, *fn;
> SVN_ERR(svn_io_temp_dir(&base_fn, pool));
> base_path = svn_path_join(base_fn, "foo", pool);
> return svn_io_open_unique_file2(fp, &fn, base_path, ".tmp",
> svn_io_file_del_on_close, pool);
> }
>
> #define ERR(expr) \
> do { \
> svn_error_t *err = (expr); \
> if (err) { \
> fprintf(stderr, "%s:%ld: (apr_err=%d)\nfoo: %s\n", \
> err->file, err->line, err->apr_err, \
> err->message); \
> svn_error_clear(err); \
> svn_pool_destroy(pool); \
> return 1; \
> } \
> } while (0)
>
> int
> main(int argc, char *argv[])
> {
> apr_pool_t *pool;
> apr_hash_t *cfg_hash;
> long revision;
> svn_ra_callbacks2_t *callbacks;
>
> if (argc < 3 || (argv[1] && argv[1][0] == '-')) {
> fputs("usage: foo URL REVISION\n", stderr);
> if (argv[1] && argv[1][0] == '-') {
> return 0;
> }
> return 2;
> }
>
> errno = 0;
> revision = strtol(argv[2], NULL, 10);
> if (errno != 0) {
> fprintf(stderr, "foo: strtol(%s): ", argv[2]);
> perror("");
> return 3;
> }
>
> /* Initialize svn. */
> {
> apr_status_t apr_status;
> char buf[1024];
>
> if ((apr_status = apr_initialize()) != APR_SUCCESS) {
> apr_strerror(apr_status, buf, sizeof(buf) - 1);
> fprintf(stderr, "foo: cannot initialize APR: %s\n", buf);
> return 1;
> }
>
> svn_dso_initialize();
>
> if (atexit(apr_terminate) != 0) {
> fputs("atexit registration failed\n", stderr);
> return 1;
> }
>
> pool = svn_pool_create(NULL);
> svn_utf_initialize(pool);
> ERR(svn_nls_init());
>
> ERR(svn_config_get_config(&cfg_hash, NULL, pool));
>
> ERR(svn_ra_initialize(pool));
>
> ERR(svn_ra_create_callbacks(&callbacks, pool));
> callbacks->open_tmp_file = open_tmp_file;
> ERR(svn_cmdline_setup_auth_baton(&(callbacks->auth_baton),
> TRUE, /* non_interactive */
> NULL, /* username */
> NULL, /* password */
> NULL, /* config_dir */
> FALSE, /* no_auth_cache */
> apr_hash_get(cfg_hash,
> SVN_CONFIG_CATEGORY_CONFIG,
> APR_HASH_KEY_STRING),
> NULL, /* cancel_func */
> NULL, /* cancel_baton */
> pool));
> }
>
> /* Try something simple enough. */
> {
> svn_ra_session_t *ra;
> svn_revnum_t head, fetched_rev;
> svn_stream_t *stdout_stream;
> apr_hash_t *props;
> ERR(svn_ra_open2(&ra, argv[1], callbacks, NULL, cfg_hash, pool));
> ERR(svn_ra_get_latest_revnum(ra, &head, pool));
> ERR(svn_stream_for_stdout(&stdout_stream, pool));
> puts("doing get_file on a dir (changes)");
> ERR(svn_ra_get_file(ra,
> "changes", /* path */
> SVN_INVALID_REVNUM,
> stdout_stream, &fetched_rev,
> &props,
> pool));
> puts("bad, should have died");
> }
>
> svn_pool_destroy(pool);
> return 0;
> }
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Yet more DAV bugs
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Wed, May 28, 2008 at 2:02 PM, C. Michael Pilato <cm...@collab.net> wrote:
> Actually, can you point out where this happens today? Using st00pid grep
ETIMEDOUT. Sorry. I remember coming across a number of cases when
writing ra_serf, but I don't have the cycles to find them again.
Looking at where ra_serf interacts with the property cache, it might
have to do with the update codepath... -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Yet more DAV bugs
Posted by "C. Michael Pilato" <cm...@collab.net>.
C. Michael Pilato wrote:
> Justin Erenkrantz wrote:
>> Of course I know that. The catch is that it's not just ne_propfind*
>> that triggers PROPFIND.
>
> Ah. Well, that would certainly put a kink in things.
Actually, can you point out where this happens today? Using st00pid grep
methods, I see that Neon appears to create PROPFIND requests only in one
interface (ne_propfind_create()). That's a public function, but is called
by ne_lock_discover(), ne_simple_propfind(), and ne_propnames(). Those are
all public, but are not called by other Neon functionality.
I can't find any code in libsvn_ra_neon that calls any of these four
interfaces. Did I overlook something?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Yet more DAV bugs
Posted by "C. Michael Pilato" <cm...@collab.net>.
Justin Erenkrantz wrote:
> On Wed, May 28, 2008 at 1:08 PM, C. Michael Pilato <cm...@collab.net> wrote:
>> You think? You know we don't use ne_propfind*, right?
>
> I don't know why you have such an antagonistic tone.
Eek. Antagonism not intended, dude. I just couldn't piece together why
this might be difficult code to write and why it would need to be done in
Neon itself unless we were using Neon's propfind interfaces (which we used
to use).
> Of course I know that. The catch is that it's not just ne_propfind*
> that triggers PROPFIND.
Ah. Well, that would certainly put a kink in things.
> You can implement a cache, but I don't expect it to be an hour's worth
> of work and get 100% efficiency. You might be able to cut out a
> reasonably good chunk, but I don't expect that you will catch all of
> the uses - to do so, you'll very likely need to dive into neon itself.
>
> I fail to see the point if ra_serf already offers the functionality
> anyway...but I have no desire in stopping you if you wish to go down
> that rabbit hole.
Okey dokey. (I'm not "wishing" to go anywhere on this, though -- just
didn't want others who might to give up before they even started due to the
belief that Neon is fundamentally flawed in some way.)
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Yet more DAV bugs
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Wed, May 28, 2008 at 1:08 PM, C. Michael Pilato <cm...@collab.net> wrote:
> You think? You know we don't use ne_propfind*, right?
I don't know why you have such an antagonistic tone.
Of course I know that. The catch is that it's not just ne_propfind*
that triggers PROPFIND.
You can implement a cache, but I don't expect it to be an hour's worth
of work and get 100% efficiency. You might be able to cut out a
reasonably good chunk, but I don't expect that you will catch all of
the uses - to do so, you'll very likely need to dive into neon itself.
I fail to see the point if ra_serf already offers the functionality
anyway...but I have no desire in stopping you if you wish to go down
that rabbit hole. -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Yet more DAV bugs
Posted by "C. Michael Pilato" <cm...@collab.net>.
Justin Erenkrantz wrote:
> On Wed, May 28, 2008 at 11:57 AM, C. Michael Pilato <cm...@collab.net> wrote:
>> I assume you mean, "the drawback and penalty of using ra_neon today".
>> ra_serf doesn't win here on any particular design merits -- it just
>> implements a propfind cache of the same variety that ra_neon could (and
>> should), too, right?
>
> I don't think it'd be feasible to implement a similar cache at the
> ra_neon layer as Neon hides too much of the underlying WebDAV
> interactions to just plug in a true property cache. So, it'd only be
> partially effective at best and be a nightmare to hook in, IMO. You'd
> probably need to rewrite large chunks of neon to do it right. --
You think? You know we don't use ne_propfind*, right? We've implemented
our own PROPFIND request parsing. So, is there something more to do than
just choosing the right keys for a cache of the results? We'd need
something like the URL, depth, and requested_props (or maybe we just always
request allprops and trust that the cache will offset the costs).
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Yet more DAV bugs
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Wed, May 28, 2008 at 11:57 AM, C. Michael Pilato <cm...@collab.net> wrote:
> I assume you mean, "the drawback and penalty of using ra_neon today".
> ra_serf doesn't win here on any particular design merits -- it just
> implements a propfind cache of the same variety that ra_neon could (and
> should), too, right?
I don't think it'd be feasible to implement a similar cache at the
ra_neon layer as Neon hides too much of the underlying WebDAV
interactions to just plug in a true property cache. So, it'd only be
partially effective at best and be a nightmare to hook in, IMO. You'd
probably need to rewrite large chunks of neon to do it right. --
justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Yet more DAV bugs
Posted by "C. Michael Pilato" <cm...@collab.net>.
Justin Erenkrantz wrote:
> On Fri, May 23, 2008 at 6:35 AM, <km...@rockwellcollins.com> wrote:
>> Haven't had time to look into it any more, and it might just be a neon
>> issue anyway... (This was with 1.4.6, but I'm pretty sure 1.5 rc5 with
>> a newer neon was the same)
>
> This is the drawback and penalty of using ra_neon. ra_serf is likely
> a *lot* smarter about not repeating the same requests (esp. PROPFIND).
> -- justin
I assume you mean, "the drawback and penalty of using ra_neon today".
ra_serf doesn't win here on any particular design merits -- it just
implements a propfind cache of the same variety that ra_neon could (and
should), too, right?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Yet more DAV bugs
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Fri, May 23, 2008 at 6:35 AM, <km...@rockwellcollins.com> wrote:
> Haven't had time to look into it any more, and it might just be a neon
> issue anyway... (This was with 1.4.6, but I'm pretty sure 1.5 rc5 with
> a newer neon was the same)
This is the drawback and penalty of using ra_neon. ra_serf is likely
a *lot* smarter about not repeating the same requests (esp. PROPFIND).
-- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Yet more DAV bugs
Posted by km...@rockwellcollins.com.
epg@google.com wrote on 05/22/2008 04:16:12 PM:
> Lieven Govaerts <sv...@mobsol.be> writes:
>
> > I'm missing something here. You're asking an API to do something that
> > it's not meant for, expecting to get an error which clearly doesn't
make
> > sense either? Why don't you use the correct API instead?
>
> Calling opendir(3) on a plain file is using "the correct API".
> So is calling svn_ra_get_file on a directory. Try-and-fail is
> better than test-first, but I'm not going to argue that here.
> So, here is what I'm doing:
>
> - I have a path which may be a file or a directory
> - I need to get its properties and contents if it's a file, or
> just its properties if it's a dir
> - Most paths are files
> => try:
> svn.ra.get_file(...)
> except NOT_FILE:
> svn.ra.get_dir(2(...)
>
> It's just more complicated than it needs to be because I also
> have to support DAV.
>
> That's the story of my life right there: "It's harder than it
> should be because of DAV."
>
> So, in most cases (files), I issue one get_file and am done; in
> the (rare) case of a modified directory, I issue two requests:
> get_file and get_dir2. Issuing a check_path on every single path
> just to find this rare case would be stupid[1].
>
> > > and we're using neon; for
> > > serf, which proceeds to write some HTML error garbage into my
> > > file,
> > >
> >
> > Well, the fact that ra_serf dumps the XML output is not correct
either.
> > If that error comes from the server, I'd at least expect the same
result
> > as you get for ra_neon now. Fixed in r31374.
> >
> > > well, I just have to declare serf unsupported.
> >
> > Sorry, no, can't have that :)
>
> Well, that's where we (gvn) are for other reasons anyway, as
> we've discussed on IRC.
>
> [1] Note that svn does this all the time, sigh. Check out what
> we do for 'svn cat':
>
> get-latest-rev
> reparent /welcome
> check-path /welcome@119767
> get-file /welcome r119767 props
> get-file /welcome r119767 text
>
> All I should see in the log for svn cat is this:
>
> get-file /welcome r119767 text props
>
> Sure, svn cat, who cares? I wonder how many other places we
> have crazy things like this...
This reminds me, I was using an http debug proxy and noticed svn repeated
the same three http requests 3 times in a row for "svn ls". (Didn't check
other commands yet.)
Same binary data in, same binary answer.
For a directory listing:
1 PROPFIND <path> props(version-controlled-configuration
baseline-relative-path
repository-uuid)
2 PROPFIND [version-controlled-configuration] props(checked-in)
3 PROPFIND [checked-in] props(baseline-collection
version-name)
4 *** Repeat 1 *** PROPFIND <path>
props(version-controlled-configuration
baseline-relative-path
repository-uuid)
5 *** Repeat 2 *** PROPFIND [version-controlled-configuration]
props(checked-in)
6 *** Repeat 3 *** PROPFIND [checked-in] props(baseline-collection
version-name)
7 *** Repeat 1 *** PROPFIND <path>
props(version-controlled-configuration
baseline-relative-path
repository-uuid)
8 *** Repeat 2 *** PROPFIND [version-controlled-configuration]
props(checked-in)
9 *** Repeat 3 *** PROPFIND [checked-in] props(baseline-collection
version-name)
10 *** Repeat 1 *** PROPFIND <path>
props(version-controlled-configuration
baseline-relative-path
repository-uuid)
11 *** Repeat 2 *** PROPFIND [version-controlled-configuration]
props(checked-in)
12 PROPFIND [baseline-collection] props(allprop)
13 *** Repeat 1 *** PROPFIND <path>
props(version-controlled-configuration
baseline-relative-path
repository-uuid)
14 *** Repeat 2 *** PROPFIND [version-controlled-configuration]
props(checked-in)
15 PROPFIND [baseline-collection] props(deadprop-count)
16 PROPFIND [baseline-collection] props(resourcetype)
17 if recursive repeat 13-16 for each directory
On a slow connection (say 400ms ping time, this might add a few seconds
to each conversation)
Haven't had time to look into it any more, and it might just be a neon
issue anyway... (This was with 1.4.6, but I'm pretty sure 1.5 rc5 with
a newer neon was the same)
Kevin R.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Yet more DAV bugs
Posted by ep...@google.com.
Lieven Govaerts <sv...@mobsol.be> writes:
> I'm missing something here. You're asking an API to do something that
> it's not meant for, expecting to get an error which clearly doesn't make
> sense either? Why don't you use the correct API instead?
Calling opendir(3) on a plain file is using "the correct API".
So is calling svn_ra_get_file on a directory. Try-and-fail is
better than test-first, but I'm not going to argue that here.
So, here is what I'm doing:
- I have a path which may be a file or a directory
- I need to get its properties and contents if it's a file, or
just its properties if it's a dir
- Most paths are files
=> try:
svn.ra.get_file(...)
except NOT_FILE:
svn.ra.get_dir(2(...)
It's just more complicated than it needs to be because I also
have to support DAV.
That's the story of my life right there: "It's harder than it
should be because of DAV."
So, in most cases (files), I issue one get_file and am done; in
the (rare) case of a modified directory, I issue two requests:
get_file and get_dir2. Issuing a check_path on every single path
just to find this rare case would be stupid[1].
> > and we're using neon; for
> > serf, which proceeds to write some HTML error garbage into my
> > file,
> >
>
> Well, the fact that ra_serf dumps the XML output is not correct either.
> If that error comes from the server, I'd at least expect the same result
> as you get for ra_neon now. Fixed in r31374.
>
> > well, I just have to declare serf unsupported.
>
> Sorry, no, can't have that :)
Well, that's where we (gvn) are for other reasons anyway, as
we've discussed on IRC.
[1] Note that svn does this all the time, sigh. Check out what
we do for 'svn cat':
get-latest-rev
reparent /welcome
check-path /welcome@119767
get-file /welcome r119767 props
get-file /welcome r119767 text
All I should see in the log for svn cat is this:
get-file /welcome r119767 text props
Sure, svn cat, who cares? I wonder how many other places we
have crazy things like this...
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Yet more DAV bugs
Posted by Lieven Govaerts <sv...@mobsol.be>.
epg@google.com wrote:
> Thanks for looking into this.
>
> Lieven Govaerts <sv...@mobsol.be> writes:
>
>> I've tested get_file_dir over ra_serf, and found that - unlike svnserve
>> - mod_dav_svn doesn't care if you're asking directory information
>> (PROPFIND allprop Depth:1) on a file. Which means that ra_serf doesn't
>> know anything was wrong.
>
> I can understand some people want to support things like this for
> compatibility for MICROS~1 Web folders or whatever. But we
> mustn't allow such side features to compromise svn itself.
>
>> Now we can correct this by adding an extra check to see of the directory
>> is really a directory. AFAIC the extra cost of the PROPFIND request to
>> the server is not worth it. Also, there's a nice workaround: if you
>> don't know if you're handling a file or a directory, use svn_ra_check_path.
>
> Extra round trips are no good, whether above or below the svn_ra
> hood.
> Right now, I'm issuing get_files with stupid code to try to figure out if it was a directory
I'm missing something here. You're asking an API to do something that
it's not meant for, expecting to get an error which clearly doesn't make
sense either? Why don't you use the correct API instead?
> and we're using neon; for
> serf, which proceeds to write some HTML error garbage into my
> file,
>
Well, the fact that ra_serf dumps the XML output is not correct either.
If that error comes from the server, I'd at least expect the same result
as you get for ra_neon now. Fixed in r31374.
> well, I just have to declare serf unsupported.
Sorry, no, can't have that :)
Lieven
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Yet more DAV bugs
Posted by ep...@google.com.
Thanks for looking into this.
Lieven Govaerts <sv...@mobsol.be> writes:
> I've tested get_file_dir over ra_serf, and found that - unlike svnserve
> - mod_dav_svn doesn't care if you're asking directory information
> (PROPFIND allprop Depth:1) on a file. Which means that ra_serf doesn't
> know anything was wrong.
I can understand some people want to support things like this for
compatibility for MICROS~1 Web folders or whatever. But we
mustn't allow such side features to compromise svn itself.
> Now we can correct this by adding an extra check to see of the directory
> is really a directory. AFAIC the extra cost of the PROPFIND request to
> the server is not worth it. Also, there's a nice workaround: if you
> don't know if you're handling a file or a directory, use svn_ra_check_path.
Extra round trips are no good, whether above or below the svn_ra
hood. Right now, I'm issuing get_files with stupid code to try
to figure out if it was a directory and we're using neon; for
serf, which proceeds to write some HTML error garbage into my
file, well, I just have to declare serf unsupported.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Yet more DAV bugs
Posted by Lieven Govaerts <sv...@mobsol.be>.
epg@google.com wrote:
> Both ra-serf and neon misbehave when doing get_file on a
> directory or get_dir on a file.
>
> # ra-svn (right)
> 0 svn% ./get_dir-file svn://svnfe-test/r 84
> doing get_dir2 on a file (welcome)
> subversion/libsvn_fs_fs/dag.c:425: (apr_err=160016)
> foo: Can't get entries of non-directory
>
> # ra-neon (wrong)
> 1 svn% ./get_dir-file https://svnfe-test/r 84
> doing get_dir2 on a file (welcome)
> bad, should have died but got 0x8084970 0x807dfd0 (1 dirents)
> d welcome
>
> # ra-serf (different wrong)
> 0 svn% ./get_dir-file https://svnfe-test/r 84
> doing get_dir2 on a file (welcome)
> bad, should have died but got 0x80c81c0 0x80c80a0 (0 dirents)
>
> # ra-svn
> 0 svn% ./get_file-dir svn://svnfe-test/r 84
> doing get_file on a dir (changes)
> subversion/libsvn_fs_fs/dag.c:984: (apr_err=160017)
> foo: Attempted to get checksum of a *non*-file node
>
> # ra-neon (wrong)
> 1 svn% ./get_file-dir https://svnfe-test/r 84
> doing get_file on a dir (changes)
> subversion/libsvn_ra_neon/util.c:549: (apr_err=175011)
> foo: Repository moved permanently to 'https://svnfe-test/r/!svn/bc/119447/changes/'; please relocate
>
> # ra-serf (different wrong)
> 1 svn% ./get_file-dir https://svnfe-test/r 84
> doing get_file on a dir (changes)
> <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
> <html><head>
> <title>301 Moved Permanently</title>
> </head><body>
> <h1>Moved Permanently</h1>
> <p>The document has moved <a href="https://svnfe-test/r/changes/">here</a>.</p>
> </body></html>
> bad, should have died
I've tested get_file_dir over ra_serf, and found that - unlike svnserve
- mod_dav_svn doesn't care if you're asking directory information
(PROPFIND allprop Depth:1) on a file. Which means that ra_serf doesn't
know anything was wrong.
Now we can correct this by adding an extra check to see of the directory
is really a directory. AFAIC the extra cost of the PROPFIND request to
the server is not worth it. Also, there's a nice workaround: if you
don't know if you're handling a file or a directory, use svn_ra_check_path.
Lieven
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org