[07:15] <cjwatson> 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] <cjwatson> 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] <cjwatson> (All proposed, so I'll bug a webop once I catch one)
[07:17] <wgrant> Can we actually deploy mirror fixes atm?
[07:17] <wgrant> There was that RT that Adam said might have been progressed or something.
[07:18] <cjwatson> Is this https://rt.admin.canonical.com/Ticket/Display.html?id=83847 ?
[07:18] <cjwatson> Ha, I think I missed the existence of that charm
[07:18] <cjwatson> Is canonical-magic-mirror not being used any more?
[07:18] <wgrant> magic-mirror is used for the split
[07:19] <wgrant> But we also need to verify that the charm syncs InRelease last.
[07:19] <wgrant> As it wasn't doing with Translations-* until that ticket.
[07:20] <cjwatson> Right
[07:20] <wgrant> The charm doesn't do any weird stuff like magic-mirror, just the two-stage sync.
[07:21] <wgrant> cjwatson: Did you look into Tarmac git support in any depth?
[07:21] <wgrant> I don't intend to excise bzrlib's command processing etc. at this stage.
[07:23] <cjwatson> wgrant: Only to the extent of satisfying myself that it isn't hard and doing some extra API exports for it
[07:23] <cjwatson> zyga said he was working on it, but I think we should time out on that soon.
[07:25] <cjwatson> I wonder where https://pastebin.canonical.com/139395/ comes from
[07:25] <cjwatson> There are similar files in puppet, but not ports-sync.
[07:38] <wgrant> cjwatson: I also intend to add BugBMP tomorrow.
[07:38] <wgrant> Initially populated by BugBranches being added, but also manageable explicitly.
[07:39] <wgrant> Eventually the diff generator should ask turnip to regexp the new commits to link bugs.
[07:39] <wgrant> I think that's the only reasonable approach for git, do you agree?
[07:40] <wgrant> cjwatson: Hm, I didn't realise ports-sync was different.
[07:40] <wgrant> cjwatson: It may be manually maintained; there are few ports mirrors.
[07:41] <cjwatson> BugBMP etc.> Agreed.
[07:41] <cjwatson> wgrant: Yeah, appears to be (see #is)
[07:41] <wgrant> Sorry, busy collecting popcorn and reading too many channels.
[07:41] <wgrant> 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] <wgrant> Heh
[07:44] <wgrant> That's an easy one, then.
[07:47] <cjwatson> Also I checked that I couldn't reproduce anything like the original apt InRelease security problems with precise and trusty
[07:47] <cjwatson> (Indeed, precise just has InRelease disabled with a big hammer)
[07:47] <wgrant> Oh, I didn't realise it was disabled.
[07:47] <wgrant> Since Debian was using it by that point.
[07:47] <wgrant> Maybe it was disabled while the sensible gpgv usage was fixed.
[07:48] <cjwatson> I guess our apt was a while out of sync.
[07:48] <cjwatson> Right.
[07:48] <wgrant> Rather than trying to parse it out manually.
[07:48] <cjwatson> In trusty, if you try tricks like appending an InRelease file, then it ignores the unsigned portions of the file
[07:49] <cjwatson> Which feels a bit odd, but not exploitably so
[07:51] <wgrant> That sounds like it's doing the right thing.
[07:51] <wgrant> 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] <wgrant> 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] <cjwatson> Yeah.  Thanks.
[08:02] <cjwatson> 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] <wgrant> Yep
[08:03] <wgrant> The fallback is rather unpleasant, but we have little choice for at least a couple of years.
[08:03] <cjwatson> 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] <wgrant> Oh, I didn't realise a-f had support.
[08:03] <wgrant> How does it handle GC?
[08:04] <cjwatson> Keeps three by default.
[08:04] <wgrant> I'd be doing it in LP.
[08:04] <wgrant> Because we need to do it for NMAF anyway.
[08:04] <cjwatson> True.
[08:04] <wgrant> It's not dissimilar from the Release code which we already reimplement.
[08:04] <cjwatson> I think we want finer-grained control than a-f offers; it only lets you control the count.
[08:04] <wgrant> We currently let a-f write directly to dists, but that's only because it's convenient.
[08:04] <cjwatson> (APT::FTPArchive::By-Hash-Keep)
[08:05] <wgrant> 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] <wgrant> I prefer to keep our dependency on a-f to a minimum.
[08:05] <cjwatson> Yeah, I guess so.
[09:34] <lifeless> cjwatson: since you're hacking on email notifications
[09:34] <lifeless> cjwatson: there is one bug (I don't have it to hand) that would make me so so so super happy
[09:35] <lifeless> cjwatson: which is that LP always sends mail to *me* even if I'm only indirectly subscribed
[09:35] <lifeless> cjwatson: which breaks GMail 'mute'
[10:00] <wgrant> lifeless: Just bugmail?
[10:00] <wgrant> There's a bug about that, let me find it.
[10:10] <lifeless> wgrant: bugmail is ~99% of my LP mail
[10:10] <lifeless> wgrant: so even if its not *just*.
[10:11] <lifeless> would still be happy happy joy joy schnaaaake time.
[10:11] <lifeless> Night all.
[10:11] <wgrant> lifeless: Oh sure, just checking there were no other cases.
[10:11] <wgrant> If we fix one source and you still whine, that's not good :)
[10:14] <wgrant> https://bugs.launchpad.net/launchpad/+bug/138592
[10:14] <mup> Bug #138592: Bug notifications have personal To: header but aren't personal <email> <lp-bugs> <notifications> <Launchpad itself:Triaged> <https://launchpad.net/bugs/138592>
[11:24] <cjwatson> 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] <cjwatson> Code at least includes the MP address
[11:31] <rpadovani> hey all :-) There is a way to login to local installation with a non-admin user?
[11:31] <cjwatson> rpadovani: Use utilities/make-lp-user
[11:31] <rpadovani> ty!
[11:31] <cjwatson> (You'll want to start a local keyserver first, probably - utilities/start-dev-soyuz.sh is overkill for that but works)
[11:33] <cjwatson> wgrant: Answers should behave the same way, I think.
[12:06] <wgrant> Hm, indeed.
[12:41] <rpadovani> 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] <rpadovani> https://code.launchpad.net/~rpadovani/launchpad/unhide-comments/+merge/270950
[12:45] <cjwatson> 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] <rpadovani> cjwatson, okay, thanks, I try
[13:18] <rpadovani> 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] <cjwatson> rpadovani: I'll keep rescanning it until it works.
[13:19] <rpadovani> thanks
[13:21] <cjwatson> rpadovani: In fact, it's there now.
[13:24] <rpadovani> oh, great
[15:49] <rpadovani> 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] <rpadovani> Another question: how stories are supposed to work? I write what is expected and then write some code that should demonstrate that?
[15:56] <cjwatson> rpadovani: Oh, right, the implementation of show_for_admin is different from how I remember.
[15:57] <cjwatson> rpadovani: Stories *are* tests.
[15:57] <cjwatson> The indented stuff is run.
[15:57] <cjwatson> https://docs.python.org/2/library/doctest.html
[15:57] <rpadovani> oooh, I see! How much things I have to learn, thanks :-)
[15:58] <cjwatson> (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] <cjwatson> 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] <rpadovani> ok, great, I add it, thanks
[15:59] <mapreri> JOOI (and OT): do you think you'll be able to move to py3 in only 5 years?
[16:00] <cjwatson> mapreri: Heh, hopefully.  Every so often we shave off another bit of the problem.
[16:00] <mapreri> looks like lot of pain from an outsider pov
[16:01] <mapreri> good luck :)
[16:01] <cjwatson> 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] <cjwatson> Right now it's only possible to attack in the abstract, which isn't very rewarding.
[16:02] <cjwatson> Also, we're still mostly deploying on 12.04, and Python 3.2 wasn't a brilliant porting target.
[16:03] <mapreri> ic
[16:05] <cjwatson> Why do you ask?
[16:05] <rpadovani> 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] <mapreri> cjwatson: just curiosity (sometimes i really act like a kid, with my curiosity) :)
[16:08] <cjwatson> rpadovani: bin/test -vvct lib/lp/bugs/stories/bugs/xx-bug-hidden-comments.txt
[16:08] <cjwatson> (or a regex that matches that)
[16:08] <rpadovani> thanks
[16:08] <cjwatson> mapreri: OK, just wondering about the choice of five years
[16:09] <mapreri> cjwatson: py2 is doomed for EOL around 2020, you know...
[16:09] <cjwatson> Oh right
[16:10] <cjwatson> It's not a timeframe I've been worrying about much
[16:10] <cjwatson> Too many closer ones :)
[16:11] <mapreri> eheh
[16:12] <cjwatson> 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] <cjwatson> So we're not ignoring it :)
[16:14] <mapreri> nice :)
[16:20] <rpadovani> 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] <cjwatson> 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] <cjwatson> 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] <rpadovani> okay, thanks
[16:30] <cjwatson> lib/lp/testing/pages.py is the file that sets up things like user_browser
[16:39] <rpadovani> cjwatson, I pushed new stories - thanks for your suggestions, I hope to become more independent with time ;-)
[16:41] <mapreri> rpadovani: maybe assign yourself the bug?
[16:42] <cjwatson> mapreri: not required
[16:43] <cjwatson> (I mean, neater perhaps, but the QA bot is about to do so anyway)
[16:43] <cjwatson> rpadovani: LGTM, thanks, landing
[16:43] <mapreri> goody bot
[16:47] <rpadovani> 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] <cjwatson> Reasonable.
[20:37] <blr> morning
[22:33] <cjwatson> rpadovani: Have you looked at your buildbot test failures?
[22:33] <cjwatson> 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] <wgrant> Morning.
[22:37] <cjwatson> 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] <wgrant> cjwatson: Nothing appeared anomalous DB-wise overnight?
[22:45] <cjwatson> wgrant: Nothing I saw, no.
[22:45] <wgrant> Lovely.
[22:48] <wgrant> 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] <wgrant> Which is slightly messy, but preserves the existing behaviour and hopefully makes things relatively unconfusing.
[22:48] <cjwatson> Yeah, I was wondering how you were going to make that not weird ...
[22:49] <wgrant> We can probably remove the UI to link a branch directly later on, and need to change bug notifications.
[22:49] <wgrant> I'm very open to suggestions if you have any...
[22:49] <cjwatson> So you'd be able to edit BugBMP, but in the bzr case it would push it through to BugBranch?
[22:49] <wgrant> But I think BugBMP is the only approach that makes sense, so we have to make it usable somehow.
[22:49] <wgrant> Right.
[22:50] <wgrant> Or maybe it only resyncs when the preview diff is updated.
[22:50] <cjwatson> Yeah, none of my half-thoughts about how to do it on GitRef directly have made any kind of sense.
[22:50] <wgrant> Hmm.
[22:50] <cjwatson> If it only resyncs when the preview diff is updated, then editing BugBMP is difficult.
[22:50] <wgrant> True.
[22:50] <cjwatson> 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] <cjwatson> Without updating a diff.
[22:51] <wgrant> Yeah.
[22:51] <wgrant> Well.
[22:51] <wgrant> No, maybe not.
[22:51] <wgrant> If we remove the BugBranch UI...
[22:51] <wgrant> 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] <cjwatson> That would also make it impossible to see bzr branches that fix bugs unless they've had MPs proposed.
[22:52] <cjwatson> Which, granted, won't work in the git case.
[22:52] <cjwatson> (Although it does work on GitHub ...)
[22:52] <wgrant> Indeed.
[22:53] <cjwatson> We could do a BugGitRepository with very weak links.
[22:53] <wgrant> I don't know how GitHub decides what is relevant.
[22:53] <cjwatson> 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] <cjwatson> Otherwise everything is attached to everything.
[22:53] <wgrant> Maybe it's only the first time each rev appears.
[22:53] <wgrant> Right, exactly.
[22:53] <wgrant> The LP logic for that is very weird.
[22:54] <cjwatson> And there are certainly some branches with implausible numbers of bug links as a result.
[22:54] <wgrant> I did at one point know why trunks sometimes got bugs linked.
[22:54] <cjwatson> It might be worth some effort working out GH's logic.
[22:54] <wgrant> I can't remember now, though.
[22:55] <cjwatson> First time each rev appears doesn't seem a terrible start, but then what if you rename the branch
[23:00] <wgrant> Yup, exactly.
[23:00] <wgrant> Will poke GitHub.
[23:00] <cjwatson> Well, we can fairly cheaply work out which branches are ancestors of which other ones in a repository
[23:00] <cjwatson> That's near enough how our merge detection works
[23:00] <wgrant> Yep, but what happens if you have two trunks?
[23:00] <cjwatson> So it could just be any links from branches that aren't merged
[23:01] <cjwatson> Doesn't matter whether they're trunks or not
[23:02] <wgrant> What does "that aren't merged" mean, if not merged into trunk?
[23:02] <cjwatson> The interesting commits are the ones that are unique to a branch
[23:02] <cjwatson> Now, the trick is handling the case where a branch is merged into a trunk and then deleted
[23:03] <cjwatson> Couple of possible approaches to that
[23:03] <wgrant> 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] <cjwatson> One promising one might be to have merge detection remember that everything older than the mergepoint is uninteresting for commit scans
[23:03] <cjwatson> Maybe
[23:05] <cjwatson> 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] <wgrant> Exactly :)
[23:05] <cjwatson> (Anything that relies on us knowing what a trunk is is probably wrong ...)
[23:06] <cjwatson> Seems like a thing worth trying out on GH, anyway.
[23:06] <wgrant> Yep, will do.
[23:31] <wgrant> cjwatson: Are you still looking at that testfix?
[23:32] <cjwatson> Yes
[23:32] <cjwatson> Nearly done, just trying to get the blasted thing to scan before pushing the fix rev
[23:32] <wgrant> Hah
[23:32] <wgrant> Thanks.
[23:32] <cjwatson> lp.bugs passes now though
[23:37] <cjwatson> wgrant: https://code.launchpad.net/~cjwatson/launchpad/testfix-unhide-comments/+merge/271043
[23:37] <wgrant> Hmm.
[23:38] <wgrant> 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] <wgrant> So it'll do.
[23:39] <cjwatson> Yeah, it's not unproblematic.  But I guess that in cases of serious abuse we'd suspend accounts anyway.
[23:39] <wgrant> Right, but it is open to other more subtle abuse.
[23:39] <cjwatson> And they can always repost, it's not like it really helps much to not let them unhide
[23:39] <wgrant> Making parts of the conversation disappear and then reappear when it's convenient.
[23:40] <wgrant> cjwatson: Did you prefer LaunchBag over self.request deliberately?
[23:41] <cjwatson> 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] <wgrant> Ah, OK, sounds reasonable.
[23:41] <cjwatson> Maybe comment hiding/unhiding should go in the activity log.
[23:41] <wgrant> No.
[23:41] <wgrant> Spam is a thing.
[23:42] <wgrant> We should fix the hiding silliness to be admin-only eventually.
[23:42] <wgrant> Feel like fixing the rest of that traversal check while you're there?
[23:42] <wgrant> It's moderately annoying for us, and it's the next line...
[23:43] <cjwatson> Sorry, I'm nearly asleep.  What's the problem with it?
[23:44] <cjwatson> Oh, it doesn't allow registry does it
[23:44] <wgrant> Right, it should probably use Bug.userCanSetcommentVisiblity
[23:44] <wgrant> +i
[23:45] <cjwatson> So should get_visible_comments, then
[23:46] <wgrant> Ah, get_visible_comments doesn't allow project-privileged users, it seems.
[23:46] <wgrant> Which is arguably correct from a spam perspective.
[23:47] <wgrant> But awkwardly inconsistent.
[23:47] <cjwatson> So they can hide spam, but can't see that they've done it?
[23:47] <wgrant> Apparently.
[23:52] <wgrant> cjwatson: Ah, so GitHub doesn't actually track this.
[23:52] <wgrant> cjwatson: The PR page doesn't directly list the issues, only by linkifying them in commit messages.
[23:52] <wgrant> 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] <wgrant> Of course in Git you don't need to know the branch, just the repo, and it's always the same repo.
[23:53] <wgrant> Well, on GitHub it's always the same repo.
[23:54] <cjwatson> Ah, right, the commit link has no ref context
[23:54]  * wgrant tries a cross-repo reference.
[23:55] <wgrant> We can't be quite so simple, as we have multiple repos.
[23:55] <cjwatson> wgrant: https://code.launchpad.net/~cjwatson/launchpad/testfix-unhide-comments/+merge/271043 brushed up slightly
[23:56] <wgrant> It seems to only show the first time it saw each commit.
[23:56] <wgrant> If I reference wgrant/test#5 in a commit in wgrant/test2, the issue shows that.
[23:57] <wgrant> But subsequently pushing that same commit to wgrant/test doesn't show anything, unless it causes the issue to be closed.
[23:58] <wgrant> cjwatson: Thanks, looks good.
[23:59] <cjwatson> Thanks, landing.
[23:59] <wgrant> Hmmmmmm.
[23:59] <wgrant> Trying to think how this should work.
[23:59] <wgrant> I guess it's not so bad if we don't link from the repo to the fixed bugs.
[23:59] <wgrant> The list on a bug will not grow large, just the list on the repo.