| jelmer | hi wgz, poolie | 00:03 |
|---|---|---|
| === mwhudson_ is now known as mwhudson | ||
| vila | hi all ! | 07:59 |
| mgz | morning! | 08:01 |
| poolie | vila, mgz, jelmer, hi | 08:02 |
| wgz | shall we get the hangout started? | 08:06 |
| poolie | yep | 08:06 |
| wgz | I want to hear how the skiing was vila :) | 08:07 |
| vila | hehe, bad during a deep learning phase and just great when I decided to enjoy it ;) | 08:08 |
| jelmer | rats, forgot about the call | 08:30 |
| wgz | http://lwn.net/Articles/485162/ | 09:12 |
| vila | jelmer: thanks for your attention to details ;) | 11:30 |
| === Amoz is now known as Afougner | ||
| === Afougner is now known as Amoz | ||
| m1sc | hi. i've setup loggerhead, i am able to load the webinterface, but it fails to load a random selection of resources like .js files etc. (loggerhead 1.18.1) | 13:46 |
| jelmer | m1sc: how are you running loggerhead? | 13:47 |
| m1sc | jelmer: /srv/bazaar/loggerhead/serve-branches --host 127.0.0.1 --port 8082 --allow-writes --prefix /bzr/devel/ repos/devel/ | 13:48 |
| m1sc | (behind apache proxy - same behaviour without proxy..) | 13:48 |
| jelmer | m1sc: does it work without the prefix perhaps? | 13:49 |
| m1sc | jelmer: let me check.. | 13:49 |
| m1sc | jelmer: seems like the proxy is the problem - apparently some GET requests don't get through.. (at least serve-branches is logging only those requests for files not "missing") | 14:25 |
| === deryck_ is now known as deryck | ||
| abentley | vila: it seems b.get_config().set_user_option('pqm_email', 'PQM <pqm@example.com>') doesn't work in bzr.dev r6480. Do I need to explicitly save or something? | 15:36 |
| vila | abentley: hmm, if pqm_email has been migrated to use config stacks, then only config stacks should be used to access it | 15:38 |
| vila | abentley: more precisely: if the writes are done by config stacks, they may be delayed and the old config reads will miss the updates | 15:38 |
| abentley | vila: this is test code that worked with 2.5 | 15:38 |
| vila | abentley: config stacks are available in 2.5 | 15:39 |
| mgz | abentley: try unlocking the branch | 15:39 |
| abentley | vila: I know. | 15:39 |
| abentley | mgz: the branch isn't locked AFAIK | 15:40 |
| mgz | depending on how your test is written, you may want that to flush the changes | 15:40 |
| mgz | there's an mp from vila where he changed a bunch of test cases that may be a useful reference | 15:40 |
| * vila nods | 15:40 | |
| mgz | most of them could just be simplified, a few needing a bit of an extra dance | 15:40 |
| vila | abentley: or re-open the branch ? | 15:41 |
| abentley | vila: I'm re-opening the branch, and it's not seeing the value I wrote. | 15:41 |
| vila | abentley: meh, I need to see more code then | 15:41 |
| abentley | vila: It's bzr-pqm, test case test_lpland.TestSubmitter.test_set_message | 15:42 |
| vila | but set_user_option() has not been changed so it should save | 15:42 |
| abentley | vila: Sorry, that should be test_lpland.TestSubmitter.test_make_submission_email | 15:44 |
| vila | no make_submission_email in pqm revno 85 | 15:45 |
| vila | ghaaa | 15:45 |
| vila | 84 | 15:45 |
| abentley | vila: It's in 89. | 15:45 |
| vila | abentley: it works if you s/get_config().set_user_option/get_config_stack().set/ | 15:50 |
| abentley | vila: that's nice, but bzr-pqm supports bzr 2.4 and earlier, so it won't work for this. | 15:51 |
| jelmer | abentley: bzr-pqm already has some code that looks as bzrlib.version_info - any reason that wouldn't work here? | 15:52 |
| abentley | jelmer: It would certainly be possible to add more compatibility code. But this looks like a bug, esp. since it worked in 2.5 | 15:53 |
| vila | abentley: well, avoiding IOs means caching values, that's what you run into, you don't lock the branch for writing but still modifies its config, I don't remember production code allowing that and I had to fix several tests that were relying on the branch being writable | 15:55 |
| abentley | vila: I believe set_option locks and unlocks. | 15:56 |
| vila | the config file only, not the branch | 15:56 |
| vila | and set_user_option also read and write the whole file | 15:57 |
| vila | which is what we want to avoid | 15:57 |
| abentley | vila: 1742: self.branch.lock_write() | 15:57 |
| vila | ECONTEXT | 15:58 |
| abentley | vila: bzrlib/config.py:1742: self.branch.lock_write() | 15:58 |
| vila | ha, right, yes, that's what I was saying, this will read and write the whole file ignoring the the fact that the option has been migrated and should use the new code | 16:00 |
| vila | you can't mix the two code paths for the *same* option | 16:00 |
| abentley | vila: You said "you don't lock the branch for writing but still modifies its config, I don't remember production code allowing that". This is production code that allows that. | 16:00 |
| vila | right, that part was wrong :) | 16:01 |
| vila | and the lines before 1742 also mentions that the approach used here needs to be fixed | 16:02 |
| abentley | vila: Yes, but it's written by you, so it's no surprise it agrees with you :-) | 16:03 |
| vila | hehe | 16:03 |
| abentley | vila: the point is, I'm writing a value, then reading it, and it's not working. If we really should not be using set_user_option, then it should be deleted from the API. | 16:04 |
| vila | in ideal world, yes, but it cannot be removed until all plugins migrate to the config stacks | 16:05 |
| abentley | vila: If it's not removed, then it needs to co-exist with config stacks properly. | 16:05 |
| vila | well, avoiding IOs means caching values | 16:06 |
| vila | you use one API to set and a different one to read | 16:06 |
| abentley | vila: That may be, but the branch is unlocked, and I haven't accessed any config stacks when I write. | 16:06 |
| wgz | I was under the impression the old apis would function as before, | 16:06 |
| wgz | and the benefits to fewer config disk reads would be limited to code using the new interface | 16:07 |
| abentley | vila: If you can't use one API to set and a different one to read, then the API should be removed. Otherwise, people are going to do it by accident or laziness. | 16:07 |
| wgz | if a (non-bogusly written) test is failing with the old interface, how sure are we plugins won't also break? | 16:07 |
| vila | I think what happens here is that 'pqm_email' is read with the new API | 16:07 |
| vila | wgz: we can't be sure but that's the first report I hear about where it fails | 16:09 |
| vila | Overall, we *read* options in 99% of the cases and migrating an option to the config stacks should be done for both the code and the tests | 16:10 |
| vila | the alternative is a big switch day which has been vetoed long ago | 16:11 |
| vila | it's also why I landed this change in 2.6 so we have a full cycle to debug the issues | 16:12 |
| wgz | that seems fair | 16:16 |
| wgz | we do want a way to write config tests that will work across multiple bzr versions though realy | 16:17 |
| wgz | jelmer: (unrelated to anything) wasn't there some bug related to bzrdir got imported? I'm now failing to find it or remember the details. | 16:18 |
| abentley | wgz: pqm-submit has its own implementation of config stacks to provide compatibility with 2.4 and lower. This doesn't seem ideal. | 16:18 |
| wgz | abentley: indeed not. | 16:19 |
| vila | wgz: avoiding the caching issues can be avoided by writing tests that don't try to cache branch/tree objects or expect them to not cache any config values | 16:20 |
| jelmer | wgz: yes, bzrlib.workingtree didn't import bzrdir for a while | 16:21 |
| wgz | mostly plugins that support multiple versions haven't switched yet (and some don't have tests...) but a way to get the benefits without breaking too badly would be really good | 16:21 |
| jelmer | wgz: that meant that if you didn't run bzrlib.plugin.load_plugins() you might end up trying to open a control directory while the format handler for .bzr/ wasn't yet registered | 16:21 |
| jelmer | wgz: for that reason, we now explicitly import bzrlib.bzrdir from bzrlib.branch, bzrlib.repository and bzrlib.workingtree | 16:22 |
| vila | wgz: my understanding is that you run into this issue only if you mix the two code paths for the *same* option and modifies it (naively) with the old code path | 16:22 |
| wgz | jelmer: ah yes, that was it, thanks, gmail search let me down. | 16:23 |
| wgz | bug 905218 | 16:23 |
| ubot5 | Launchpad bug 905218 in Bazaar "bzrlib.initialize() misses out the default prober" [Critical,Fix released] https://launchpad.net/bugs/905218 | 16:23 |
| jelmer | wgz: we could move the probers out of bzrlib.bzrdir into bzrlib.bzrformats or something (and leave the actual implementation in bzrlib.bzrdir); that would allow us to access foreign formats without loading any of the bzr-specific code | 16:23 |
| jelmer | s/any/barely any/ | 16:23 |
| wgz | that would be neat. | 16:23 |
| wgz | vila: that seems quite likely during transition | 16:25 |
| vila | wgz: which part ? | 16:26 |
| wgz | mixing up old and new config calls on the same option | 16:26 |
| vila | *and* modifies the option ? | 16:27 |
| vila | early enough to fall into the trap ? | 16:27 |
| vila | not in a test ? | 16:27 |
| wgz | having some clear guidelines on The Right Way seems useful | 16:27 |
| vila | yup | 16:27 |
| abentley | vila: _get_config_store seems to remember cache the BranchStore, but the BranchStore doesn't seem to care about locks. Could this be the cause of the problem? | 16:28 |
| abentley | vila: I think it should not set self.conf_store for unlocked branches, or perhaps it should be needs_read_lock. | 16:29 |
| vila | abentley: more likely the issue is that the branch does not clear the cached config when unlocking when that was deliberate to avoid a read at the next option access | 16:30 |
| vila | remove the second when | 16:30 |
| abentley | vila: I thought I saw code to clear it in Branch.unlock. | 16:31 |
| vila | hmm, I see self.conf_store.save_changes() in unlock (branch.py line 2564) | 16:33 |
| abentley | vila: Maybe it should be in Branch._clear_cached_state ? | 16:33 |
| abentley | vila: Yeah, but I don't actually see it clearing it. | 16:33 |
| vila | because clearing it lose half of the optimization, save_changes() already read/write the file, throwing it away it a waste | 16:34 |
| abentley | vila: if you don't clear it when you really unlock the branch, you run the risk of using stale data, don't you? | 16:35 |
| abentley | vila: If the branch is being locked for the right duration, it shouldn't throw away the optimization, I would think. | 16:36 |
| vila | hmm, sounds convincing... but I expect more test bugs in that case | 16:38 |
| abentley | vila: Also, it looks like you're calling save_changes on every call to Branch.unlock, even if the branch has been locked multiple times. | 16:40 |
| abentley | vila: If so, you are doing more I/O than necessary. | 16:41 |
| abentley | vila: Though this may be theoretical since setting config is rare. | 16:42 |
| vila | if there is no changes there is no IO but the other point is worth investigating | 16:42 |
| vila | I thought unlock was protected by some other mean but imbw | 16:43 |
| abentley | vila: right. To do extra IO, you'd need to Branch.lock_write(); Branch.get_config().set(); Branch.lock_write(); Branch.get_config.set(); Branch.unlock(); Branch.unlock() | 16:43 |
| abentley | sorry, s/get_config/get_config_stack/ | 16:44 |
| vila | yeah, if unlock is always called, I thought there was some mechanism to call it only for the outer call but I don't see the relevant code (or I'm confused by another implementation) | 16:44 |
| abentley | vila: Or another case would be Branch.lock_write(); Branch.get_config_stack().set(); Branch.lock_read(); Branch.unlock(); Branch.get_config_stack.set(); Branch.unlock(); | 16:45 |
| abentley | vila: I believe Branch.unlock will actually get called, but "if not self.control_files.is_locked" prevents action if we haven't actually unlocked. | 16:46 |
| vila | in any case, save_changes() should be called *before* the real unlocking happen | 16:47 |
| abentley | vila: most definitely. | 16:48 |
| vila | oh crap the texinfo/sphinx issue is still pending for bzr.dev | 16:51 |
| === zyga-xchat is now known as zyga | ||
| mgz | vila: yeah. Gordan worked around it by renaming the bzr bundled texinfo. | 16:54 |
| vila | neat | 16:55 |
| abentley | vila: Here is a test that demonstrates the caching problems: http://pastebin.ubuntu.com/871738/ | 17:07 |
| vila | abentley: thanks | 17:09 |
| vila | I always known that there are issues with caching the config values, the real question has always been: which use case will it break ? | 17:10 |
| vila | s/known/knew/ | 17:10 |
| vila | In practice, once you open a branch in read-mode, another process can open it in write mode and change the config values, is there a case where we should care ? | 17:11 |
| abentley | vila: Arguably when we read-lock the branch, we should cache all cacheable values. | 17:13 |
| vila | abentley: right, the old code didn't do that, the new one does | 17:13 |
| vila | but that doesn't give us a use case where it breaks :) | 17:13 |
| abentley | vila: No, it doesn't do it when we read-lock the branch. | 17:13 |
| abentley | vila: It does it afterward. | 17:14 |
| vila | ha, right | 17:14 |
| vila | yeah | 17:14 |
| vila | but it's very early (stacked_on_location) | 17:14 |
| vila | or some other option | 17:14 |
| abentley | vila: But that's *before* we read-lock the branch. | 17:14 |
| vila | mwhahahah | 17:14 |
| vila | even worse :) | 17:15 |
| abentley | vila: I've been wanting us to open all branches in locked mode for years. | 17:15 |
| abentley | vila: e.g. Brach.open_read_locked(), Branch.open_write_locked(). | 17:15 |
| vila | the fact that we didn't encounter bugs around this (yet) hints that it's not that vital either | 17:16 |
| abentley | vila: I recall hitting plenty of bugs that this would have solved. | 17:16 |
| vila | but I agree it would be far cleaner | 17:16 |
| vila | ha | 17:17 |
| vila | did you file some ? | 17:17 |
| abentley | vila: But like I say, it's been years. | 17:17 |
| abentley | vila: I think it would also fix the bug I'm showing here. | 17:17 |
| vila | right | 17:18 |
| abentley | vila: Well, it would if we cleared the store on unlock. | 17:18 |
| vila | the fact that we can use unlocked branches means many tests just did that | 17:19 |
| vila | yeah | 17:19 |
| vila | If no test fails doing that it's a no-brainer | 17:19 |
| vila | abentley: can you file a bug and I'll look into it tomorrow ? (too many juggling eggs right now) | 17:20 |
| abentley | vila: For sure. I was going to land a couple of expectedFailure tests. | 17:20 |
| vila | abentley: there are several bugs (not clearing, saving only for the outer call) but a single one should do for the discussion about the balance between some caching and no caching at all | 17:23 |
| vila | abentley: deprecating set_user_option for already migrated options may be an option too | 17:24 |
| vila | already migrated == registered | 17:24 |
| abentley | vila: deprecation implies that it works, but you shouldn't use it. This is a case where it doesn't work. Because you can call set_user_option, and then invoke code that does get. | 17:25 |
| vila | abentley: strictly speaking, yes, I was thinking about a warning to guide people during the migration | 17:26 |
| vila | abentley: the check is cheap and the output would help | 17:27 |
| abentley | vila: IMO, we expect APIs to break during betas, so we should just remove set_user_option. Keeping it around is unnecessary pain. | 17:27 |
| vila | abentley: that would mean breaking all plugins that still use the old API, sounds a bit painful too | 17:28 |
| vila | a bit *too* painful even | 17:28 |
| abentley | vila: Well, maybe in this case, but in general, removing APIs is going to break all plugins that use it. | 17:30 |
| vila | especially given that *setting* options programatically (sp?) is the exception rather than the norm and the bug is triggered only when you mix both APIs | 17:30 |
| vila | both APIs for the *same* option | 17:30 |
| abentley | vila: I dunno. There are certainly going to be plugins that set options and then expect bzrlib to respect them. | 17:31 |
| vila | so far you can still use both APIs as long as you don't mix them which is a softer path for migration (but jelmer already said he was for deprecating the old config during the 2.6 cycle while I was targetting 2.7) | 17:32 |
| abentley | vila: I don't think it's practical to avoid mixing them, because you can use config stacks indirectly via bzrlib. | 17:33 |
| vila | abentley: I'm not that sure about that, options are rarely set by plugins themselves, *users* set options | 17:33 |
| === deryck is now known as deryck[lunch] | ||
| abentley | vila: In which case, it shouldn't be to painful to remove set_user_option? :-) | 17:34 |
| vila | hehe | 17:35 |
| abentley | vila: AFAICT, self.branch.get_config_stack().set('foo', 'bar') does IO. | 17:39 |
| vila | abentley: yes :-/ | 17:39 |
| vila | abentley: but | 17:39 |
| vila | b.lock() ; b.get_config_stack().set('foo', 'bar') doesn;t until b.unlock() | 17:40 |
| vila | this was changed during the review as it made easier to use | 17:40 |
| vila | s/made/made it/ | 17:41 |
| abentley | vila: I'm doing it in a locked branch. | 17:41 |
| vila | <cough> It's what the bug you're about to file is about now ? :) | 17:42 |
| vila | s/now/no/ | 17:42 |
| vila | s/if self.conf_store is not None/if self._control_files._lock_count == 1 and self.conf_store is not None/ | 17:42 |
| abentley | vila: Well, this is the fact that branch.unlock unconditionally does IO | 17:43 |
| vila | s/unconditionally/if there are changes to save/ | 17:43 |
| abentley | vila: Right. BranchStack.set is needs_write_lock, so it implicitly does Branch.unlock. | 17:46 |
| vila | abentley: yup and it should *not* trigger save_changes() if the branch is already locked, that's a bug | 17:47 |
| vila | abentley: then, save_changes() itself will trigger an IO *only* if there have been changes since the last read | 17:48 |
| abentley | vila: Right. | 17:48 |
| vila | abentley: it's unfortunate I missed the save-on-last-unlock part given how many tests failed during this change :-/ | 17:49 |
| vila | abentley: I will add the missing tests while fixing this | 17:49 |
| vila | abentley: but the plan is definitely to *not* write until the last unlock | 17:50 |
| abentley | vila: this means Branch.get_config_stack.set() will interoperate with Branch.get_config.get_user_option correctly. | 17:50 |
| vila | indeed | 17:50 |
| abentley | vila: Until the bug is fixed. | 17:50 |
| vila | indeed again, which is why I expect for test bugs | 17:51 |
| vila | the weird thing is that I'm pretty sure there are some hpss test that should have failed if each set() was trigerring an IO, but maybe the remote branch object does calls the branch.unlock() only for the outer unlock... | 17:53 |
| vila | abentley: EOD here, thanks for the feedback | 17:54 |
| abentley | vila: np | 17:54 |
| === zyga is now known as zyga-afk | ||
| === yofel_ is now known as yofel | ||
| === deryck[lunch] is now known as deryck | ||
| abentley | vila: Bug #948344 and #948339 | 19:50 |
| ubot5 | Launchpad bug 948344 in Bazaar "BranchConfig and BranchStack do not interoperate correctly" [Medium,Triaged] https://launchpad.net/bugs/948344 | 19:51 |
| ubot5 | Launchpad bug 948339 in Bazaar "BranchStack caching misbehaves" [Medium,Triaged] https://launchpad.net/bugs/948339 | 19:51 |
| sarrvesh | I need help with pulling a branch using bazaar.I tried executing the command bzr branch lp:ubuntu/unity-greeter | 20:57 |
| sarrvesh | in terminal | 20:57 |
| sarrvesh | but I got the following error message | 20:57 |
| sarrvesh | Agent admitted failure to sign using the key. | 20:57 |
| sarrvesh | Permission denied (publickey). | 20:57 |
| sarrvesh | ConnectionReset reading response for 'BzrDir.open_2.1', retrying | 20:57 |
| sarrvesh | Agent admitted failure to sign using the key. | 20:57 |
| sarrvesh | Permission denied (publickey). | 20:57 |
| sarrvesh | bzr: ERROR: Connection closed: Unexpected end of message. Please check connectivity and permissions, and report a bug if problems persist. | 20:57 |
| sarrvesh | How do I overcome this? | 20:57 |
| sarrvesh | Please help | 20:57 |
| jelmer | hi sarrvesh | 20:59 |
| sarrvesh | hi | 20:59 |
| jelmer | sarrvesh: it sounds like you haven't set up a SSH key but have specified your launchpad login with "bzr lp-login" | 20:59 |
| sarrvesh | no but I pasted my ssh public key into launchpad | 21:00 |
| sarrvesh | any idea how to overcome this? | 21:08 |
| jelmer | sarrvesh: do you have your ssh key added locally? | 21:12 |
| jelmer | Did you tell bzr about your launchpad login name using "bzr lp-login" ? | 21:12 |
| sarrvesh | yea i did | 21:13 |
| sarrvesh | first i did bzr whoami "name <email_id>" | 21:13 |
| fullermd | The error message sounds like something with the agent. | 21:13 |
| sarrvesh | then i did bzr launchpad-login <username> | 21:14 |
| thomi | Hi everyone - I'm having an issue related to bzrlib - in sloecode we do this: "to_transport = repo.user_transport.clone('trunk')" in order to create a repo for a new project, but I get this exception: AttributeError: 'CHKInventoryRepository' object has no attribute 'user_transport' | 22:05 |
| thomi | however I think this is because the repositories on disk may have become corrupted. Any hints as to what that exception means, and how I can fix it? | 22:06 |
| lifeless | bzrlib version skew I suspect | 22:07 |
| thomi | so... the repositories were created with version X, but the system has version Y installed? | 22:08 |
| lifeless | thomi: so your *code* was created for version X and your *code* doesn't work with version Y | 22:13 |
| thomi | lifeless: I see, that makes more sense. :) | 22:13 |
| thomi | Thanks | 22:14 |
| thomi | ok, so my code uses bzrlib v2.5, but doesn't work with ver 2.1. At least now I know what the issue is | 22:17 |
| richo | I'm looking for a unicode version of the bzr logo- like ± for git or ☿ for hg. Is there such a thing? | 22:50 |
| lifeless | probably not | 22:50 |
| jelmer | I'm sure there must be a unicode symbol for merging traffic ? | 22:53 |
| richo | that's what I thought | 22:54 |
| richo | but I spent ages looking through glyph tables and didn't find anything, thought it might be easier to just ask | 22:54 |
| jelmer | U+26D7 is in the right sort of area | 23:01 |
| spiv | ⤖ perhaps? | 23:07 |
| spiv | Or ⤴ | 23:07 |
| jelmer | ⛗ | 23:10 |
| spiv | That's a very tidy rendering of 2, 6, D, and 7 inside a rectangle :P | 23:11 |
| jelmer | not here, here it is rendered as ⛗ :P | 23:12 |
| jelmer | I guess you're using a different font than I am | 23:12 |
| spiv | Inconsolata, but it typically falls back to another font if necessary for a particular glyph | 23:13 |
| spiv | But I am on a lucid system. | 23:13 |
| jelmer | ah, ok | 23:13 |
| jelmer | you're running gooboontoo? | 23:13 |
| spiv | I'm not sure it has quite that many "o"s in it :) | 23:14 |
| james_w | U+26D8: BLACK LEFT LANE MERGE sounds promising | 23:14 |
| spiv | Bikeshedding: Unicode Edition! | 23:15 |
| === mlh_ is now known as mlh | ||
| jelmer | spiv: I'm sure you mean 🚲shedding | 23:16 |
| spiv | REPLACEMENT CHARACTERshedding? Yes, yes I do! | 23:16 |
| * mwhudson is failing at fonts for this conversation too | 23:16 | |
| jelmer | me too actually | 23:17 |
| * mwhudson is amused by the way the character map applies bold and italic to unrendered characters | 23:18 | |
| * jelmer quickly closes Character Map before he finds more unicode characters representing things like KISSING CAT FACE WITH CLOSED EYES | 23:18 | |
| mwhudson | i now have a bold, italic 2, 6, D, and 8 in a box! | 23:18 |
| jelmer | mwhudson: I wonder what a bold bicycle looks like :P | 23:19 |
| SamB | jelmer: I think that one is made using several JIS characters | 23:20 |
| SamB | the cat one | 23:20 |
| jelmer | ah | 23:22 |
| richo | damn, this charset doesn't have glyphs for any of those : | 23:28 |
| richo | :( | 23:28 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!