=== poolie is now known as pooliex [00:12] is this the one causing all the oopses? [00:14] no [00:17] Hi. [00:17] someone sent me a diff file to apply to my tree... [00:18] they said use the 'patch' command, however there is no patch command in 1.6 [00:18] edencane: The patch command is part of bzrtools. [00:19] pooliex: sorry, was in the zonr [00:19] ok... (-: [00:19] thanks wgrant [00:32] mwhudson, i'm going to look at #261315 today [00:32] lifeless, np [00:36] pooliex: cool [00:38] hah, pushing the exact same branch from scratch over both bzr+ssh and svn+ssh to the same server is faster over svn+ssh now >-) [00:39] the difference is not very big, and the bzr server side is running bzr 1.0 (bzr.dev on the client) [00:43] spiv: how did you go with streams? [00:50] * jml submits a patch [00:55] classic - http://steve-yegge.blogspot.com/2008/08/business-requirements-are-bullshit.html?showComment=1218561660000#c5713392683478153083 [00:58] hmmm, does bzr have anything like git where you can export checkins as diff files with metadata? [00:58] warren, 'bzr help send' [00:59] poolie: hello [00:59] poolie: wb [00:59] we had it first i think [00:59] thx [00:59] poolie: welcome back. [00:59] thanks, both of you [01:03] thx [01:07] lifeless: hi [01:07] nice rant if somewhat longwinded [01:07] ho [01:07] is the comment directed at me though? [01:07] poolie: no [01:07] poolie: but it did make me smile [01:08] i would say the main article is somewhat in the category of 'intentionally misconstruing the question' [01:15] anyone know of a way to interactively tweak where 'bzr shelve' decides to place hunk boundaries? [01:15] quite often I need to break up the changeset differently :/ [01:16] teratorn: I think shelve gets it from diff. [01:18] you're probably right [01:18] one day when I'm rich I'm gonna hire someone to follow me around on IRC and implement all the stuff I complain about [01:21] you jus gotta complain to the right people! [01:21] complaining is hard work [01:21] abentley: ping [01:22] abentley: I want hunk-editing and stuff in bzr core [01:22] abentley: what do you think of the shelf editor code - is it lovely, and what you'd build for bzr core, or should I take the concepts and write something new? [01:23] lifeless: I've never had much need to look at the shelf editor code. So it must be good. [01:24] what is this shelf editor code of which you speak? [01:24] (Originally written Michael Ellerman. I just maintain it.) [01:24] abentley: I poked at it, I found it a little obstruse, but not terribly so; I don't know if it will scale to hunk splitting and so on, which I do want us to get to. [01:24] teratorn: its the hunk selector in 'bzr shelve' [01:25] teratorn: bzr shelve's hunks are those output by 'bzr diff' [01:26] *nod* [01:26] yeah that would be way cool [01:26] lifeless: I've always figured that sub-hunk territory was best handled in an editor. [01:28] abentley: so fire up an editor with some markers, perhaps the basis & current text seperate, let them edit till it has what they want,then use that to do a new diff and shuffle stuff ? [01:28] basically all you need to do is to be able to split the hunks [01:28] a simple line editor that lets you add some "<<<<<" markers, or something [01:28] and move them around.. up or down [01:28] lifeless: Currently, for that purpose, I stop using shelve, and start using bzr vimdiff [01:30] poolie, spiv: Good to have you guys back. Today is "Labor Day" in the US. [01:30] abentley: I'm sneaking up on tree marks. I want something that doesn't need specific things like vimdiff that may not be readily available for some users, or which will not bind well for IDE's and GUI's. [01:35] poolie, spiv, jam: In Canada, today is "Labour Day" ;-) [01:36] abentley: I guess that means u work harder [01:36] jam: lol [01:37] abentley: I guess I'll send a mail to the list about hunk editing/selection. Using a callback may be the right interface. [01:38] it seems like you could evolve the "shelve" interface if you really wanted to go that route [01:38] just have a "split hunk" option [01:39] though I agree, vimdiff (or an editor) is probably a more comfortable ui for it [01:39] anyway, I'm only just walking by, back to family time [01:40] oh, and poolie, spiv, igc, lifeless, abentley, vila, beuno, jelmer, LarstiQ, fullermd, etc, etc.... Feature freeze is tomorrow, so I encourage you to do some reviewing [01:41] jam, oh, right [01:41] re Labo[u]r Day [01:43] abentley: the other thing we should discuss - rich-root; *I* thought it was waiting on a patch from you, you're saying its blocked on me. Lets sync [01:43] oh damn [01:46] Reviewing? You must have me confused with somebody who knows what they're doing :p [01:56] jelmer: best version of bzr to use with TracBzr is 1.5 ? [01:56] rocky, probably [01:56] jelmer: does TracBzr support trac 0.11 ? [01:57] rocky, yeah [01:57] latest release of TracBzr or do i need to grab a branch? [02:09] rocky, there are no releases, you'll have to grab a branch [02:09] just grabbed trunk [02:12] jam: why does branch.get_revno_map use the old graph abstraction? [02:14] jelmer: any particular reason you haven't merged over the trac0.11 branch into trunk? [02:15] rocky, it is merged into trunk [02:15] oh... sorry i'm still getting familiar with launchpad's very very complicated interface [02:15] seemed to indicate it hadn't been merged [02:16] https://code.launchpad.net/~trac-bzr-team/trac-bzr/trunk/+merges seems to show it hasn't been merged [02:16] rocky: I'm not the maintainer of trac-bzr btw, I do the occasional merge when I need it myself though [02:16] oh gotcha [02:16] rocky, since nobody appears to be particularly looking after it these days [02:16] i might be interested in helping ... particularly if i integrate it with cluemapper [02:16] ah, cool === mw is now known as mw|out === Snaury__ is now known as Snaury [02:33] lifeless: because merge_sort doesn't allow ghosts [02:34] lifeless: please use more contentful subject lines than 'review feedback' thx [02:35] yes [02:35] like [02:36] poolie: please patch bzr send to give me more contentful subject lines! [02:36] du du du [02:37] lifeless: is this the bug about not being able to override the subject line if you use the builtin mail-sender? [02:37] poolie: actually, I didn't look at the subject at all [02:37] poolie: and as there is one, evo didn't bitch at me [02:38] if you're sending through evo i think you can override it and i'm asking that you do [02:38] besides which, BB knows its a follow on patch, it can give extra content. [02:38] poolie: I normally do. I forgot, once. [02:38] and got jumped on. [02:38] oh np [02:39] no stomping was intended [02:55] lifeless, when you have time, any chance you can reply to the "[MERGE] Move install_revision(s) onto the Repository object." thread? [02:55] (http://news.gmane.org/find-root.php?message_id=%3C20080804032617.GA19075%40vernstok.nl%3E) [02:55] poolie: ok [02:56] jelmer: is it stuck? [02:56] lifeless: Sortof - you voted resubmit but I wasn't entirely sure what I had to change [02:57] jelmer: right found it. [02:57] jelmer: what was unclear? [02:57] lifeless: The tests for install_revisions() were already inside repository_implementations [02:57] even though install_revisions() didn't live on Repository but was generic [02:57] ah [02:58] wow [02:58] we have 1 test [02:58] and its bare bones [02:59] uhm [02:59] it really needs to obey the full protocol on per-file graph support etc [02:59] I hesistate to ask you to extend it to be sure of that, but I can think of so many ways it could be broken and pass the current test. [03:00] poolie: I've dropped you a mail, there is a little bit of administrivia needed fairly urgently [03:00] lifeless: Ok, so it just needs more tests? [03:01] jelmer: yeah, I think so [03:01] * poolie chases indirections [03:01] jelmer: I think I'd structure them self-referentially - thusly [03:01] lifeless: is this at google again? [03:01] jelmer: use the default format to build a branch with a file, dir, symlink (only where symlinks are present), that have the following occur to them [03:01] created [03:01] modified [03:02] modified in two branches and merged [03:02] modified in one branch merged to the other branch [03:02] these are the basic corner cases for last-modified revision assignment [03:02] then copy those revisions over to the repository format that is being tested [03:03] they should end up identical [03:04] thanks [03:07] I'll leave it alone for now though, it seems like too much trouble to avoid 20 lines of code duplication in bzr-svn [03:08] I'll note that the bugs in bzr-svn recently about last-changed etc will be avoided if you do this [03:09] its not a good idea to leave it alone and hope :( [03:09] lifeless, last-changed? [03:09] you mean the text revids? That would be the bit I would override in bzr-svn [03:09] its the bit you need to make sure the rules are absolutely in sync with bzr [03:10] Sure, but this is just code sharing, it's not test sharing [03:10] right [03:12] I'm looking for a review on http://bundlebuggy.aaronbentley.com/project/bzr/request/%3C1219204485.31549.89.camel%40lifeless-64%3E [03:12] So I don't see how that would help here; I would override install_revisions() for bzr-svn anyway, but that would allow using an existing InterRepository implementation that uses install_revisions() rather than writing my own. [03:13] jelmer: so, I guess I'm thinking about test sharing more [03:13] I'm hesitant about install_revisions as it stands, there are still #XXX's in it [03:13] last I checked [03:14] sure, but it's already being used at the moment. I'm just moving it. [03:15] so my comment was, that by moving it we're making it be at higher risk - bugs in it will be more exposed [03:15] and anyone implementing Repository will be expected to implement it, so test coverage matters substantially more [03:16] lifeless: I can review that [03:17] lifeless: as a general comment, I'm in favour of bringing things in incrementally though, so don't share jam's concern there. [03:32] lifeless, ah, ok [03:32] lifeless: review sent. [03:33] spiv: thanks [03:36] * beuno -> sleep [03:36] spiv: replied [03:36] g'night all [03:37] spiv: I'm not sure where the 'if in tests is a bad smell' meme is coming from, but its either too simplistically phrased, or wrong. [03:37] I've seen it a couple of times recently, I think its new. [03:39] lifeless: well, I can elaborate why I think this particular if is bad; I agree they can be ok. But by default treating with some suspicion is justified. [03:39] why is it justified? [03:39] lifeless: as to the source of the meme, the xUnit Test Patterns book lists it ("Conditional Test Logic") [03:39] or, whats the justification [03:42] In this case I think it's basically just that you're using the if statement to squeeze multiple tests into one test method. [03:42] you've seen my reply? [03:42] Which isn't as clear at communicating intent as it would be as two separate tests. [03:42] Not yet; I'll do that now. [03:42] because I'm not doing that [03:43] I'm testing the single behaviour of an interface with optional output [03:43] Ok. [03:43] the first column must be either all None, or not None at all [03:43] So I think you can express that intent more clearly. [03:43] as I've defined it [03:43] (And without an if statement in the test method) [03:44] By: [03:44] names = sorted([result[1] for result in read_result]) [03:44] self.assertEqual(expected_names, names) [03:44] # (so far, same as before) [03:44] self.assertAllNoneOrAllNotNone([result[0] for result in read_result]) [03:45] ugh [03:45] There would be an if statement in the assertion method, obviously. [03:45] Thats less clear [03:46] (The xUnit book points out that this is a good thing because you can write tests for your test helpers, whereas writing tests for test methods leads to madness) [03:46] I'll change the test around a bit, see if you like it [03:46] I'll be interested to see what it looks like. [03:47] well, the xUnit book, frankly, should smoke less camel toenails. Its equally untested even with a helper test, because you may simply fail to call the right helper, or pass incorrect parameters in [03:47] at a certain point you have to assess the code in front of you on its own merits [03:47] :) [03:48] I do write helper tests, for helpers that deserve it [03:48] Right, I agree. [03:48] so its not a critique of doing that. Its a critique of using that facility to support an argument that 'if' is bad in test methods [03:48] I nearly said so originally, but I felt like I was already waffling :) [03:48] bet you the authors don't like 'goto' either. [03:49] Perhaps, but the index gives no clue either way :) [03:49] :> [03:49] (goto is very good practice in C for function cleanups) [03:50] (agreed) [03:52] Skimming the Conditional Test Logic section of the book, it is a touch vague but boils down to "conditional logic == complex code == harder to understand/trust == less confidence". [03:54] sounds like an indictment of both 'if' and 'objects' all at once [03:55] Which I think is right. It's a mild smell for me; it can be a sign that the test could be better, but I don't consider it to be a 100% reliable indicator of trouble. Sometimes the test is clear enough even though it has conditional. [03:55] Well, complexity sucks. :) [03:55] see, this is the bit where I think its process insanity [03:55] thats the sort of thing thats so inane, I'd never put it in a talk [03:56] One of the neat thing about tests is that you can often write several simple tests that you individually trust to gain confidence in a larger, complex system. [03:56] "I find your test hard to read" >>>> "your test has if in it" [03:56] spiv: that ignores the potential for skew [03:56] (Keeping in mind that you may have gaps between your simple bits...) [03:57] you have to be sure they are aligned right, and thats hard to test [03:57] Right. [03:57] I said it's a "neat thing", not a "magic bullet" :) [03:58] Anyway, back to the case in hand... I *did* find the purpose of the test a little confusing. [03:58] which is fine, fixing. [04:02] Partly because I worry a little that a casual reader might not pick up on the fact that that test is parameterised, which is helpful in figuring out why the "if" might matter. I wonder if we should have some convention to flag parameterised tests that are outside the per_* and *_implementations directories? [04:03] Perhaps the answer is just that a casual reader that gets confused should just read more closely :) [04:03] well, the test, when it runs, shows the parameters [04:04] spiv: mail sent to the list [04:04] Yeah. So people reading the test because of test failures will know. [04:04] Which is probably most casual readers. [04:08] lifeless: that tweak is better, thanks. [04:08] the difference from a helper function is that this is clearer [04:09] a non-reused helper is just a new function to remember, and the name would signal what it does, not why it does it [04:09] why is what matters [04:09] Sure. [04:10] I wasn't wedded to using a new function, but it was a lot easier to type into IRC :) [04:10] wow [04:10] i got sucked in too [04:12] poolie: ? [04:12] to the 'if in tests' thing [04:12] hehe [04:12] smell doesn't mean 'never do this' [04:13] poolie: not sucked into wow, i hope [04:13] i guess overall, why not hide the sorting by inodes inside the function you're testing [04:13] nup [04:13] poolie: performance [04:13] heh [04:13] care to expand on that? [04:13] sure [04:14] sorting costs, its cheaper not to sort unless you need to [04:15] so if a caller wants the directory without caring about sorting by inode, surely they'd like to also avoid consing and then removing all the tuples? [04:16] poolie: I'll be a second, buffer overflow [04:22] poolie: so, I think the thing about smells, is that if you bring them up in a comment, it muddies the actual discussion that should take place. [04:23] the reviewer should keep inside their head the fact that it's a smell and just either give a concrete reason or say nothing? [04:23] yes [04:23] unless they want to just say 'it smelt to me, if it smells to you too please fix it' [04:24] So I mentioned it as a shorthand way of explaining what I thought was problematic. [04:24] Clearly this failed as a way to explain myself concisely :) [04:24] like pattern languages, unless its shared, it fails [04:24] it may bifurcate the conversation into a discussion of whether it's a general problem or not, and what to do in this case [04:24] as has happened here [04:25] they may go in different directions [04:26] anyhow, coming back to the specific thing here [04:27] it seems to me that callers want either a directory listing in random order, or a listing sorted for optimal access to the files (if possible) [04:27] just waiting for a push to finish then I'll have the code up in front of me again [04:29] dear lazyspiv: getattr won't be called if the attribute is defined in a base class? [04:30] Heh. [04:30] The __getattr__ hook? Right. [04:30] poolie: you mean __getattr__ right? and yes, pretty sure [04:30] i do [04:31] thanks [04:31] that was my reading of the docs, it just contradicts something jam said so i was checking [04:32] lifeless: the other thing here, which is kind of mission creep, is that i think you can actually return the file file kind from readdir, which your comment waves towards [04:32] poolie: its not a win today [04:32] poolie: I ripped that code out [04:32] but it might take a bit of code change to use it, and it probably isn't enough to avoid stat [04:32] * spiv -> lunch [04:34] poolie: so, sometimes we want to sort, sometimes we don't care. Having multiple interfaces is less appealing than a single if its cheap === lamont` is now known as lamont [04:34] poolie: we would allocate inode, string tables to sort anyway, so there isn't of a saving in hiding that [04:38] my point is that if you care about avoiding the sort when the client doesn't care, shouldn't you want to avoid allocating all the data structures needed to sort too? [04:39] poolie: yes, I didn't disagree with your point [04:40] poolie: but we'll need another tested interface to implement it, and that has its own cost; I don't really want to duplicate the C code unnecessarily [04:42] i think it would be clearest to have list_dir(..., sort_hint=None) or something [04:44] that will be quite tricky to test === mario_ is now known as pygi [04:58] poolie: so, spiv has given bb:approve, i'm blocked on a little clearer messaging from you === abentley1 is now known as abentley === abentley1 is now known as abentley === abentley1 is now known as abentley [05:05] lifeless: ok [05:05] in what way is that trickier to test? [05:06] at the moment you're not testing that it is sorted in any way at all [05:06] i think if you do this it will address john's just-posted concern that this is not a good api for pyrex [05:13] he doesn't say its not a good api for pyrex; hes saying its not all-thats-needed-to-be-really-fast [05:13] because we need to fix the overhead of stat [05:13] and we may actually want a full pyrexed walkdirs implementation [05:16] so is that clear or not? [05:17] so, I'm having a problem with this bzrdir patch. [05:18] so the change you propose is really unrelated to John's concern about pyrex, which I've replied to [05:18] If I add another BzrDirMetaFormat1 to the tested formats, I get this error: http://pastebin.ubuntu.com/42594/ [05:19] jml: you shouldn't add another BzrDirMetaFormat1 to the tests [05:19] jml: as its already tested [05:19] lifeless: ok. [05:19] jml: like I said yesterday, just change the formats it tests with to be the latest ones, rather than the defaults [05:19] lifeless: changing the existing format to use stackable branches and repositories makes the 'does default work' tests fail [05:20] * jml looks for the right shelf to get the test output [05:20] jml: give me a diff somewhere [05:20] poolie: so, I can do it, but I think we're polishing the knob a little early [05:20] http://pastebin.ubuntu.com/42595/ [05:20] lifeless: ^^ [05:21] poolie: what I have is the shrunken version of 'get the file kind from C's readdir()' [05:22] jml: what for are you fiddling around in there [05:22] jml: BzrDirFormat.known_formats() is the function to change [05:23] poolie: no its not really clear; I can't tell if you are voting comment: or tweak: [05:23] lifeless: I'm sorry, but I have no idea how to change that in a sane way [05:23] lifeless: what's a control format? [05:24] jml: BzrDir4/5/6/7/BzrDirMetaDir/RemoteBzrDir/SvnDir [05:24] HgDir [05:24] GitDir [05:24] etc [05:25] ok. I still don't get how to change known_formats. [05:25] so [05:25] I can paraphrase the problem differently I think. [05:26] the reason your tests did not fail after you wrote them is that the BzrDirMeta1 type supports stacking, even though the bzr 'default' format does not [05:28] * jml nods [05:28] I am saying, change the default, for all these tests [05:28] ok, i reviewed the new one [05:30] lifeless: how do I change the default for the tests without changing the default for bzr? [05:31] so, known_formats is only used for testing [05:31] if you look at the loop [05:32] its returning all the specific types known by each control type [05:32] so, all the flavours of BzrDir, all the Flavours of GitDir etc [05:33] you want the BzrDirMeta1 flavour returned to be parameterised with a branch and repo type [05:33] basically, I think you change [05:34] __default_format = BzrDirMetaFormat1 [05:34] way down in the file [05:34] ok. [05:34] and check 'bzr init' doesn't break surprisingly. (I don't think it will) [05:34] oh, or upgrade [05:34] it shouldn't actually change the default, due to various things [05:36] poolie: where did you see the copyright thing? [05:36] readdir.h [05:37] and whats wrong with it (other than it saying Bazaar-NG, which I've fixed) [05:39] it's also 2008 now :) [05:39] that file has not changed since 2006 [05:40] ok [05:40] it's newly added so i thought it was new [05:40] Am I misunderstanding (C) rules? [05:40] but maybe it's been sitting on your disk since then [05:40] in which case it's fine [05:41] import sucks [05:45] jml: ? [05:47] lifeless: would you expect http://pastebin.ubuntu.com/42600/ to cause http://pastebin.ubuntu.com/42601/ [05:48] lifeless: usually copyright relates to publication date rather than creation date [05:48] so a 2008 copyright might be appropriate if that's when it first gets published [05:49] its been on the interwebs since london 2006 [05:50] why is something so simple such a long process [05:50] 1/2 a day on one patch [05:50] jml: yes, I would [05:50] lifeless: for my part, it's because of a lack of familiarity :) [05:50] jml: I wasn't referring to your patch [05:51] this readdir one of mine [05:51] EFRUSTRATION [05:51] :) [05:51] I feel like saying, meh, I'll come back to it in another 2 years [05:51] (This patch was the reason I wrote the pyrex stuff for setup.py in the first place, way way way back - it was to enable bringing in such extensions) [05:52] jml: anyhow, your patch. bzrlib.branch is probably lazy importing bzrdir [05:52] and those default setting lines are too high up [05:52] lifeless: i feel like that sometimes too [05:53] jml: try changing bzrlib.branch.BranchFormat.default [05:53] jml: instead, I think it delegates by default [05:55] lifeless: want to talk? any ideas about what to do differently? [05:55] poolie: I'm feeling a tad time pressured. The idea of a process chat right now is not appealing [06:10] lifeless: take 3 is ok with me if you really don't want to change it [06:10] i guess i'd be more relaxed if it were marked private [06:11] poolie: it is now, its _read_dir [06:12] am i one patch behind? [06:12] no [06:12] you're probably confused by the modules [06:12] _readdir_pyx.pyx -> thats a private module [06:12] it has read_dir() in it [06:12] oh, you mean _readdir [06:12] bzrlib.osutils._readdir [06:12] * jml looks into changing the repository default [06:12] fairy nuf [06:12] is where it is exposed, and its exposed _ prefixed. [06:13] http://bundlebuggy.aaronbentley.com/project/bzr/request/%3C1219204485.31549.89.camel%40lifeless-64%3E has osutils republishing it as read_dir [06:13] try: [06:13] from bzrlib._readdir_pyx import read_dir as _read_dir [06:13] except ImportError: [06:13] from bzrlib._readdir_py import read_dir as _read_dir [06:14] ok [06:14] wrong tab [06:15] so my view is - there is one user; it obvious that it is doing a sort, so the costs are visible to readers [06:15] we can triangulate when there are multiple users of the interface [06:22] jam: bug 262565 [06:23] Launchpad bug 262565 in bzr "Old resolved conflicts resurrected" [Undecided,New] https://launchpad.net/bugs/262565 [06:23] jam: I suspect you've spent the most time looking at that set of problems.. [06:24] poolie: for squid patches, in the web ui you can retarget them rather then rejecting them [06:24] poolie: it would be nice if you could change your vote to comment [06:24] http://bundlebuggy.aaronbentley.com/project/squid/request/<1220137897.6703.4.camel%40henriknordstrom.net> [06:25] sure [06:28] thanks [07:00] out to the shops, biab [07:06] jonnydee: sorry that your patch wasn't merged. It was approved, just not sent to the gatekeeper [08:16] hi [08:17] i can't manage to checkout a bzr:// branch via a ssh tunnel.. is there something specific needed to setup that? [08:17] to setup the tunnel i do: ssh -nNL 4155:bling:4155 sshhost [08:18] philn, hi, do you want to tunnel ssh over ssh, or have bzr listening on a tcp port on the other side of the tunnel [08:18] I wouldn't think anything special would be needed. [08:18] i have bzr serve listening on the remote side [08:18] try adding -v to ssh and see if it tries to connect? [08:18] when i do telnet localhost 4155 it wors [08:18] potentially silly question, why not just use bzr+ssh? [08:18] ok [08:19] jonnydee: your bugfix has landed in bzr.dev [08:19] err, dunno ;) i should try that maybe ;) [08:19] philn: and so you're trying to checkout using "bzr co bzr://localhost/..."? [08:19]