[MERGE][#175520] use historical context for file logging
Vincent Ladeuil
v.ladeuil+lp at free.fr
Wed Jan 28 13:52:02 GMT 2009
Some comments on the discussions before the review itself.
The outcome is a BB:tweak but ask for some tests rewriting.
>>>>> "Ian" == Ian Clatworthy <ian.clatworthy at internode.on.net> writes:
Ian> Adrian Wilkins wrote:
>>> If we start supporting file-ids in the UI, I'd like to do so
>>> across every command and allow you to specify "fileid:..."
>>> anywhere you can currently specify a file.
I disagree, I can't think of another command that needs file-id
like log does. Care to elaborate ?
>>>
>>
>> +1 on supporting file-id ; when the FAQ of "how do I find the revision a
>> file got deleted in" came up for the Nth time in IRC, my first impulse
>> was to implement a --file-id option for `bzr log` which is relatively
>> easy since it just resolves them internally anyway.
Ian> FWIW, if this patch and my faster-log patch land, log
Ian> will begin showing revisions that deleted a file
Ian> *provided* one specifies a lower revision limit of some
Ian> kind and excludes the merge revisions.
Gee, if I'm supposed to remember those constraints before firing
the command... I'm sure I will fail each time...
Ian> The latter can be achieve via --short, --line or the
Ian> proposed --no-include-merges option (yet another log
Ian> patch). So there will be an workaround of a kind until
Ian> the problem is addressed via a later format.
:-/
Ian> Technically, my faster-log patch makes a choice wrt
Ian> file-matching algorithm: per-file graph (current) or
Ian> delta-lookup. The latter is currently more accurate and
Ian> faster at incrementally showing results. However, it is
Ian> unacceptably slow across a large number of revisions,
Ian> (ala log -v now) so unfortunately it can't be the
Ian> default until the split-inventory format lands. I guess
Ian> we could be smarter still and use a combination: delta
Ian> until the delete is found then per-file graph? Our time
Ian> may be better spent elsewhere though, e.g. getting split
Ian> inventory landed.
Let's do that, yes.
<snip/>
Ian> So the UI question becomes "What was that file called and
Ian> when did it get created/renamed/deleted?" My proposed answer
Ian> is that is adding an option to log that let's you filter on
Ian> status flags something like ...
Ian> bzr log --status-flags NRD
Meh. *I* don't care about filtering that, I care about the file
life cycle and history (that's why I'm using log), but filtering
*inside* that file history... that's a bit overkill for me.
I want one precise file (hence a file-id) that I have identified
one way or another (be it bzr status --show-ids -rX..Y, annotate
--show-ids, you name it).
>>>>> "Ian" == Ian Clatworthy <ian.clatworthy at internode.on.net> writes:
Ian> Ian Clatworthy wrote:
>> This patch fixes log so that the end revision is used
>> for mapping a FILE to a file-id. This means log can
>> now find deleted files. It also means renamed files
>> are matched against their name at the time.
Ian> This updated version searches the start revision if the
Ian> file doesn't exist in the end revision.
Right, I guess that's the best we can do *without* a file-id
option... but I really think you should update the log help with
a complete explanation there...(Did *you* say HDD ? :-)
Ian> Ian C.
Ian> # Bazaar merge directive format 2 (Bazaar 0.90)
Ian> # revision_id: ian.clatworthy at canonical.com-20090118061118-\
Ian> # 21mjqrjiaxwz1rdl
Ian> # target_branch: http://people.ubuntu.com/~ianc/bzr/ianc-integration
Ian> # testament_sha1: 95bfe6ac2ed04d777a8a5d57cbf1ed14379e1b0d
Ian> # timestamp: 2009-01-18 16:11:34 +1000
Ian> # base_revision_id: pqm at pqm.ubuntu.com-20090115233242-4bxyn4zcj2a0ksfk
Ian> #
Ian> # Begin patch
Ian> === modified file 'NEWS'
Ian> --- NEWS 2009-01-15 22:51:27 +0000
Ian> +++ NEWS 2009-01-18 06:11:18 +0000
Ian> @@ -35,6 +35,13 @@
Ian> a change to FILE when the ``--short`` and ``--line`` log formats
Ian> are used. (Ian Clatworthy, #317417)
Ian> + * ``bzr log -rX..Y FILE`` now shows the history of FILE provided
Ian> + it existed in Y or X, even if the file has since been deleted or
Ian> + renamed. If no range is given, the current/basis tree and
Ian> + initial tree are searched in that order. More generally, log
Ian> + now interprets filenames in their historical context.
Ian> + (Ian Clatworthy, #175520)
Ian> +
Ian> * Fix a problem with CIFS client/server lag on Windows colliding with
Ian> an invariant-per-process algorithm for generating AtomicFile names
Ian> (Adrian Wilkins, #304023)
Ian> === modified file 'bzrlib/builtins.py'
Ian> --- bzrlib/builtins.py 2009-01-15 05:18:22 +0000
Ian> +++ bzrlib/builtins.py 2009-01-18 06:11:18 +0000
Ian> @@ -1895,12 +1895,10 @@
Ian> tree, b, fp = bzrdir.BzrDir.open_containing_tree_or_branch(
Ian> location)
Ian> if fp != '':
Ian> - if tree is None:
Ian> - tree = b.basis_tree()
Ian> - file_id = tree.path2id(fp)
Ian> + file_id = _get_fileid_to_log(revision, tree, b, fp)
Ian> if file_id is None:
Ian> raise errors.BzrCommandError(
Ian> - "Path does not have any revision history: %s" %
Ian> + "Path unknown at end or start of revision range: %s" %
Ian> location)
Right at least that gives some hints and may help me remember how
that should be used.
Ian> else:
Ian> # local dir only
Ian> @@ -1935,6 +1933,45 @@
Ian> finally:
Ian> b.unlock()
Ian> +def _get_fileid_to_log(revision, tree, b, fp):
Doc string please.
And how about putting that helper in the log module ? No need to
make builtins.py bigger than it currently is.
Ian> + if revision is None:
Ian> + if tree is None:
Ian> + tree = b.basis_tree()
Ian> + file_id = tree.path2id(fp)
Ian> + if file_id is None:
Ian> + # go back to when time began
Ian> + rev1 = b.get_rev_id(1)
Ian> + tree = b.repository.revision_tree(rev1)
Ian> + file_id = tree.path2id(fp)
Wow, that's really desperate :-)
I frankly doubt it will often succeeds... but since failures to
find the file id are trapped at the caller level, I'm fine with
that.
As long as you add some direct tests for it (one more reason to
put it in the log module).
Ian> === modified file 'bzrlib/tests/blackbox/test_log.py'
Ian> --- bzrlib/tests/blackbox/test_log.py 2009-01-15 14:05:13 +0000
Ian> +++ bzrlib/tests/blackbox/test_log.py 2009-01-18 06:11:18 +0000
Ian> @@ -188,7 +188,7 @@
<snip/>
Ian> + def test_log_file_historical(self):
Ian> + """File matched against revision range, not current tree."""
Ian> + self.prepare_tree(complex=True)
Ian> +
Ian> + # Check logging a deleted file gives an error if the
Ian> + # file isn't found at the end or start of the revision range
Ian> + err_msg = "Path unknown at end or start of revision range: file2"
Ian> + err = self.run_bzr('log file2', retcode=3)[1]
Ian> + self.assertContainsRe(err, err_msg)
Ian> +
Ian> + # Check logging a deleted file is ok if the file existed
Ian> + # at the end the revision range
Ian> + log, err = self.run_bzr('log -r..4 file2')
Ian> + self.assertEquals('', err)
Ian> + self.assertNotContainsRe(log, 'revno: 1\n')
Ian> + self.assertContainsRe(log, 'revno: 2\n')
Ian> + self.assertNotContainsRe(log, 'revno: 3\n')
Ian> + self.assertContainsRe(log, 'revno: 3.1.1\n')
Ian> + self.assertContainsRe(log, 'revno: 4\n')
Ian> +
Ian> + # Check logging a deleted file is ok if the file existed
Ian> + # at the start of the revision range
Ian> + log, err = self.run_bzr('log file1')
Ian> + self.assertEquals('', err)
Ian> + self.assertContainsRe(log, 'revno: 1\n')
Ian> + self.assertNotContainsRe(log, 'revno: 2\n')
Ian> + self.assertNotContainsRe(log, 'revno: 3\n')
Ian> + self.assertNotContainsRe(log, 'revno: 3.1.1\n')
Ian> + self.assertNotContainsRe(log, 'revno: 4\n')
I'd really really prefer if we can define some dumb log formatter
that just records the revisions it displays so that the tests can
then use some 'rev in list' conditions.
And this test is at least three different ones (you have the
comment to give them a name), split it please.
BB:tweak
Vincent
More information about the bazaar
mailing list