[07:21] <poolie_> hi vila
[07:21] <vila> poolie_: hey, not fully here yet ;)
[07:21] <poolie_> i'm not fully gone, but i will
[07:21] <poolie_> be
[07:21] <vila> reading the thread about locales on python...
[07:22] <vila> finger syndrom
[07:22] <poolie_> L
[07:22] <poolie_> ?
[07:22] <vila> the user is wrong, python is wrong, bzr get the finger :)
[07:23] <poolie_> i like the way python and linux are pointing at each other
[07:23] <poolie_> oh well
[07:23] <poolie_> Joyeux Noël
[07:23] <fullermd_> Linux?  People still use that thing?
[07:23] <vila> http://paste.ubuntu.com/778346/
[07:24] <vila> poolie_: hehe, thanks :)
[07:26] <poolie_> more seriously perhaps we should back out gz's hack
[07:26] <poolie_> anyhow
[07:26] <poolie_> night all
[08:14] <fullermd> Man, it sure did get quiet this week.
[08:19] <vila> It all happened while you slept ?
[08:20] <fullermd> All sorts of things happen while I sleep.  Like, where did all the trilobites go?
[08:25] <vila> no doubt about that: australia
[10:49] <jelmer> vila: let me know if you'd like to discus feature flags some more
[10:50] <jelmer> I've updated the docs
[10:51] <vila> I'll look into it, I was reviewing switch-possible-transports and there are still 2 test failures :-/
[10:51] <vila> comment posted
[10:52] <jelmer> vila: switch-possible-transports has two prerequisites, one of which isn't approved either.. so there are other branches I'm more interested in tbh :)
[10:53] <vila> yeah, well...
[11:01] <jelmer> vila: that's not to say I don't appreciate the review..
[11:05] <vila> so, if the feature name is global, _present_features should contain all features provided by the installed code base (ie. bzr and plugins) right ?
[11:06] <jelmer> yep
[11:06] <vila> hmm, I was about to ask for some help associated with the feature but that's useless for features that are not "installed"
[11:07] <vila> (thinking out aloud)
[11:08] <vila> lock_write raises for write acces, but what about read access ?
[11:09] <vila> if the format talks about feature, why does each line says feature ? Didn't we settle to put all features on the same line for each kind of necessity ? And consider all unknown necessities as required ?
[11:10] <jelmer> vila: All unknown necessities are considered required by the existing code
[11:10] <vila> (given that when a plugin provides a feature it's present even if the necessity s unknown)
[11:10] <jelmer> vila: I don't think we discussed the layout
[11:10] <vila> ha, misunderstanding then :)
[11:11] <jelmer> vila: I agree there are in concept two lists at the moment - the optional and the required features
[11:11] <vila> and optional additional lists for unknown necessities
[11:12] <vila> the core issues are still the same whatever format we chose :)
[11:12] <vila> I'm trying to agree with you on the simplest one that allows the most robust parser :)
[11:13] <vila> we're close to a simple scanner and scanners are more robust than parsers
[11:13] <jelmer> what's the difference?
[11:14] <vila> wow, many
[11:14] <vila> roughly a scanner doesn't need to take a context into account nor handle error recovery
[11:14] <vila> it sees a stream of text and split it into tokens
[11:15] <vila> a parser sees a stream of tokens and recognizes more complex constructs
[11:15] <jelmer> vila: right, I don't see how we can really avoid a parser?
[11:15] <jelmer> we'll need one in either case, even if it's very simple
[11:16] <vila> right, scanners are often called parsers even where there is no grammar to parse
[11:16] <jelmer> I think there is some really basic grammar here :)
[11:17] <vila> no, only tokens separated by spaces
[11:17] <vila> and newlines
[11:17] <jelmer> grammar exists over those tokens
[11:18] <vila> well, if you insist... but it's so trivial it doesn't need a parser
[11:18] <jelmer> we only allow "optional feature foo", not "foo bar bla"
[11:18] <vila> anyway, forget about the distinction
[11:18] <jelmer> sure
[11:18] <vila> I'd prefer: optional foo, bar
[11:18] <vila> required biz, bob
[11:19] <vila> 'feature' is just noise here
[11:19] <jelmer> vila: I've considered that as well, but it's less extendable in the future
[11:19] <jelmer> it doesn't allow adding extra data at the end (like per-feature parameters), or data other than features
[11:20] <vila> we're back to the start of yesterday's discussion ?
[11:20] <jelmer> maybe a bit
[11:20] <vila> we ruled out per-feature parameters are being under plugin responsibility
[11:21] <vila> I fail to see why you want to open for data other than features on the basis that your actual parser is robust without desmonstrating that
[11:22] <vila> i.e. if you don't clarify the unknowns I can't see what you're talking about
[11:22] <jelmer> vila: I don't want to make extension of the format file in the future impossible
[11:22] <jelmer> I can imagine us wanting to add other things to the format file than features, or to add more data per feature
[11:23] <jelmer> "optional index bar"
[11:24] <vila> same as optional bar
[11:24] <vila> index is under bar responsibility
[11:24] <jelmer> vila: no, because it's different from "optional feature bar"
[11:25] <vila> does it change the fact that the format can be read if the feature is not there ?
[11:25] <jelmer> vila: it's something entirely different, unrelated to features
[11:25] <jelmer> vila: "bar" is not a feature in "optional index bar". it's something entirely different
[11:26] <jelmer> vila: it can be read, as it's optional too - and the parser won't barf on that
[11:26] <vila> what's the API for that ?
[11:26] <vila> who benefits from it ?
[11:27] <jelmer> vila: there is no API, it's to allow future extension of the format file
[11:28] <jelmer> vila: we didn't foresee features in the past, but the reason they're possible is that older versions of bzr read the entire format file, not just the first line
[11:29] <jelmer> vila: I'd like to keep that flexibility present in case we have something similar in the future, that's why the format is a bit more verbose
[11:29] <jelmer> vila: perhaps we can fold it into one line still, e.g.:
[11:29] <jelmer> "optional features = foo, bar, bla"
[11:29] <jelmer> "required features = blie"
[11:30] <jelmer> that'll still be a bit ugly if there are going to be many and the line doesn't wrap, but I could live with that.. :)
[11:30] <jelmer> I do like it more verbose though, also because it means we can add more per-feature things in the future if we wanted to for some reason
[11:31] <vila> that's a path I don't want to follow, you're opening a can of worms with no idea about what kind of worms will come out
[11:31] <vila> why ?
[11:31] <jelmer> vila: how is this opening a can of worms?
[11:32] <vila> interop, upgrades
[11:32] <jelmer> I'm not proposing we add anything else. I just don't want to paint ourselves into a corner.
[11:33] <vila> why don't you want to get out the corner we're in right now with small and safe steps ?
[11:33] <jelmer> vila: we're not in a corner now - we have plenty of room to extend the existing format
[11:33] <vila> "anythin else" == the unknowns I'm referring to,
[11:34] <vila> allowing optional features and required features is already a huge opportunity worth exploring with limited risks,
[11:34] <vila> what's the point of raising the risks ?
[11:34] <jelmer> how is this raising the risks? I don't see how there are any additional risks unless we actually extend the format further.
[11:34] <vila> then why allowing it ?
[11:35] <jelmer> The only thing I want to prevent is that we can't extend it further if we want to, in the future.
[11:36] <jelmer> how is that raising the risks?
[11:36] <vila> we've already discussed that, I proposed an already supported format allowing a huge kind of tricks and we couldn't agree on what should be recognized there to allow a single backport
[11:37] <vila> designing an extensible format is more than splitting lines (or hairs ;)
[11:37] <vila> ruling out ini format on the basis that it's too much overhead but not comparing it to an existing implementation... is questionable
[11:38] <vila> and almost irrelevant until what is taken out from it clearly defined
[11:38] <vila> and my understanding was that you wanted to extract only: is there a required feature there or something that should be considered like a required feature
[11:38] <vila> nothing else
[11:39] <jelmer> vila: that is what I want to extract, and the only thing the current parser does today
[11:40] <vila> but what guarantees does it offer that your unknown future additions will still be parsed and ignored or considered required ?
[11:41] <jelmer> vila: it means we *can* add more stuff in the future apart from features, without having older clients blow up in our face too badly
[11:42] <jelmer> I have no idea what that stuff is, but it seems sensible to keep the option open
[11:42] <vila> meh, I'm saying your parser doesn't allow that while the ini format does and you're just replying: no
[11:43] <vila> how can you say 'I have no idea' and 'without blowing' ?
[11:43] <jelmer> vila: how does my parser not allow that? It allows opening things if there is a line that starts with "optional " that it doesn't understand
[11:44] <jelmer> anything that doesn't start with optional it *will* complain about
[11:46] <jelmer> so let's back up
[11:46] <jelmer> you would prefer something like "Bzr format x\noptional foo,bar,bla\n" ?
[11:47] <vila> yes, preferably allowing 'required biz,bob\n' and everything else being handled as 'required you-can-t-represent-me-anyway'
[11:48] <vila> which give us the ability to support older bzr release and experimenting like mad with bzr.dev
[11:49] <vila> no doubt we will gather a lot of understanding
[11:49] <jelmer> vila: could we do
[11:49] <vila> one point I don't get:
[11:49] <jelmer> "required features biz,box\n" ?
[11:49] <vila> are you trying to never require format bump at all ?
[11:49] <vila> s/ features// :-p
[11:50] <jelmer> vila: I would really like to have the namespace just in case we need it
[11:50] <jelmer> I can understand the redundancy in having each feature on a separate line and repeating "optional feature ..."
[11:50] <vila> we're adding one, you're asking for two or mores ;)
[11:51] <vila> meh, we're adding two: optional, required
[11:52] <jelmer> vila: that's just one - they share a namespace
[11:52] <vila> meh again, yup
[11:52] <jelmer> vila: we don't want to bail on everything we encounter that doesn't start with required or optional
[11:52] <vila> with an attribute being 'optional, required' but let's add 'read-optional' at least :)
[11:53] <vila> we won't bail, we will handle it as required
[11:53] <vila> so even corruptions will be seen as required...
[11:53] <jelmer> vila: but then we don't have the ability to ever extend the file again
[11:53] <jelmer> and this is why I would really like to have that word "features" in there
[11:53] <vila> what difference will that make for older bzrs ?
[11:54] <vila> none if nothing is backported ?
[11:54] <jelmer> vila: if we add something other than "features" in the future, but it's optional, they will know to ignore it
[11:55] <vila> why can't a plugin does that with only optional ?
[11:55] <vila> s/does/do/
[11:56] <jelmer> vila: why can't a plugin do what with only optional?
[11:56] <vila> adding something other than features
[11:57] <vila> that *is* optional
[11:58] <spiv> Add bugs rather than features, maybe? ;)
[11:58] <jelmer> vila: it's not really plugins that I have in mind, it's whatever we as bzr devs add in the future
[11:59] <spiv> I'm very new to this topic, but I'm not sure what sorts of flags you could have in a format that couldn't be sensibly called "features" — parameters maybe?
[11:59] <jelmer> spiv: possibly
[12:00] <spiv> (Although parameters strike me as something I'd probably prefer to have in a separate file, rather than encouraging anyone from editing format files!)
[12:00] <jelmer> spiv: I'm mostly concerned that we if we only have one namespace, we can't ever add anything else
[12:00] <spiv> Hmm.
[12:01] <spiv> So if they are one-per-line, I'm not sure there's much to worry about
[12:01] <vila> format bumps allow adding anything, are you trying to entirely avoid format bumps ?
[12:01] <jelmer> spiv: the discussion is (among other things) about whether they should be one line :)
[12:01] <spiv> Because "optional non-feature=17" would still be usable backwards and forwards.
[12:01] <jelmer> vila: yes, I would realy like to avoid another one
[12:02] <spiv> vila: "entirely" is an extreme goal, but "greatly minimise" sounds like a good (and plausible) goal to me
[12:02] <vila> spiv: precisely my question
[12:03] <spiv> Well, I'm not sure the distinction matters really?
[12:03] <jelmer> vila: if we have them in a list, it greatly limits what can be in there
[12:03] <spiv> Because we already have the fallback to format bumps if we find no other way, but if we try hard to not have any more we might just achieve that.
[12:04] <vila> well, I see feature flags as a way to *prepare* format bumps
[12:04] <spiv> vila: I like the way ext2 → ext3 → ext4 has evolved surprisingly smoothly with in-place upgrades
[12:05] <spiv> vila: and it's in part due to having something quite like feature flags AIUI
[12:05] <vila> format bumps indeed imply smooth upgrades
[12:05] <jelmer> http://pastebin.ubuntu.com/778530/
[12:05] <vila> but saying feature flags will allow avoiding format bumps without having experimenting with them...
[12:05] <spiv> Do you mean “imply we need to add support for smooth upgrades?”
[12:05] <jelmer> that's basically the different options as I see them
[12:06] <vila> spiv: we need to improve the upgrade experience, there is a consensus that we are not there yet
[12:06] <spiv> Because that's not what we have now, nor quite how I feel the ext2/3/4 example has worked
[12:06] <jelmer> or http://pastebin.ubuntu.com/778531/, with read-optional
[12:06] <spiv> I'd say that for years now, format bumps have implied very rocky upgrades in practice
[12:07] <spiv> And rather than just saying "well we'll fix that next time", perhaps we should look for a different path entirely
[12:07] <jelmer> spiv: (1) is the current format, (2) is what vila is suggesting, (3) and (4) are alternatives that I could also live with
[12:08] <vila> spiv: yup, but the code has evolve a bit since the last bump but some more polish is still needed
[12:08] <jelmer> spiv: Yeah, I think ext2/3/4 is indeed a good example of a nicer way of doing this
[12:08] <vila> jelmer: you didn't mention required ?
[12:09] <jelmer> spiv: (and they don't have the problem of people sending each other disk blocks over the interwebz)
[12:09] <spiv> We may need to do something to make the bump to with-feature-flags less rocky than in the past, but my point is that feature-flags is a highly plausible route for much smoother upgrades in future
[12:09] <vila> spiv: we all agree on that
[12:09] <jelmer> spiv: enabling feature flags doesn't require a format bump, the only real bump is when you actually enable a feature
[12:09] <jelmer> vila: yeah, this is mostly an example - I cna add some if you like
[12:09] <vila> lunch time here, daughters calling
[12:10] <spiv> jelmer: my initial preference is for 4
[12:10] <vila> jelmer: nope, just checking (the first line may vary too)
[12:10] <spiv> jelmer: if it's flexibility you want, that way has got it:
[12:10] <jelmer> http://pastebin.ubuntu.com/778535/
[12:11] <spiv> jelmer: you could e.g. imagine instead of a registry of feature-flag keys, a registry of "can-you-interpret-feature-line" functions
[12:11] <spiv> jelmer: initially all we might have are one-word keys, and indeed a single function that looks for and understands those
[12:12] <jelmer> spiv: yeah, that makes sense
[12:12] <spiv> jelmer: but it leaves scope for future readers (including the same version of bzrlib with shiny new plugin) to do arbitrary fancy stuff if you really want
[12:12] <vila> jelmer: bbl, using examples is far clearer, add garbage in all places where it would be ignored or seen as required and we should get a far better picture
[12:12] <vila> and I'm a big fan of registries too ;)
[12:13] <jelmer> spiv: that's indeed the sort of option I would like to keep open
[12:13] <spiv> jelmer: I'd think probably we don't need to worry about the slight bloat vs. all-one-line, because it'd take quite a few features to have this exceed even 1k
[12:14] <spiv> The one-per-line format is also dead-simple to parse (or tokenise ;)
[12:15] <jelmer> spiv: yep :)
[12:15] <spiv> And also easier to add new flags to than a comma-separated format, assuming no ordering restrictions: just "echo 'optional new-feature' >> format"
[12:16] <spiv> (unless you're on a platform with a non-\n EOL :)
[12:16] <spiv> Hmm, one other comment:
[12:16] <spiv> I'd actually suggest changing the first line
[12:17] <spiv> For the zero-flags case, where it's the same as today, sure keep it the same.
[12:17] <spiv> But otherwise a new format would be good as a hint to users with older bzrs trying to google for errors
[12:18] <jelmer> spiv: I'm not sure
[12:18] <spiv> It'll be slightly inconvenient I'm sure
[12:19] <jelmer> spiv: It makes things hard as features are removed again
[12:19] <jelmer> spiv: and we'd backport this to older series, so hopefully most people will end up with a version of bzr that understands it
[12:19] <spiv> But it will avoid giving confusing errors where the user looks at a big long string that has the first 90% the same as another big long string that works
[12:20] <spiv> Which is very much the sort of error that will confuse people, because which users actually read every byte of the format string in that sort of error?
[12:20]  * vila passing around, jelmer: isn't this turning into something a prober should handle ?
[12:21] <spiv> jelmer: It definitely does suck for the "remove last feature flag from format" case.
[12:21] <spiv> jelmer: but that's a modest code inconvenience in exchange for confusing unlucky users less
[12:22] <spiv> Think of it as part of the "making upgrades smooth" goal that we all agree on ;)
[12:23] <spiv> Anyway, that's just some quick opinions before bedtime.  You're welcome to disagree, I know you've been thinking about this for more than the 10 minutes that I have been!
[12:24] <spiv> jelmer: parting thought: do you have concrete examples of feature-flags you think we might have?
[12:25] <spiv> jelmer: the case that springs to mind for me is some of the optional look-aside indices that we've sometimes talked about adding to repos (and in some cases have plugins for, like search or revnocache)
[12:26] <vila> spiv: nested-trees, git-cache, bzr-search-index
[12:26] <spiv> jelmer: I could imagine e.g. an "optional revision-graph-heads-cache"
[12:27] <jelmer> spiv: yep
[12:27] <jelmer> spiv: that also seems like something that could be read-optional - i.e. you can access the repository without having it installed, but if you write to it you need to have the feature
[12:27] <spiv> jelmer: (readers that want to use that cache would need to do something like compare the current pack-names with the pack-names at the time the cache was last regenerated, in case a writer without the flag added revisions)
[12:28] <spiv> jelmer: or that!  But that's less smooth ;)
[12:28] <spiv> (great for safe prototyping though before adding the extra backwards-compat polish)
[12:29] <spiv> jelmer, vila: it intrigues me that with the possible exception of nested-trees these all seem like primarily flags on the repo component
[12:29] <jelmer> spiv: search indexes are on branches
[12:29] <spiv> Ah!  Interesting.
[12:30] <spiv> Anyway, bedtime over here.
[12:30] <jelmer> g'night :)
[12:30] <spiv> Merry winter solstice and/or other season-appropriate greetings, and good night :)
[12:32] <jelmer> hehe, you too !
[12:58] <vila> jelmer: back
[12:59] <vila> jelmer: so, how about probers ? If they can't handle that, why ?
[13:01] <vila> jelmer: and is there a link with all the refactoring you did related to foreign plugins and feature flags ? If not, why ?
[13:12] <jelmer> vila: back too
[13:12] <jelmer> vila: what about probers? The bzr prober only checks for the .bzr/branch-format file, it doesn't try to parse it, that's all in Bzr(Dir)Format
[13:12] <jelmer> vila: feature flags are bzr-specific
[13:13] <vila> but isn't it the part that decide whether or not a format can be opened ?
[13:13] <vila> (and already implement a registry with all the associated flexibility)
[13:14] <vila> I realize a lot of ideas are thrown around against an *existing* proposal which is frustrating but I think it's worth it
[13:15] <jelmer> vila: the prober just returns the format if it can find it
[13:16] <jelmer> vila: if it can't, it raises UnknownFormatError
[13:16] <jelmer> whether or not the format is supported isn't checked until later
[13:17] <vila> ha, ok, and all formats inherit from BzrFormat (with your proposal at least, may be worth splitting :-})
[13:17] <vila> right, there is the check_support_status
[13:18] <vila> and the parsing, sorry, not familiar with this code yet ;)
[13:20] <vila> jelmer: and the other question ?
[13:20] <jelmer> vila: what was the other question (sorry) ?
 jelmer: and is there a link with all the refactoring you did related to foreign plugins and feature flags ? If not, why ?
[13:22] <jelmer> vila: ah
[13:22] <vila> http://paste.ubuntu.com/778596/ (4) with garbage added
[13:23] <jelmer> vila: there is no link, because feature flags are specific to bzr formats
[13:24] <jelmer> vila: that last line would be treated as requiring a feature name "not starting with (optional, read-optional or required) is handled as if a required feature was not present"
[13:24] <jelmer> vila: (so we can add new necessities in the future, while not breaking old versions of bzr too badly)
[13:24] <vila> good for bzr but not good enough for foreign plugins ?
[13:25] <jelmer> vila: it's a format-specific feature
[13:25] <jelmer> vila: we can't write to .svn/format and have svn clients honor that and not break :)
[13:25] <vila> ha :)
[13:27] <vila> hmm, so better use http://paste.ubuntu.com/778603/ then
[13:28] <jelmer> vila: no, that would break the ability to add things to the end of the line
[13:28] <jelmer> vila: what's wrong with the interpretation I mentioned earlier?
[13:29] <vila> feature names should not contain spaces
[13:29] <jelmer> vila: but what does swapping the name and the necessity help?
[13:29] <vila> it makes it clearer what the most important stuff is: the feature name
[13:30] <vila> and matches your implementation, key=name, value=necessity
[13:30] <jelmer> sigh, I fear we're back where we started :(
[13:30] <jelmer> vila: that breaks extensibility again
[13:30] <vila> well, you still haven't answered my initial objections...
[13:30] <jelmer> vila: what were those?
[13:31] <jelmer> vila: you were reasonably happy with (4) right?
[13:31] <jelmer> vila: if you don't see us needing the extensibility, what's the problem with interpreting "everything not starting with (optional, read-optional or required) is handled as if a required feature was not present
[13:31] <jelmer> "
[13:31] <jelmer> as a feature named "not starting with (optional, read-optional or required) is  handled as if a required feature was not present
[13:32] <jelmer> "
[13:32] <jelmer> with necessity "everything" (which, because it is unknown, gets treated as required)
[13:32] <vila> I haven't agreed yet on the format, I'm discussing it
[13:34] <jelmer> I didn't say you had
[13:34] <vila> ha, sorry, misread reasonably happy, let me clarify then
[13:34] <vila> (4) seems like a good alternate for (2)
[13:35] <vila> I want to see where garbage is interpreted
[13:35] <vila> meh
[13:35] <vila> I want to see where garbage is accepted and how it is interpreted
[13:36] <vila> if you want extensibility at the feature level, it makes sense to start with line format where only two things are required: the feature name and the necessity
[13:36] <jelmer> vila: I agree
[13:37] <jelmer> vila: I think the reason I like the necessity first is that it makes the file read like a check list
[13:37] <vila> from there, since additional stuff is feature specific, it makes sense to make the necessity a feature attribute (which your implementation already does)
[13:37] <jelmer> vila: and it means it's possible to allow spaces in feature names (and do fancy stuff later)
[13:38] <vila> meh bad idea, if you start with spaces you want to allow what more ? unicode ?
[13:38] <jelmer> vila: allow spaces, not encourage spaces :) It means we can tack on stuff at the end later /if we want to/
[13:39] <vila> re-read my paste, it allows more optional stuff after the necessity, making it mandatory will require a backport though
[13:40] <jelmer> vila: for now, everything after the necessity would be part of the feature name
[13:40] <vila> and anyway we said parameters should not be part of the format or do we disagree on that ?
[13:40] <jelmer> vila: that way we wouldn't need a backport later
[13:40] <vila> huh ?
[13:40] <jelmer> vila: yeah, parameters shouldn't be added to the format, at least not now
[13:40] <vila> you won't recognize the feature anymore
[13:41] <jelmer> vila: right, and that's the point
[13:41] <vila> shudder
[13:41] <jelmer> better to not recognize a feature than to load it with the wrong data
[13:42] <vila> would you mind mention your motives when you know them instead of waiting for me to discover it  by poking randomly? :-D
[13:42] <vila> s/it/them/
[13:42] <vila> you can get the same effect with less magic by *changing* the feature name when it needs more stuff
[13:43] <jelmer> vila: right, I'm just saying there's no particular reason spaces couldn't be part of the name
[13:43] <vila> I have one such reason: identifiers cannot contain spaces :)
[13:44] <jelmer> vila: who says they'll just be identifiers forever?
[13:44] <vila> what would be the need ?
[13:45] <vila> on the other hand, enforcing identifiers allows useful stuff like mapping to the file system, mapping to python modules, etc
[13:45] <jelmer> vila: as I said earlier - I don't know, but I would like to keep the ability to add stuff open
[13:45] <vila> I don't know either but I won't forbid such uses for no good reasons
[13:46] <jelmer> vila: you mean, you would ? :)
[13:47] <vila> yeah :) I don't want to allow things that will make my life harder
[13:47] <vila> handling spaces in identifiers has always make my life harder
[13:47] <jelmer> vila: again, I'm not saying it would be good practice to allow spaces in feature names
[13:48] <jelmer> vila: and we're not mapping features to anything, they're just arbitrary strings
[13:49] <jelmer> vila: s/allow/use/
[13:49] <vila> you're using them in a set() and a dict() already, the later *is* a mapping
[13:49] <jelmer> vila: not in the sense you meant
[13:51] <vila> ?
[13:51] <jelmer> vila: either way, spaces after feature names wouldn't usually happen.. this is just a corner case of what we do in the parser with when we encounter them
[13:51] <vila> but that's the whole point of defining a format
[13:51] <jelmer> vila: we don't need to map features to python modules, file systems, etc
[13:51] <vila> if it's not defined you can't write a robust parser
[13:51] <jelmer> vila: so you would just ignore everything after a space?
[13:53] <vila> so you can use split(' ', 2) yes or even split(' ', 1) and delegate to a registry as spiv suggested
[13:53] <jelmer> vila: if you don't see the use of ever having spaces in the feature data, what's the problem with just including the in the feature name for the moment?
[13:53] <jelmer> vila: but we're not doing that (delegating to a registry) just yet
[13:54] <jelmer> vila: when we do, then we'll start splitting and passing the rest of the string (after the space) to the feature
[13:54] <vila> that's still a property of the format we're discussing
[13:54] <jelmer> vila: but old bzr versions shouldn't try to open a feature while discarding data in the file
[13:54] <vila> who forbids adding new stuff now ?
[13:55] <jelmer> vila: what do you mean with now?
[13:56] <vila> if old bzr versions should not ignore adding data you can't add it
[13:56] <vila> added
[13:57] <jelmer> vila: why not? I'd rather have the old version of bzr not be able to load the feature at all than load it without the extra data
[13:57] <vila> you lost me here
[13:58] <jelmer> vila: let's say we do add feature parameters in bzr 2.7
[13:58] <vila> I've been advocating a robust format (which we don't have yet) for this reason (don't open what you don't understand) and you've been complaining that it will forbid adding more unknown stuff
[13:59] <jelmer> vila: and bzr-search starts setting "optional search with-authors-data"
[14:00] <vila> bzr-search business, since that requires additional data put 'with-authors-data' there and deal with it
[14:00] <jelmer> vila: we don't want older versions of bzr to open that branch and not see the "with-authors-data" flag when they write to the index - because they wouldn't add the authors data and make the index incomplete
[14:01] <jelmer> vila: sorry, but I still don't still what's not robust about the current format.
[14:02] <vila> it won't be robust until we know what is recognized and what is not and what happens in all cases
[14:02] <vila> there should be no unknown as far as (format + parser) is concerned
[14:02] <jelmer> it's pretty well defined what is recognized and what isn't
[14:02] <vila> there are ways to allow some unknown in the format
[14:02] <vila> where is that defined ?
[14:03] <jelmer> ok, that's a fair point
[14:03] <jelmer> I was going to say.. in the parser :)
[14:03] <vila> pfew
[14:03] <jelmer> the spaces handling isn't defined in the docs or the tests, but all the other stuff is
[14:03] <vila> progress ! :)
[14:04] <vila> yes, many things are defined, some better than others but I don't mind that too much,
[14:05] <vila> what I care about is that there is *no* unknown
[14:05] <vila> and the simplest the format is, the less unknowns we have to care about
[14:06] <jelmer> vila: again, I think that's true for the current format :)
[14:06] <vila> for current == ?
[14:06] <vila> http://paste.ubuntu.com/778603/ ?  http://pastebin.ubuntu.com/778535/ (4) ?
[14:06] <jelmer> vila: "format 4" - basically lp:~jelmer/bzr/feature-flags with " feature " removed
[14:07] <jelmer> in short:
[14:07] <vila> haaa
[14:07] <jelmer> we allow additional lines in the format file, each consisting of: a necessity (not containing a space), a space, and a feature name (anything but a newline), and a newline
[14:07] <vila> hmm, that was as in: 'ok, let's look at a summary to re-sync'
[14:09] <vila> anything but a newline is too magic to my taste without being the only way to provide something
[14:09] <jelmer> vila: it's the simplest and allows for the most extensibility in the future
[14:10] <jelmer> vila: we can disallow registering features with spaces in their name?
[14:10] <vila> better disallow at the source
[14:11] <vila> allowing stuff and then saying "no, you can't" is frustrating :)
[14:11] <jelmer> vila: "be liberal in what you accept, conservative in what you send"
[14:11] <vila> yup, we generate the format file :)
[14:12] <jelmer> vila: I don't want us writing feature names with spaces in them, I want us to be able to deal with and not misinterpret format files that we receive
[14:12] <vila> but me too damn it, why do you think I'm discussing ?
[14:13] <jelmer> ok, perhaps there's some miscommunication here
[14:14] <jelmer> vila: I think we're agreed that we shouldn't allow spaces in feature names?
[14:14] <jelmer> vila: the question is what to do with spaces we find in files we receive
[14:14] <vila> yes (no spaces)
[14:15] <vila> spaces we receive..
[14:15] <vila> first,
[14:15] <vila> we have lines
[14:15] <jelmer> vila: I think we should basically treat anything with a space in it as unknown
[14:15] <vila> as long as we recognize a line, we continue,
[14:16] <vila> when we encounter a line we don't recognize => as if a required feature was not there
[14:16] <vila> ha
[14:16] <vila> what kind of unknown ?
[14:17] <vila> if all unknowns are considered as an absent required feature, it's easy
[14:17] <jelmer> vila: the feature flags doc describes this
[14:17] <vila> ooooh, are you saying you don't want to allow optional parts after the feature name ? Never ?
[14:18] <jelmer> vila: if we encounter "optional foo bar" I want to still load the file, because there is an optional feature we don't know about
[14:18] <jelmer> vila: even if foo is registered
[14:19] <jelmer> vila: if we encounter "required foo bar" I think we should abort
[14:19] <jelmer> vila: the first word is always interpreted as the necessity - unknown necessities are considered equal to "required"
[14:20] <vila> could you write tests for that ? I have a gut feeling there is a hole there
[14:21] <jelmer> vila: there are tests for this
[14:21] <vila> <"optional foo bar" I want to still load the file > but it's an unknown
[14:21] <jelmer> vila: not for the space handling, but for the unknown necessities
[14:21] <jelmer> I'll write some for the space handling
[14:22] <vila> isolating the parser and writing direct tests will probably help
[14:22] <vila> the expected output being: can/can't open
[14:22] <jelmer> the parser is literally three lines
[14:23] <jelmer> but I can split it out
[14:24] <vila> from_string ?
[14:24] <vila> good enough then
[14:25] <vila> but even with 3 lines I think it will choke on 'not-asingle-space-your-split-will-blowup'
[14:26] <jelmer> vila: I'm using .split(" ", 1)
[14:26] <vila> you changed it since the version I'm looking at then :)
[14:27] <jelmer> I've just pushed a version that disallows registering features with spaces in their name
[14:31] <vila> jelmer: on the basis that a feature should not use spaces so can't be recognized if more stuff appears after it in the format ?
[14:32] <vila> grr diverged branches
[14:32] <jelmer> vila: yeah
[14:36] <vila> far simpler :)
[14:38] <vila> and necessity first to be allows to add more in the future (could have worked if it was second too ;)
[14:39] <vila> ordereddict requires python-2.7
[14:41] <jelmer> oh, ok
[14:41] <jelmer> I guess it's not the worst thing in the world if we write things back in the wrong order
[14:42] <vila> when do you write them and from what ? as_string() ? How are they *created* then ?
[14:43] <jelmer> vila: some other API (probably e.g. Repository.update_features) updates format.features and then writes out format.as_string() to its format file
[14:43] <vila> I may miss something but the only place I see self.features modified is by from_string()
[14:43] <vila> ha, but that's not addressed by your proposal right ?
[14:44] <jelmer> vila: indeed
[14:45] <vila> right. well, that would help the discussion but the format looks simpler now, let me recap:
[14:45] <vila> a line is <necessity<space><feature><newline> if feature is not registered => can't open (msg to the user ? NotABranch and such ?)
[14:46] <vila> if a line doesn't match that => can't open (msg ?)
[14:47] <vila> ha no, MissingFeature
[14:47] <vila> jelmer: correct ?
[14:48] <jelmer> vila: if the feature isn't registered (and required) the user indeed gets MissingFeature
[14:48] <jelmer> an invalid line (ie one missing a space) causes ValueError at the moment, that should probably be a UnknownFormatError instead
[14:49] <vila> hmm, more probably broken, the first line have been recognized
[14:50] <jelmer> well, who knows what extensions we might add in the future :)
[14:50] <vila> FormatCorrupt ?
[14:50] <vila> hungh
[14:50] <jelmer> hmm, I think corrupt sounds a bit alarming
[14:50] <vila> I want this case to be covered :)
[14:50] <jelmer> vila: perhaps just leaving it as Valueerror for the moment?
[14:51] <jelmer> vila: test_from_string tests it
[14:51] <vila> so we can indeed not emit a FormatCorrupt unless the content is indeed broken
[14:51] <vila> We have DirstateCorrupt for this reason
[14:52] <vila> and LockCorrupt (if you want a case closer to the format file)
[14:52] <vila> I mean covered as in: we open or not and if not because a feature is missing
[14:53] <vila> don't add a new unknown again :)
[14:54] <jelmer> vila: so, I'm not particularly concerned about this either way. I think a ValueError isn't the worst thing in the world for this, but I'd be happy to add FormatCorrupt too
[14:56] <vila> And the ValueError is caught ?
[14:56] <vila> Or does it bubble up to the user ?
[14:57] <jelmer> vila: I'm happy for it to bubble up - we do that in a lot of other cases too, and it provides the best data to report back to us developers
[14:57] <jelmer> e.g. we have the same when we read the last-revision file
[14:58] <vila> meh that's not a good reason
[14:59] <jelmer> vila: either way, I don't really care what we do in this case
[14:59] <jelmer> vila: let's focus on the core stuff
[14:59] <vila> I do
[14:59] <vila> This code will be backported
[15:00] <vila> (necessity, feature) = 'foo'.split(' ', 1)  breaks
[15:00] <jelmer> vila: that only occurs if people somehow have a corrupted format file
[15:00] <vila> exactly
[15:01] <vila> we keep getting bug reports about corrupted files, if we can diagnose them in a 3 lines parser why not do it ?
[15:01] <jelmer> vila: we don't catch ValueErrors in a lot of other cases either if there are corrupt files
[15:02] <jelmer> vila: I think adding CorruptFormat will make it harder to debug, not easier
[15:03] <jelmer> vila: unless I'm missing something?
[15:03] <vila> I've never see a finer error handling render debug harder... the code is fresh, there is an issue, handling it later will cost more (probably less than the time we already spent discussing it)
[15:04] <vila> hey wait,
[15:04] <vila> invalid format header is already a ValueError ?
[15:04] <vila> oh, that's what is presented to the user for unknown formats ? Naaaah
[15:04] <jelmer> vila: no
[15:04] <vila> then it's caught somewhere ?
[15:05] <jelmer> vila: we already check the first line earlier, because we look that up in a registry
[15:05] <vila> raise ValueError("Invalid format header %r" % format_string)
[15:05] <vila> err
[15:05] <jelmer> vila: line 1175
[15:05] <vila> raise ValueError("Invalid format header %r" % text) in your proposal
[15:05] <jelmer> I guess this could be an AssertionError
[15:06] <vila> yes, better than ValueError
[15:06] <vila> the object can't be open anyway
[15:06] <vila> especially if the line is displayed
[15:07] <vila> but wait
[15:07] <vila> where is the ValueError caught ?
[15:07] <jelmer> vila: nowhere
[15:07] <jelmer> vila: that first line is looked up in the format registry
[15:08] <jelmer> vila: and if that raises a KeyError, we raise UnknownFormatError
[15:08] <vila> only later ?
[15:08] <jelmer> see line 1171-1175
[15:08] <jelmer> vila: before
[15:08] <vila> I'm there but that's where the exception is raised not caught
[15:09] <jelmer> vila: where UnknownFormatError is raised you mean?
[15:09] <vila> 'invalid format header' is informative, 'need more than 1 value to unpack' is not, especially if the line is not displayed
[15:09] <vila> no no, ValueError
[15:09] <jelmer> vila: sorry, 1243
[15:09] <vila> KeyError here
[15:10] <vila> oh, I see what you mean
[15:10] <vila> right, probe_transport :)
[15:11] <vila> hmm, UnknownFormatError(line, kind='feature-flag') ?
[15:12] <jelmer> vila: who says the unparseable line is a feature flag?
[15:12] <jelmer> vila: I think the format as a whole that is unknown is the bzrdir/repository/branch/workingtree format
[15:13] <vila> it's where feature flags are defined and should indicate a corruption ? May be FormatCorrupt is better ?
[15:13] <jelmer> vila: sure
[15:14] <vila> anyway, we agree it's a case where we can't open right ?
[15:14] <jelmer> yes
[15:15] <vila> so as long as the message is better than 'need more than 1 value to unpack' with no context...
[15:15] <vila> AssertionError will do
[15:16] <vila> if it's corrupted, the line should give enough hints, if it's not (a mythical unknwon :) it will be clear too
[15:16] <jelmer> right
[15:17] <vila> the plan to backport is to call the parser code everywhere it's needed in old bzr right ? (As in: without backporting BzrFormat related changes)
[15:18] <vila> hmm, a bit more than the parser, the set() and dict() handling too right ?
[15:18] <jelmer> vila: depends on how much we want to backport
[15:19] <jelmer> vila: actually, yeah
[15:19] <jelmer> vila: at least backporting the parser and calling that everywhere
[15:19] <jelmer> and then probably ignoring optional lines and raising MissingFeature for each non-optional feature we encounter
[15:20] <vila> that's check_support_status right ?
[15:20] <jelmer> vila: yep
[15:21] <jelmer> or a simplified version of it
[15:21] <vila> not by much if you want to allow support for required features but may be that could be done separately
[15:22] <vila> or may be we only provide support for optional features which would be ok with me
[15:22] <jelmer> vila: I'm happy if old bzrs can just skip optional features and print a sane error if they encounter required features
[15:23] <vila> yup, which a robust parser should provide
[15:24]  * vila looks if the parser can be made even more robust by deleting one line or two
[15:24] <vila> :-p
[15:26] <jelmer> :)
[15:29] <vila> jelmer: hmm, you haven't included read-optional yet ?
[15:29] <jelmer> vila: no, that would be too big a change, and there's no particular reason to already include it
[15:29] <vila> If that requires a backport and then a release and then an install...
[15:30] <vila> I'm lost, what change ?
[15:30] <jelmer> vila: hooking into lock_write
[15:30] <vila> right
[15:30] <vila> but
[15:30] <vila> oh, to forbid writes
[15:31] <vila> ok
[15:33] <jelmer> vila: pushed use of ParseFormatError
[15:39] <vila> hehe lineno+2
[15:40] <vila> oooh, and you added format to the previous ValueError too !
[15:41] <jelmer> vila: s/ValueError/AssertionError/ :)
[15:42] <vila> no, I'm referring to the one for invalid header, the format was there already ?
[15:42] <vila> oh no, you turned it into an AssertionError !
[15:42] <jelmer> yes
[15:42] <vila> lp incremental diff is totally broken
[15:44] <vila> ghaa, no, it conflates all the revisions, nm
[15:44] <vila> and splitted tests ! It's Christmas !
[15:45] <vila> jelmer: do you want a second review ?
[15:45]  * vila ducks
[15:46] <vila> ghaaa, you resubmitted it ? I voted on an old one...
[15:46] <jelmer> whoops
[15:46] <jelmer> I resubmitted it because I wanted a new diff, lp was taking too long to update..
[15:46] <jelmer> vila: and THANKS
[15:48] <vila> pfew, thanks for your patience, I hope you don't mind the time spent here but I feel confident that we would have spend far more in backporting and handling spurious bugs otherwise
[15:48] <jelmer> that was probably the longest IRC discussion I have ever had about a single MP, but it seems to've worked :)
[15:48] <vila> hehe
[15:50] <vila> jelmer: there is no urgency (hmm) but don't forget that we'll need to release whatever you backport so it could take a while before we can safely deploy even optional features
[15:50] <vila> it may not matter if we start experimenting with people already using 2.5 but...
[15:51] <vila> ...the sooner the minimal stuff is backported the sooner we can experiment freely
[15:53] <jelmer> vila: yeah
[17:25] <smoser> is this a known bug ? http://paste.ubuntu.com/778868/
[17:25] <smoser> is think i saw poolie mention issues with etc keeper and locale on a list.
[17:27] <smoser> that is a pristine precise install then some automated 'late_command' run after installer.
[17:28] <vila> smoser: yeah, depending who you ask, the bug can be in bzr/python/ubuntu or yourself for not getting the file system encoding right
[17:28] <smoser> well i certainly blame anyone but me.
[17:28] <vila> since the locale is C...
[17:28] <vila> sorry, was kidding as there was a heated discussion around a patch proposal for python
[17:28] <smoser> :)
[17:28] <smoser> i'm joking too.
[17:29] <smoser> i understood.
[17:29] <vila> the python guys said: tell your users to set the right locale
[17:29] <smoser> i was fairly surprised that apt didn't fail
[17:29] <smoser> that in itself is almost a bug.  something tried to run bzr (etc-keeper i think). i would have thought it would cause install failure.
[17:29] <vila> well, it probably doesn't try to find a valid unicode representation of the path
[17:31] <smoser> so what bug would i subscribe to just to follow?
[17:31] <vila> the surprising bit is that several packagers (in a related but less heated discussion) mentioned that utf8_something was the default for ubuntu
[17:31] <vila> ha, mgz knows
[17:31] <vila> but he've been pretty quiet today so I'm not sure he's around
[17:32] <vila> justasec
[17:32] <vila> Bug #794353
[17:33] <vila> ha, fix released :) Let's check where
[17:33] <vila> 2.5b5
[17:35] <vila> smoser: <http://bugs.python.org/issue13643> for the attempt at fixing python itself
[17:37] <vila> smoser: in other words, this is in flux but a fix is currently in bzr.dev (2.5b5 will be released  around 2012-01-16)
[17:37] <smoser> vila, thanks.
[17:37] <smoser> good enough for me
[20:16] <mgedmin> help? 'bzr commit' used to show me the diff in the commit message template, but now it stopped
[20:16] <mgedmin> I'm yak shaving already, can anybody point me to a quick setting I could check?
[20:17]  * mgedmin looks at ~/.bazaar/bazaar.conf and notices [ALIASES] ci = ci --show-diff
[20:17] <mgedmin> oh, so I used to type 'bzr ci' and now I started typing 'bzr commit', apparently
[20:18] <mgedmin> I thought I read in a recent message to the mailing list that show-diff was on by default?
[20:18] <jelmer> hi mgedmin
[20:18] <jelmer> mgedmin: I'm pretty sure --show-diff isn't on by default
[20:20] <mgedmin> hi!