Re: [Monotone-devel] Kicking around ideas

[Thomas Keller]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Derek Scherger schrieb:
> Thomas Keller wrote:
>> does not. The latter was "fixed" by me already in
>> 3a9c12f498b2446ff8b570ffc254367287203189 simply by adding another
>> argument to the path_restriction ctor which optionally skips the path
>> validity checks, but I think this is merely a hack.
> 
> Yeah, with this I can presumably say 'mtn automate inventory blarf' and
> it won't complain or produce any output if I've mis-spelled the name I'm
> actually interested in. That was the original rationale when these
> checks went in anyway. I can't remember who, but at least one user
> complained that it had not produced any output when they had restricted
> to a bogus name.

Oh, restricting on bogus paths still does not work, because they are not
part of any roster.

>> Anyways, even with this fix, there are some doh's:
>>
>> * mtn au inventory source correctly outputs the source/ and source/a
>> nodes, but lists target/ and target/a as missing with fs_type set to
>> "none". This is clearly wrong (both exist) and can be checked by
>> restricting mtn au inventory to target.
>>
>> * On the other hand, an mtn au inventory target now lists target/ and
>> target/a correctly, but fails to notice f.e. if source/ is an existing,
>> unknown directory.
>>
>> The reason why both cases fail is because the node_restriction includes
>> two paths for each node, while the path_restriction only one of the
>> path, so the tree walker later misses to get the fs state of the
>> "additional" paths.
> 
> I don't completely follow this. I would expect the tree walker to notice
> the target nodes because they do exist on the filesystem. I would expect
> it to not notice the source nodes because they don't exist on the
> filesystem.

$ ls -la
insgesamt 16
drwxr-xr-x  5 tkeller users   43 21. Jan 18:13 .
drwxr-xr-x 22 tkeller users 4096 21. Jan 18:20 ..
drwxr-xr-x  2 tkeller users   58 22. Jan 09:38 _MTN
drwxr-xr-x  2 tkeller users    6 21. Jan 18:13 source
drwxr-xr-x  2 tkeller users   14 18. Jan 17:34 target

$ mtn au inventory target
    path "source"
old_type "directory"
new_path "target"
 fs_type "none"
  status "rename_source"

    path "source/a"
old_type "file"
new_path "target/a"
 fs_type "none"
  status "rename_source"

    path "target"
new_type "directory"
old_path "source"
 fs_type "directory"
  status "rename_target" "known"

    path "target/a"
new_type "file"
old_path "source/a"
 fs_type "file"
  status "rename_target" "known"

When I restrict on "target" like above, "source" is not part of the
include paths and therefor gets the wrong fs_type (none instead of
directory) and also misses one status (unknown).

The same happens the other way around: If I restrict on "source",
"target" and "target/a" are not scanned, thus listed as missing:

$ mtn au inventory source
    path "source"
old_type "directory"
new_path "target"
 fs_type "directory"
  status "rename_source" "unknown"

    path "source/a"
old_type "file"
new_path "target/a"
 fs_type "none"
  status "rename_source"

    path "target"
new_type "directory"
old_path "source"
 fs_type "none"
  status "rename_target" "missing"

    path "target/a"
new_type "file"
old_path "source/a"
 fs_type "none"
  status "rename_target" "missing"

Now, the "source" node is correct, but the "target" nodes are not obviously.

>> An obvious way to "fix" this problem on the user side is to include both
>> paths right from the start in the restriction, i.e. "mtn au inventory
>> source target", but then the user has to know _beforehand_ that one of
>> the nodes he's restricting is part of a rename. If he doesn't know,
>> he'll simply get wrong output like the above.
> 
> This has come up a few times but I would be surprised if people didn't
> complain loudly if they had to list both names. I could be wrong though. ;)

Yeah, I don't like to do that kind of "fix" either...

>> Thinking on our side, there are two possible ways to fix it:
>>
>> a) add all paths which are included by node_restriction as additional
>> include paths for the path_restriction - this is expanding "source" or
>> "target" to "source target" through the "backdoor".
> 
> This doesn't sound so bad actually. At least the set of paths that would
> be considered valid would be the same for both the path and node
> restrictions.

The problem here is that I think this implementation is rather hacky,
because I'd have to go over the the rosters again and check

a) if one of the include paths matches an existing node in either roster
b) if yes, get the name of the node in the other roster and add it to
the paths

>> I'm tending to implement b), what about you?
> 
> Another option might be to require that names be listed with some form
> of OLD/NEW revision specifier so that listing either side of a rename
> can be done unambiguously.
> 
> i.e.
>     mtn auto inventory address@hidden address@hidden
> 
> This might solve some problems but it also wouldn't surprise me if it
> created some new ones. ;)

Daniel Carosone proposed on IRC two more options for automate,
- --only-new and --only-old, which would make inventory output either set
of nodes. I have to say I'd rather like to stick with the "relative"
simplicity inventory has now, than sticking more things into it...

Thomas.

- --
GPG-Key 0x160D1092 | address@hidden | http://thomaskeller.biz
Please note that according to the EU law on data retention, information
on every electronic information exchange might be retained for a period
of six months or longer: http://www.vorratsdatenspeicherung.de/?lang=en

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.4-svn0 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

iD8DBQFHla5/af7NlBYNEJIRAtUhAJ44LBAdTT8AMCx7gyquimMJ/oCqKgCgqL0q
/uBRlM1Jg8jIh7VnD/ZRkxo=
=iX+8
-----END PGP SIGNATURE-----