Closed
Bug 1363533
Opened 7 years ago
Closed 7 years ago
Setup a build configuration to build firefox without built-in DevTools
Categories
(DevTools :: General, enhancement, P3)
DevTools
General
Tracking
(firefox55 fixed)
RESOLVED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: ochameau, Assigned: ochameau)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 3 obsolete files)
In order to ease testing the upcoming setup where mozilla-central doesn't ship DevTools, we should introduce a build configuration allowing to build Firefox without DevTools. MOZ_DEVTOOLS variable could be an option. Today it can be set to: server or all. In order to ship only the server part, or everything. We could introduce a third option: "addon", which would only ship binary components and any shim module that is meant to stay on mozilla-central.
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8866073 [details] Bug 1363533 - Introduce MOZ_DEVTOOLS=addon to allow building Firefox without DevTools. https://reviewboard.mozilla.org/r/137672/#review140836 At a high level, I think this seems like a resonable approach so you can start to move to the configuration you're expecting. However, it might be simpler if you move these native pieces to a location like `devtools/platform` or something. Then you'd modify `devtools/moz.build` so that in `addon` mode, it builds only that dir (the existing modes would also include it). That way the changes are contained to only `devtools/moz.build`, and you wouldn't need to list out each native / platform thing you want to include. I assume you need to leave these things behind in m-c somewhere anyway, so it probably makes sense for them to move somewhere other than where they are now. Where are you expecting the shim modules to live? That might also help clarify things. When you move DevTools code to GitHub, will there still be a /devtools directory in m-c at all? (I am curious what the final location of platform things would look like after the code moves.)
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #2) > However, it might be simpler if you move these native pieces to a location > like `devtools/platform` or something. Then you'd modify > `devtools/moz.build` so that in `addon` mode, it builds only that dir (the > existing modes would also include it). That way the changes are contained > to only `devtools/moz.build`, and you wouldn't need to list out each native > / platform thing you want to include. Yes I thought about moving heapsnapshot to js/ or somewhere in platform as it sounds closer to that rather than DevTools. > Where are you expecting the shim modules to live? That might also help > clarify things. We are about to introduce some new code that is going to stay in mozilla-central: - compatibility shim, replacing gDevTools.jsm (bug 1356244) Julian suggests to put them in /devtools/shim/ I suggested to put them at /devtools/ - new startup components which triggers the install UI (bug 1361080) I originaly put it at /devtools/, but could move them into /devtools/shim as well > When you move DevTools code to GitHub, will there still be a /devtools > directory in m-c at all? (I am curious what the final location of platform > things would look like after the code moves.) Yes. To keep things simple. The D-Day, we would just toggle the MOZ_DEVTOOLS to "addon" in /browser/confvars.sh Once we are confident the move to github is stable (i.e. we won't backout this change), we can remove all devtools files that are now on github. (all but shim and c++) And after that, we can shuffle things around in m-c. We could imagine moving the shim to /browser/ and C++ to js/, but tbh, I don't see a big value in doing this. We should just ensure that /devtools folder is simple, clear and isn't complex just for historical reasons. Here is what I had in mind as final layout, post file removal. /devtools/moz.build /devtools/README.MD (explain add-on + github story) /devtools/DevToolsShim.jsm (compatiblity shim) /devtools/devtools-startup.js (startup xpcom, addon-installation) /devtools/InstallDialog.{jsm,xhtml,js,css} (to come, define the addon installation dialog, may be hosted online) /devtools/heapsnapshot/ We could add a /devtools/shim/ folder to ease comprehension in today's layout.
Thanks, that helps! So I'd say make devtools/platform for the native bits you want to keep in all modes, and a devtools/shim for the new shim layer. That should make it easier to follow what's included where, and the moz.build changes should change to just whichever directories you need in each case. I agree you could move native bits to /js, the shim to /browser, etc., but leaving them in subdirs of /devtools seems fine too. I don't have a preference either way myself.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 9•7 years ago
|
||
Comment on attachment 8866073 [details] Bug 1363533 - Introduce MOZ_DEVTOOLS=addon to allow building Firefox without DevTools. I introduce empty moz.build in platform and shim folder. Second patch starts contributing to platform one, while shim should be modifed by bug 1361080 or bug 1356244 (whatever lands first)
Assignee | ||
Comment 10•7 years ago
|
||
Comment on attachment 8866736 [details] Bug 1363533 - Move heapsnapshot native components to /devtools/platform/heapsnapshot. I'll move this patch as followup as I expect more discussion about the js dependencies of this code and its tests.
Assignee | ||
Updated•7 years ago
|
Blocks: dt-addon-uishim
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8866073 [details] Bug 1363533 - Introduce MOZ_DEVTOOLS=addon to allow building Firefox without DevTools. https://reviewboard.mozilla.org/r/137672/#review141586 Thanks, looks good to me!
Attachment #8866073 -
Flags: review?(jryans) → review+
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8866735 [details] Bug 1363533 - Move nsIJSInspector from devtools/server/ to devtools/platform/. https://reviewboard.mozilla.org/r/138342/#review141588 Looks good! :)
Attachment #8866735 -
Flags: review?(jryans) → review+
Assignee | ||
Comment 13•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce4bcf33d56972aa170c93255928f2c5907a499e
Comment 14•7 years ago
|
||
mozreview-review |
Comment on attachment 8866735 [details] Bug 1363533 - Move nsIJSInspector from devtools/server/ to devtools/platform/. https://reviewboard.mozilla.org/r/138342/#review141588 Looks good! :)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8866073 -
Attachment is obsolete: true
Attachment #8866073 -
Flags: review?(jdescottes)
Assignee | ||
Updated•7 years ago
|
Attachment #8866736 -
Attachment is obsolete: true
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 18•7 years ago
|
||
mozreview-review |
Comment on attachment 8866793 [details] Bug 1363533 - Introduce MOZ_DEVTOOLS=addon to allow building Firefox without DevTools. https://reviewboard.mozilla.org/r/138400/#review141598
Attachment #8866793 -
Flags: review?(jryans) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 22•7 years ago
|
||
mozreview-review |
Comment on attachment 8866793 [details] Bug 1363533 - Introduce MOZ_DEVTOOLS=addon to allow building Firefox without DevTools. https://reviewboard.mozilla.org/r/138400/#review141608
Attachment #8866793 -
Flags: review?(jdescottes) → review+
Assignee | ||
Comment 23•7 years ago
|
||
Comment on attachment 8866735 [details] Bug 1363533 - Move nsIJSInspector from devtools/server/ to devtools/platform/. Hi gps, do you mind looking at these moz.build changes? The story here is to use MOZ_DEVTOOLS=addon to build firefox without devtools. That to anticipate the upcoming "devtools as an add-on" project. But we have to keep just a few bits related to devtools: * any c++/native code, like heapsnapshot * some glue code, here called "shims", that will connect mozilla-central/firefox codebase with the add-on.
Attachment #8866735 -
Flags: review?(gps)
Assignee | ||
Updated•7 years ago
|
Attachment #8866793 -
Flags: review?(gps)
Comment 24•7 years ago
|
||
mozreview-review |
Comment on attachment 8866793 [details] Bug 1363533 - Introduce MOZ_DEVTOOLS=addon to allow building Firefox without DevTools. https://reviewboard.mozilla.org/r/138400/#review141990 ::: devtools/moz.build:31 (Diff revision 2) > - 'server', > + 'server', > - 'shared', > + 'shared', > -] > + ] > +else: > + DIRS += [ > + 'shim', I didn't pay enough attention to the changes in the 2nd revision. My initial idea was to put the about:debugging registration code under shim/ as I thought it would be processed in any case. This way I could get rid of the current registration of about:debugging in nsAboutRedirector.cpp and and nsDocShellModule.cpp
Assignee | ||
Comment 25•7 years ago
|
||
I went back and forth with that, I originaly put it next to platform, so always included. But move it there for devtools-startup.js needs, I can move this if MOZ_DEVTOOLS==addon within shim/moz.build.
Comment 26•7 years ago
|
||
mozreview-review |
Comment on attachment 8866793 [details] Bug 1363533 - Introduce MOZ_DEVTOOLS=addon to allow building Firefox without DevTools. https://reviewboard.mozilla.org/r/138400/#review142270 Usually we don't add moz.build files until they are necessary. Why do you add them in this commit? Also, AFAICT the "shim" directory is empty at the end of this series. Or am I missing something?
Attachment #8866793 -
Flags: review?(gps)
Comment 27•7 years ago
|
||
mozreview-review |
Comment on attachment 8866735 [details] Bug 1363533 - Move nsIJSInspector from devtools/server/ to devtools/platform/. https://reviewboard.mozilla.org/r/138342/#review142272 ::: devtools/platform/tests/unit/test_nsjsinspector.js:1 (Diff revision 3) > +/* Any copyright is dedicated to the Public Domain. This file seems to have lost its rename annotation.
Attachment #8866735 -
Flags: review?(gps)
Comment 28•7 years ago
|
||
mozreview-review |
Comment on attachment 8866794 [details] Bug 1363533 - Move heapsnapshot native components to /devtools/platform/heapsnapshot. https://reviewboard.mozilla.org/r/138402/#review142274 This series seems pretty reasonable to me from a build system perspective.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8866794 -
Attachment is obsolete: true
Assignee | ||
Comment 31•7 years ago
|
||
I removed the empty moz.build, it was to help understanding where I'm going with this new flag. I also removed heapsnapshot for a dedicated followup. But there is something broken, still. I can't understand why nsIJSInspector interface isn't registered. I thought it was about missing modification to dumbmake-dependencies, but no. This interface is still missing. If you have any idea... Components.interfaces.nsIJSInspector is undefined.
(In reply to Alexandre Poirot [:ochameau] from comment #31) > I removed the empty moz.build, it was to help understanding where I'm going > with this new flag. > I also removed heapsnapshot for a dedicated followup. > > But there is something broken, still. > I can't understand why nsIJSInspector interface isn't registered. > I thought it was about missing modification to dumbmake-dependencies, but no. > This interface is still missing. If you have any idea... > Components.interfaces.nsIJSInspector is undefined. Did the .xpt change paths[1]? Maybe you need "@RESPATH@/components/jsinspector.xpt" instead of "@RESPATH@/browser/components/jsinspector.xpt"? [1]: http://searchfox.org/mozilla-central/source/browser/installer/package-manifest.in#253
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #32) > Did the .xpt change paths[1]? > > Maybe you need "@RESPATH@/components/jsinspector.xpt" instead of > "@RESPATH@/browser/components/jsinspector.xpt"? It still _seems_ like the old path should be correct, since devtools/moz.build is still setting `DIST_SUBDIR = 'browser'`... Anyway, I'd say inspect your objdir/dist directory to see where files like the .xpt are ending up.
Assignee | ||
Comment 34•7 years ago
|
||
The xpt path doesn't change. I tried to do some diff on the objdir and can't find anything significantly different :/ I tried chanding the IDs in the idl and c++, but got the same result.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → poirot.alex
Have you tried a full build, or only artifact builds? You might need a full build to cover these changes.
Assignee | ||
Comment 36•7 years ago
|
||
Full build, with wipping my objdir, this is really weird. Now I'm trying to move pieces bit by bit to see when it breaks.
Comment 37•7 years ago
|
||
mozreview-review |
Comment on attachment 8866793 [details] Bug 1363533 - Introduce MOZ_DEVTOOLS=addon to allow building Firefox without DevTools. https://reviewboard.mozilla.org/r/138400/#review144356
Attachment #8866793 -
Flags: review?(gps) → review+
Comment 38•7 years ago
|
||
mozreview-review |
Comment on attachment 8866735 [details] Bug 1363533 - Move nsIJSInspector from devtools/server/ to devtools/platform/. https://reviewboard.mozilla.org/r/138342/#review144358
Attachment #8866735 -
Flags: review?(gps) → review+
Comment 39•7 years ago
|
||
The browser/ prefix for the xpt comes from the export('DIST_SUBDIR') in devtools/moz.build. As long as the moz.build defining the XPIDL interface isn't having its DIST_SUBDIR reset by itself or in a calling moz.build, the xpt path should remain the same. Note that devtools/server/shims/toolkit/moz.build and devtools/shared/shims/moz.build reset DIST_SUBDIR to '', thus removing the browser/ prefix. But I don't think this comes into play here unless I'm missing something.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 44•7 years ago
|
||
Finally figured it out, I was missing firefox-appdir = browser in xpcshell.ini
Comment 45•7 years ago
|
||
Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/40eda96da608 Introduce MOZ_DEVTOOLS=addon to allow building Firefox without DevTools. r=gps,jdescottes,jryans https://hg.mozilla.org/integration/autoland/rev/ebb6631b83fc Move nsIJSInspector from devtools/server/ to devtools/platform/. r=gps,jryans
Comment 46•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/40eda96da608 https://hg.mozilla.org/mozilla-central/rev/ebb6631b83fc
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•