Re: [Monotone-devel] basic_io inventory

[Thomas Keller]

Stephen Leake schrieb:
> Stephen Leake <address@hidden> writes:
> 
>>>> I can help with the tests and documentation.
> 
> I've taken a first stab at updating
> tests/automate_inventory/__driver__.lua, and adding some comments in
> automate.cc.

Wow... you've been busy! You might want to provide a patch for
everything if you don't have write access to venge.net.

> As an aside, I found the lua test framework quite nice, once I figured
> out how to run it on an individual test.

For the tests, I think you should implement the actual matching after
you've parsed the basic_io properly. For this there exists a function
named parse_basic_io which takes the basic_io string as input and
returns a lua table. The automate_keys test includes an example how to
cope with that.

> //    old_node "<old_node_id>" "<old_node path::status>"
> //    new_node "<new_node_id>" "<new_node path::status>"
> //     fs_type "<current path::status>"

path::status could be simply resolved to "file|directory", and for
fs_type "file|directory|none" (none if the file is missing). This makes
it easier to read.

> //      status "<status>"
> //     changes "{content|attrs}..."

changes can be one of "content", "content attrs" or "attrs".

> // 'fs_type' gives the state of the item in the workspace filesystem.
> //
> // 'old_node' and 'new_node' give the status of the item in the manifest.
> // The 'old_node' and 'new_node' lines are not output if the corresponding
> // node state is 'nonexistent'. FIXME: better to output 'none'?
> //
> // '<status>' is one of 'known', 'missing', 'unknown'

There is also the 'ignored' state.

> // Error conditions: If no workspace book keeping _MTN directory is found,
> //   prints an error message to stderr, and exits with status 1.
> 
> Is this correct so far?

You might want to add a short note about the possible PATH argument and
the options, but this is nit-picking here and could also just go into
monotone.texi.

> Here's the first set of checks in
> tests/automate_inventory/__driver__.lua. I left the original output
> for comparison. I added FIXME: comments where I think the output could
> be improved.

Now since the format is more complete, you may want to add checks for
corner cases as well which weren't properly supported by the old format
in the past, f.e. renames which change the type of a node (drop a dir,
add a file with the same name and vice versa, stuff like that).

> --  AP 0 0 added
> --  FIXME: status should indicate 'added, needs commit'
> check(grep('^    path "added"$', "stdout"), 0, false, false)
> check(grep('^new_node "2147483648" "file"$', "stdout"), 0, false, false)
> check(grep('^ fs_type "file"$', "stdout"), 0, false, false)
> check(grep('^  status "known"$', "stdout"), 0, false, false)
> check(grep('^$', "stdout"), 0, false, false)

The new_node ids for added files are unique, but I've forgot how they're
calculated (MAX_INTEGER - ... maybe), anyways

a) its not a good idea to test for specific values here
b) the explicit note that this file is added is indeed missing; as long
as there is no other node outputted which has the new_node id set in its
old_node id (which practically makes the add a rename) we can however
assume that this is an add

> -- D   0 0 dropped
> --  FIXME: no 'status'; should indicate 'dropped, needs commit'
> check(grep('^    path "dropped"$', "stdout"), 0, false, false)
> check(grep('^old_node "2" "file"$', "stdout"), 0, false, false)
> check(grep('^ fs_type "none"$', "stdout"), 0, false, false)
> check(grep('^$', "stdout"), 0, false, false)

Same like the above, just the other way around.

> --   I 0 0 ignored
> --  FIXME: need an option to not output these
> check(grep('^   path "ignored~"$', "stdout"), 0, false, false)
> check(grep('^fs_type "file"$', "stdout"), 0, false, false)
> check(grep('^ status "ignored"$', "stdout"), 0, false, false)
> check(grep('^$', "stdout"), 0, false, false)

You like to have an option to hide ignored files from basic_io output?
What if a user explicitely wants to add an ignored file? (Maybe it was
ignored by accident, f.e. a wrong .mtn-ignore entry?)

> -- not in original output (why not?)
> check(grep('^   path "inventory_hooks.lua"$', "stdout"), 0, false, false)
> check(grep('^fs_type "file"$', "stdout"), 0, false, false)
> check(grep('^ status "unknown"$', "stdout"), 0, false, false)
> check(grep('^$', "stdout"), 0, false, false)

You don't need to check things more than once, right? The complete test
suite already takes a long time to finish, no need to make it take even
longer =)

> 1) clearly indicate whether the item needs committing or not. 
> 2) clearly indicate renamed and dropped.

If we'd bring the additional "added", "dropped", "renamed_from" and
"renamed_to" states back, I guess it should be fairly easy to determine
if an item needs to be committed or not (either it has one of those
states or the changes stanza is present).

> 3) option to not output unchanged and ignored files (to speed processing)

I guess restriction support alone should speed up processing a lot, if
there are still speed issues one could think about adding an
"--show-tracked-only" option or something along this line.

> It would be more consistent to output 'old_node <id> "none"' rather
> than not outputing the old_node line; same for new_node. But that's
> not hard to handle in the Emacs parser.
> 
> I'm not clear what the rationale is for the node ids; what subsequent
> commands would use them?

F.e. automate get_file (obviously there is a version available now which
takes the file path and revision_id).

Thomas.

-- 
ICQ: 85945241 | SIP: 1-747-027-0392 | http://www.thomaskeller.biz
> Guitone, a frontend for monotone: http://guitone.thomaskeller.biz
> Music lyrics and more: http://musicmademe.com