=== StevenK_ is now known as StevenK [07:15] apt by-hash is going to need at least https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=798919 fixing before we can use it [07:16] I believe I have all the LP changes done for InRelease, but I need to get some internal mirroring etc. fixes landed first. [07:16] (All proposed, so I'll bug a webop once I catch one) [07:17] Can we actually deploy mirror fixes atm? [07:17] There was that RT that Adam said might have been progressed or something. [07:18] Is this https://rt.admin.canonical.com/Ticket/Display.html?id=83847 ? [07:18] Ha, I think I missed the existence of that charm [07:18] Is canonical-magic-mirror not being used any more? [07:18] magic-mirror is used for the split [07:19] But we also need to verify that the charm syncs InRelease last. [07:19] As it wasn't doing with Translations-* until that ticket. [07:20] Right [07:20] The charm doesn't do any weird stuff like magic-mirror, just the two-stage sync. [07:21] cjwatson: Did you look into Tarmac git support in any depth? [07:21] I don't intend to excise bzrlib's command processing etc. at this stage. [07:23] wgrant: Only to the extent of satisfying myself that it isn't hard and doing some extra API exports for it [07:23] zyga said he was working on it, but I think we should time out on that soon. [07:25] I wonder where https://pastebin.canonical.com/139395/ comes from [07:25] There are similar files in puppet, but not ports-sync. [07:38] cjwatson: I also intend to add BugBMP tomorrow. [07:38] Initially populated by BugBranches being added, but also manageable explicitly. [07:39] Eventually the diff generator should ask turnip to regexp the new commits to link bugs. [07:39] I think that's the only reasonable approach for git, do you agree? [07:40] cjwatson: Hm, I didn't realise ports-sync was different. [07:40] cjwatson: It may be manually maintained; there are few ports mirrors. [07:41] BugBMP etc.> Agreed. [07:41] wgrant: Yeah, appears to be (see #is) [07:41] Sorry, busy collecting popcorn and reading too many channels. [07:41] Haven't caught up. [07:43] * cjwatson checks whether cdimage's two-stage sync handles InRelease and finds he did that in 2011 [07:44] Heh [07:44] That's an easy one, then. [07:47] Also I checked that I couldn't reproduce anything like the original apt InRelease security problems with precise and trusty [07:47] (Indeed, precise just has InRelease disabled with a big hammer) [07:47] Oh, I didn't realise it was disabled. [07:47] Since Debian was using it by that point. [07:47] Maybe it was disabled while the sensible gpgv usage was fixed. [07:48] I guess our apt was a while out of sync. [07:48] Right. [07:48] Rather than trying to parse it out manually. [07:48] In trusty, if you try tricks like appending an InRelease file, then it ignores the unsigned portions of the file [07:49] Which feels a bit odd, but not exploitably so [07:51] That sounds like it's doing the right thing. [07:51] It is a little odd, and there is an argument to be made for failing on non-whitespace outside the sig, but it doesn't really worry me. [07:53] I don't remember how I found it the first time, but I'll have a poke around the code too this week. [08:01] Yeah. Thanks. [08:02] On apt by-hash, I think we probably can in fact do most of it without the bug above being fixed, just not the Acquire-By-Hash flag in Release. [08:02] Yep [08:03] The fallback is rather unpleasant, but we have little choice for at least a couple of years. [08:03] It'll need an apt-ftparchive backport (or we could do it in LP directly, but I don't think it's worth it). Fortunately a small one. [08:03] Oh, I didn't realise a-f had support. [08:03] How does it handle GC? [08:04] Keeps three by default. [08:04] I'd be doing it in LP. [08:04] Because we need to do it for NMAF anyway. [08:04] True. [08:04] It's not dissimilar from the Release code which we already reimplement. [08:04] I think we want finer-grained control than a-f offers; it only lets you control the count. [08:04] We currently let a-f write directly to dists, but that's only because it's convenient. [08:04] (APT::FTPArchive::By-Hash-Keep) [08:05] It would be entirely reasonable to have it write to a temporary directory, then we grab the files and do whatever we want with them. [08:05] I prefer to keep our dependency on a-f to a minimum. [08:05] Yeah, I guess so. [09:34] cjwatson: since you're hacking on email notifications [09:34] cjwatson: there is one bug (I don't have it to hand) that would make me so so so super happy [09:35] cjwatson: which is that LP always sends mail to *me* even if I'm only indirectly subscribed [09:35] cjwatson: which breaks GMail 'mute' [10:00] lifeless: Just bugmail? [10:00] There's a bug about that, let me find it. [10:10] wgrant: bugmail is ~99% of my LP mail [10:10] wgrant: so even if its not *just*. [10:11] would still be happy happy joy joy schnaaaake time. [10:11] Night all. [10:11] lifeless: Oh sure, just checking there were no other cases. [10:11] If we fix one source and you still whine, that's not good :) [10:14] https://bugs.launchpad.net/launchpad/+bug/138592 [10:14] Bug #138592: Bug notifications have personal To: header but aren't personal

[11:24] lifeless: Mm, that wouldn't be too hard, I guess. Some code review stuff already does something kind of similar (well, it actually does a weird thing where it tries to work out which of the people the notification's being sent to that you're allowed to know about, but I see even less justification for that for bugs than for code) [11:24] Code at least includes the MP address [11:31] hey all :-) There is a way to login to local installation with a non-admin user? [11:31] rpadovani: Use utilities/make-lp-user [11:31] ty! [11:31] (You'll want to start a local keyserver first, probably - utilities/start-dev-soyuz.sh is overkill for that but works) [11:33] wgrant: Answers should behave the same way, I think. [12:06] Hm, indeed. [12:41] cjwatson, o/ I found a very trivial bug to fix, so I wrote the one line change needed, but I'm a bit confused about how writing the related test. To don't bother you right now, I wrote all in the description of the MR - when you have some spare time, could you please take a look? Thanks so much :-) [12:41] https://code.launchpad.net/~rpadovani/launchpad/unhide-comments/+merge/270950 [12:45] rpadovani: You'll want to add an optional comment_owner argument that's passed to makeBugComment as owner=, like TestBugHideCommentControls.getContext just below. Then extend TestMessageVisibilityMixin with a test saying that the comment owner can see the comment. [12:46] cjwatson, okay, thanks, I try [13:18] well, I pushed the test, but lp doesn't want to update my branch information. Time to study I think, I'll take a look later [13:18] rpadovani: I'll keep rescanning it until it works. [13:19] thanks [13:21] rpadovani: In fact, it's there now. [13:24] oh, great [15:49] hi cjwatson, thanks for your time :-) I have a couple of questions about your review: you wrote hidden comments need admin style - is that the one with gray background? If yes, it's already present, without having to edit BugComment.show_for_admin [15:50] Another question: how stories are supposed to work? I write what is expected and then write some code that should demonstrate that? [15:56] rpadovani: Oh, right, the implementation of show_for_admin is different from how I remember. [15:57] rpadovani: Stories *are* tests. [15:57] The indented stuff is run. [15:57] https://docs.python.org/2/library/doctest.html [15:57] oooh, I see! How much things I have to learn, thanks :-) [15:58] (They are mostly very irritating and we prefer not to write more of them, but it's OK to extend existing ones a little bit if there isn't an immediate alternative. Rewriting as unit tests is usually better.) [15:58] rpadovani: I think a pagetest (== .../stories/...) addition is a good idea here even if you don't need to change show_for_admin. [15:59] ok, great, I add it, thanks [15:59] JOOI (and OT): do you think you'll be able to move to py3 in only 5 years? [16:00] mapreri: Heh, hopefully. Every so often we shave off another bit of the problem. [16:00] looks like lot of pain from an outsider pov [16:01] good luck :) [16:01] After we move off buildout, we'll be able to upgrade to a current ZTK, and then it might actually be possible to see how terrible the situation is. [16:02] Right now it's only possible to attack in the abstract, which isn't very rewarding. [16:02] Also, we're still mostly deploying on 12.04, and Python 3.2 wasn't a brilliant porting target. [16:03] ic [16:05] Why do you ask? [16:05] sorry to bother again, how could I run the story test? Using python -m doctest -v lib/lp/bugs/stories/bugs/xx-bug-hidden-comments.txt doesn't work [16:07] cjwatson: just curiosity (sometimes i really act like a kid, with my curiosity) :) [16:08] rpadovani: bin/test -vvct lib/lp/bugs/stories/bugs/xx-bug-hidden-comments.txt [16:08] (or a regex that matches that) [16:08] thanks [16:08] mapreri: OK, just wondering about the choice of five years [16:09] cjwatson: py2 is doomed for EOL around 2020, you know... [16:09] Oh right [16:10] It's not a timeframe I've been worrying about much [16:10] Too many closer ones :) [16:11] eheh [16:12] I did go through recently and get rid of most of our use of class advice, which was a multi-thousand line patch needed for py3 [16:12] So we're not ignoring it :) [16:14] nice :) [16:20] cjwatson, how can I use user_browser.open() to log in with a different user? My edits broke test in xx-bug-hidden-comments.txt at line 40 because now the comment is visible - to test if a ordinary user doesn't see the hidden comment, I need to change user - how? (or, what can I read to understand the solution?) [16:28] rpadovani: You'll need to get hold of a different browser instance that's authenticated as a different user. The pagetests often rely on the sample data shipped with Launchpad, so the easiest way is probably something like >>> test_browser = setupBrowser(auth="Basic test@canonical.com:test") [16:29] Then you'll want to test that the comment appears for the non-admin user that posted the comment (with basically the same set of tests we perform for admin users), and doesn't appear for a different non-admin user. [16:30] okay, thanks [16:30] lib/lp/testing/pages.py is the file that sets up things like user_browser [16:39] cjwatson, I pushed new stories - thanks for your suggestions, I hope to become more independent with time ;-) [16:41] rpadovani: maybe assign yourself the bug? [16:42] mapreri: not required [16:43] (I mean, neater perhaps, but the QA bot is about to do so anyway) [16:43] rpadovani: LGTM, thanks, landing [16:43] goody bot [16:47] cjwatson, thanks, I'll take a look to another bug then - usually I assign bugs to myself only when I'm sure I know how to fix them, to don't block any other user that wants to fix it [16:48] Reasonable. === Guest39525 is now known as anthonyjf === anthonyf is now known as Guest95842 === Guest95842 is now known as anthonyjf [20:37] morning [22:33] rpadovani: Have you looked at your buildbot test failures? [22:33] rpadovani: Some are spurious, but the failure in lib/lp/bugs/stories/bugs/xx-bug-hidden-comments.txt is real, which is weird because I thought you said you'd run that. [22:36] Morning. [22:37] rpadovani: Ah, you need to fix BugTaskNavigation.traverse_comments too [22:42] * cjwatson assumes rpadovani has gone to bed, so fixes locally and runs the bugs tests [22:44] cjwatson: Nothing appeared anomalous DB-wise overnight? [22:45] wgrant: Nothing I saw, no. [22:45] Lovely. [22:48] So my plan for BugBMP is that in the bzr case it will be tied exactly to BugBranch while the MP is open. [22:48] Which is slightly messy, but preserves the existing behaviour and hopefully makes things relatively unconfusing. [22:48] Yeah, I was wondering how you were going to make that not weird ... [22:49] We can probably remove the UI to link a branch directly later on, and need to change bug notifications. [22:49] I'm very open to suggestions if you have any... [22:49] So you'd be able to edit BugBMP, but in the bzr case it would push it through to BugBranch? [22:49] But I think BugBMP is the only approach that makes sense, so we have to make it usable somehow. [22:49] Right. [22:50] Or maybe it only resyncs when the preview diff is updated. [22:50] Yeah, none of my half-thoughts about how to do it on GitRef directly have made any kind of sense. [22:50] Hmm. [22:50] If it only resyncs when the preview diff is updated, then editing BugBMP is difficult. [22:50] True. [22:50] I don't think that makes sense. After all, today, if you link a branch to a bug then that's reflected immediately in the BMP. [22:51] Without updating a diff. [22:51] Yeah. [22:51] Well. [22:51] No, maybe not. [22:51] If we remove the BugBranch UI... [22:51] Then you can only link by pushing a new rev, which will update the diff, or by clicking the link on the BMP directly, or by using the BugBranch API. [22:52] That would also make it impossible to see bzr branches that fix bugs unless they've had MPs proposed. [22:52] Which, granted, won't work in the git case. [22:52] (Although it does work on GitHub ...) [22:52] Indeed. [22:53] We could do a BugGitRepository with very weak links. [22:53] I don't know how GitHub decides what is relevant. [22:53] I'm not sure how pulling from one branch to another would work. It would have to be tied into merge detection somehow. [22:53] Otherwise everything is attached to everything. [22:53] Maybe it's only the first time each rev appears. [22:53] Right, exactly. [22:53] The LP logic for that is very weird. [22:54] And there are certainly some branches with implausible numbers of bug links as a result. [22:54] I did at one point know why trunks sometimes got bugs linked. [22:54] It might be worth some effort working out GH's logic. [22:54] I can't remember now, though. [22:55] First time each rev appears doesn't seem a terrible start, but then what if you rename the branch [23:00] Yup, exactly. [23:00] Will poke GitHub. [23:00] Well, we can fairly cheaply work out which branches are ancestors of which other ones in a repository [23:00] That's near enough how our merge detection works [23:00] Yep, but what happens if you have two trunks? [23:00] So it could just be any links from branches that aren't merged [23:01] Doesn't matter whether they're trunks or not [23:02] What does "that aren't merged" mean, if not merged into trunk? [23:02] The interesting commits are the ones that are unique to a branch [23:02] Now, the trick is handling the case where a branch is merged into a trunk and then deleted [23:03] Couple of possible approaches to that [23:03] Or where I push a branch, then push a superset of the same branch to a different name because the first name was wrong. [23:03] One promising one might be to have merge detection remember that everything older than the mergepoint is uninteresting for commit scans [23:03] Maybe [23:05] It's hard to tell the difference between your case and the case of the creation of a new trunk. Whatever a trunk is. [23:05] Exactly :) [23:05] (Anything that relies on us knowing what a trunk is is probably wrong ...) [23:06] Seems like a thing worth trying out on GH, anyway. [23:06] Yep, will do. [23:31] cjwatson: Are you still looking at that testfix? [23:32] Yes [23:32] Nearly done, just trying to get the blasted thing to scan before pushing the fix rev [23:32] Hah [23:32] Thanks. [23:32] lp.bugs passes now though [23:37] wgrant: https://code.launchpad.net/~cjwatson/launchpad/testfix-unhide-comments/+merge/271043 [23:37] Hmm. [23:38] I'm not actually sure I like randoms being able to unhide their comments, but I guess until we have a way to fix mistakes it's awkward. [23:38] So it'll do. [23:39] Yeah, it's not unproblematic. But I guess that in cases of serious abuse we'd suspend accounts anyway. [23:39] Right, but it is open to other more subtle abuse. [23:39] And they can always repost, it's not like it really helps much to not let them unhide [23:39] Making parts of the conversation disappear and then reappear when it's convenient. [23:40] cjwatson: Did you prefer LaunchBag over self.request deliberately? [23:41] I didn't know that API well; I noticed that self.request was =None in Navigation's constructor, and found ILaunchBag used elsewhere [23:41] Ah, OK, sounds reasonable. [23:41] Maybe comment hiding/unhiding should go in the activity log. [23:41] No. [23:41] Spam is a thing. [23:42] We should fix the hiding silliness to be admin-only eventually. [23:42] Feel like fixing the rest of that traversal check while you're there? [23:42] It's moderately annoying for us, and it's the next line... [23:43] Sorry, I'm nearly asleep. What's the problem with it? [23:44] Oh, it doesn't allow registry does it [23:44] Right, it should probably use Bug.userCanSetcommentVisiblity [23:44] +i [23:45] So should get_visible_comments, then [23:46] Ah, get_visible_comments doesn't allow project-privileged users, it seems. [23:46] Which is arguably correct from a spam perspective. [23:47] But awkwardly inconsistent. [23:47] So they can hide spam, but can't see that they've done it? [23:47] Apparently. [23:52] cjwatson: Ah, so GitHub doesn't actually track this. [23:52] cjwatson: The PR page doesn't directly list the issues, only by linkifying them in commit messages. [23:52] And the issue page tells you that a bug was closed by merging into the default branch, or that it was referenced in a commit (not a branch) [23:53] Of course in Git you don't need to know the branch, just the repo, and it's always the same repo. [23:53] Well, on GitHub it's always the same repo. [23:54] Ah, right, the commit link has no ref context [23:54] * wgrant tries a cross-repo reference. [23:55] We can't be quite so simple, as we have multiple repos. [23:55] wgrant: https://code.launchpad.net/~cjwatson/launchpad/testfix-unhide-comments/+merge/271043 brushed up slightly [23:56] It seems to only show the first time it saw each commit. [23:56] If I reference wgrant/test#5 in a commit in wgrant/test2, the issue shows that. [23:57] But subsequently pushing that same commit to wgrant/test doesn't show anything, unless it causes the issue to be closed. [23:58] cjwatson: Thanks, looks good. [23:59] Thanks, landing. [23:59] Hmmmmmm. [23:59] Trying to think how this should work. [23:59] I guess it's not so bad if we don't link from the repo to the fixed bugs. [23:59] The list on a bug will not grow large, just the list on the repo.