=== spiv_ is now known as spiv [01:10] hi all [01:13] Hi poolie [01:13] hi spiv [01:13] spiv, today, starting quite soon, i'm going to try to improve the broken lock bug [01:14] bug 619872? [01:14] Launchpad bug 619872 in Bazaar "bzr: ERROR: exceptions.ValueError: tag/value separator not found in line '' reading lockdir info file (affected: 1, heat: 6)" [Medium,Confirmed] https://launchpad.net/bugs/619872 [01:14] sorry, no, the failure to release locks when we're interrupted [01:15] Ah, good, because I'm working on 619872 :) [01:15] great [01:15] so you said the file is full of nuls? or has a nul? [01:15] Well, rio.read_stanza(['\0']) is the minimal way to reproduce that exact error text. [01:16] But any number of nuls would do it too. [01:16] :/ [01:16] i'm glad we know why it's happening but that's a bit disturbing [01:16] i wonder how they get in there? [01:16] (pedant: any natural number...) [01:16] ooh, that's high grade pedantry [01:16] :) [01:16] it reminds me of the other duped bug that was active recently where the dirstate file is truncated [01:17] well, it's not very similar, but both are "don't trust the filesystem too much" [01:17] I think some filesystems like XFS have been known to leave files full of nuls after a crash? [01:17] many unix things can [01:17] if the inode update is not strictly ordered with writing data [01:17] xfs may be more prone too it [01:18] some have had the bug that you could get bits of other files [01:18] but that's a bit of a security bug and should be uncommon [01:18] Right. And ext3/4 has a bunch of different options that affect that sort of thing. [01:19] The original bug report in this case appears to be windows. [01:24] * fullermd supposed you mean "... appears to be ON windows", but it's funnier read the other way 8-} [05:13] hi spiv, teddy? [05:14] i'm getting in to the locking bugs [05:14] i might try just closing a terminal with bzr running, holding a lock [05:14] and see what signal it gets and what it does after that [05:14] bam [05:14] presumably sighup [05:14] ? [05:14] mental image [05:15] row of bzr's lined up [05:15] in terminals [05:15] and a firing squad [05:15] mm [05:15] trying to get them at just the right moment [05:15] <- strange [05:15] I think thats a great way to get a handle on what is happening to python/bzr [05:16] in some ways i think coping with broken locks by detecting them and asking if you want to steal them, would be better [05:16] something you may have already considered is adding some instrumentation to capture in .bzr.log signal info [05:16] poolie: we should definitely do that for same-machine locks [05:16] i know [05:17] * lifeless was emitting moral support [05:17] poolie: I'm pretty sure it's SIGHUP [05:18] poolie: I did an experiment along those lines a while back, registering a SIGHUP handler in a simple python script, blocking on raw_input(), then closing the terminal [05:18] it is [05:19] i'm not confident that unwinding will really fix all cases but it's worth a try [05:20] Hmm, if you want to be fancy you could subclass KeyboardInterrupt as SIGHUPInterrupt, and raise that from a SIGHUP handler. All code that expects to let KeyboardInterrupt propagate would still work, and then at the top level you could potentially distinguish from KI in cause you didn't want to print 'bzr: interrupted' or whatever. [05:22] poolie: one more suggesting [05:22] poolie: when stealing, we should run some recovery / check code for half-done alterations to the repository in particular (think pack-names). [05:31] good idea spiv [05:31] and lifeless [05:31] i might hook it from library_state.py [07:10] hi, I managed to break bzr http://paste.pocoo.org/show/257906/ [07:13] all I did was 'bzr remove --keep ' for a dir with versioned files and then it all went breaking here and there [07:24] CcxCZ: hmm, that sounds familiar [07:28] CcxCZ: hmm, sounds like another instance of https://bugs.edge.launchpad.net/bzr/+bug/377261 [07:28] Launchpad bug 377261 in Bazaar ""AssertionError: name u'COMPILING' already in parent" in _generate_inventory when running "bzr update" (affected: 4, heat: 2)" [High,Confirmed] [07:31] CcxCZ: FWIW I can't reproduce with simple experiments locally with 'bzr remove --keep' of a dir, if you can figure out a recipe to reproduce your scenario that would be very helpful [07:32] CcxCZ: also there's a comment on https://bugs.edge.launchpad.net/bzr/+bug/504836 (a dupe of 377361) that may help you recover [07:32] Launchpad bug 504836 in Bazaar "AssertionErorr "name .. already in parent" during _generate_inventory (dup-of: 377261)" [Undecided,New] [07:32] Launchpad bug 377261 in Bazaar ""AssertionError: name u'COMPILING' already in parent" in _generate_inventory when running "bzr update" (affected: 4, heat: 2)" [High,Confirmed] [07:35] ok, I'll mark myself affected and I'll post my dirstate [07:36] CcxCZ: thanks! Please give as much info as you can about how you got into this state, e.g. any uncommitted changes or merges before you ran bzr remove --keep. [07:41] hi all ! [07:47] Hi vila [07:47] \o_ [07:55] spiv: submitted. Right now I don't have time to try to reproduce it, but I have the state archived. === spm` is now known as spm [08:08] CcxCZ: great, thank you! [08:44] spiv, so as i feared, once we try handling sighup as an exception, we get back into ERESTARTSYS [08:53] hi vila? [08:53] poolie: hey ! [08:53] hey there, how are you? [08:54] poolie: I'm fine [08:54] i just saw, catching up on my mail from last week, that the january epic is already settled [08:54] that's truly epic organization :) [08:54] ;) [08:57] Yeah, found your pm only this morning, we need to discuss before end of September (we should have booked the flights by then) [08:58] poolie: :/ [09:00] poolie: one option would be a C module like https://code.edge.launchpad.net/~spiv/bzr/sigwinch-without-signalmodule-583941 proposed, or only enabling this on sufficiently new CPython. (>= 2.6.6, IIRC) [09:00] (2.6.6 is the version where signal.siginterrupt actually works) [09:01] :/ [09:01] that seems like quite a heavy solution considering that this probably won't fix most unlocking cases [09:01] IIRC the C module would provide basically the same behaviour as Python gives us for SIGINT [09:01] i feel a bit more convinced now it would be better to just let the lock break and then cope gracefully later [09:02] poolie: as in better stealing a broken lock ? [09:02] FWIW I'm trying to be pretty rigorous with tests and corner cases with my lock work atm. [09:02] vila, yes- [09:02] spiv, i'm glad, but what's the connection? [09:03] poolie: lock :) [09:03] ? [09:03] Well, coping with perhaps damaged locks, e.g. incompletely written. [09:03] oh ok [09:04] i was just confused because in the cases i'm thinking about, there's not much we can do about them [09:04] these are stale locks left behind by bzr dying, or the connection dropping [09:04] Right. [09:04] or, for example, it being killed by sighup [09:06] My hope is that what I'm working on will make the recovery from those situations very reliable, because as you say they are hard to avoid. (And in the worst case the network connection drops midway and there's nothing bzr can do to tidy up the interrupted op) [09:07] Or after a messy system crash. [09:07] It's a bit of a shame to have to code to cope gracefully with non-bzr things failing, but that's life :) [09:08] oh, maybe we're overlapping then? [09:08] i was going to put this aside and give you a ui to break locks interactively [09:08] and to detect locks probably owned by dead processes [09:09] Hmm, I don't think there's much overlap, I think probably what I'm doing is more complementary. [09:09] Partly test coverage, partly giving better exceptions in previously unexpected cases. [09:11] Anyway, it's past 6 on a Friday, and V is letting me know by reaching for my laptop, so I should go :) [09:13] :) [09:13] i hope you're all getting better? [09:22] meh, how can "error: (32, 'Broken pipe')" be uncaught by a "except (OSError, IOError), e:" ? [09:23] err, followed by: if e.errno not in (errno.EPIPE,): [09:23] raise [09:26] vila, i'm guessing it's actually a socket.error [09:26] which is a different beast [09:26] annoyingly [09:26] i think that's what it was [09:36] gimme my bazaar.launchpad,net back [09:36] i know! [09:37] ha, planned hardware maintenance: http://identi.ca/launchpadstatus/ [09:37] one hour to go [09:39] hmm, I can delay writes, but reads...WIBNI a read-only version was always available... [09:43] poolie: hmm, socket.error, sounds like it. We have some latent bugs then [09:44] there's a bug asking for readonly mode [09:44] though if it's a hardware fix, this might be a case where we can't do it [09:44] there's also a bug about socket.error [09:45] vila, i'm thinking of giving the ui a concept of "does the user want interaction on topic x" [09:45] then we can just turn that off for break-lock [09:46] indeed perhaps that's overcomplicated and they just want "is this interactive at all?" [09:46] poolie: yup, that matches up with having config variables accepting y/n/ask instead of being pure boolenas [09:47] booleans even damn delete key being too close to enter ;) [09:47] I thought about that recently, we ould then have a '-y' generic option that would always reply yes without any user interaction [09:48] s/ould/could/ [09:48] right, i'm thinking we should just have that poke some state in to the ui factory [09:48] we could also add configuration options to control them [09:49] yup, that could be implemented as a ui factory, which will be well suited for script usage [09:50] then you could chose between the config variable and the generic option or any combination that fits [10:05] should we have a separate concept of asking whether you want to be interactive, or should the core ui code perhaps just ask "should i break the lock" [10:05] it's a bit look-before-you-leap [11:09] vila, what do you think of http://pastebin.ubuntu.com/487726/ [11:09] i might sign off in a bti [11:13] poolie: at first glance: I was thinking of a specific type for config values but I don't have a clear answer about how that should interact with the ui factory, having to register all interaction messages won't scale right [11:15] I think the idea was that the code still calls get_boolean, but if the value is 'ask' then the user is prompted, unless the ui is non-interactive [11:16] that doesn't say what the default should be though, but I think get_boolean can already return None [11:16] I kind of feel that the message and the default value should stay in the caller code... [11:18] it seems useful to me to put the message into the ui so that guis or whatever can present it differently [11:18] poolie: and when I mentioned the '-y' option, I was vaguely thinking about a '--no' option too, but the consequences are unclear... [11:18] rather than just getting a string [11:18] right, and i18n issues too, but doesn't get_boolean accept a prompt parameter (or should it ?) [11:18] i was going to then make the ui look up that value in the config [11:19] it does take a prompt parameter [11:19] but it folds the other parameters in to it [11:19] whereas really we want to do i18n before that [11:21] Well, if we reach the point where every constant is a config variable, then, msgs are constants... [11:22] so what do you think? [11:22] so your idea will fly too but turning all messages into constants... will not rejoice everybody :-/ [11:23] heh, i wouldn't like that [11:23] but i have a feeling that ui interactions are a bit different [11:23] i suppose you're right though that it does make things more indirect that you can't just put them inline in the code [11:23] I think I prefer get_boolean(prompt=xxx) rather than interaction_messages, the prompt is indeed an interaction message [11:26] i think for setting interactivity and defaults [11:26] to step back a bit, my primary example was uncommit's 'Are you sure' , where I'd like a config variable [11:27] and for letting the ui do something smarter than just showing the string [11:27] we want a symbolic name, not just a string to match against [11:27] but perhaps not? [11:27] err, as far as config variables are concerned, strings *are* symbolic names [11:28] so what would you anticipate having in the config file? [11:28] "Are you sure" = no? [11:28] ho no ! You mean that get_boolean is called with a prompt and without referring to a config variable [11:29] i thought that's what you were asking for [11:29] in this case, we need config.get_uncommit_xxx() which implementation check for a conf var and fallback to get_boolean('Are you sure') [11:30] to ui.get_boolean('Are you sure') so GUIs still have a chance to trigger [11:33] i'd like to keep it at still just one line for application code [11:34] so how about ui.get_boolean('Really uncommit?', id='bzr.uncommit.confirm') [11:35] could be: config.get_user_option('bzrlib.builtins.cmd_uncommit.confirm') hehe [11:35] heh, something like that [11:35] aliases could be defined [11:36] and then the other thing to change is to put the variables to insert into it as kwargs to the ui factory [11:36] so, it depends on whether config can use ui or the opposite [11:36] right [11:36] i could go either way [11:37] but i think of ui as being a somewhat higher-level thing [11:37] so far I think there is only get_user_option_as_bool that uses config, I don't know if ui uses config [11:38] I don't know, a user should be able to configure the ui to use but that doesn't forbid all uis to implement all the needed features [11:39] istm that making them symbolic would also possibly help later with testing [11:39] The uis use env variables which can be seen as config vars as well [11:39] right, they should be unified [11:41] indeed, partly by defining where they are in the config layering and partly by defining either aliases (to the corresponding config var) or a rule to go from one to the other [11:42] just keeping things at a more symbolic layer rather than going straight down to strings would be good [11:42] oh, rules from env variables to config things? yes [11:42] yes [11:44] I think my (slight) disagreement is the same regarding the show_xxx and warn_xxx additions to ui, my feeling is that this shouldn't be part of ui but stay in application code [11:45] I understand the benefits to group them in a single place but... dunno, something feels wrong [11:48] do you mean things like show_cross_format_fetch? [11:48] yes [11:48] ah ok, so show_user_warning is already going in this direction isn't it, of having a warning_id [11:49] by the way, 'bzrlib.builtins.cmd_uncommit.confirm' is accepted as a key by configobj (I just checked) [11:49] that's what I'm saying, yes [11:50] ok [11:50] on the other hand, we also have a config.suppress_warnings [11:51] which is to say, we may need more data points to triangulate :) [11:51] ok so it seems like we agree there should be an id passed through [11:51] and the arguments should be separate === poolie changed the topic of #bzr to: Bazaar version control | try https://answers.launchpad.net/bzr for more help | http://irclogs.ubuntu.com/ | Patch pilot: jam | bzr 2.2.0 is out [11:52] (still needs an announcement i think) [11:52] and this id is probably the config var [11:52] right [11:53] so there's the question of whether the standard text for it should be inline in the caller [11:53] stepping back to the application code, I want a boolean, I may describe its intent to the user with a string (which could be the docstring for the default value of the config var) [11:53] which will make it easy to see how to add new ones [11:53] hehe [11:54] ok? [11:54] all this proposals came from my thinkings about config vars to be honest [11:55] i think it would be nice to have purely declarative stuff for config vars [11:55] no methods per var [11:55] exactly [11:55] except those that have a calculated default [11:55] even them [11:55] and then describe where it's read from and what env var it may map to [11:55] well, no method, just a callable [11:55] so in this particular one [11:55] we can declare them essentially inline at the place that uses them [11:56] which may be reasonable if they are mostly used from one call site [11:56] or we could have a central registry [11:56] not even a callable, we can manage to have declarations in bzrlib.config or just a registry so anybody can declare them inline [11:56] hehe [11:56] well, if it's for example determined by reading /etc/mailname then at some level there needs to be a callable [11:57] right, the default value can be a callable [11:57] which in turn can use othe conf vars, haha, only serious [11:58] and of course plugins can override the default value... endless fun [11:58] ok, so [11:58] in this context, the user can speak too, hence the 'ask' special value [11:59] adding id and args parameters to get_boolean and friends is a small change for the callers [11:59] and probably makes it maximally easy to implemetn new uis [11:59] ohhhhh, yeah ! That's the ui side ! [12:00] hmm, so the conf var docstring can be a format, hence the kwargs [12:00] then i guess if for some thing we want to take the default text out of the application code, we could just make the text parameter optional [12:00] what's the conf var docstring? [12:00] so there's the question of whether the standard text for it should be inline in the caller [12:00] stepping back to the application code, I want a boolean, I may describe its intent to the user with a string (which could be the docstring for the default value of the config var) [12:01] hm, and where would that be in the code? [12:01] I don't know if this will work for all cases but the ui level don't have to care [12:01] in the registry [12:02] that is, if you use conf vars, you pass the docstring to the ui [12:02] if you get straight to the ui, well, you provide it, so it will be inline in the caller [12:03] if you go straight to the ui, well, you provide it, so it will be inline in the caller [12:03] when you said ui higher than config, I thought, well, orthogonal [12:06] so do you still see the app code calling a method on the ui? [12:06] if there is no config variable, yes [12:06] i would have thought we'd have a clear distinction between things that are meant to behave like config variables, and things that are possibly user interactions [12:07] ISTM that some/most? of the user interactions can be linked to a conf var [12:08] mm, but i think specifically blocking and asking the user something needs to be taken account in the code in a way that looking up a config variable does not [12:08] maybe not [12:08] warnings about format deprecations, confirming uncommit, forcing push/pull/commit [12:09] that's why I say conf vars, newbies may want to be asked whether their push is valid, experienced users hate that [12:09] oh, i definitely agree that should be configurable [12:09] i just think that the intent of the app code is "here's something to possibly warn about" not "look up a configuration variable" [12:10] hmm [12:11] ok [12:11] i'm tired; i should stop [12:12] but the code flow may depends on the user answer or a conf var... I don't have a definitive answer, both config and ui are involved it may be clearer for the application code to refer to ui, may be the call itself should be ui and the parameters from config, or just hidden in the call [12:13] istm there are going to be a number of callers [12:13] and we want them to have the simplest experience we can [12:13] so at least a facade that covers up checking with either config or the ui [12:15] sounds good === ddaa1 is now known as ddaa === rml_ is now known as rml [13:25] hi can i hook a shell script which would be executed on the server as soon a rev is pushed to it? (bzr+ssh) === Ursinha is now known as Ursinha-lunch === Ursinha-lunch is now known as Ursinha === basic_ is now known as basic` === beuno is now known as beuno-lunch [18:11] starenka: look into the "post_branch_tip_changed" hook. It is a python hook, but you can have it hook into a script [18:11] I think jelmer also had a plugin that exposed bzr hooks to shell scripts === beuno-lunch is now known as beuno [19:08] jam: thanks === james_w` is now known as james_w === deryck is now known as deryck[lunch] === deryck[lunch] is now known as deryck [20:31] For all the guys who asked for testtools on FreeBSD... Here it is: http://www.freshports.org/devel/py-testtools/ [20:31] Just ported it and it got accepted today :-) [20:32] this is in particular news for fullermd :-) [20:57] C-S: \o/ [20:57] jelmer: what you guys wanted, right? [20:58] C-S: Well, I don't use FreeBSD but it's great to have testtools available on more systems. [20:58] jelmer: You should give it a try! FreeBSD is just a dream :-) [21:13] :) [21:13] I've used it in the past and quite like it [21:17] C-S: Sweet :) === _thumper_ is now known as thumper