Re: Phabricator Update, July 2017
Mark Côté wrote: > It was announced in May > (https://groups.google.com/forum/#!topic/mozilla.tools/4qroY2Iia9I), > linked to in this forum: > https://groups.google.com/d/msg/mozilla.dev.platform/qh5scX3Gk2U/xCWe8jrOAQAJ I stand corrected, thanks. I would've thought that'd be put in moz.dev.planning or at least cross-posted since it is a planning issue. Edmund ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
Hi Joe, I just want to publicly apologize for being sarcastic in my original post to you. I could've found a better voice and the frustration clouded my judgement. I'm sorry. Edmund ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: More Rust code
BTW, speaking of training: Jason's and my book, "Programming Rust" will be available on paper from O'Reilly on August 29th! Steve Klabnik's book with No Starch Press is coming out soon as well, but I don't know the details there. On Mon, Jul 17, 2017 at 12:43 PM, Ted Mielczarek wrote: > Nick, > > Thanks for kicking off this discussion! I felt like a broken record > talking to people about this in SF. From my perspective Rust is our > single-biggest competitive advantage for shipping Firefox, and every > time we choose C++ over Rust we throw that away. We know the costs of > shipping complicated C++ code: countless hours of engineering time spent > chasing down hard-to-reproduce crashes, exploitable security holes, and > threading issues. Organizationally we need to get to a point where every > engineer has the tools and training they need to make Rust their first > choice for writing code that ships with Firefox. > > On Mon, Jul 10, 2017, at 09:15 PM, Bobby Holley wrote: > > I think this is pretty uncontroversial. The high-level strategic decision > > to bet on Rust has already been made, and the cost of depending on the > > language is already sunk. Now that we're past that point, I haven't heard > > anyone arguing why we shouldn't opt for memory safety when writing new > > standalone code. If there are people out there who disagree and think > > they > > have the arguments/clout/allies to make the case, please speak up. > > From my anecdotal experiences, I've heard two similar refrains: > 1) "I haven't learned Rust well enough to feel confident choosing it for > this code." > 2) "I don't want to spend time learning Rust that I could be spending > just writing the code in C++." > > I believe that every developer that writes C++ at Mozilla should be > given access to enough Rust training and work hours to spend learning it > beyond the training so that we can eliminate case #1. With the Rust > training sessions at prior All-Hands and self-motivated learning, I > think we've pretty well saturated the group of early adopters. These > people are actively writing new Rust code. We need to at least get the > people that want to learn Rust but don't feel like they've had time to > that same place. > > For case #2, there will always be people that don't want to learn new > languages, and I'm sympathetic to their perspective. Learning Rust well > does take a large investment of time. I don't know that I would go down > the road of making Rust training mandatory (yet), but we are quickly > going to hit a point where "I don't feel like learning Rust" is not > going to cut it anymore. I would hope that by that point we will have > trained everyone well enough that case #2 no longer exists, but if not > we will have to make harder choices. > > > > The tradeoffs come when the code is less standalone, and we need to weigh > > the integration costs. This gets into questions like whether/how Rust > > code > > should integrate into the cycle collector or into JS object reflection, > > which is very much a technical decision that should be made by experts. I > > have a decent sense of who some of those experts might be, and would like > > to find the most lightweight mechanism for them to steer this ship. > > We definitely need to figure out an ergonomic solution for writing core > DOM components in Rust, but I agree that this needs a fair bit of work > to be feasible. Most of the situations I've seen recently were not that > tightly integrated into Gecko. > > -Ted > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On 2017-07-17 8:46 PM, Edmund Wong wrote: Mike Hoye wrote: Given that we've been talking about this stuff for years now, I think it's very clear that we haven't come to this point by "somebody at the top issuing an edict that they want something modern"; the decision to commit to Phabricator was ultimately announced on May 11th, and that decision wasn't made unilaterally or lightly. It's hardly fair to say that "we've been talking about this stuff for years now" when this is the very first time it's been posted. There was no public notification of the intention of doing away with splinter/MozReview. Sure, there may have been talks but were they public and not just hidden in some obscure thread? It was announced in May (https://groups.google.com/forum/#!topic/mozilla.tools/4qroY2Iia9I), linked to in this forum: https://groups.google.com/d/msg/mozilla.dev.platform/qh5scX3Gk2U/xCWe8jrOAQAJ Mark ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On Mon, Jul 17, 2017 at 12:27 PM, Gregory Szorc wrote: > If the bug is only serving as an anchor to track code review, then the > question we should be asking is "do we even need a bug." > In my experience the answer to this is "yes, we need a bug". I very rarely have a one-to-one mapping between bugs and review requests. Sometimes there are many reviews per bug. Sometimes there are no reviews in a bug, but it still contains useful information. All the meta information around bug blockers/dependents is orthogonal to reviews. If you want to get rid of bugzilla in favor of your new tool, you are really creating a new bug tracker with a built-in review tool, IMO. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Status of Closing Intermittents
Two Sundays ago I went through and closed the first batch of intermittent bugs with no new OrangeFactor (or other) comments. That was 5,000 bugs and cause an bugmail storm. I'm holding off on closing the next batch, because we need to run a script to do this. https://bugzilla.mozilla.org/show_bug.cgi?id=1381567 is tracking it. Intermittent bugs don't count against totals in my reports, but I'd still like to get these cleaned up. -- Emma ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
Mike Hoye wrote: > > Given that we've been talking about this stuff for years now, I think > it's very clear that we haven't come to this point by "somebody at the > top issuing an edict that they want something modern"; the decision to > commit to Phabricator was ultimately announced on May 11th, and that > decision wasn't made unilaterally or lightly. It's hardly fair to say that "we've been talking about this stuff for years now" when this is the very first time it's been posted. There was no public notification of the intention of doing away with splinter/MozReview. Sure, there may have been talks but were they public and not just hidden in some obscure thread? > > We've been discussing how to improve our tooling and processes in open > forums for years now, most frequently on dev-platform. Despite that, > it's true that a lot of the major influences of this decision have been > mostly invisible to people who aren't involved in the day to day work on > infrastructure and tooling, in the way that icebergs are mostly invisible. Not exactly a good analogy there, considering icebergs can also sink ships. So this discussion has been pretty much invisible from people who use the tools and only for the select few. > Some of the things that haven't made it into this thread that have > influenced this decision include: > > - a real and pressing requirement to update our commit access policies > and practices, and backstop them with reliable tooling (a discussion you > participated in, as I recall), Yes. I did participate in that. Though I'm not entirely sure what came off that discussion. > - the significant engineering burdens (including opportunity cost and > bus-factor risk) of being the sole owner/user of a major piece of > infrastructure software that's not our core product, and Oh hell yeah. This truck/bus factor is a definite PITA, and I applaud any effort in fixing this. > - the much more pernicious set of hidden costs we incur from _failing_ > to standardize on certain tools and processes. I agree as well. > > It's a very serious drag on product development and contributor > participation if everyone - paid senior engineers and new contributors > alike - needs to use _this_ tool and process and coding style (and and > and) to participate in one part of the project, and _that_ etc. etc. to > participate in another. Or, sometimes, even other corners of the same > codebase. Oh... hell yeah. You've mentioned quite a lot of pain points in the whole process, and I certainly agree with them. However, it would've been a better process if there was some sort of public declaration of intention of moving to a tool which would streamline the whole review and commit process. While there may have been a smattering of discussions here of commit policies, there wasn't a concise, for lack of a better word, glue to point out that there was going to be this change. The point is this. We have bugzilla and having a review process that enable people to review patches without needing to jump to another tool, would be more effective than needing to jump from one tool (bugzilla) to another one. If Phabricator does the job well, then so be it. Thank you for your clarifications, Mike. Edmund ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposed W3C Charter: Web Commerce Interest Group
On Mon, Jul 17, 2017 at 4:11 PM, L. David Baron wrote: > The W3C is proposing a new charter for: > > Web Commerce Interest Group > https://www.w3.org/2017/03/commerce-charter.html > https://lists.w3.org/Archives/Public/public-new-work/2017Jul/0008.html Context: while technically with a new name "Commerce", this is the replacement/follow-on for the existing Web Payments Interest Group (Payments IG) which expires around the same time Commerce IG is to start. https://www.w3.org/2014/04/payments/webpayments_charter.html The chairs are the same set of three people. And while W3C's HTML diff tool has limitations, it does seem that most of the charter is dramatically different: http://services.w3.org/htmldiff?doc1=https%3A%2F%2Fwww.w3.org%2F2014%2F04%2Fpayments%2Fwebpayments_charter.html&doc2=https%3A%2F%2Fwww.w3.org%2F2017%2F03%2Fcommerce-charter.html > Mozilla has the opportunity to send support, comments, or objections > through Monday, August 28. If this is work that we want to see > happen at W3C, we should explicitly support it; if there are things > we think should be different about the charter, this is the time to > say so. > > Please reply to this thread if you think there's something we should > say as part of this charter review, or if you think we should > support or oppose it. One glaring difference is that the previous/current Payments IG has a long "Dependencies and Liaisons" section, while the Commerce IG has none (not even an explicit "liason" with the Web Payments Working Group), that seems like an odd omission. I'd like to hear feedback from Marcos (cc'd) on how/why this group will complement or help our work in the Web Payments WG (does not seem to be addressed by the proposed Commerce IG charter). Thanks, Tantek ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Care in the use of MOZ_CRASH_UNSAFE_* macros: data review needed
On Mon, Jul 17, 2017 at 10:01 AM, Kris Maglione wrote: > On Mon, Jul 17, 2017 at 09:52:55AM -0700, Bobby Holley wrote: > >> On Mon, Jul 17, 2017 at 9:42 AM, Benjamin Smedberg > > >> wrote: >> >> I don't know really anything about how rust panics get reflected into >>> crash-data. Who would be the right person to talk to about that? >>> >>> >> Rust panics are equivalent to MOZ_CRASHES, and we treat them as such (or >> at >> least try to, see bug 1379857). >> >> Rust makes it easier to put non-constant things in the crash strings, >> which >> can be quite useful (see [1] as an example). They're not used often, and >> it >> seems unlikely that the existing use-cases would pose privacy issues, but >> I >> don't have a good proposal for enforcing that. >> > > It would be nice if we could add a commit hook that required a data-r tag > for any changes that add MOZ_CRASH_UNSAFE_PRINTF or a Rust panic with a > non-static string. I suspect it's something that most people will tend to > overlook. For the cases that are clearly not data privacy issues, we could > accept data-r=trivial, or something like that. > Phabricator allows you to create rules when certain events occur. You can react to changes to certain files or when certain content within files changes. Phabricator has the concept of a "blocking reviewer." If set, you must have a review from a "blocking reviewer" before the patch can be accepted (r+'d). A "blocking reviewer" can be an individual or a group. If a group, a member of that group must accept the patch. Putting the two together, Phabricator can be configured to automatically flag diffs containing changes to MOZ_CRASH_UNSAFE_PRINTF so they set a "blocking reviewer" from a "data review" group, thus ensuring data review is conducted before the patch lands. (This also means we could formalize module review in Phabricator if we wanted. And I predict there will be some interesting debates about how formal we want the module review policy to be in a post-Phabricator world and how "blocking reviewer" should be used. But that's for a different thread.) Phabricator's features are superior to custom server-side hooks for various reasons and I look forward to the day when we can define "commit policy" as part of Phabricator rules instead of custom hooks. Of course, we need all commits landing via Phabricator/Autoland before we can talk about abandoning server-side hooks completely. That's in the works for post-Phabricator. I'm unsure of timetable. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Proposed W3C Charter: Web Commerce Interest Group
The W3C is proposing a new charter for: Web Commerce Interest Group https://www.w3.org/2017/03/commerce-charter.html https://lists.w3.org/Archives/Public/public-new-work/2017Jul/0008.html Mozilla has the opportunity to send support, comments, or objections through Monday, August 28. If this is work that we want to see happen at W3C, we should explicitly support it; if there are things we think should be different about the charter, this is the time to say so. Please reply to this thread if you think there's something we should say as part of this charter review, or if you think we should support or oppose it. -David -- 𝄞 L. David Baron http://dbaron.org/ 𝄂 𝄢 Mozilla https://www.mozilla.org/ 𝄂 Before I built a wall I'd ask to know What I was walling in or walling out, And to whom I was like to give offense. - Robert Frost, Mending Wall (1914) signature.asc Description: PGP signature ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
I filed a central tracker bug for production Phabricator deployment: https://bugzilla.mozilla.org/show_bug.cgi?id=1381498. I have filed blockers and dependencies for a variety of related tasks as discussed in these threads. Mark On 2017-07-14 11:33 AM, Milan Sreckovic wrote: Replying in general, to a random message :) I don't have the numbers, but I imagine reviews are happening in the hundreds every day (if we land almost 300 patches.) So, I wouldn't expect the conversation about adding/removing/changing tools involved in reviews to be any less complicated, passionate and involved as what we're having here :) I believe Mark is putting together an e-mail to give us a better place to continue these conversations; something with meta bugs for "enable phabricator", "disable splinter", etc. This should let us track the issues that are raised, discuss decisions as to which of those should be blockers, and have a better record of what actually gets resolved and how. This should leave us *just* with the decision making and communication rollout issues that were raised; I know we are continuously discussing those and trying to get better at it, but it is a separate conversation from the technical issues, so it's probably OK that it remains separate. On 13-Jul-17 15:41, Randell Jesup wrote: To answer the other part of your question, MozReview will be disabled for active use across the board, but it is currently used by a small number of projects. Splinter will be disabled on a per-product basis, as there may be some projects that can't, won't, or shouldn't be migrated to Phabricator. Splinter is still a nice UI to look at patches already attached to bugs. Please don't disable it. excellent point; thanks for that feedback. instead of disabling splinter for phabricator backed products, we could make it a read-only patch viewer. I would consider it a massive fail if we disabled at least read-only splinter viewing of patches. As noted before. let's make sure we don't lose things we agreed on as issues. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: More Rust code
Nick, Thanks for kicking off this discussion! I felt like a broken record talking to people about this in SF. From my perspective Rust is our single-biggest competitive advantage for shipping Firefox, and every time we choose C++ over Rust we throw that away. We know the costs of shipping complicated C++ code: countless hours of engineering time spent chasing down hard-to-reproduce crashes, exploitable security holes, and threading issues. Organizationally we need to get to a point where every engineer has the tools and training they need to make Rust their first choice for writing code that ships with Firefox. On Mon, Jul 10, 2017, at 09:15 PM, Bobby Holley wrote: > I think this is pretty uncontroversial. The high-level strategic decision > to bet on Rust has already been made, and the cost of depending on the > language is already sunk. Now that we're past that point, I haven't heard > anyone arguing why we shouldn't opt for memory safety when writing new > standalone code. If there are people out there who disagree and think > they > have the arguments/clout/allies to make the case, please speak up. >From my anecdotal experiences, I've heard two similar refrains: 1) "I haven't learned Rust well enough to feel confident choosing it for this code." 2) "I don't want to spend time learning Rust that I could be spending just writing the code in C++." I believe that every developer that writes C++ at Mozilla should be given access to enough Rust training and work hours to spend learning it beyond the training so that we can eliminate case #1. With the Rust training sessions at prior All-Hands and self-motivated learning, I think we've pretty well saturated the group of early adopters. These people are actively writing new Rust code. We need to at least get the people that want to learn Rust but don't feel like they've had time to that same place. For case #2, there will always be people that don't want to learn new languages, and I'm sympathetic to their perspective. Learning Rust well does take a large investment of time. I don't know that I would go down the road of making Rust training mandatory (yet), but we are quickly going to hit a point where "I don't feel like learning Rust" is not going to cut it anymore. I would hope that by that point we will have trained everyone well enough that case #2 no longer exists, but if not we will have to make harder choices. > The tradeoffs come when the code is less standalone, and we need to weigh > the integration costs. This gets into questions like whether/how Rust > code > should integrate into the cycle collector or into JS object reflection, > which is very much a technical decision that should be made by experts. I > have a decent sense of who some of those experts might be, and would like > to find the most lightweight mechanism for them to steer this ship. We definitely need to figure out an ergonomic solution for writing core DOM components in Rust, but I agree that this needs a fair bit of work to be feasible. Most of the situations I've seen recently were not that tightly integrated into Gecko. -Ted ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: More Rust code
On Wed, Jul 12, 2017, at 07:41 PM, Mike Hommey wrote: > On Wed, Jul 12, 2017 at 04:06:39PM -0700, Eric Rahm wrote: > > Interesting points. > > > >- *using breakpad* - was the problem that creating wrappers to access > >the c/c++ code was too tedious? Could bindgen help with that, if not it > >would be interesting gather some details about why it wouldn't work and > >file bugs against it. > >- *pingsender* - was something like https://hyper.rs/ not around when > >you were working on it or is this a case of finding the things you want > > can > >be difficult in rust-land? Either way it might be a good idea for us to > > put > >together a list of "sanctioned" crates for various tasks. > > Note that pingsender is a small self-contained binary. I'm afraid writing > in > in rust would make it very much larger. While this is probably true, it's also a best-case scenario for writing a component in Rust: it doesn't have to call into *any* Gecko C++ code. We also have zero visibility into failures in helper programs like this, so the stability gains we get from using Rust instead of C++ are outsized in this case. -Ted ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On 7/16/17 11:10 PM, Edmund Wong wrote: Joe Hildebrand wrote: I'm responding at the top of the thread here so that I'm not singling out any particular response. We didn't make clear in this process how much work Mark and his team did ahead of the decision to gather feedback from senior engineers on both Selena and my teams, and how deeply committed the directors have been in their support of this change. Really? So? What exactly is your point? This sarcasm is unwarranted. Given that we've been talking about this stuff for years now, I think it's very clear that we haven't come to this point by "somebody at the top issuing an edict that they want something modern"; the decision to commit to Phabricator was ultimately announced on May 11th, and that decision wasn't made unilaterally or lightly. We've been discussing how to improve our tooling and processes in open forums for years now, most frequently on dev-platform. Despite that, it's true that a lot of the major influences of this decision have been mostly invisible to people who aren't involved in the day to day work on infrastructure and tooling, in the way that icebergs are mostly invisible. Some of the things that haven't made it into this thread that have influenced this decision include: - a real and pressing requirement to update our commit access policies and practices, and backstop them with reliable tooling (a discussion you participated in, as I recall), - the significant engineering burdens (including opportunity cost and bus-factor risk) of being the sole owner/user of a major piece of infrastructure software that's not our core product, and - the much more pernicious set of hidden costs we incur from _failing_ to standardize on certain tools and processes. It's a very serious drag on product development and contributor participation if everyone - paid senior engineers and new contributors alike - needs to use _this_ tool and process and coding style (and and and) to participate in one part of the project, and _that_ etc. etc. to participate in another. Or, sometimes, even other corners of the same codebase. Each of those on their own is a huge deal, probably big enough to warrant a major change in our tooling and processes. And it's true that we haven't done a great job of explicitly spelling out what a big deal they are, and how much they've informed our decision-making. But while we support - and will always support - user choice in our products, there aren't enough engineering hours in a year for us to support a bunch of different ways of shipping, securing, measuring and improving the tools and processes that provide those choices _to_ our users. So, yes, we could have done a better job of communicating here and making our process more transparent, but I'm very confident that this decision will make life a lot better for Mozilla's casual contributors and broader community as much as for Mozilla's full-time engineers. If you'd like to dig into the details of "why" I'd be happy to do so. And if we can do that without unnecessary snark, all the better. - mhoye ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Care in the use of MOZ_CRASH_UNSAFE_* macros: data review needed
On Mon, Jul 17, 2017 at 09:52:55AM -0700, Bobby Holley wrote: On Mon, Jul 17, 2017 at 9:42 AM, Benjamin Smedberg wrote: I don't know really anything about how rust panics get reflected into crash-data. Who would be the right person to talk to about that? Rust panics are equivalent to MOZ_CRASHES, and we treat them as such (or at least try to, see bug 1379857). Rust makes it easier to put non-constant things in the crash strings, which can be quite useful (see [1] as an example). They're not used often, and it seems unlikely that the existing use-cases would pose privacy issues, but I don't have a good proposal for enforcing that. It would be nice if we could add a commit hook that required a data-r tag for any changes that add MOZ_CRASH_UNSAFE_PRINTF or a Rust panic with a non-static string. I suspect it's something that most people will tend to overlook. For the cases that are clearly not data privacy issues, we could accept data-r=trivial, or something like that. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Care in the use of MOZ_CRASH_UNSAFE_* macros: data review needed
On Mon, Jul 17, 2017 at 9:42 AM, Benjamin Smedberg wrote: > I don't know really anything about how rust panics get reflected into > crash-data. Who would be the right person to talk to about that? > Rust panics are equivalent to MOZ_CRASHES, and we treat them as such (or at least try to, see bug 1379857). Rust makes it easier to put non-constant things in the crash strings, which can be quite useful (see [1] as an example). They're not used often, and it seems unlikely that the existing use-cases would pose privacy issues, but I don't have a good proposal for enforcing that. [1] http://searchfox.org/mozilla-central/rev/01d27fdd3946f7210da91b18fa01d7324fe2/servo/components/style/thread_state.rs#74 > > --BDS > > On Mon, Jul 17, 2017 at 12:40 PM, Emilio Cobos Álvarez > wrote: > > > On 07/17/2017 05:18 PM, Benjamin Smedberg wrote:> Unlike MOZ_CRASH, > > which only annotates crashes with compile-time constants > > > which are inherently not risky, both MOZ_CRASH_UNSAFE_OOL and > > > MOZ_CRASH_UNSAFE_PRINTF can annotate crashes with arbitrary data. Crash > > > reasons are publicly visible in crash reports, and in general it is not > > > appropriate to send any kind of user data as part of the crash reason. > > > > I suppose the same happens with rust panics, should those also be > reviewed? > > > > -- Emilio > > ___ > > dev-platform mailing list > > dev-platform@lists.mozilla.org > > https://lists.mozilla.org/listinfo/dev-platform > > > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Care in the use of MOZ_CRASH_UNSAFE_* macros: data review needed
As of bug 1275780, rust panic text gets reported as a MOZ_CRASH reason. On Mon, Jul 17, 2017 at 12:42 PM, Benjamin Smedberg wrote: > I don't know really anything about how rust panics get reflected into > crash-data. Who would be the right person to talk to about that? > > --BDS > > On Mon, Jul 17, 2017 at 12:40 PM, Emilio Cobos Álvarez > wrote: > > > On 07/17/2017 05:18 PM, Benjamin Smedberg wrote:> Unlike MOZ_CRASH, > > which only annotates crashes with compile-time constants > > > which are inherently not risky, both MOZ_CRASH_UNSAFE_OOL and > > > MOZ_CRASH_UNSAFE_PRINTF can annotate crashes with arbitrary data. Crash > > > reasons are publicly visible in crash reports, and in general it is not > > > appropriate to send any kind of user data as part of the crash reason. > > > > I suppose the same happens with rust panics, should those also be > reviewed? > > > > -- Emilio > > ___ > > dev-platform mailing list > > dev-platform@lists.mozilla.org > > https://lists.mozilla.org/listinfo/dev-platform > > > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Care in the use of MOZ_CRASH_UNSAFE_* macros: data review needed
I don't know really anything about how rust panics get reflected into crash-data. Who would be the right person to talk to about that? --BDS On Mon, Jul 17, 2017 at 12:40 PM, Emilio Cobos Álvarez wrote: > On 07/17/2017 05:18 PM, Benjamin Smedberg wrote:> Unlike MOZ_CRASH, > which only annotates crashes with compile-time constants > > which are inherently not risky, both MOZ_CRASH_UNSAFE_OOL and > > MOZ_CRASH_UNSAFE_PRINTF can annotate crashes with arbitrary data. Crash > > reasons are publicly visible in crash reports, and in general it is not > > appropriate to send any kind of user data as part of the crash reason. > > I suppose the same happens with rust panics, should those also be reviewed? > > -- Emilio > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Care in the use of MOZ_CRASH_UNSAFE_* macros: data review needed
On 07/17/2017 05:18 PM, Benjamin Smedberg wrote:> Unlike MOZ_CRASH, which only annotates crashes with compile-time constants > which are inherently not risky, both MOZ_CRASH_UNSAFE_OOL and > MOZ_CRASH_UNSAFE_PRINTF can annotate crashes with arbitrary data. Crash > reasons are publicly visible in crash reports, and in general it is not > appropriate to send any kind of user data as part of the crash reason. I suppose the same happens with rust panics, should those also be reviewed? -- Emilio ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
> On Jul 15, 2017, at 23:36, Gabriele Svelto wrote: > >> On 14/07/2017 05:39, Boris Zbarsky wrote: >> I should note that with GitHub what this means is that you get >> discussion on the PR that should have gone in the issue, with the result >> that people following the issue don't see half the relevant discussion. >> In particular, it's common to go off from "reviewing this line of code" >> to "raising questions about what the desired behavior is", which is >> squarely back in issue-land, not code-review land. >> >> Unfortunately, I don't know how to solve that problem without >> designating a "central point where all discussion happens" and ensuring >> that anything outside it is mirrored there... > > Yeah, we frequently had that problem with Gaia issues as part of Firefox > OS. Reviews were done on GitHub's PR so the bugzilla entries were often > empty (even for bugs with huge, complex patch-sets). To gather any kind > of information one had to go and check the comments on the PR itself > which not only was less-than-optimal but made life a lot harder for > those not directly involved with development. If the bug is only serving as an anchor to track code review, then the question we should be asking is "do we even need a bug." I've explored this a bit in https://gregoryszorc.com/blog/2015/01/16/bugzilla-and-the-future-of-firefox-development/. That post was written 2.5 years ago. If you replace "MozReview" with "Phabricator," many of the points I made are still relevant. We're just taking a drastically different path. Also, it was written very early in MozReview's development and reflected more of a long term workflow we wanted to enable. It became apparent that many things would not be easily achievable or were not reasonable at that time. That's why we dropped/paused many of the more drastic ideas. Making the decision to decouple review from issue tracking with Phabricator brings many of them back to the table. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Care in the use of MOZ_CRASH_UNSAFE_* macros: data review needed
Please take care when using the MOZ_CRASH_UNSAFE_* macros. Because of the risk involved and because this constitutes data collection, I would like Firefox data stewards to review any new usages of the MOZ_CRASH_UNSAFE_* macros. Unlike MOZ_CRASH, which only annotates crashes with compile-time constants which are inherently not risky, both MOZ_CRASH_UNSAFE_OOL and MOZ_CRASH_UNSAFE_PRINTF can annotate crashes with arbitrary data. Crash reasons are publicly visible in crash reports, and in general it is not appropriate to send any kind of user data as part of the crash reason. If you are interested in collecting diagnostic data which is not appropriate for public data collection, it is possible to use crash annotations to store this information more securely. --BDS ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On Thursday, July 13, 2017 at 11:39:38 PM UTC-4, Boris Zbarsky wrote: > On 7/13/17 9:04 PM, Mark Côté wrote: > > It is also what newer systems > > do today (e.g. GitHub and the full Phabricator suite) > > I should note that with GitHub what this means is that you get > discussion on the PR that should have gone in the issue, with the result > that people following the issue don't see half the relevant discussion. > In particular, it's common to go off from "reviewing this line of code" > to "raising questions about what the desired behavior is", which is > squarely back in issue-land, not code-review land. > > Unfortunately, I don't know how to solve that problem without > designating a "central point where all discussion happens" and ensuring > that anything outside it is mirrored there... So I for one, tend to like to read commentary on interesting bugs inline in my e-mail with said bug, so that I can get some context for the "why" along with "patch created" --> "comment on patch" --> "[context of patch] why did this line change" ---> ! Aha, I can help with context there! (or Maybe changing that hunk of that file is interesting to me and I can provide useful feedback). So, a few concrete suggestions on how I imagine we can meet my desire of having patch comment/review/etc in bug, without having the downsides called out in this thread (no deep thought on difficulty of implementation for these ideas, I trust those reading to evaluate utility against the difficulty): * Comments on patches get emailed to the people in the bug contact lists (using similar X-Bugzilla Headers) * Comments/Changes in phabricator get viewable in bugzilla directly. - Patch Creation/Obsolescence (options below) * New patches in patch list, like mozreview/traditional * An integrated table that merely queries a phabricator API * A bmo comment * An entry inline with bmo comments to indicate phabricator patch created/obsoleted, but without being part of bmo in any other way. - Patch flag setting (Depends partly on solution(s) chosen above) * Actual bmo flag flipped * Table of phabricator reviews updated * A bmo comment saying state of review changed * An entry inline with bmo comments but not actually a comment - Patch commentary * Actual BMO comment * A `phabricator` comment viewed in bmo (iframe/direct paste/whatever) as a read-only thing * A BMO comment but `reply` opens in phabricator * A BMO comment that says "review [x] updated in phabricator: [link]" * A inline history entry showing that a review comment happened in phabricator, with link. * ...etc? The bottom line for me, is that I'd like to be able to see, at a glance, that "stuff" happened in the review tool when I'm looking at a bug in bmo. There are various degrees of this that could make me happier than unhappy, though I'm aware that some of my own desires conflict directly with desires of others, and in relative difficulty to implement/maintain. I'd love to hear the outcome of this subthread, regardless of what choice (if any) of what I proposed, is made. ~Justin Wood (Callek) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
