Re: [Monotone-devel] Kicking around ideas

[Thomas Keller]

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

Stephen Leake schrieb:
> Thomas Keller <address@hidden> writes:
> 
>> I haven't tested the mainline version, but is it correct that
>>
>> $ mtn au inventory --no-unchanged
>>
>> will also skip added/dropped/renamed nodes in the output? At least from
>> what I can see in the code this is the case... is this intentional?
> 
> That was not the intent; those nodes need attention, and should be
> considered "changed".

Ok, I thought so as well ;)

>> If not, I'm having my branch (net.venge.monotone.inventory-fixes)
>> ready here which fixes that and the other mentioned things in the
>> previous emails. Please check
>>
>> 96b39ed0cc6a70b472dd80d35c6fb6a0bd2c4dbc
>>
>> for more information.
>>
>> I haven't yet propagated from mainline, so I guess there will be a
>> conflict wrt the recent tests Stephe added (I implemented them as well
>> on my branch) - if I fix that, is it ok if I merge nvm.inventory-fixes
>> to mainline?
> 
> I see you have now propagated from mainline.
> 
> I like the new option --no-corresponding-renames.
> 
> I'm getting a test failure:
> 
>  78 automate_inventory_options                    FAIL (line 13)
> 
> all of the "expected-renames-*.stdout" files are missing.
> 
> I assume this passes on your system.

Yes, sorry, added them in ce4ffef538886e199240be25c2433c1ebf1b4cad

> In automate.cc, you replaced the tests of the boolean outputs from
> inventory_determine_states with tests of the result of a linear search
> thru a short vector of strings. I guess it won't really affect the
> speed, since that's dominated by disk accesses and memory allocations
> for the rosters, but that seems horribly inefficient :).

Well, having a couple of more boolean values just make the code more
unreadable. You see that the boolean values for "is_tracked" and
"has_changed" are rather complex - but as they are now the conditions
for either one can be seen at a glance. If we'd do that with boolean /
bit values inside the inventory_determine_states and friends, this would
get highly obfuscated again. Sure, we compare strings. But they're
almost fixed size and not of arbitrarily length, so take a constant
execution time. If you think this hurts performance too much (and are
able to prove that), feel free to improve that part.

> I see now what you meant by "continue early"; it is a much better
> structure.
> 
> This looks ok to me for merging to mainline.

Cool!

> We should use 'samefile' in automate_inventory_options; I didn't know
> about that Lua function when I wrote that test.
> 
> Do you prefer the "expected output file" style used in
> automate_inventory_options, or the "check_inventory" style used in
> automate_inventory_restricted? I prefer the files; they are easier to
> create. I wish I'd thought of that first :).

Well, depends which of both formats is rather error-prone when it comes
to whitespace changes or stanza reordering (if this ever happens). I
like the check_inventory style more because I don't have to supply /
update extra files if something changes, but have everything in one
place: the test file.

> I don't see an explicit test of --no-unchanged with added/dropped
> nodes (there is one with renames). I can work on adding those tests,
> after the merge.

There are a couple of other open tests at the end of
automate_inventory/__driver__.lua, if you are in the mood, it would be
cool if you could add those as well. F.e. the test to verify the
"invalid" state is completly missing. I've updated monotone.texi with
some new examples already and rewrote / improved some parts, so the
tests are the only thing missing for this now.

> Just out of curiosity, when do you use "--exclude" with inventory?

Well, I do not, but since it is an allowed option if we allow
restrictions, I had to include some code and tests to support that as
well. I haven't tested both, include as well as exclude patterns, with
globs so far, and I believe its perfectly possible that all the
nitty-gritty things do not work properly if such a restriction applys,
but then again: who restricts on a renamed node with those inputs? If
this day comes, we'll have again a chance to look into resolving this
issue. (I'm a huge fan of Nathaniel's dogma "do not produce dead code".)

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

iD8DBQFHl0KAaf7NlBYNEJIRAujQAKDK24VFivtFb9MBhVIbKkIR6vMnSACg+ERD
cIgNGtZ+FvbGamiGVo93nMQ=
=JeZO
-----END PGP SIGNATURE-----