Re: Please do NOT hand-edit web platform test MANIFEST.json files
> Fun fact: lots of JSON documents also evaluate as Python data structures. > So if you prepend "foo = " and throw that into eval(), you can > magically evaluate a JSON document into a Python variable. Of course, > eval() is a security concern. But people blindly execute code in > mozilla-central (like the build system) all the time. So perhaps this is > tolerable. from ast import literal_eval literal_eval does not execute code. Only accepts literals. Works in all Python versions. Please don't use eval! ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Please do NOT hand-edit web platform test MANIFEST.json files
On 3/22/17 8:11 PM, gsquel...@mozilla.com wrote: (Is the ~10s extra build time unacceptable?) I just checked on my local machine, and a full manifest update takes over a minute. I rather doubt your machine is actually 6x faster than mine, so I have to assume that you didn't actually time a full manifest update. -Boris P.S. My 1-minute time is consistent with the times we were seeing before James added the checked-in hashes, which is why he added them. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Please do NOT hand-edit web platform test MANIFEST.json files
On Saturday, March 18, 2017 at 5:01:17 AM UTC+11, Boris Zbarsky wrote: > We have tools for this: "mach wpt-manifest-update" will do the right thing. > > The typical result of hand-edits is that later changesets that do use > the tools end up conflicting with each other, as they all fix up the > incorrect hand-edits. Please don't cause this pain for other developers > and the sheriffs. > > Thanks, > Boris Oh hey, another day, another silly thought: If this file can be generated, why is it even checked-in? :-) (Is the ~10s extra build time unacceptable?) Gerald ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Please do NOT hand-edit web platform test MANIFEST.json files
On Tue, Mar 21, 2017 at 7:44 PM, Boris Zbarsky wrote: > On 3/21/17 6:41 PM, Jeff Gilbert wrote: > >> JSON allows comments if all the JSON processors we use handle comments. :) >> > > JSON.parse in JS does not. > > The Python "json" module does not as far as I can tell. > > What JSON processors are you thinking of? > > -Boris > > P.S. The Python "json" module is most relevant here, since it's the thing > actually being used to deal with MANIFEST.json. Fun fact: lots of JSON documents also evaluate as Python data structures. So if you prepend "foo = " and throw that into eval(), you can magically evaluate a JSON document into a Python variable. Of course, eval() is a security concern. But people blindly execute code in mozilla-central (like the build system) all the time. So perhaps this is tolerable. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Please do NOT hand-edit web platform test MANIFEST.json files
On 3/21/17 6:41 PM, Jeff Gilbert wrote: JSON allows comments if all the JSON processors we use handle comments. :) JSON.parse in JS does not. The Python "json" module does not as far as I can tell. What JSON processors are you thinking of? -Boris P.S. The Python "json" module is most relevant here, since it's the thing actually being used to deal with MANIFEST.json. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Please do NOT hand-edit web platform test MANIFEST.json files
JSON allows comments if all the JSON processors we use handle comments. :) On Tue, Mar 21, 2017 at 8:52 AM, James Graham wrote: > On 20/03/17 22:15, gsquel...@mozilla.com wrote: > >> Sorry if it's a silly suggestion: >> Could the current tool insert some helpful reminders *everywhere* in the >> generated file (so it's can't be missed)? >> E.g., every 2nd line would read: "// PSA: This file is auto-generated by >> ./mach wpt-manifest-update, please don't edit!" ;-) > > > It is of course possible but it's not trivial since JSON doesn't allow > comments, so there would have to be some preprocessing magic and so on. > Probably better to spend the time on a good solution. > > > ___ > 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: Please do NOT hand-edit web platform test MANIFEST.json files
On 20/03/17 22:15, gsquel...@mozilla.com wrote: Sorry if it's a silly suggestion: Could the current tool insert some helpful reminders *everywhere* in the generated file (so it's can't be missed)? E.g., every 2nd line would read: "// PSA: This file is auto-generated by ./mach wpt-manifest-update, please don't edit!" ;-) It is of course possible but it's not trivial since JSON doesn't allow comments, so there would have to be some preprocessing magic and so on. Probably better to spend the time on a good solution. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Please do NOT hand-edit web platform test MANIFEST.json files
On Tuesday, March 21, 2017 at 2:58:17 AM UTC+11, Boris Zbarsky wrote: > On 3/19/17 12:36 AM, Nils Ohlmeier wrote: > > Wouldn’t it make more sense to let the build system detect and reject/warn > > about (?) such a manual modification? > > That would be ideal, but there are some issues with doing it. We tried > adding a lint, but it was orange _all the time_ because the sanest > possible workflow of "edit a test file" or "add a test file using the > mach command we have for it" looked like manual modification. > > We're working on a better setup for this, but in the meantime it would > be good if people would try to use the tools we have to make life a bit > less painful. > > -Boris Sorry if it's a silly suggestion: Could the current tool insert some helpful reminders *everywhere* in the generated file (so it's can't be missed)? E.g., every 2nd line would read: "// PSA: This file is auto-generated by ./mach wpt-manifest-update, please don't edit!" ;-) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Please do NOT hand-edit web platform test MANIFEST.json files
On 3/19/17 12:36 AM, Nils Ohlmeier wrote: Wouldn’t it make more sense to let the build system detect and reject/warn about (?) such a manual modification? That would be ideal, but there are some issues with doing it. We tried adding a lint, but it was orange _all the time_ because the sanest possible workflow of "edit a test file" or "add a test file using the mach command we have for it" looked like manual modification. We're working on a better setup for this, but in the meantime it would be good if people would try to use the tools we have to make life a bit less painful. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Please do NOT hand-edit web platform test MANIFEST.json files
On Sun, Mar 19, 2017, at 12:36 AM, Nils Ohlmeier wrote: > Wouldn’t it make more sense to let the build system detect and > reject/warn about (?) such a manual modification? > My assumption here is that mailing list archives are not a good place to > document processes or systems. I think some additional forward progress may now have been made. There's some extra IRC discussion of this issue at: http://logs.glob.uno/?c=mozilla%23content&s=17+Mar+2017&e=17+Mar+2017#c430993 In the discussion, https://bugzilla.mozilla.org/show_bug.cgi?id=1333433 "Consider making the wpt manifest a product of the build system" was referenced. And as a result of the discussion, https://bugzilla.mozilla.org/show_bug.cgi?id=1333433#c8 was logged on trying to use build artifacts to deal with the current 1+ minutes slowness of manifest generation (which is why it gets checked into the tree and leads to conflicts in the first place). Andrew ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Please do NOT hand-edit web platform test MANIFEST.json files
> On Mar 17, 2017, at 11:01, Boris Zbarsky wrote: > > We have tools for this: "mach wpt-manifest-update" will do the right thing. > > The typical result of hand-edits is that later changesets that do use the > tools end up conflicting with each other, as they all fix up the incorrect > hand-edits. Please don't cause this pain for other developers and the > sheriffs. Wouldn’t it make more sense to let the build system detect and reject/warn about (?) such a manual modification? My assumption here is that mailing list archives are not a good place to document processes or systems. Best Nils signature.asc Description: Message signed with OpenPGP ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Please do NOT hand-edit web platform test MANIFEST.json files
We have tools for this: "mach wpt-manifest-update" will do the right thing. The typical result of hand-edits is that later changesets that do use the tools end up conflicting with each other, as they all fix up the incorrect hand-edits. Please don't cause this pain for other developers and the sheriffs. Thanks, Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
