Roland McGrath | 26 Sep 14:09
Favicon

[PATCH] fix use of hostname from parsed uri

This looks like an obvious fix.  Without this, the local variable 'host' is
set and then never used.  For me, this seems to make:

	mtn sync mtn://server/

work happily.  By making that:

	mtn sync mtn://server/foobar

I think this is already all that was needed for usher to get really useful
for sensible multi-db hosting.  So I hope this fix goes in ASAP!

(Beyond this, it seems like there ought to be some actual checking on the
uri scheme, instead of treating any scheme name not handle by the lua hook
as meaning netsync.)

Thanks,
Roland

#
# old_revision [6341444c06293469f66f1c431063282004c5be9d]
#
# patch "netsync.cc"
#  from [faa9a58eb3dd3f07ec771969251ba8c5c8f2d91d]
#    to [500fec0e1c25b6ed2e1cf03ee6bd25039e1a1e01]
#
============================================================
--- netsync.cc	faa9a58eb3dd3f07ec771969251ba8c5c8f2d91d
+++ netsync.cc	500fec0e1c25b6ed2e1cf03ee6bd25039e1a1e01
@@ -2461,8 +2461,7 @@ build_stream_to_server(options & opts, l
(Continue reading)

Zack Weinberg | 26 Sep 18:40

Re: [PATCH] fix use of hostname from parsed uri

On Fri, Sep 26, 2008 at 5:09 AM, Roland McGrath <roland <at> redhat.com> wrote:
> This looks like an obvious fix.  Without this, the local variable 'host' is
> set and then never used.  For me, this seems to make:
>
>        mtn sync mtn://server/
>
> work happily.

Ok ...

>  By making that:
>
>        mtn sync mtn://server/foobar

... huh?

Is it possible you could write a test case or two for this bug?

zw
Roland McGrath | 27 Sep 01:41
Favicon

Re: [PATCH] fix use of hostname from parsed uri

> >        mtn sync mtn://server/foobar
> 
> ... huh?

In every other networkable SCM today, scm://server/name URIs can be used in
straightforward ways with a single server host to select entirely separate
databases (with distinct local auth controls) based what "name" the client
specified.

AIUI, the usher commands in the netsync protocol send the whole original
command line string (here "mtn://server/foobar") to the server, so usher
can use that however it likes to disambiguate what database to talk to.
The fix that allows "mtn://server" to work also allows "mtn://server/name"
so that the rest of the story can be done quite nicely just with more minor
hacking on usher.

> Is it possible you could write a test case or two for this bug?

The test is any push/pull/sync operation using "mtn://server" instead of
"server" (or "mtn://server:port/" instead of "server:port") as the ADDRESS
parameter.

Thanks,
Roland
Zack Weinberg | 27 Sep 02:27

Re: [PATCH] fix use of hostname from parsed uri

On Fri, Sep 26, 2008 at 4:41 PM, Roland McGrath <roland <at> redhat.com> wrote:
>> >        mtn sync mtn://server/foobar
>>
>> ... huh?
...
> The fix that allows "mtn://server" to work also allows "mtn://server/name"
> so that the rest of the story can be done quite nicely just with more minor
> hacking on usher.

Oh, I see.  I thought you might be saying that your patch somehow
caused "mtn://server" to get rewritten to "mtn://server/name" which
didn't make any sense.

>> Is it possible you could write a test case or two for this bug?
>
> The test is any push/pull/sync operation using "mtn://server" instead of
> "server" (or "mtn://server:port/" instead of "server:port") as the ADDRESS
> parameter.

I understood this; I was asking you to write an automated test that
could be added to the test suite, to ensure that it doesn't get broken
in the future.

zw
Roland McGrath | 27 Sep 02:59
Favicon

Re: [PATCH] fix use of hostname from parsed uri

> > The test is any push/pull/sync operation using "mtn://server" instead of
> > "server" (or "mtn://server:port/" instead of "server:port") as the ADDRESS
> > parameter.
> 
> I understood this; I was asking you to write an automated test that
> could be added to the test suite, to ensure that it doesn't get broken
> in the future.

I understood this.  I was hoping that among experienced monotone hackers
there might be some sympathy for the guy who sent in the fix, and someone
might take the minute it took to commit the fix and throw in the five more
minutes someone well-versed in the monotone tree would need to add the
simple test case.  But thanks for the invitation to spend all day trying to
figure out the monotone test suite setup just to get any useful action on
fixing something that should always have worked and that any random user
walking up to use mtn can't fathom wouldn't be entirely reliable and
thoroughly tested in any SCM that isn't a joke, it's awesome for building
community spirit.

I can't figure out why tests/netsync_mtn_uri_scheme doesn't get run
already, or if it does, why it doesn't already fail the same way that I see
when I try any simple manual use of a mtn:// uri.  It's too bad if no one
else cares to look into this.  I think I'll spend what little time I can
spare for improving monotone on trying to get a sensible packaged usher
hosting server setup to work out of the box, rather than on this.

Thanks,
Roland
Zack Weinberg | 27 Sep 05:25

Re: [PATCH] fix use of hostname from parsed uri

On Fri, Sep 26, 2008 at 5:59 PM, Roland McGrath <roland <at> redhat.com> wrote:
>> > The test is any push/pull/sync operation using "mtn://server" instead of
>> > "server" (or "mtn://server:port/" instead of "server:port") as the ADDRESS
>> > parameter.
>>
>> I understood this; I was asking you to write an automated test that
>> could be added to the test suite, to ensure that it doesn't get broken
>> in the future.
>
> I understood this.  I was hoping that among experienced monotone hackers
> there might be some sympathy for the guy who sent in the fix, and someone
> might take the minute it took to commit the fix and throw in the five more
> minutes someone well-versed in the monotone tree would need to add the
> simple test case.  But thanks for the invitation to spend all day trying to
> figure out the monotone test suite setup just to get any useful action on
> fixing something that should always have worked and that any random user
> walking up to use mtn can't fathom wouldn't be entirely reliable and
> thoroughly tested in any SCM that isn't a joke, it's awesome for building
> community spirit.

I apologize; I didn't mean the request to be that off-putting.  The
thing is, I doubt anyone other than you or me will do it, and I was
guessing that it would be *easier* for you than me, because you know
exactly what doesn't work, whereas I have never tried mtn:// urls.  --
You may have noticed that there hasn't been a lot of activity here
lately.  I think the only person who's done serious development in a
while is Stephen Leake.  I want to spend more time on it but my day
job is eating all my hacking energy.

Anyway, I'll find time this weekend to merge your patch and some tests for it.
(Continue reading)

Roland McGrath | 28 Sep 00:32
Favicon

Re: [PATCH] fix use of hostname from parsed uri

> Anyway, I'll find time this weekend to merge your patch and some tests for it.

Thanks very much.

> (Did it seriously take all day to figure out the test suite?  I know
> it's not well documented but I didn't think it could be *that*
> confusing.)

I didn't spend all day!  I haven't figured it out!

> > I can't figure out why tests/netsync_mtn_uri_scheme doesn't get run
> > already, or if it does, why it doesn't already fail the same way that I see
> > when I try any simple manual use of a mtn:// uri.
> 
> Hmm.  Did you try urls of the form mtn://host?branch instead of
> mtn://host/branch ?  That's the only thing I can think of.

I tried by hand what appear to be the cases that test does and they don't
work either.  In no example anywhere (mine or anyone's) is mtn://host/branch
a proper syntax.  I mentioned mtn://host/name, where "name" is entirely
something to be considered by usher.

> You might find the debian/monotone-server.* scripts helpful, although
> I have no idea how much of that is applicable to Redhat-style systems
> or to usher setups.  It would be great to make that stuff less
> distro-specific.

The Fedora monotone-server package already does the equivalent to that.
(I'm not sure which of us did it first, in fact.)  That is a setup for
running "mtn serve" from one db.  What I'm talking about, as a sensible
(Continue reading)

Stephen Leake | 27 Sep 07:24
Favicon

Re: [PATCH] fix use of hostname from parsed uri

Roland McGrath <roland <at> redhat.com> writes:

>> > The test is any push/pull/sync operation using "mtn://server" instead of
>> > "server" (or "mtn://server:port/" instead of "server:port") as the ADDRESS
>> > parameter.
>> 
>> I understood this; I was asking you to write an automated test that
>> could be added to the test suite, to ensure that it doesn't get broken
>> in the future.
>
> I understood this.  I was hoping that among experienced monotone hackers
> there might be some sympathy for the guy who sent in the fix, and someone
> might take the minute it took to commit the fix and throw in the five more
> minutes someone well-versed in the monotone tree would need to add the
> simple test case.  But thanks for the invitation to spend all day trying to
> figure out the monotone test suite setup just to get any useful action on
> fixing something that should always have worked and that any random user
> walking up to use mtn can't fathom wouldn't be entirely reliable and
> thoroughly tested in any SCM that isn't a joke, it's awesome for building
> community spirit.

I think it's always appropriate to _ask_ if the bug submitter can
write a test. For example, I have no idea what your level of expertise
in the mtn testsuite is; asking is the only way to find out.

You could have just said "no, that would be too hard for me; I don't
understand how the test suite works".

I thought the request was phrased quite gently:

(Continue reading)


Gmane