Remove debugger devtools-utils package
Categories
(DevTools :: Debugger, task, P3)
Tracking
(firefox99 fixed)
| Tracking | Status | |
|---|---|---|
| firefox99 | --- | fixed |
People
(Reporter: jdescottes, Assigned: jdescottes)
References
(Blocks 1 open bug)
Details
Attachments
(4 files, 1 obsolete file)
I was poking at how sourcemaps were working between devtools & the debugger, and I think it should be relatively easy to remove devtools-utils.
We can inline the networkRequest helper in devtools-source-map and move the worker helpers as regular devtools modules.
|
Assignee | |
Updated•2 years ago
|
|
|
Assignee | |
Comment 1•2 years ago
|
||
No significant gain to expect as we are mostly moving things around, but this could reduce the confusion around the debugger.
|
|
Assignee | |
Comment 2•2 years ago
|
||
Depends on D138759
|
|
Assignee | |
Comment 3•2 years ago
|
||
Depends on D138763
|
||
Comment 4•2 years ago
|
||
Comment on attachment 9263918 [details]
Bug 1755434 - [devtools] Add some documentation for debugger bin/bundle artifacts
Revision D138764 was moved to bug 1755529. Setting attachment 9263918 [details] to obsolete.
|
|
Assignee | |
Comment 5•2 years ago
|
||
As mentioned on Phabricator, the issue with this approach is that bundling the sourcemap-helper & the various debugger workers would depend on a regular DevTools module. This means that whenever we update the module, we should run node devtools/client/debugger/bin/bundle.js.
Without any safeguards, it's very likely we will forget to do that. It could be acceptable if we had a CI job to check if the bundles need to be regenerated.
Unassigning for now, I won't have time to complete this.
|
|
Assignee | |
Updated•2 years ago
|
|
|
||
Updated•2 years ago
|
|
|
Assignee | |
Comment 6•2 years ago
|
||
Hubert, Nicolas, just curious to get your feeling about the issue I faced here.
I was trying to remove devtools-utils. Doing that I extracted the worker-related modules to a regular DevTools module called worker-utils. The issue is that the workers built by devtools/client/debugger/bin/bundle.js rely on worker-utils, so each modification to this file means we should regenerate the bundles.
For me that's a deal breaker because I feel like we will forget to update the bundle.
One option would be to add a CI step which runs devtools/client/debugger/bin/bundle.js, then runs hg diff and fails if anything has changed, but I get the feeling it would be flaky?
Another option would be documentation + herald rules to have a mandatory review whenever any file which impacts a bundle is updated.
We can also just postpone this until we have a better idea of what to do with our build steps. I thought it would be an "easy small task while I have to look at source maps" so I don't really have to land this work :)
What do you think?
|
||
Comment 7•2 years ago
|
||
(In reply to Julian Descottes [:jdescottes] from comment #6)
One option would be to add a CI step which runs devtools/client/debugger/bin/bundle.js, then runs
hg diffand fails if anything has changed, but I get the feeling it would be flaky?
That kinda look similar to what we do for console stubs, and it could work well as a node test I guess (if we can execute the hg diff from CI)
I don't think it would be flaky, the only file that is modified "randomly" is devtools/client/debugger/bin/module-manifest.json , but I don't think we take advantage of it anywhere, so we can probably remove it.
|
|
Assignee | |
Comment 8•2 years ago
|
||
Oh good point, module-manifest.json was the one I was concerned about, but if we hgignore it we should be fine!
|
|
||
Comment 9•2 years ago
|
||
it's added by: devtools/client/debugger/webpack.config.js#27
And given the documentation on recordspath: https://webpack.js.org/configuration/other-options/#recordspath I don't think we have use for it, so we can probably remove the property in webpack.config.js and it should be fine?
|
|
Assignee | |
Comment 10•2 years ago
•
|
||
As discussed on Slack, it seems webpack.config.js actually brings some persistence to the bundles, otherwise they might significantly differ from one build to another. So let's go ahead and simply ignore this file from the diff.
For the record I have a very dumb test for this: https://treeherder.mozilla.org/jobs?repo=try&revision=cd2076d03e54afb07dde3f024edbf324eaf1b1d7&selectedTaskRun=QdXI2Y6jTMC1uplm5e7Low.0
It's just failing if there is any hg diff output, but that's a start.
In our case a hg status would probably be enough and easier to work with. Just split all the lines, ignore devtools/client/debugger/bin/module-manifest.json (as well as anything outside of devtools).
|
|
Assignee | |
Comment 11•2 years ago
|
||
(clearing the ni for bomsy, I think we have good next steps for now)
|
|
Assignee | |
Comment 12•2 years ago
|
||
Depends on D138763
|
|
Assignee | |
Comment 13•2 years ago
|
||
depends on D139737
|
||
Comment 14•2 years ago
|
||
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a81965b4dfa3 [devtools] Remove debugger devtools-utils package r=bomsy https://hg.mozilla.org/integration/autoland/rev/aeab90b13a07 [devtools] Replace netmonitor duplicated worker-utils with shared worker-utils r=bomsy https://hg.mozilla.org/integration/autoland/rev/bd9e34c0a3d9 [devtools] Regenerate debugger and source-map bundles r=bomsy https://hg.mozilla.org/integration/autoland/rev/b6a5caccd1d8 [devtools] Add job to check devtools bundles don't need to be regenerated r=jmaher
|
||
Comment 15•2 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/a81965b4dfa3
https://hg.mozilla.org/mozilla-central/rev/aeab90b13a07
https://hg.mozilla.org/mozilla-central/rev/bd9e34c0a3d9
https://hg.mozilla.org/mozilla-central/rev/b6a5caccd1d8
Description
•