Re: [Monotone-devel] review of nvm.connection_info_cleanup

[Stephen Leake]

Thomas Keller <address@hidden> writes:

> The branch nvm.connection_info_cleanup should be feature-complete from
> my side, all tests pass and the docs have been updated accordingly.
> Please read over it and give me feedback if you find gotchas or think of
> other improvements.

I reviewed monotone.texi, but not the code.

The documentation is pretty clear. However, I think in this syntax:

mtn automate pull [--set-default] address@hidden address@hidden [...] 
address@hidden

the 'glob' should only be there if the 'uri-or-address' is the old style
host address? If true, I think it would be better to list this as two lines:

mtn automate pull [--set-default] address@hidden [...] address@hidden

mtn automate pull [--set-default] address@hidden address@hidden [...] 
address@hidden

Then it is easier to say the second one is deprecated. Similarly for
push and sync, of course.

There needs to be a NEWS entry.

I compiled the code on MinGW; parse_date.cc needed the '#include
"sanity.hh"' patch.

Running 'mtn sync' in an existing monotone workspace gave no warnings;
that's good.

However, running with an explicit old-style address:

mtn  pull  monotone.ca  net.venge.monotone*

gave these messages:

mtn: connecting to mtn://monotone.ca
mtn: first time connecting to server mtn://monotone.ca
...
mtn: setting default include pattern for server 'mtn://monotone.ca' to 
'net.venge.monotone*'
mtn: setting default exclude pattern for server 'mtn://monotone.ca' to ''

This would be better if there was an explicit "you specified a
deprecated argument" message first.

In addition, the default-server and known-server db vars are now
inconsistent:

database: default-server monotone.ca
known-servers: monotone.ca 3e6f5225bc2fffacbc20c9de37ff2dae1e20892e
known-servers: mtn://monotone.ca 3e6f5225bc2fffacbc20c9de37ff2dae1e20892e

Is this intended?

mtn 0.46 is ok with these db vars, so I think this is ok in general.


I also compiled on Cygwin to test ssh: and file: syncs. g++ reported
missing '\' in uri.cc:

../monotone.connection_info_cleanup/uri.cc: In function 'bool 
try_parse_bare_authority(const std::string&, uri_t&, origin::type)':
../monotone.connection_info_cleanup/uri.cc:86: error: unknown escape sequence 
'\]'

this: 

                       "\[[:0-9a-fA-F]+\](:[0-9]+)?" // bracketed ipv6

should be:

                       "\\[[:0-9a-fA-F]+\\](:[0-9]+)?" // bracketed ipv6


With this current ssh: command line:

mtn --db /Stephe/monotone-dbs/gds.db sync \
ssh://address@hidden/home/stephe/monotone-dbs/gds.db "*"

I get the error:

mtn: misuse: explicit port-number specification in URI has no digits

This syntax should be accepted. I was hoping for a 'deprecated' warning,
that also gave the equivalent command in the new syntax.

Changing the command to:

mtn --db /Stephe/monotone-dbs/gds.db sync \
"ssh://address@hidden/home/stephe/monotone-dbs/gds.db?*"

works.

I get the same error for file:

mtn --db /Stephe/monotone-dbs/gds.db sync --ticker=count 
file:/cygdrive/f/gds.db "*"

mtn: misuse: explicit port-number specification in URI has no digits

Again, changing to the new URI syntax works.

Both of the above sync commands set server vars in the db:

database: default-server 
ssh://address@hidden/export/services/monotone/gds/gds.db
server-exclude: file:/cygdrive/f/gds.db 
server-exclude: ssh://address@hidden/home/stephe/monotone-dbs/gds.db 
server-include: file:/cygdrive/f/gds.db *
server-include: ssh://address@hidden/home/stephe/monotone-dbs/gds.db *

I guess that makes sense, but it did not happen with the current version
of mtn.

The paragraph that describes saving these defaults talks about 'address'
and 'glob'; it needs to be updated:

    The @command{pull}, @command{push}, and @command{sync} commands only
    require you pass @var{address} and @var{glob} the first time you use
    one of them; monotone will memorize this use and in the future
    default to the same server and glob. For instance, if Bob wants to
    @command{sync} with Alice again, he can simply run:


 
-- Stephe